[dpdk-dev] [PATCH v4 09/10] net/mlx5: add parameter for port representors

[Shahaf Shuler]

Tuesday, July 10, 2018 1:58 PM, Adrien Mazarguil:
> Subject: Re: [PATCH v4 09/10] net/mlx5: add parameter for port
> representors
> 
> On Tue, Jul 10, 2018 at 10:16:03AM +0000, Shahaf Shuler wrote:
> > Tuesday, July 10, 2018 12:37 PM, Adrien Mazarguil:
> > > Subject: Re: [PATCH v4 09/10] net/mlx5: add parameter for port
> > > representors
> > >
> > > On Mon, Jul 09, 2018 at 11:57:37AM +0000, Shahaf Shuler wrote:
> > > > Thursday, July 5, 2018 11:46 AM, Adrien Mazarguil:
> > > > > Subject: [PATCH v4 09/10] net/mlx5: add parameter for port
> > > > > representors
> > > > >
> > > > > Prior to this patch, all port representors detected on a given
> > > > > device were probed and Ethernet devices instantiated for each of
> them.
> > > > >
> > > > > This patch adds support for the standard "representor"
> > > > > parameter, which implies that port representors are not probed
> > > > > by default anymore, except for the list provided through device
> arguments.
> > > > >
> > > > > (Patch based on prior work from Yuanhan Liu)
> > > > >
> > > > > Signed-off-by: Adrien Mazarguil <adrien.mazarguil at 6wind.com>
> > > > > Reviewed-by: Xueming Li <xuemingl at mellanox.com>
> > > > > --
> > > > > v3 changes:
> > > > >
> > > > > - Adapted representor detection to the reworked
> mlx5_dev_spawn().
> > > <snip>
> > > > > @@ -672,7 +679,9 @@ mlx5_uar_init_secondary(struct rte_eth_dev
> > > *dev)
> > > > >   *
> > > > >   * @return
> > > > >   *   A valid Ethernet device object on success, NULL otherwise and
> > > rte_errno
> > > > > - *   is set.
> > > > > + *   is set. The following error is defined:
> > > > > + *
> > > > > + *   EBUSY: device is not supposed to be spawned.
> > > > >   */
> > > > >  static struct rte_eth_dev *
> > > > >  mlx5_dev_spawn(struct rte_device *dpdk_dev, @@ -723,6 +732,26
> > > > > @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
> > > > >  	int own_domain_id = 0;
> > > > >  	unsigned int i;
> > > > >
> > > > > +	/* Determine if this port representor is supposed to be
> spawned. */
> > > > > +	if (switch_info->representor && dpdk_dev->devargs) {
> > > > > +		struct rte_eth_devargs eth_da;
> > > > > +
> > > > > +		err = rte_eth_devargs_parse(dpdk_dev->devargs-
> >args,
> > > > > &eth_da);
> > > > > +		if (err) {
> > > > > +			rte_errno = -err;
> > > > > +			DRV_LOG(ERR, "failed to process device
> arguments:
> > > > > %s",
> > > > > +				strerror(rte_errno));
> > > > > +			return NULL;
> > > > > +		}
> > > > > +		for (i = 0; i < eth_da.nb_representor_ports; ++i)
> > > > > +			if (eth_da.representor_ports[i] ==
> > > > > +			    (uint16_t)switch_info->port_name)
> > > > > +				break;
> > > > > +		if (i == eth_da.nb_representor_ports) {
> > > > > +			rte_errno = EBUSY;
> > > >
> > > > Why EBUSY is the correct errno? Will another attempts to probe the
> > > > device
> > > can be successful?
> > >
> > > That's the definition of EAGAIN :)
> > >
> > > I thought EBUSY in the sense of "don't disturb" would be
> > > appropriate. This value was also chosen because it is not likely to
> > > be returned by any intermediate function calls. I've defined EBUSY
> > > along with the return value of this function for clarity (see above). Any
> suggestion?
> >
> > How about ENODEV ?
> 
> Already used by many internal functions, typically returned if the associated
> netdevice doesn't exist (e.g. sent to another netns; a fatal error when
> probing representors).
> 
> We need a unique error code that says "OK, no problem, just not this one".

OK, we can keep the EBUSY. 

> 
> --
> Adrien Mazarguil
> 6WIND