From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ferruh Yigit Subject: Re: [PATCH] net/vdev_netvsc: fix erronous cast Date: Mon, 25 Mar 2019 13:51:42 +0000 Message-ID: <631bc9fd-5ed9-af31-1a52-7de9ab0205aa@intel.com> References: <20190313021253.525-1-stephen@networkplumber.org> <20190314084435.2cca9735@shemminger-XPS-13-9360> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: "dev@dpdk.org" To: Matan Azrad , Stephen Hemminger Return-path: Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by dpdk.org (Postfix) with ESMTP id C52E82BD3 for ; Mon, 25 Mar 2019 14:51:44 +0100 (CET) In-Reply-To: Content-Language: en-US List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 >>>> --- >>>> 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 Fixes: 56252de779a6 ("net/vdev_netvsc: add automatic probing") Cc: stable@dpdk.org Applied to dpdk-next-net/master, thanks.