From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pa0-f44.google.com (mail-pa0-f44.google.com [209.85.220.44]) by dpdk.org (Postfix) with ESMTP id 5D02EB5D2 for ; Wed, 18 Feb 2015 02:55:25 +0100 (CET) Received: by padet14 with SMTP id et14so10725768pad.11 for ; Tue, 17 Feb 2015 17:55:24 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:message-id:date:from:user-agent:mime-version:to :cc:subject:references:in-reply-to:content-type :content-transfer-encoding; bh=O+0b6PtG1eRmeYyvbxPQlZoWe24CaYSDOQVXD9RnY3s=; b=lbhjv0zByyET4yZ31ojlIvKoI/AqVnyHk/i2vnbu0pg4Q6L+VzW2GurXb8sHlm5dDU Jqg9u6WXJJojR1lQbKrKV7lc8fTYGXudv5jmbkMzVaX1g0sV4EPgn9tlhXJZV+sA8TTn 51bN15kyOYJRUj/LrXNJm+7Vdb5PpI53ft3ZA87BoPrP6y0ew61xPwqUoYhhHznCuhRN J1zXhACq+t5fqqsxDRNGtW8Q4+hXuDckL1K7UMX2ggP9QtI3XUtcPqvai74C3A917yGn lt0XcDFDXVaqnzERG5o6js1uh/DOADQ5SUPhwUPXXddQVYHdf5IvHzFb+ifUAZpZXvug mutw== X-Gm-Message-State: ALoCoQn2HB4b+lKqa1uyih/3gqJosF9jVYGasSa//MD07RTSQYD8BQPDLihVVvLx12wskdtqHQby X-Received: by 10.70.54.69 with SMTP id h5mr52875256pdp.37.1424224524681; Tue, 17 Feb 2015 17:55:24 -0800 (PST) Received: from [10.16.129.101] (napt.igel.co.jp. [219.106.231.132]) by mx.google.com with ESMTPSA id s4sm18941046pdd.40.2015.02.17.17.55.23 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 17 Feb 2015 17:55:24 -0800 (PST) Message-ID: <54E3F10A.4090303@igel.co.jp> Date: Wed, 18 Feb 2015 10:55:22 +0900 From: Tetsuya Mukawa User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 MIME-Version: 1.0 To: Thomas Monjalon References: <1423470639-15744-2-git-send-email-mukawa@igel.co.jp> <4367974.mVXCMbsChG@xps13> <54E2DC61.9010804@igel.co.jp> <1684102.trQSmodf0H@xps13> In-Reply-To: <1684102.trQSmodf0H@xps13> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit 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:55:25 -0000 On 2015/02/18 10:02, Thomas Monjalon wrote: > 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. > Sure I will add it. Thanks, Tetsuya