[dpdk-dev] [PATCH v2 6/6] app/testpmd: adjust ethdev port ownership

[Gaëtan Rivet]

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.

-- 
Gaëtan Rivet
6WIND