Re: [dpdk-dev] [PATCH] net/vdev_netvsc: fix erronous cast

[Matan Azrad <matan@mellanox.com>]

Hi

From: Stephen Hemminger
> > Hi
> >
> > From: Stephen Hemminger
> > > The return value from bus->find_device is a rte_device which is not
> > > safe to cast to a rte_vdev_device structure.
> > > It doesn't really matter since only being checked for NULL but
> > > static checkers might find a bug here.
> > >
> >
> > Fix line is missing.
> >
> > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > > ---
> > >  drivers/net/vdev_netvsc/vdev_netvsc.c | 7 ++++---
> > >  1 file changed, 4 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/net/vdev_netvsc/vdev_netvsc.c
> > > b/drivers/net/vdev_netvsc/vdev_netvsc.c
> > > index ba63fac2a598..801f54c96e01 100644
> > > --- a/drivers/net/vdev_netvsc/vdev_netvsc.c
> > > +++ b/drivers/net/vdev_netvsc/vdev_netvsc.c
> > > @@ -808,7 +808,7 @@ vdev_netvsc_cmp_rte_device(const struct
> > > rte_device *dev1,  static void
> > > vdev_netvsc_scan_callback(__rte_unused
> > > void *arg)  {
> > > -	struct rte_vdev_device *dev;
> > > +	struct rte_device *dev;
> > >  	struct rte_devargs *devargs;
> > >  	struct rte_bus *vbus = rte_bus_find_by_name("vdev");
> > >
> > > @@ -816,8 +816,9 @@ vdev_netvsc_scan_callback(__rte_unused void
> *arg)
> > >  		if (!strncmp(devargs->name, VDEV_NETVSC_DRIVER_NAME,
> > >  			     VDEV_NETVSC_DRIVER_NAME_LEN))
> > >  			return;
> > > -	dev = (struct rte_vdev_device *)vbus->find_device(NULL,
> > > -		vdev_netvsc_cmp_rte_device,
> > > VDEV_NETVSC_DRIVER_NAME);
> > > +
> > > +	dev = vbus->find_device(NULL, vdev_netvsc_cmp_rte_device,
> > > +				VDEV_NETVSC_DRIVER_NAME);
> >
> > Since the device must be vdev here,
> > It is better to use explicit cast to make the checker happy.
> 
> The cast is converting from rte_device to rte_vdev_device which incorrect.
> 
> The device returned by vbus->find_device is a rte_device not a
> rte_vdev_device:
> 
> vbus->find_device points to rte_vdev_find_device
> 
> struct rte_device *
> rte_vdev_find_device(const struct rte_device *start, rte_dev_cmp_t cmp,
> 		     const void *data)
> 
> but definition of rte_vdev_device is:
> 
> struct rte_vdev_device {
> 	TAILQ_ENTRY(rte_vdev_device) next;      /**< Next attached vdev */
> 	struct rte_device device;               /**< Inherit core device */
> };
> 
> The only safe way to get rte_vdev_device would be to use container_of().
> 
> 	dev = vbus->find_device(NULL, vdev_netvsc_cmp_rte_device,
> 				VDEV_NETVSC_DRIVER_NAME);
> 	vdev = dev ? container_of(dev, struct rte_vdev_device, device) :
> NULL;
> 
> But as the PATCH demonstrated, this is not necessary.

Yes, you right.
Please add fix line to the patch + stable line.


Matan