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

[Matan Azrad]

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