From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id E40661B472; Thu, 12 Jul 2018 11:25:00 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga105.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 12 Jul 2018 02:25:00 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.51,342,1526367600"; d="scan'208";a="71729649" Received: from aburakov-mobl.ger.corp.intel.com (HELO [10.237.220.102]) ([10.237.220.102]) by fmsmga001.fm.intel.com with ESMTP; 12 Jul 2018 02:24:45 -0700 To: Qi Zhang , thomas@monjalon.net Cc: konstantin.ananyev@intel.com, dev@dpdk.org, bruce.richardson@intel.com, ferruh.yigit@intel.com, benjamin.h.shelton@intel.com, narender.vangati@intel.com, stable@dpdk.org 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> From: "Burakov, Anatoly" Message-ID: <20058c30-7587-7c23-d783-b7a8153592b8@intel.com> Date: Thu, 12 Jul 2018 10:24:44 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.9.0 MIME-Version: 1.0 In-Reply-To: <20180712011514.45006-3-qi.z.zhang@intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit 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 09:25:01 -0000 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") > 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 = { > }; > 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 == addr2->domain && > + addr1->bus == addr2->bus && > + addr1->devid == addr2->devid && > + addr1->function == 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? > + > 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 = 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