[dpdk-dev,v2,6/6] app/testpmd: adjust ethdev port ownership
Checks
Commit Message
Testpmd should not use ethdev ports which are managed by other DPDK
entities.
Set Testpmd ownership to each port which is not used by other entity and
prevent any usage of ethdev ports which are not owned by Testpmd.
Signed-off-by: Matan Azrad <matan@mellanox.com>
---
app/test-pmd/cmdline.c | 88 +++++++++++++++++++--------------------------
app/test-pmd/cmdline_flow.c | 2 +-
app/test-pmd/config.c | 37 +++++++++----------
app/test-pmd/parameters.c | 4 +--
app/test-pmd/testpmd.c | 63 ++++++++++++++++++++------------
app/test-pmd/testpmd.h | 3 ++
6 files changed, 102 insertions(+), 95 deletions(-)
Comments
Hi Matan,
On Sun, Jan 07, 2018 at 09:45:51AM +0000, Matan Azrad wrote:
> Testpmd should not use ethdev ports which are managed by other DPDK
> entities.
>
> Set Testpmd ownership to each port which is not used by other entity and
> prevent any usage of ethdev ports which are not owned by Testpmd.
>
This patch should not be necessary.
Ideally, your API evolution should not impact the default case. As such,
the default iterator RTE_ETH_FOREACH_DEV should still be used in
testpmd.
RTE_ETH_FOREACH_DEV should call RTE_ETH_FOREACH_DEV_OWNED_BY, with the
default owner (meaning that it would thus iterate on the
application-owned set of device).
This new API should avoid breaking the current code as much as possible.
From: Gaëtan Rivet, Monday, January 8, 2018 1:40 PM
> Hi Matan,
>
> On Sun, Jan 07, 2018 at 09:45:51AM +0000, Matan Azrad wrote:
> > Testpmd should not use ethdev ports which are managed by other DPDK
> > entities.
> >
> > Set Testpmd ownership to each port which is not used by other entity
> > and prevent any usage of ethdev ports which are not owned by Testpmd.
> >
>
> This patch should not be necessary.
>
> Ideally, your API evolution should not impact the default case. As such, the
> default iterator RTE_ETH_FOREACH_DEV should still be used in testpmd.
>
Why? We want to adjust testpmd to the port ownership.
> RTE_ETH_FOREACH_DEV should call RTE_ETH_FOREACH_DEV_OWNED_BY,
> with the default owner (meaning that it would thus iterate on the
> application-owned set of device).
>
It will break the API (we already talked about it).
There is not any default owner:
Any DPDK entity includes applications must to allocate an owner ID and use it to own the ports they wants to use.
The application can include more than 1 owner depends on the user needs.
Each DPDK entity which can synchronize all its port usage can be a valid DPDK entity for the ownership mechanism.
> This new API should avoid breaking the current code as much as possible.
>
Yes, but there is a real big problem in testpmd regarding ownership issue - it must be changed.
The previous testpmd thought any port is for it in many places in the code.
Please see a lot of discussions about port ownership in the previous version.
> --
> Gaëtan Rivet
> 6WIND
On Mon, Jan 08, 2018 at 12:30:19PM +0000, Matan Azrad wrote:
>
>
> From: Gaëtan Rivet, Monday, January 8, 2018 1:40 PM
> > Hi Matan,
> >
> > On Sun, Jan 07, 2018 at 09:45:51AM +0000, Matan Azrad wrote:
> > > Testpmd should not use ethdev ports which are managed by other DPDK
> > > entities.
> > >
> > > Set Testpmd ownership to each port which is not used by other entity
> > > and prevent any usage of ethdev ports which are not owned by Testpmd.
> > >
> >
> > This patch should not be necessary.
> >
> > Ideally, your API evolution should not impact the default case. As such, the
> > default iterator RTE_ETH_FOREACH_DEV should still be used in testpmd.
> >
> Why? We want to adjust testpmd to the port ownership.
>
This adjustment should be seamless for existing application.
> > RTE_ETH_FOREACH_DEV should call RTE_ETH_FOREACH_DEV_OWNED_BY,
> > with the default owner (meaning that it would thus iterate on the
> > application-owned set of device).
> >
>
> It will break the API (we already talked about it).
> There is not any default owner:
> Any DPDK entity includes applications must to allocate an owner ID and use it to own the ports they wants to use.
> The application can include more than 1 owner depends on the user needs.
> Each DPDK entity which can synchronize all its port usage can be a valid DPDK entity for the ownership mechanism.
>
That's the point of my remark: you did not include a default owner.
I think there should be one, and that all ports should pertain to this
default owner by default when created.
This would not prevent a user or application from adding new owners
specific to their use and specialize ports if need be.
However, for other applications that do not care for this
specialization, they should run with the current API and avoid the ports
that are configured by other third parties.
I'm thinking about applications already written that would be used with
fail-safe ports: they would use RTE_ETH_FOREACH_DEV, and would thus
iterate over every ports, including those owned by the fail-safe, unless
they start following the new API.
This is unnecessary: adding a default owner for all created ports and
redefining RTE_ETH_FOREACH_DEV as follow
#define RTE_ETH_FOREACH_DEV(i)
RTE_ETH_FOREACH_DEV_OWNED_BY(i, RTE_ETH_DEFAULT_OWNER)
Is simple enough and will simplify the work of DPDK users. Moreover, it
would make fail-safe compatible with all applications using
RTE_ETH_FOREACH_DEV without additional evolution. It would actually
make any code using your API supported by those same applications, which
I think would help its adoption.
> > This new API should avoid breaking the current code as much as possible.
> >
> Yes, but there is a real big problem in testpmd regarding ownership issue - it must be changed.
> The previous testpmd thought any port is for it in many places in the code.
>
Sure, then update the code with RTE_ETH_FOREACH_DEV().
> Please see a lot of discussions about port ownership in the previous version.
You did not address this remark in the previous thread. I'm thus
reiterating it with the new version of your patchset.
Hi Gaetan
From: Gaëtan Rivet, Monday, January 8, 2018 3:30 PM
> On Mon, Jan 08, 2018 at 12:30:19PM +0000, Matan Azrad wrote:
> >
> >
> > From: Gaëtan Rivet, Monday, January 8, 2018 1:40 PM
> > > Hi Matan,
> > >
> > > On Sun, Jan 07, 2018 at 09:45:51AM +0000, Matan Azrad wrote:
> > > > Testpmd should not use ethdev ports which are managed by other
> > > > DPDK entities.
> > > >
> > > > Set Testpmd ownership to each port which is not used by other
> > > > entity and prevent any usage of ethdev ports which are not owned by
> Testpmd.
> > > >
> > >
> > > This patch should not be necessary.
> > >
> > > Ideally, your API evolution should not impact the default case. As
> > > such, the default iterator RTE_ETH_FOREACH_DEV should still be used in
> testpmd.
> > >
> > Why? We want to adjust testpmd to the port ownership.
> >
>
> This adjustment should be seamless for existing application.
>
> > > RTE_ETH_FOREACH_DEV should call
> RTE_ETH_FOREACH_DEV_OWNED_BY, with
> > > the default owner (meaning that it would thus iterate on the
> > > application-owned set of device).
> > >
> >
> > It will break the API (we already talked about it).
> > There is not any default owner:
> > Any DPDK entity includes applications must to allocate an owner ID and use
> it to own the ports they wants to use.
> > The application can include more than 1 owner depends on the user needs.
> > Each DPDK entity which can synchronize all its port usage can be a valid
> DPDK entity for the ownership mechanism.
> >
>
> That's the point of my remark: you did not include a default owner.
> I think there should be one, and that all ports should pertain to this default
> owner by default when created.
>
> This would not prevent a user or application from adding new owners specific
> to their use and specialize ports if need be.
>
> However, for other applications that do not care for this specialization, they
> should run with the current API and avoid the ports that are configured by
> other third parties.
>
RTE_ETH_FOREACH_DEV means iterate over all devices and should stay as is in my opinion.
I understand your concern about changes in current application,
But your "default" suggestion will cause to "non-default" applications to reset all the default owners and will complicate them and hurt semantics.
> I'm thinking about applications already written that would be used with fail-
> safe ports: they would use RTE_ETH_FOREACH_DEV, and would thus iterate
> over every ports, including those owned by the fail-safe, unless they start
> following the new API.
>
They should start, it is really not complicated.
What's about application which use count=rte_eth_dev_count and iterate over all ports from 0 to count-1?
We cannot save all the wrong application options.
> This is unnecessary: adding a default owner for all created ports and
> redefining RTE_ETH_FOREACH_DEV as follow
>
> #define RTE_ETH_FOREACH_DEV(i)
> RTE_ETH_FOREACH_DEV_OWNED_BY(i, RTE_ETH_DEFAULT_OWNER)
>
> Is simple enough and will simplify the work of DPDK users. Moreover, it
> would make fail-safe compatible with all applications using
> RTE_ETH_FOREACH_DEV without additional evolution. It would actually make
> any code using your API supported by those same applications, which I think
> would help its adoption.
>
Will break API, will hurt semantic of FOREACH , and will complicate ownership care applications as I wrote above.
> --
> Gaëtan Rivet
> 6WIND
On Mon, Jan 08, 2018 at 01:55:52PM +0000, Matan Azrad wrote:
> Hi Gaetan
>
> From: Gaëtan Rivet, Monday, January 8, 2018 3:30 PM
> > On Mon, Jan 08, 2018 at 12:30:19PM +0000, Matan Azrad wrote:
> > >
> > >
> > > From: Gaëtan Rivet, Monday, January 8, 2018 1:40 PM
> > > > Hi Matan,
> > > >
> > > > On Sun, Jan 07, 2018 at 09:45:51AM +0000, Matan Azrad wrote:
> > > > > Testpmd should not use ethdev ports which are managed by other
> > > > > DPDK entities.
> > > > >
> > > > > Set Testpmd ownership to each port which is not used by other
> > > > > entity and prevent any usage of ethdev ports which are not owned by
> > Testpmd.
> > > > >
> > > >
> > > > This patch should not be necessary.
> > > >
> > > > Ideally, your API evolution should not impact the default case. As
> > > > such, the default iterator RTE_ETH_FOREACH_DEV should still be used in
> > testpmd.
> > > >
> > > Why? We want to adjust testpmd to the port ownership.
> > >
> >
> > This adjustment should be seamless for existing application.
> >
> > > > RTE_ETH_FOREACH_DEV should call
> > RTE_ETH_FOREACH_DEV_OWNED_BY, with
> > > > the default owner (meaning that it would thus iterate on the
> > > > application-owned set of device).
> > > >
> > >
> > > It will break the API (we already talked about it).
> > > There is not any default owner:
> > > Any DPDK entity includes applications must to allocate an owner ID and use
> > it to own the ports they wants to use.
> > > The application can include more than 1 owner depends on the user needs.
> > > Each DPDK entity which can synchronize all its port usage can be a valid
> > DPDK entity for the ownership mechanism.
> > >
> >
> > That's the point of my remark: you did not include a default owner.
> > I think there should be one, and that all ports should pertain to this default
> > owner by default when created.
> >
> > This would not prevent a user or application from adding new owners specific
> > to their use and specialize ports if need be.
> >
> > However, for other applications that do not care for this specialization, they
> > should run with the current API and avoid the ports that are configured by
> > other third parties.
> >
>
> RTE_ETH_FOREACH_DEV means iterate over all devices and should stay as is in my opinion.
> I understand your concern about changes in current application,
> But your "default" suggestion will cause to "non-default" applications to reset all the default owners and will complicate them and hurt semantics.
Why should an application be able to iterate over all ports? Leave this
capability to the EAL (or ethdev layer) alone, while other components should
be restricted to their specific set.
And if a need for this general iterator appears, solutions could be
found very easily.
RTE_ETH_FOREACH_DEV currently does not iterate over deferred ports, it
iterates over the base set of ports available. Changing this behavior is
not necessary, you could introduce your API while keeping it.
>
> > I'm thinking about applications already written that would be used with fail-
> > safe ports: they would use RTE_ETH_FOREACH_DEV, and would thus iterate
> > over every ports, including those owned by the fail-safe, unless they start
> > following the new API.
> >
>
> They should start, it is really not complicated.
The point is not whether developpers downstream would be able to grasp
such complexity, but whether a project like DPDK should foster an
unstable environment for its currently still limited ecosystem.
> What's about application which use count=rte_eth_dev_count and iterate over all ports from 0 to count-1?
> We cannot save all the wrong application options.
>
> > This is unnecessary: adding a default owner for all created ports and
> > redefining RTE_ETH_FOREACH_DEV as follow
> >
> > #define RTE_ETH_FOREACH_DEV(i)
> > RTE_ETH_FOREACH_DEV_OWNED_BY(i, RTE_ETH_DEFAULT_OWNER)
> >
> > Is simple enough and will simplify the work of DPDK users. Moreover, it
> > would make fail-safe compatible with all applications using
> > RTE_ETH_FOREACH_DEV without additional evolution. It would actually make
> > any code using your API supported by those same applications, which I think
> > would help its adoption.
> >
>
> Will break API, will hurt semantic of FOREACH , and will complicate ownership care applications as I wrote above.
Well, breaking an API is best before such API is integrated anyway.
I disagree regarding the added complexity for applications that would
use ownership. With your proposal, most applications will only add a
single user and register all their ports with this user, then be forced
to iterate upon their registered user.
You can save all of them the hassle of adding this code, by taking care
of the most common case, avoiding redundant code downstream and simplifying
possible future update to this default case.
So if anything, this would greatly simplify ownership for the vast
majority of applications.
From: Gaëtan Rivet, Monday, January 8, 2018 4:22 PM
> On Mon, Jan 08, 2018 at 01:55:52PM +0000, Matan Azrad wrote:
> > Hi Gaetan
> >
> > From: Gaëtan Rivet, Monday, January 8, 2018 3:30 PM
> > > On Mon, Jan 08, 2018 at 12:30:19PM +0000, Matan Azrad wrote:
> > > >
> > > >
> > > > From: Gaëtan Rivet, Monday, January 8, 2018 1:40 PM
> > > > > Hi Matan,
> > > > >
> > > > > On Sun, Jan 07, 2018 at 09:45:51AM +0000, Matan Azrad wrote:
> > > > > > Testpmd should not use ethdev ports which are managed by other
> > > > > > DPDK entities.
> > > > > >
> > > > > > Set Testpmd ownership to each port which is not used by other
> > > > > > entity and prevent any usage of ethdev ports which are not
> > > > > > owned by
> > > Testpmd.
> > > > > >
> > > > >
> > > > > This patch should not be necessary.
> > > > >
> > > > > Ideally, your API evolution should not impact the default case.
> > > > > As such, the default iterator RTE_ETH_FOREACH_DEV should still
> > > > > be used in
> > > testpmd.
> > > > >
> > > > Why? We want to adjust testpmd to the port ownership.
> > > >
> > >
> > > This adjustment should be seamless for existing application.
> > >
> > > > > RTE_ETH_FOREACH_DEV should call
> > > RTE_ETH_FOREACH_DEV_OWNED_BY, with
> > > > > the default owner (meaning that it would thus iterate on the
> > > > > application-owned set of device).
> > > > >
> > > >
> > > > It will break the API (we already talked about it).
> > > > There is not any default owner:
> > > > Any DPDK entity includes applications must to allocate an owner ID
> > > > and use
> > > it to own the ports they wants to use.
> > > > The application can include more than 1 owner depends on the user
> needs.
> > > > Each DPDK entity which can synchronize all its port usage can be a
> > > > valid
> > > DPDK entity for the ownership mechanism.
> > > >
> > >
> > > That's the point of my remark: you did not include a default owner.
> > > I think there should be one, and that all ports should pertain to
> > > this default owner by default when created.
> > >
> > > This would not prevent a user or application from adding new owners
> > > specific to their use and specialize ports if need be.
> > >
> > > However, for other applications that do not care for this
> > > specialization, they should run with the current API and avoid the
> > > ports that are configured by other third parties.
> > >
> >
> > RTE_ETH_FOREACH_DEV means iterate over all devices and should stay as
> is in my opinion.
> > I understand your concern about changes in current application, But
> > your "default" suggestion will cause to "non-default" applications to reset
> all the default owners and will complicate them and hurt semantics.
>
> Why should an application be able to iterate over all ports? Leave this
> capability to the EAL (or ethdev layer) alone, while other components should
> be restricted to their specific set.
>
Yes, you right.
> And if a need for this general iterator appears, solutions could be found very
> easily.
>
> RTE_ETH_FOREACH_DEV currently does not iterate over deferred ports, it
> iterates over the base set of ports available. Changing this behavior is not
> necessary, you could introduce your API while keeping it.
>
Right.
> >
> > > I'm thinking about applications already written that would be used
> > > with fail- safe ports: they would use RTE_ETH_FOREACH_DEV, and would
> > > thus iterate over every ports, including those owned by the
> > > fail-safe, unless they start following the new API.
> > >
> >
> > They should start, it is really not complicated.
>
> The point is not whether developpers downstream would be able to grasp
> such complexity, but whether a project like DPDK should foster an unstable
> environment for its currently still limited ecosystem.
>
> > What's about application which use count=rte_eth_dev_count and iterate
> over all ports from 0 to count-1?
> > We cannot save all the wrong application options.
> >
> > > This is unnecessary: adding a default owner for all created ports
> > > and redefining RTE_ETH_FOREACH_DEV as follow
> > >
> > > #define RTE_ETH_FOREACH_DEV(i)
> > > RTE_ETH_FOREACH_DEV_OWNED_BY(i, RTE_ETH_DEFAULT_OWNER)
> > >
> > > Is simple enough and will simplify the work of DPDK users. Moreover,
> > > it would make fail-safe compatible with all applications using
> > > RTE_ETH_FOREACH_DEV without additional evolution. It would actually
> > > make any code using your API supported by those same applications,
> > > which I think would help its adoption.
> > >
> >
> > Will break API, will hurt semantic of FOREACH , and will complicate
> ownership care applications as I wrote above.
>
> Well, breaking an API is best before such API is integrated anyway.
>
> I disagree regarding the added complexity for applications that would use
> ownership. With your proposal, most applications will only add a single user
> and register all their ports with this user, then be forced to iterate upon their
> registered user.
>
> You can save all of them the hassle of adding this code, by taking care of the
> most common case, avoiding redundant code downstream and simplifying
> possible future update to this default case.
>
> So if anything, this would greatly simplify ownership for the vast majority of
> applications.
>
OK, got you.
I will just document the API with the new semantic and will use the NO_OWNER for the old API.
But actually I think testpmd should use the ownership mechanism as a good example for it.
Thanks!
> --
> Gaëtan Rivet
> 6WIND
Hi Matan,
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Matan Azrad
> Sent: Sunday, January 7, 2018 5:46 PM
> To: Thomas Monjalon <thomas@monjalon.net>; Gaetan Rivet
> <gaetan.rivet@6wind.com>; Wu, Jingjing <jingjing.wu@intel.com>
> Cc: dev@dpdk.org; Neil Horman <nhorman@tuxdriver.com>; Richardson,
> Bruce <bruce.richardson@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>
> Subject: [dpdk-dev] [PATCH v2 6/6] app/testpmd: adjust ethdev port
> ownership
>
> Testpmd should not use ethdev ports which are managed by other DPDK
> entities.
>
> Set Testpmd ownership to each port which is not used by other entity and
> prevent any usage of ethdev ports which are not owned by Testpmd.
Sorry I don't follow all the discussion as there's too much. So it may be a silly question.
Testpmd already has the parameter " --pci-whitelist" to only use the assigned devices. When using this parameter, all the devices are owned by the current APP. So I don't know why need to set/check the ownership.
BTW, in this patch, seem all the change is for ownership checking. I don't find the setting code. Do I miss something?
Hi Lu
From: Lu, Wenzhuo, Tuesday, January 16, 2018 7:54 AM
> Hi Matan,
>
>
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Matan Azrad
> > Sent: Sunday, January 7, 2018 5:46 PM
> > To: Thomas Monjalon <thomas@monjalon.net>; Gaetan Rivet
> > <gaetan.rivet@6wind.com>; Wu, Jingjing <jingjing.wu@intel.com>
> > Cc: dev@dpdk.org; Neil Horman <nhorman@tuxdriver.com>; Richardson,
> > Bruce <bruce.richardson@intel.com>; Ananyev, Konstantin
> > <konstantin.ananyev@intel.com>
> > Subject: [dpdk-dev] [PATCH v2 6/6] app/testpmd: adjust ethdev port
> > ownership
> >
> > Testpmd should not use ethdev ports which are managed by other DPDK
> > entities.
> >
> > Set Testpmd ownership to each port which is not used by other entity
> > and prevent any usage of ethdev ports which are not owned by Testpmd.
> Sorry I don't follow all the discussion as there's too much. So it may be a silly
> question.
No problem, I'm here for any question :)
> Testpmd already has the parameter " --pci-whitelist" to only use the assigned
> devices.
It is an EAL parameter. No? just say to EAL which devices to create..
> When using this parameter, all the devices are owned by the current
> APP.
No, what's about vdev? vdevs may manage devices(even whitlist PCI devices) by themselves and want to prevent any app to use these devices(see fail-safe PMD).
> So I don't know why need to set/check the ownership.
> BTW, in this patch, seem all the change is for ownership checking. I don't find
> the setting code. Do I miss something?
Yes, see in main function (the first FOREACH).
Thanks, Matan.
Hi Matan,
> -----Original Message-----
> From: Matan Azrad [mailto:matan@mellanox.com]
> Sent: Tuesday, January 16, 2018 4:16 PM
> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Thomas Monjalon
> <thomas@monjalon.net>; Gaetan Rivet <gaetan.rivet@6wind.com>; Wu,
> Jingjing <jingjing.wu@intel.com>
> Cc: dev@dpdk.org; Neil Horman <nhorman@tuxdriver.com>; Richardson,
> Bruce <bruce.richardson@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>
> Subject: RE: [dpdk-dev] [PATCH v2 6/6] app/testpmd: adjust ethdev port
> ownership
>
> Hi Lu
> From: Lu, Wenzhuo, Tuesday, January 16, 2018 7:54 AM
> > Hi Matan,
> >
> >
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Matan Azrad
> > > Sent: Sunday, January 7, 2018 5:46 PM
> > > To: Thomas Monjalon <thomas@monjalon.net>; Gaetan Rivet
> > > <gaetan.rivet@6wind.com>; Wu, Jingjing <jingjing.wu@intel.com>
> > > Cc: dev@dpdk.org; Neil Horman <nhorman@tuxdriver.com>; Richardson,
> > > Bruce <bruce.richardson@intel.com>; Ananyev, Konstantin
> > > <konstantin.ananyev@intel.com>
> > > Subject: [dpdk-dev] [PATCH v2 6/6] app/testpmd: adjust ethdev port
> > > ownership
> > >
> > > Testpmd should not use ethdev ports which are managed by other DPDK
> > > entities.
> > >
> > > Set Testpmd ownership to each port which is not used by other entity
> > > and prevent any usage of ethdev ports which are not owned by Testpmd.
> > Sorry I don't follow all the discussion as there's too much. So it may
> > be a silly question.
>
> No problem, I'm here for any question :)
>
> > Testpmd already has the parameter " --pci-whitelist" to only use the
> > assigned devices.
>
> It is an EAL parameter. No? just say to EAL which devices to create..
>
> > When using this parameter, all the devices are owned by the current
> > APP.
>
> No, what's about vdev? vdevs may manage devices(even whitlist PCI devices)
> by themselves and want to prevent any app to use these devices(see fail-
> safe PMD).
I'm not an expert of EAL and vdev. Suppose this would be discussed in other patches.
I don't want to bother you again here as testpmd is only used to show the result.
So I think if this patch is needed just depends on if other patches are accepted :)
>
> > So I don't know why need to set/check the ownership.
> > BTW, in this patch, seem all the change is for ownership checking. I
> > don't find the setting code. Do I miss something?
>
> Yes, see in main function (the first FOREACH).
I think you mean this change,
@@ -2394,7 +2406,12 @@ uint8_t port_is_bonding_slave(portid_t slave_pid)
rte_pdump_init(NULL);
#endif
- nb_ports = (portid_t) rte_eth_dev_count();
+ if (rte_eth_dev_owner_new(&my_owner.id))
+ rte_panic("Failed to get unique owner identifier\n");
+ snprintf(my_owner.name, sizeof(my_owner.name), TESTPMD_OWNER_NAME);
+ RTE_ETH_FOREACH_DEV_OWNED_BY(port_id, RTE_ETH_DEV_NO_OWNER)
+ if (rte_eth_dev_owner_set(port_id, &my_owner) == 0)
+ nb_ports++;
if (nb_ports == 0)
RTE_LOG(WARNING, EAL, "No probed ethernet devices\n");
But I thought about some code to assign a specific device to a specific APP explicitly.
This code looks like just occupying the devices with no owner. So, it means the first APP will occupy all the devices? It makes me confused as I don't see the benefit or the difference than before.
>
> Thanks, Matan.
Hi Lu
From: Lu, Wenzhuo, Wednesday, January 17, 2018 2:47 AM
> Hi Matan,
>
> > -----Original Message-----
> > From: Matan Azrad [mailto:matan@mellanox.com]
> > Sent: Tuesday, January 16, 2018 4:16 PM
> > To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Thomas Monjalon
> > <thomas@monjalon.net>; Gaetan Rivet <gaetan.rivet@6wind.com>; Wu,
> > Jingjing <jingjing.wu@intel.com>
> > Cc: dev@dpdk.org; Neil Horman <nhorman@tuxdriver.com>; Richardson,
> > Bruce <bruce.richardson@intel.com>; Ananyev, Konstantin
> > <konstantin.ananyev@intel.com>
> > Subject: RE: [dpdk-dev] [PATCH v2 6/6] app/testpmd: adjust ethdev port
> > ownership
> >
> > Hi Lu
> > From: Lu, Wenzhuo, Tuesday, January 16, 2018 7:54 AM
> > > Hi Matan,
> > >
> > >
> > > > -----Original Message-----
> > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Matan Azrad
> > > > Sent: Sunday, January 7, 2018 5:46 PM
> > > > To: Thomas Monjalon <thomas@monjalon.net>; Gaetan Rivet
> > > > <gaetan.rivet@6wind.com>; Wu, Jingjing <jingjing.wu@intel.com>
> > > > Cc: dev@dpdk.org; Neil Horman <nhorman@tuxdriver.com>;
> Richardson,
> > > > Bruce <bruce.richardson@intel.com>; Ananyev, Konstantin
> > > > <konstantin.ananyev@intel.com>
> > > > Subject: [dpdk-dev] [PATCH v2 6/6] app/testpmd: adjust ethdev port
> > > > ownership
> > > >
> > > > Testpmd should not use ethdev ports which are managed by other
> > > > DPDK entities.
> > > >
> > > > Set Testpmd ownership to each port which is not used by other
> > > > entity and prevent any usage of ethdev ports which are not owned by
> Testpmd.
> > > Sorry I don't follow all the discussion as there's too much. So it
> > > may be a silly question.
> >
> > No problem, I'm here for any question :)
> >
> > > Testpmd already has the parameter " --pci-whitelist" to only use the
> > > assigned devices.
> >
> > It is an EAL parameter. No? just say to EAL which devices to create..
> >
> > > When using this parameter, all the devices are owned by the current
> > > APP.
> >
> > No, what's about vdev? vdevs may manage devices(even whitlist PCI
> > devices) by themselves and want to prevent any app to use these
> > devices(see fail- safe PMD).
> I'm not an expert of EAL and vdev. Suppose this would be discussed in other
> patches.
> I don't want to bother you again here as testpmd is only used to show the
> result.
> So I think if this patch is needed just depends on if other patches are
> accepted :)
>
Sure!
> >
> > > So I don't know why need to set/check the ownership.
> > > BTW, in this patch, seem all the change is for ownership checking. I
> > > don't find the setting code. Do I miss something?
> >
> > Yes, see in main function (the first FOREACH).
> I think you mean this change,
>
> @@ -2394,7 +2406,12 @@ uint8_t port_is_bonding_slave(portid_t
> slave_pid)
> rte_pdump_init(NULL);
> #endif
>
> - nb_ports = (portid_t) rte_eth_dev_count();
> + if (rte_eth_dev_owner_new(&my_owner.id))
> + rte_panic("Failed to get unique owner identifier\n");
> + snprintf(my_owner.name, sizeof(my_owner.name),
> TESTPMD_OWNER_NAME);
> + RTE_ETH_FOREACH_DEV_OWNED_BY(port_id,
> RTE_ETH_DEV_NO_OWNER)
> + if (rte_eth_dev_owner_set(port_id, &my_owner) == 0)
> + nb_ports++;
> if (nb_ports == 0)
> RTE_LOG(WARNING, EAL, "No probed ethernet devices\n");
Yes.
> But I thought about some code to assign a specific device to a specific APP
> explicitly.
> This code looks like just occupying the devices with no owner. So, it means
> the first APP will occupy all the devices? It makes me confused as I don't see
> the benefit or the difference than before.
Remember that in EAL init some drivers may create ports and take ownership of them, so this code owns all the ownerless ports.
So, ports which are already owned by another DPDK entity will not success to take ownership here.
Since Testpmd wanted to manage all the ports before this feature( by mistake ), I guess this is the right behavior now, just use the ports which are not used.
>
> >
> > Thanks, Matan.
Hi Matan,
> -----Original Message-----
> From: Matan Azrad [mailto:matan@mellanox.com]
> Sent: Wednesday, January 17, 2018 4:51 PM
> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Thomas Monjalon
> <thomas@monjalon.net>; Gaetan Rivet <gaetan.rivet@6wind.com>; Wu,
> Jingjing <jingjing.wu@intel.com>
> Cc: dev@dpdk.org; Neil Horman <nhorman@tuxdriver.com>; Richardson,
> Bruce <bruce.richardson@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>
> Subject: RE: [dpdk-dev] [PATCH v2 6/6] app/testpmd: adjust ethdev port
> ownership
>
> Hi Lu
>
> From: Lu, Wenzhuo, Wednesday, January 17, 2018 2:47 AM
> > Hi Matan,
> >
> > > -----Original Message-----
> > > From: Matan Azrad [mailto:matan@mellanox.com]
> > > Sent: Tuesday, January 16, 2018 4:16 PM
> > > To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Thomas Monjalon
> > > <thomas@monjalon.net>; Gaetan Rivet <gaetan.rivet@6wind.com>; Wu,
> > > Jingjing <jingjing.wu@intel.com>
> > > Cc: dev@dpdk.org; Neil Horman <nhorman@tuxdriver.com>; Richardson,
> > > Bruce <bruce.richardson@intel.com>; Ananyev, Konstantin
> > > <konstantin.ananyev@intel.com>
> > > Subject: RE: [dpdk-dev] [PATCH v2 6/6] app/testpmd: adjust ethdev
> > > port ownership
> > >
> > > Hi Lu
> > > From: Lu, Wenzhuo, Tuesday, January 16, 2018 7:54 AM
> > > > Hi Matan,
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Matan Azrad
> > > > > Sent: Sunday, January 7, 2018 5:46 PM
> > > > > To: Thomas Monjalon <thomas@monjalon.net>; Gaetan Rivet
> > > > > <gaetan.rivet@6wind.com>; Wu, Jingjing <jingjing.wu@intel.com>
> > > > > Cc: dev@dpdk.org; Neil Horman <nhorman@tuxdriver.com>;
> > Richardson,
> > > > > Bruce <bruce.richardson@intel.com>; Ananyev, Konstantin
> > > > > <konstantin.ananyev@intel.com>
> > > > > Subject: [dpdk-dev] [PATCH v2 6/6] app/testpmd: adjust ethdev
> > > > > port ownership
> > > > >
> > > > > Testpmd should not use ethdev ports which are managed by other
> > > > > DPDK entities.
> > > > >
> > > > > Set Testpmd ownership to each port which is not used by other
> > > > > entity and prevent any usage of ethdev ports which are not owned
> > > > > by
> > Testpmd.
> > > > Sorry I don't follow all the discussion as there's too much. So it
> > > > may be a silly question.
> > >
> > > No problem, I'm here for any question :)
> > >
> > > > Testpmd already has the parameter " --pci-whitelist" to only use
> > > > the assigned devices.
> > >
> > > It is an EAL parameter. No? just say to EAL which devices to create..
> > >
> > > > When using this parameter, all the devices are owned by the
> > > > current APP.
> > >
> > > No, what's about vdev? vdevs may manage devices(even whitlist PCI
> > > devices) by themselves and want to prevent any app to use these
> > > devices(see fail- safe PMD).
> > I'm not an expert of EAL and vdev. Suppose this would be discussed in
> > other patches.
> > I don't want to bother you again here as testpmd is only used to show
> > the result.
> > So I think if this patch is needed just depends on if other patches
> > are accepted :)
> >
> Sure!
>
> > >
> > > > So I don't know why need to set/check the ownership.
> > > > BTW, in this patch, seem all the change is for ownership checking.
> > > > I don't find the setting code. Do I miss something?
> > >
> > > Yes, see in main function (the first FOREACH).
> > I think you mean this change,
> >
> > @@ -2394,7 +2406,12 @@ uint8_t port_is_bonding_slave(portid_t
> > slave_pid)
> > rte_pdump_init(NULL);
> > #endif
> >
> > - nb_ports = (portid_t) rte_eth_dev_count();
> > + if (rte_eth_dev_owner_new(&my_owner.id))
> > + rte_panic("Failed to get unique owner identifier\n");
> > + snprintf(my_owner.name, sizeof(my_owner.name),
> > TESTPMD_OWNER_NAME);
> > + RTE_ETH_FOREACH_DEV_OWNED_BY(port_id,
> > RTE_ETH_DEV_NO_OWNER)
> > + if (rte_eth_dev_owner_set(port_id, &my_owner) == 0)
> > + nb_ports++;
> > if (nb_ports == 0)
> > RTE_LOG(WARNING, EAL, "No probed ethernet devices\n");
>
> Yes.
>
> > But I thought about some code to assign a specific device to a
> > specific APP explicitly.
> > This code looks like just occupying the devices with no owner. So, it
> > means the first APP will occupy all the devices? It makes me confused
> > as I don't see the benefit or the difference than before.
>
> Remember that in EAL init some drivers may create ports and take
> ownership of them, so this code owns all the ownerless ports.
> So, ports which are already owned by another DPDK entity will not success to
> take ownership here.
> Since Testpmd wanted to manage all the ports before this feature( by
> mistake ), I guess this is the right behavior now, just use the ports which are
> not used.
Thanks for the explanation. Sounds good to me :)
>
> >
> > >
> > > Thanks, Matan.
@@ -1357,7 +1357,7 @@ struct cmd_config_speed_all {
&link_speed) < 0)
return;
- RTE_ETH_FOREACH_DEV(pid) {
+ RTE_ETH_FOREACH_DEV_OWNED_BY(pid, my_owner.id) {
ports[pid].dev_conf.link_speeds = link_speed;
}
@@ -1851,7 +1851,7 @@ struct cmd_config_rss {
struct cmd_config_rss *res = parsed_result;
struct rte_eth_rss_conf rss_conf = { .rss_key_len = 0, };
int diag;
- uint8_t i;
+ uint16_t pid;
if (!strcmp(res->value, "all"))
rss_conf.rss_hf = ETH_RSS_IP | ETH_RSS_TCP |
@@ -1885,12 +1885,12 @@ struct cmd_config_rss {
return;
}
rss_conf.rss_key = NULL;
- for (i = 0; i < rte_eth_dev_count(); i++) {
- diag = rte_eth_dev_rss_hash_update(i, &rss_conf);
+ RTE_ETH_FOREACH_DEV_OWNED_BY(pid, my_owner.id) {
+ diag = rte_eth_dev_rss_hash_update(pid, &rss_conf);
if (diag < 0)
printf("Configuration of RSS hash at ethernet port %d "
"failed with error (%d): %s.\n",
- i, -diag, strerror(-diag));
+ pid, -diag, strerror(-diag));
}
}
@@ -3681,10 +3681,8 @@ struct cmd_csum_result {
int hw = 0;
uint16_t mask = 0;
- if (port_id_is_invalid(res->port_id, ENABLED_WARN)) {
- printf("invalid port %d\n", res->port_id);
+ if (port_id_is_invalid(res->port_id, ENABLED_WARN))
return;
- }
if (!strcmp(res->mode, "set")) {
@@ -4282,8 +4280,8 @@ struct cmd_gso_show_result {
{
struct cmd_gso_show_result *res = parsed_result;
- if (!rte_eth_dev_is_valid_port(res->cmd_pid)) {
- printf("invalid port id %u\n", res->cmd_pid);
+ if (port_id_is_invalid(res->cmd_pid, ENABLED_WARN)) {
+ printf("invalid/not owned port id %u\n", res->cmd_pid);
return;
}
if (!strcmp(res->cmd_keyword, "gso")) {
@@ -5293,7 +5291,12 @@ static void cmd_create_bonded_device_parsed(void *parsed_result,
port_id);
/* Update number of ports */
- nb_ports = rte_eth_dev_count();
+ if (rte_eth_dev_owner_set(port_id, &my_owner) != 0) {
+ printf("Error: cannot own new attached port %d\n",
+ port_id);
+ return;
+ }
+ nb_ports++;
reconfig(port_id, res->socket);
rte_eth_promiscuous_enable(port_id);
}
@@ -5402,10 +5405,8 @@ static void cmd_set_bond_mon_period_parsed(void *parsed_result,
struct cmd_set_bond_mon_period_result *res = parsed_result;
int ret;
- if (res->port_num >= nb_ports) {
- printf("Port id %d must be less than %d\n", res->port_num, nb_ports);
+ if (port_id_is_invalid(res->port_num, ENABLED_WARN))
return;
- }
ret = rte_eth_bond_link_monitoring_set(res->port_num, res->period_ms);
@@ -5463,11 +5464,8 @@ struct cmd_set_bonding_agg_mode_policy_result {
struct cmd_set_bonding_agg_mode_policy_result *res = parsed_result;
uint8_t policy = AGG_BANDWIDTH;
- if (res->port_num >= nb_ports) {
- printf("Port id %d must be less than %d\n",
- res->port_num, nb_ports);
+ if (port_id_is_invalid(res->port_num, ENABLED_WARN))
return;
- }
if (!strcmp(res->policy, "bandwidth"))
policy = AGG_BANDWIDTH;
@@ -5726,7 +5724,7 @@ static void cmd_set_promisc_mode_parsed(void *parsed_result,
/* all ports */
if (allports) {
- RTE_ETH_FOREACH_DEV(i) {
+ RTE_ETH_FOREACH_DEV_OWNED_BY(i, my_owner.id) {
if (enable)
rte_eth_promiscuous_enable(i);
else
@@ -5806,7 +5804,7 @@ static void cmd_set_allmulti_mode_parsed(void *parsed_result,
/* all ports */
if (allports) {
- RTE_ETH_FOREACH_DEV(i) {
+ RTE_ETH_FOREACH_DEV_OWNED_BY(i, my_owner.id) {
if (enable)
rte_eth_allmulticast_enable(i);
else
@@ -6540,31 +6538,31 @@ static void cmd_showportall_parsed(void *parsed_result,
struct cmd_showportall_result *res = parsed_result;
if (!strcmp(res->show, "clear")) {
if (!strcmp(res->what, "stats"))
- RTE_ETH_FOREACH_DEV(i)
+ RTE_ETH_FOREACH_DEV_OWNED_BY(i, my_owner.id)
nic_stats_clear(i);
else if (!strcmp(res->what, "xstats"))
- RTE_ETH_FOREACH_DEV(i)
+ RTE_ETH_FOREACH_DEV_OWNED_BY(i, my_owner.id)
nic_xstats_clear(i);
} else if (!strcmp(res->what, "info"))
- RTE_ETH_FOREACH_DEV(i)
+ RTE_ETH_FOREACH_DEV_OWNED_BY(i, my_owner.id)
port_infos_display(i);
else if (!strcmp(res->what, "stats"))
- RTE_ETH_FOREACH_DEV(i)
+ RTE_ETH_FOREACH_DEV_OWNED_BY(i, my_owner.id)
nic_stats_display(i);
else if (!strcmp(res->what, "xstats"))
- RTE_ETH_FOREACH_DEV(i)
+ RTE_ETH_FOREACH_DEV_OWNED_BY(i, my_owner.id)
nic_xstats_display(i);
else if (!strcmp(res->what, "fdir"))
- RTE_ETH_FOREACH_DEV(i)
+ RTE_ETH_FOREACH_DEV_OWNED_BY(i, my_owner.id)
fdir_get_infos(i);
else if (!strcmp(res->what, "stat_qmap"))
- RTE_ETH_FOREACH_DEV(i)
+ RTE_ETH_FOREACH_DEV_OWNED_BY(i, my_owner.id)
nic_stats_mapping_display(i);
else if (!strcmp(res->what, "dcb_tc"))
- RTE_ETH_FOREACH_DEV(i)
+ RTE_ETH_FOREACH_DEV_OWNED_BY(i, my_owner.id)
port_dcb_info_display(i);
else if (!strcmp(res->what, "cap"))
- RTE_ETH_FOREACH_DEV(i)
+ RTE_ETH_FOREACH_DEV_OWNED_BY(i, my_owner.id)
port_offload_cap_display(i);
}
@@ -10484,10 +10482,8 @@ struct cmd_flow_director_mask_result {
struct rte_eth_fdir_masks *mask;
struct rte_port *port;
- if (res->port_id > nb_ports) {
- printf("Invalid port, range is [0, %d]\n", nb_ports - 1);
+ if (port_id_is_invalid(res->port_id, ENABLED_WARN))
return;
- }
port = &ports[res->port_id];
/** Check if the port is not started **/
@@ -10685,10 +10681,8 @@ struct cmd_flow_director_flex_mask_result {
uint16_t i;
int ret;
- if (res->port_id > nb_ports) {
- printf("Invalid port, range is [0, %d]\n", nb_ports - 1);
+ if (port_id_is_invalid(res->port_id, ENABLED_WARN))
return;
- }
port = &ports[res->port_id];
/** Check if the port is not started **/
@@ -10839,12 +10833,10 @@ struct cmd_flow_director_flexpayload_result {
struct cmd_flow_director_flexpayload_result *res = parsed_result;
struct rte_eth_flex_payload_cfg flex_cfg;
struct rte_port *port;
- int ret = 0;
+ int ret;
- if (res->port_id > nb_ports) {
- printf("Invalid port, range is [0, %d]\n", nb_ports - 1);
+ if (port_id_is_invalid(res->port_id, ENABLED_WARN))
return;
- }
port = &ports[res->port_id];
/** Check if the port is not started **/
@@ -11560,7 +11552,7 @@ struct cmd_config_l2_tunnel_eth_type_result {
entry.l2_tunnel_type = str2fdir_l2_tunnel_type(res->l2_tunnel_type);
entry.ether_type = res->eth_type_val;
- RTE_ETH_FOREACH_DEV(pid) {
+ RTE_ETH_FOREACH_DEV_OWNED_BY(pid, my_owner.id) {
rte_eth_dev_l2_tunnel_eth_type_conf(pid, &entry);
}
}
@@ -11676,7 +11668,7 @@ struct cmd_config_l2_tunnel_en_dis_result {
else
en = 0;
- RTE_ETH_FOREACH_DEV(pid) {
+ RTE_ETH_FOREACH_DEV_OWNED_BY(pid, my_owner.id) {
rte_eth_dev_l2_tunnel_offload_set(pid,
&entry,
ETH_L2_TUNNEL_ENABLE_MASK,
@@ -14203,10 +14195,8 @@ struct cmd_ddp_add_result {
int file_num;
int ret = -ENOTSUP;
- if (res->port_id > nb_ports) {
- printf("Invalid port, range is [0, %d]\n", nb_ports - 1);
+ if (port_id_is_invalid(res->port_id, ENABLED_WARN))
return;
- }
if (!all_ports_stopped()) {
printf("Please stop all ports first\n");
@@ -14285,10 +14275,8 @@ struct cmd_ddp_del_result {
uint32_t size;
int ret = -ENOTSUP;
- if (res->port_id > nb_ports) {
- printf("Invalid port, range is [0, %d]\n", nb_ports - 1);
+ if (port_id_is_invalid(res->port_id, ENABLED_WARN))
return;
- }
if (!all_ports_stopped()) {
printf("Please stop all ports first\n");
@@ -14600,10 +14588,8 @@ struct cmd_ddp_get_list_result {
#endif
int ret = -ENOTSUP;
- if (res->port_id > nb_ports) {
- printf("Invalid port, range is [0, %d]\n", nb_ports - 1);
+ if (port_id_is_invalid(res->port_id, ENABLED_WARN))
return;
- }
#ifdef RTE_LIBRTE_I40E_PMD
size = PROFILE_INFO_SIZE * MAX_PROFILE_NUM + 4;
@@ -15821,7 +15807,7 @@ struct cmd_cmdfile_result {
if (id == (portid_t)RTE_PORT_ALL) {
portid_t pid;
- RTE_ETH_FOREACH_DEV(pid) {
+ RTE_ETH_FOREACH_DEV_OWNED_BY(pid, my_owner.id) {
/* check if need_reconfig has been set to 1 */
if (ports[pid].need_reconfig == 0)
ports[pid].need_reconfig = dev;
@@ -2652,7 +2652,7 @@ static int comp_vc_action_rss_queue(struct context *, const struct token *,
(void)ctx;
(void)token;
- RTE_ETH_FOREACH_DEV(p) {
+ RTE_ETH_FOREACH_DEV_OWNED_BY(p, my_owner.id) {
if (buf && i == ent)
return snprintf(buf, size, "%u", p);
++i;
@@ -155,7 +155,7 @@ struct rss_type_info {
if (port_id_is_invalid(port_id, ENABLED_WARN)) {
printf("Valid port range is [0");
- RTE_ETH_FOREACH_DEV(pid)
+ RTE_ETH_FOREACH_DEV_OWNED_BY(pid, my_owner.id)
printf(", %d", pid);
printf("]\n");
return;
@@ -235,7 +235,7 @@ struct rss_type_info {
if (port_id_is_invalid(port_id, ENABLED_WARN)) {
printf("Valid port range is [0");
- RTE_ETH_FOREACH_DEV(pid)
+ RTE_ETH_FOREACH_DEV_OWNED_BY(pid, my_owner.id)
printf(", %d", pid);
printf("]\n");
return;
@@ -252,10 +252,9 @@ struct rss_type_info {
struct rte_eth_xstat_name *xstats_names;
printf("###### NIC extended statistics for port %-2d\n", port_id);
- if (!rte_eth_dev_is_valid_port(port_id)) {
- printf("Error: Invalid port number %i\n", port_id);
+
+ if (port_id_is_invalid(port_id, ENABLED_WARN))
return;
- }
/* Get count */
cnt_xstats = rte_eth_xstats_get_names(port_id, NULL, 0);
@@ -320,7 +319,7 @@ struct rss_type_info {
if (port_id_is_invalid(port_id, ENABLED_WARN)) {
printf("Valid port range is [0");
- RTE_ETH_FOREACH_DEV(pid)
+ RTE_ETH_FOREACH_DEV_OWNED_BY(pid, my_owner.id)
printf(", %d", pid);
printf("]\n");
return;
@@ -439,7 +438,7 @@ struct rss_type_info {
if (port_id_is_invalid(port_id, ENABLED_WARN)) {
printf("Valid port range is [0");
- RTE_ETH_FOREACH_DEV(pid)
+ RTE_ETH_FOREACH_DEV_OWNED_BY(pid, my_owner.id)
printf(", %d", pid);
printf("]\n");
return;
@@ -724,10 +723,15 @@ struct rss_type_info {
int
port_id_is_invalid(portid_t port_id, enum print_warning warning)
{
+ struct rte_eth_dev_owner owner;
+ int ret;
+
if (port_id == (portid_t)RTE_PORT_ALL)
return 0;
- if (rte_eth_dev_is_valid_port(port_id))
+ ret = rte_eth_dev_owner_get(port_id, &owner);
+
+ if (ret == 0 && owner.id == my_owner.id)
return 0;
if (warning == ENABLED_WARN)
@@ -2310,7 +2314,7 @@ struct igb_ring_desc_16_bytes {
return;
}
nb_pt = 0;
- RTE_ETH_FOREACH_DEV(i) {
+ RTE_ETH_FOREACH_DEV_OWNED_BY(i, my_owner.id) {
if (! ((uint64_t)(1ULL << i) & portmask))
continue;
portlist[nb_pt++] = i;
@@ -2449,10 +2453,9 @@ struct igb_ring_desc_16_bytes {
void
setup_gro(const char *onoff, portid_t port_id)
{
- if (!rte_eth_dev_is_valid_port(port_id)) {
- printf("invalid port id %u\n", port_id);
+ if (port_id_is_invalid(port_id, ENABLED_WARN))
return;
- }
+
if (test_done == 0) {
printf("Before enable/disable GRO,"
" please stop forwarding first\n");
@@ -2511,10 +2514,9 @@ struct igb_ring_desc_16_bytes {
param = &gro_ports[port_id].param;
- if (!rte_eth_dev_is_valid_port(port_id)) {
- printf("Invalid port id %u.\n", port_id);
+ if (port_id_is_invalid(port_id, ENABLED_WARN))
return;
- }
+
if (gro_ports[port_id].enable) {
printf("GRO type: TCP/IPv4\n");
if (gro_flush_cycles == GRO_DEFAULT_FLUSH_CYCLES) {
@@ -2532,10 +2534,9 @@ struct igb_ring_desc_16_bytes {
void
setup_gso(const char *mode, portid_t port_id)
{
- if (!rte_eth_dev_is_valid_port(port_id)) {
- printf("invalid port id %u\n", port_id);
+ if (port_id_is_invalid(port_id, ENABLED_WARN))
return;
- }
+
if (strcmp(mode, "on") == 0) {
if (test_done == 0) {
printf("before enabling GSO,"
@@ -428,7 +428,7 @@
if (port_id_is_invalid(port_id, ENABLED_WARN) ||
port_id == (portid_t)RTE_PORT_ALL) {
printf("Valid port range is [0");
- RTE_ETH_FOREACH_DEV(pid)
+ RTE_ETH_FOREACH_DEV_OWNED_BY(pid, my_owner.id)
printf(", %d", pid);
printf("]\n");
return -1;
@@ -489,7 +489,7 @@
if (port_id_is_invalid(port_id, ENABLED_WARN) ||
port_id == (portid_t)RTE_PORT_ALL) {
printf("Valid port range is [0");
- RTE_ETH_FOREACH_DEV(pid)
+ RTE_ETH_FOREACH_DEV_OWNED_BY(pid, my_owner.id)
printf(", %d", pid);
printf("]\n");
return -1;
@@ -136,6 +136,11 @@
lcoreid_t nb_lcores; /**< Number of probed logical cores. */
/*
+ * My port owner structure used to own Ethernet ports.
+ */
+struct rte_eth_dev_owner my_owner; /**< Unique owner. */
+
+/*
* Test Forwarding Configuration.
* nb_fwd_lcores <= nb_cfg_lcores <= nb_lcores
* nb_fwd_ports <= nb_cfg_ports <= nb_ports
@@ -483,7 +488,7 @@ static int eth_event_callback(portid_t port_id,
portid_t pt_id;
int i = 0;
- RTE_ETH_FOREACH_DEV(pt_id)
+ RTE_ETH_FOREACH_DEV_OWNED_BY(pt_id, my_owner.id)
fwd_ports_ids[i++] = pt_id;
nb_cfg_ports = nb_ports;
@@ -607,7 +612,7 @@ static int eth_event_callback(portid_t port_id,
fwd_lcores[lc_id]->cpuid_idx = lc_id;
}
- RTE_ETH_FOREACH_DEV(pid) {
+ RTE_ETH_FOREACH_DEV_OWNED_BY(pid, my_owner.id) {
port = &ports[pid];
rte_eth_dev_info_get(pid, &port->dev_info);
@@ -733,7 +738,7 @@ static int eth_event_callback(portid_t port_id,
queueid_t q;
/* set socket id according to numa or not */
- RTE_ETH_FOREACH_DEV(pid) {
+ RTE_ETH_FOREACH_DEV_OWNED_BY(pid, my_owner.id) {
port = &ports[pid];
if (nb_rxq > port->dev_info.max_rx_queues) {
printf("Fail: nb_rxq(%d) is greater than "
@@ -1027,9 +1032,8 @@ static int eth_event_callback(portid_t port_id,
uint64_t tics_per_1sec;
uint64_t tics_datum;
uint64_t tics_current;
- uint8_t idx_port, cnt_ports;
+ uint16_t idx_port;
- cnt_ports = rte_eth_dev_count();
tics_datum = rte_rdtsc();
tics_per_1sec = rte_get_timer_hz();
#endif
@@ -1044,11 +1048,10 @@ static int eth_event_callback(portid_t port_id,
tics_current = rte_rdtsc();
if (tics_current - tics_datum >= tics_per_1sec) {
/* Periodic bitrate calculation */
- for (idx_port = 0;
- idx_port < cnt_ports;
- idx_port++)
+ RTE_ETH_FOREACH_DEV_OWNED_BY(idx_port,
+ my_owner.id)
rte_stats_bitrate_calc(bitrate_data,
- idx_port);
+ idx_port);
tics_datum = tics_current;
}
}
@@ -1386,7 +1389,7 @@ static int eth_event_callback(portid_t port_id,
portid_t pi;
struct rte_port *port;
- RTE_ETH_FOREACH_DEV(pi) {
+ RTE_ETH_FOREACH_DEV_OWNED_BY(pi, my_owner.id) {
port = &ports[pi];
/* Check if there is a port which is not started */
if ((port->port_status != RTE_PORT_STARTED) &&
@@ -1404,7 +1407,7 @@ static int eth_event_callback(portid_t port_id,
portid_t pi;
struct rte_port *port;
- RTE_ETH_FOREACH_DEV(pi) {
+ RTE_ETH_FOREACH_DEV_OWNED_BY(pi, my_owner.id) {
port = &ports[pi];
if ((port->port_status != RTE_PORT_STOPPED) &&
(port->slave_flag == 0))
@@ -1453,7 +1456,7 @@ static int eth_event_callback(portid_t port_id,
if(dcb_config)
dcb_test = 1;
- RTE_ETH_FOREACH_DEV(pi) {
+ RTE_ETH_FOREACH_DEV_OWNED_BY(pi, my_owner.id) {
if (pid != pi && pid != (portid_t)RTE_PORT_ALL)
continue;
@@ -1634,7 +1637,7 @@ static int eth_event_callback(portid_t port_id,
printf("Stopping ports...\n");
- RTE_ETH_FOREACH_DEV(pi) {
+ RTE_ETH_FOREACH_DEV_OWNED_BY(pi, my_owner.id) {
if (pid != pi && pid != (portid_t)RTE_PORT_ALL)
continue;
@@ -1677,7 +1680,7 @@ static int eth_event_callback(portid_t port_id,
printf("Closing ports...\n");
- RTE_ETH_FOREACH_DEV(pi) {
+ RTE_ETH_FOREACH_DEV_OWNED_BY(pi, my_owner.id) {
if (pid != pi && pid != (portid_t)RTE_PORT_ALL)
continue;
@@ -1728,7 +1731,7 @@ static int eth_event_callback(portid_t port_id,
printf("Resetting ports...\n");
- RTE_ETH_FOREACH_DEV(pi) {
+ RTE_ETH_FOREACH_DEV_OWNED_BY(pi, my_owner.id) {
if (pid != pi && pid != (portid_t)RTE_PORT_ALL)
continue;
@@ -1773,6 +1776,12 @@ static int eth_event_callback(portid_t port_id,
if (rte_eth_dev_attach(identifier, &pi))
return;
+ if (rte_eth_dev_owner_set(pi, &my_owner) != 0) {
+ printf("Error: cannot own new attached port %d\n", pi);
+ return;
+ }
+ nb_ports++;
+
socket_id = (unsigned)rte_eth_dev_socket_id(pi);
/* if socket_id is invalid, set to 0 */
if (check_socket_id(socket_id) < 0)
@@ -1780,8 +1789,6 @@ static int eth_event_callback(portid_t port_id,
reconfig(pi, socket_id);
rte_eth_promiscuous_enable(pi);
- nb_ports = rte_eth_dev_count();
-
ports[pi].port_status = RTE_PORT_STOPPED;
printf("Port %d is attached. Now total ports is %d\n", pi, nb_ports);
@@ -1795,6 +1802,9 @@ static int eth_event_callback(portid_t port_id,
printf("Detaching a port...\n");
+ if (port_id_is_invalid(port_id, ENABLED_WARN))
+ return;
+
if (!port_is_closed(port_id)) {
printf("Please close port first\n");
return;
@@ -1808,7 +1818,7 @@ static int eth_event_callback(portid_t port_id,
return;
}
- nb_ports = rte_eth_dev_count();
+ nb_ports--;
printf("Port '%s' is detached. Now total ports is %d\n",
name, nb_ports);
@@ -1826,7 +1836,7 @@ static int eth_event_callback(portid_t port_id,
if (ports != NULL) {
no_link_check = 1;
- RTE_ETH_FOREACH_DEV(pt_id) {
+ RTE_ETH_FOREACH_DEV_OWNED_BY(pt_id, my_owner.id) {
printf("\nShutting down port %d...\n", pt_id);
fflush(stdout);
stop_port(pt_id);
@@ -1858,7 +1868,7 @@ struct pmd_test_command {
fflush(stdout);
for (count = 0; count <= MAX_CHECK_TIME; count++) {
all_ports_up = 1;
- RTE_ETH_FOREACH_DEV(portid) {
+ RTE_ETH_FOREACH_DEV_OWNED_BY(portid, my_owner.id) {
if ((port_mask & (1 << portid)) == 0)
continue;
memset(&link, 0, sizeof(link));
@@ -1948,6 +1958,8 @@ struct pmd_test_command {
switch (type) {
case RTE_ETH_EVENT_INTR_RMV:
+ if (port_id_is_invalid(port_id, ENABLED_WARN))
+ break;
if (rte_eal_alarm_set(100000,
rmv_event_callback, (void *)(intptr_t)port_id))
fprintf(stderr, "Could not set up deferred device removal\n");
@@ -2083,7 +2095,7 @@ struct pmd_test_command {
portid_t pid;
struct rte_port *port;
- RTE_ETH_FOREACH_DEV(pid) {
+ RTE_ETH_FOREACH_DEV_OWNED_BY(pid, my_owner.id) {
port = &ports[pid];
port->dev_conf.rxmode = rx_mode;
port->dev_conf.fdir_conf = fdir_conf;
@@ -2394,7 +2406,12 @@ uint8_t port_is_bonding_slave(portid_t slave_pid)
rte_pdump_init(NULL);
#endif
- nb_ports = (portid_t) rte_eth_dev_count();
+ if (rte_eth_dev_owner_new(&my_owner.id))
+ rte_panic("Failed to get unique owner identifier\n");
+ snprintf(my_owner.name, sizeof(my_owner.name), TESTPMD_OWNER_NAME);
+ RTE_ETH_FOREACH_DEV_OWNED_BY(port_id, RTE_ETH_DEV_NO_OWNER)
+ if (rte_eth_dev_owner_set(port_id, &my_owner) == 0)
+ nb_ports++;
if (nb_ports == 0)
RTE_LOG(WARNING, EAL, "No probed ethernet devices\n");
@@ -2442,7 +2459,7 @@ uint8_t port_is_bonding_slave(portid_t slave_pid)
rte_exit(EXIT_FAILURE, "Start ports failed\n");
/* set all ports to promiscuous mode by default */
- RTE_ETH_FOREACH_DEV(port_id)
+ RTE_ETH_FOREACH_DEV_OWNED_BY(port_id, my_owner.id)
rte_eth_promiscuous_enable(port_id);
/* Init metrics library */
@@ -79,6 +79,8 @@
#define NUMA_NO_CONFIG 0xFF
#define UMA_NO_CONFIG 0xFF
+#define TESTPMD_OWNER_NAME "TestPMD"
+
typedef uint8_t lcoreid_t;
typedef uint16_t portid_t;
typedef uint16_t queueid_t;
@@ -409,6 +411,7 @@ struct queue_stats_mappings {
* nb_fwd_ports <= nb_cfg_ports <= nb_ports
*/
extern portid_t nb_ports; /**< Number of ethernet ports probed at init time. */
+extern struct rte_eth_dev_owner my_owner; /**< Unique owner. */
extern portid_t nb_cfg_ports; /**< Number of configured ports. */
extern portid_t nb_fwd_ports; /**< Number of forwarding ports. */
extern portid_t fwd_ports_ids[RTE_MAX_ETHPORTS];