From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by dpdk.org (Postfix) with ESMTP id AC05C1B430; Thu, 12 Jul 2018 13:58:32 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga107.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 12 Jul 2018 04:58:31 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.51,342,1526367600"; d="scan'208";a="66354374" Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by fmsmga002.fm.intel.com with ESMTP; 12 Jul 2018 04:57:57 -0700 Received: from fmsmsx157.amr.corp.intel.com (10.18.116.73) by FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 12 Jul 2018 04:57:56 -0700 Received: from shsmsx101.ccr.corp.intel.com (10.239.4.153) by FMSMSX157.amr.corp.intel.com (10.18.116.73) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 12 Jul 2018 04:57:56 -0700 Received: from shsmsx103.ccr.corp.intel.com ([169.254.4.100]) by SHSMSX101.ccr.corp.intel.com ([169.254.1.57]) with mapi id 14.03.0319.002; Thu, 12 Jul 2018 19:57:54 +0800 From: "Zhang, Qi Z" To: =?iso-8859-1?Q?Ga=EBtan_Rivet?= , "Burakov, Anatoly" CC: "thomas@monjalon.net" , "Ananyev, Konstantin" , "dev@dpdk.org" , "Richardson, Bruce" , "Yigit, Ferruh" , "Shelton, Benjamin H" , "Vangati, Narender" , "stable@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH v13 02/19] bus/pci: fix PCI address compare Thread-Index: AQHUGX3GFOrtg/RtIUaT9Tilrmm5/KSKyz4AgAACE4CAAK64cA== Date: Thu, 12 Jul 2018 11:57:53 +0000 Message-ID: <039ED4275CED7440929022BC67E70611532597D7@SHSMSX103.ccr.corp.intel.com> References: <20180607123849.14439-1-qi.z.zhang@intel.com> <20180712011514.45006-1-qi.z.zhang@intel.com> <20180712011514.45006-3-qi.z.zhang@intel.com> <20058c30-7587-7c23-d783-b7a8153592b8@intel.com> <20180712093209.x7r4lzazlxsvvcwu@bidouze.vm.6wind.com> In-Reply-To: <20180712093209.x7r4lzazlxsvvcwu@bidouze.vm.6wind.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNzBlODg3MzUtYmE4Ny00ZTQxLTlkYTMtMzY3MzJjY2NmOTgxIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiSlwvektlMFFiaVBSNFpiT093OUJvMDJLbFhyeE9UZEtoeXlaK2hQeVdzU0JkeTFUWXdYZEpiYTJPa3RSZktvUUMifQ== x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.200.100 dlp-reaction: no-action x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v13 02/19] bus/pci: fix PCI address compare X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 12 Jul 2018 11:58:33 -0000 > -----Original Message----- > From: Ga=EBtan Rivet [mailto:gaetan.rivet@6wind.com] > Sent: Thursday, July 12, 2018 5:32 PM > To: Burakov, Anatoly > Cc: Zhang, Qi Z ; thomas@monjalon.net; Ananyev, > Konstantin ; dev@dpdk.org; Richardson, > Bruce ; Yigit, Ferruh ; > Shelton, Benjamin H ; Vangati, Narender > ; stable@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v13 02/19] bus/pci: fix PCI address compar= e >=20 > Hi, >=20 > On Thu, Jul 12, 2018 at 10:24:44AM +0100, Burakov, Anatoly wrote: > > On 12-Jul-18 2:14 AM, Qi Zhang wrote: > > > When use memcmp to compare two PCI address, sizeof(struct > > > rte_pci_addr) is 4 bytes aligned, and it is 8. While only 7 byte of > > > struct rte_pci_addr is valid. So compare the 8th byte will cause the > > > unexpected result, which happens when repeatedly attach/detach a > device. > > > > > > Fixes: c752998b5e2e ("pci: introduce library and driver") >=20 > Shouldn't be the original commit be >=20 > Fixes: 94c0776b1bad ("vfio: support hotplug") >=20 > instead? You are right, this should be one that introduce the issue, thanks! >=20 > > > Cc: stable@dpdk.org > > > > > > Signed-off-by: Qi Zhang > > > --- > > > drivers/bus/pci/linux/pci_vfio.c | 13 ++++++++++++- > > > 1 file changed, 12 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/bus/pci/linux/pci_vfio.c > > > b/drivers/bus/pci/linux/pci_vfio.c > > > index aeeaa9ed8..dd25c3542 100644 > > > --- a/drivers/bus/pci/linux/pci_vfio.c > > > +++ b/drivers/bus/pci/linux/pci_vfio.c > > > @@ -43,6 +43,17 @@ static struct rte_tailq_elem rte_vfio_tailq =3D { > > > }; > > > EAL_REGISTER_TAILQ(rte_vfio_tailq) > > > +/* Compair two pci address */ > > > +static int pci_addr_cmp(struct rte_pci_addr *addr1, struct > > > +rte_pci_addr *addr2) { > > > + if (addr1->domain =3D=3D addr2->domain && > > > + addr1->bus =3D=3D addr2->bus && > > > + addr1->devid =3D=3D addr2->devid && > > > + addr1->function =3D=3D addr2->function) > > > + return 0; > > > + return 1; > > > +} > > > > Generally, change looks OK to me, but I think we already have this > > function in PCI library - rte_pci_addr_cmp(). Is there a specific > > reason to reimplement it here? > > >=20 > +1 >=20 > > > + > > > int > > > pci_vfio_read_config(const struct rte_intr_handle *intr_handle, > > > void *buf, size_t len, off_t offs) @@ -642,7 +653,7 @@ > > > pci_vfio_unmap_resource(struct rte_pci_device *dev) > > > vfio_res_list =3D RTE_TAILQ_CAST(rte_vfio_tailq.head, > mapped_pci_res_list); > > > /* Get vfio_res */ > > > TAILQ_FOREACH(vfio_res, vfio_res_list, next) { > > > - if (memcmp(&vfio_res->pci_addr, &dev->addr, sizeof(dev->addr))) > > > + if (pci_addr_cmp(&vfio_res->pci_addr, &dev->addr)) > > > continue; > > > break; > > > } > > > > > > > > > -- > > Thanks, > > Anatoly >=20 > -- > Ga=EBtan Rivet > 6WIND