Re: [dpdk-dev] [PATCH v2 2/4] ethdev: add siblings iterators

["Gaëtan Rivet" <gaetan.rivet@6wind.com>]

Hi Thomas,

On Wed, Feb 20, 2019 at 11:10:49PM +0100, Thomas Monjalon wrote:
> If multiple ports share the same hardware device (rte_device),
> they are siblings and can be found thanks to the new functions
> and loop macros.
> One iterator takes a port id as reference,
> while the other one directly refers to the parent device.
> 
> The ownership is not checked because siblings may have
> different owners.
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
>  lib/librte_ethdev/rte_ethdev.c           | 20 +++++++++++
>  lib/librte_ethdev/rte_ethdev.h           | 46 ++++++++++++++++++++++++
>  lib/librte_ethdev/rte_ethdev_version.map |  2 ++
>  3 files changed, 68 insertions(+)
> 
> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> index b3b2fb1dba..42154787f8 100644
> --- a/lib/librte_ethdev/rte_ethdev.c
> +++ b/lib/librte_ethdev/rte_ethdev.c
> @@ -340,6 +340,26 @@ rte_eth_find_next(uint16_t port_id)
>  	return port_id;
>  }
>  
> +uint16_t __rte_experimental
> +rte_eth_find_next_of(uint16_t port_id, const struct rte_device *parent)
> +{
> +	while (port_id < RTE_MAX_ETHPORTS &&
> +			rte_eth_devices[port_id].state == RTE_ETH_DEV_UNUSED &&
> +			rte_eth_devices[port_id].device != parent)
> +		port_id++;

Why not call rte_eth_find_next directly from this function, and
add your specific test on top of it?

Something like:

    	while (port_id < RTE_MAX_ETHPORTS &&
    	       rte_eth_devices[port_id].device != parent)
    		port_id = rte_eth_find_next(port_id + 1);

this way you won't have to rewrite the test on the device state. Having the
logic expressed in several places would make reworking the device states more
complicated than necessary if it were to happen (just as you did when switching
the test from !(ATTACHED || REMOVED) to (UNUSED).

-- 
Gaëtan Rivet
6WIND