On Thu, Nov 30, 2017 at 02:30:20PM +0000, Matan Azrad wrote: > Hi all > > > -----Original Message----- > > From: Gaëtan Rivet [mailto:gaetan.ri...@6wind.com] > > Sent: Thursday, November 30, 2017 3:25 PM > > To: Neil Horman <nhor...@tuxdriver.com> > > Cc: Matan Azrad <ma...@mellanox.com>; Thomas Monjalon > > <tho...@monjalon.net>; Jingjing Wu <jingjing...@intel.com>; > > dev@dpdk.org > > Subject: Re: [dpdk-dev] [PATCH 2/5] ethdev: add port ownership > > > > Hello Matan, Neil, > > > > I like the port ownership concept. I think it is needed to clarify some > > operations and should be useful to several subsystems. > > > > This patch could certainly be sub-divided however, and your current 1/5 > > should probably come after this one. > > Can you suggest how to divide it? >
Adding first the API to add / remove owners, then in a second patch set / get / unset. (by the way, remove / delete is pretty confusing, I'd suggest renaming those.) You can also separate the introduction of the new owner-wise iterator. Ultimately, you are the author, it's your job to help us review your work. > 1/5 could be actually outside of this series, it is just better behavior to > use the right function to do release port :) > > > > > Some comments inline. > > > > On Thu, Nov 30, 2017 at 07:36:11AM -0500, Neil Horman wrote: > > > On Tue, Nov 28, 2017 at 11:57:58AM +0000, Matan Azrad wrote: > > > > The ownership of a port is implicit in DPDK. > > > > Making it explicit is better from the next reasons: > > > > 1. It may be convenient for multi-process applications to know which > > > > process is in charge of a port. > > > > 2. A library could work on top of a port. > > > > 3. A port can work on top of another port. > > > > > > > > Also in the fail-safe case, an issue has been met in testpmd. > > > > We need to check that the user is not trying to use a port which is > > > > already managed by fail-safe. > > > > > > > > Add ownership mechanism to DPDK Ethernet devices to avoid multiple > > > > management of a device by different DPDK entities. > > > > > > > > A port owner is built from owner id(number) and owner name(string) > > > > while the owner id must be unique to distinguish between two > > > > identical entity instances and the owner name can be any name. > > > > The name helps to logically recognize the owner by different DPDK > > > > entities and allows easy debug. > > > > Each DPDK entity can allocate an owner unique identifier and can use > > > > it and its preferred name to owns valid ethdev ports. > > > > Each DPDK entity can get any port owner status to decide if it can > > > > manage the port or not. > > > > > > > > The current ethdev internal port management is not affected by this > > > > feature. > > > > > > > > The internal port management is not affected, but the external interface is, > > however. In order to respect port ownership, applications are forced to > > modify their port iterator, as shown by your testpmd patch. > > > > I think it would be better to modify the current RTE_ETH_FOREACH_DEV to > > call RTE_FOREACH_DEV_OWNED_BY, and introduce a default owner that > > would represent the application itself (probably with the ID 0 and an owner > > string ""). Only with specific additional configuration should this default > > subset of ethdev be divided. > > > > This would make this evolution seamless for applications, at no cost to the > > complexity of the design. > > As you can see in patch code and in testpmd example I added option to iterate > over all valid ownerless ports which should be more relevant by owner_id = 0. > So maybe the RTE_ETH_FOREACH_DEV should be changed to run this by the new > iterator. That is precisely what I am suggesting. Ideally, you should not have to change anything in testpmd, beside some bug fixing regarding port iteration to avoid those with a specific owner. RTE_ETH_FOREACH_DEV must stay valid, and should iterate over ownerless ports (read: port owned by the default owner). > By this way current applications don't need to build their owners but the ABI > will be broken. > ABI is broken anyway as you will add the owner to rte_eth_dev_data. > Actually, I think the old iterator RTE_ETH_FOREACH_DEV should be unexposed or > just removed, I don't think so. Using RTE_ETH_FOREACH_DEV should allow keeping the current behavior of iterating over ownerless ports. Applications that do not care for this API should not have to change anything to their code. > also the DEFFERED state should be removed, Of course. > I don't really see any usage to iterate over all valid ports by DPDK entities > different from ethdev itself. > I just don't want to break it now. > [snip] -- Gaëtan Rivet 6WIND
