Re: [PATCH] bus/vdev: automatically add eth alias for net drivers

[Bruce Richardson <bruce.richardson@intel.com>]

On Thu, Oct 20, 2022 at 10:23:27AM +0200, Thomas Monjalon wrote:
> 19/10/2022 15:20, Bruce Richardson:
> > On Wed, Oct 19, 2022 at 02:11:18PM +0100, Bruce Richardson wrote:
> > > For historical reasons, a number of net vdev drivers also add a driver
> > > alias using the "eth_" prefix. Since this is done on a per-driver basis,
> > > the use of the alias in inconsistent and is spread across multiple
> > > files. We can remove the per-driver aliases by just adding the alias
> > > automatically at the vdev bus level.
> > > 
> > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > > ---
> > >  drivers/bus/vdev/vdev.c | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > > 
> > > diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c
> > > index f5b43f1930..bfd7ce60c1 100644
> > > --- a/drivers/bus/vdev/vdev.c
> > > +++ b/drivers/bus/vdev/vdev.c
> > > @@ -54,6 +54,12 @@ static rte_spinlock_t vdev_custom_scan_lock = RTE_SPINLOCK_INITIALIZER;
> > >  void
> > >  rte_vdev_register(struct rte_vdev_driver *driver)
> > >  {
> > > +	/* For net driver vdevs, add an automatic alias using "eth" prefix */
> > > +	if (strncmp(driver->driver.name, "net_", 4) == 0 && driver->driver.alias == NULL) {
> > > +		char *alias = strdup(driver->driver.name);
> > > +		memcpy(alias, "eth_", 4);
> > > +		driver->driver.alias = alias;
> > > +	}
> > >  	TAILQ_INSERT_TAIL(&vdev_driver_list, driver, next);
> > >  }
> > >  
> > 
> > As a self-review comment, I realise this solution has got an issue that it
> > leaks memory if drivers are constantly being registered or unregistered. I
> > find it hard to see situations where this can occur, but it is a potential
> > issue.
> > 
> > A second solution that does not have this problem is to move the aliasing
> > to EAL, as below:
> 
> Honestly I think the status quo is OK:
> We have some aliases in some PMD for some historical reason
> and everybody looks OK with that. Isn't it?
> 

Well, the inconsistency bugs me a little, but if others feel the status quo
is ok, I'm ok with that.