On 3/17/2019 6:16 AM, Matan Azrad wrote:
> 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 <step...@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.
Adding explicit cast from Matan:
Acked-by: Matan Azrad <ma...@mellanox.com>
Fixes: 56252de779a6 ("net/vdev_netvsc: add automatic probing")
Cc: sta...@dpdk.org
Applied to dpdk-next-net/master, thanks.
