From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-we0-f171.google.com (mail-we0-f171.google.com [74.125.82.171]) by dpdk.org (Postfix) with ESMTP id CA2FCB5D5 for ; Wed, 18 Feb 2015 02:02:57 +0100 (CET) Received: by wesq59 with SMTP id q59so1740620wes.1 for ; Tue, 17 Feb 2015 17:02:56 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:organization :user-agent:in-reply-to:references:mime-version :content-transfer-encoding:content-type; bh=ZP37pvO/ay1MOiavra6l96BpJdxGQmUzILBZhQXeZN0=; b=HVoT3dqaZbGq9WkOIvqrBMaIzkQ5rs96SkNlAg/DbozSTNcMFvJVkHYpb1iFtD/2QS dkGMUsAhi53sZ2vEilwZKCvjeBrNIQwazAwlNeW7eYtRHrVL9Fp2V0D8/7Ii12Yvsqdm r5UKUZ5pO9P/0NSDTenpy8P2MIFz2qVC5736q+fV6ojjcA/AhOgQvweFDqfWHbmljeEI F1pO6cna7gKW/DwYswIZgGfunlFBOaBOTPtxIPBiuq8ypDd3210mTi+TgH5xhNOnyKi/ pNxHcDtP7RvPmdO6Ztv/roGAgz9mYlFrS29NS41QUX+nlPTv5+LMcJ1n2q+Gg4yvawxq mBRQ== X-Gm-Message-State: ALoCoQlhbem2EkorJbmzU8OOit0DeMrIAvmZd2WdJ2upMPb8/hNmiIzoXo6NVgS7uFyEQV9VnBJp X-Received: by 10.180.198.110 with SMTP id jb14mr44719713wic.57.1424221376668; Tue, 17 Feb 2015 17:02:56 -0800 (PST) Received: from xps13.localnet (guy78-1-82-235-116-147.fbx.proxad.net. [82.235.116.147]) by mx.google.com with ESMTPSA id er13sm29796121wjc.11.2015.02.17.17.02.55 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 17 Feb 2015 17:02:55 -0800 (PST) From: Thomas Monjalon To: Tetsuya Mukawa Date: Wed, 18 Feb 2015 02:02:23 +0100 Message-ID: <1684102.trQSmodf0H@xps13> Organization: 6WIND User-Agent: KMail/4.14.4 (Linux/3.18.4-1-ARCH; KDE/4.14.4; x86_64; ; ) In-Reply-To: <54E2DC61.9010804@igel.co.jp> References: <1423470639-15744-2-git-send-email-mukawa@igel.co.jp> <4367974.mVXCMbsChG@xps13> <54E2DC61.9010804@igel.co.jp> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH v8 04/14] eal/pci: Consolidate pci address comparison APIs X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 18 Feb 2015 01:02:58 -0000 2015-02-17 15:14, Tetsuya Mukawa: > On 2015/02/17 9:44, Thomas Monjalon wrote: > > 2015-02-16 13:14, Tetsuya Mukawa: > >> @@ -356,13 +342,24 @@ pci_scan_one(int dev_pci_fd, struct pci_conf *conf) > >> } > >> else { > >> struct rte_pci_device *dev2 = NULL; > >> + int ret; > >> > >> TAILQ_FOREACH(dev2, &pci_device_list, next) { > >> - if (pci_addr_comparison(&dev->addr, &dev2->addr)) > >> + ret = eal_compare_pci_addr(&dev->addr, &dev2->addr); > >> + if (ret > 0) > >> continue; > >> - else { > >> + else if (ret < 0) { > >> TAILQ_INSERT_BEFORE(dev2, dev, next); > >> return 0; > >> + } else { /* already registered */ > >> + /* update pt_driver */ > >> + dev2->pt_driver = dev->pt_driver; > >> + dev2->max_vfs = dev->max_vfs; > >> + memmove(dev2->mem_resource, > >> + dev->mem_resource, > >> + sizeof(dev->mem_resource)); > >> + free(dev); > >> + return 0; > > Could you comment this "else part" please? > > PCI device list is allocated when rte_eal_init() is called. At the time, > to fill pci device information, sysfs value is used. > If sysfs values written by kernel device driver will not be changed by > igb_uio, vfio or pci_uio_genereic, above code isn't needed. > But actually above values are changed or added by them. > > Here is a step to cause issue. > 1. Boot linux. > 2. Start DPDK application without any physical NIC ports. > - Here, some sysfs values are read, and store to pci device list. > 3. igb_uio starts managing a port. > - Here, some sysfs values are changed. > 4. Add a NIC port to DPDK application using hotplug functions. > - Here, we need to replace some values. I think that you are showing that something is wrongly designed in these EAL structures. I suggest to try cleaning this mess instead of workarounding. [...] > >> - if (memcmp(&uio_res->pci_addr, &dev->addr, sizeof(dev->addr))) > >> + if (eal_compare_pci_addr(&uio_res->pci_addr, &dev->addr)) > > Why memcmp is not sufficient to compare PCI addresses? > > The only exception I see is endianness for natural sorting. > > Here is the definition. > > struct rte_pci_addr { > uint16_t domain; /**< Device domain */ > uint8_t bus; /**< Device bus */ > uint8_t devid; /**< Device ID */ > uint8_t function; /**< Device function. */ > }; > > But, sizeof(struct rte_pci_addr) will be 6. > If rte_pci_addr is allocated in a function without bzero, last 1 byte > may have some value. > As a result, memcmp may not work. To avoid such a case, I checked like > above. OK thanks. That's the kind of information which is valuable in a commit log.