From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f51.google.com (mail-wm0-f51.google.com [74.125.82.51]) by dpdk.org (Postfix) with ESMTP id 3B5B02BE2 for ; Thu, 21 Apr 2016 09:27:38 +0200 (CEST) Received: by mail-wm0-f51.google.com with SMTP id n3so117440723wmn.0 for ; Thu, 21 Apr 2016 00:27:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=Iiia9Yhk+4EIrczbXyi3rHF5fiBQa2wRdSpHS5aRln0=; b=rVfm3Od4SN3mFMDez0dcDh+u5LgJQEgaKYxnDQhl0MMn5m7aqIaX9QuXG+zoypDgng 1mboUy5TBRzGWDEceIM1xzsE4tEJDbGBvgkG1Dr7H7kR5E9C7w9mUksPZCDfiN/LbANy g2RQawaS/by1fM5iOiTG+qKAXCeqppaPCzNnt9kIzOYeX0c+9uzq1l+cZXaHVmBc0reh pDVDhLCPHm8oejnKoqjUJORWECKKTVNzB2orkOctotHb1VNfD2Ki0fo3qAR7oCmTFHRC +0JdpqXakvKXjI97B2xqVokhsQ9ABo7C2oPVt6QGjITmkgGO9fa3dipxXbWQbCtqPaED WvUg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=Iiia9Yhk+4EIrczbXyi3rHF5fiBQa2wRdSpHS5aRln0=; b=j92+t/K8SUZqmmTtfb3lXdjfyHN2Gx+MT6BctmlQtQhiujDfHRFgtMFDWh82ZUBBMe /3VgqQQZLUIfQtN2OgVzDlbAwXjlcv50zLYDN87wZ+CE9zPlbrkN+O6lQTYkB7GC9ryJ 3Gn7JeHjHc5yNIdg8j3j8xPfn9/Z3PB6bG3zoDFtykwmbACr5dnJiA/QcFqPfoqxf5r3 ET/xiE1Ez7MaWy30cS8j2QChyp/TIfVRU9qnJ2rErMsZ7jSR7HrHqzBiCt0hCtihofIf +jdikm3T6BlVPXtyYryYNa0/YBjSZGDkbT38RcCjcqK9npPP3dgWSI4SYTZm2BO01gR0 9xDQ== X-Gm-Message-State: AOPr4FUzMJfYhy5Fqrqv147MMkb7Cd9FahdS4Y7uKdtktwMwx4j1jBd3RFXwnzwbLNiaMv2xdLlkEL6B0suNImyg X-Received: by 10.28.140.12 with SMTP id o12mr33817394wmd.19.1461223658002; Thu, 21 Apr 2016 00:27:38 -0700 (PDT) MIME-Version: 1.0 Received: by 10.28.96.2 with HTTP; Thu, 21 Apr 2016 00:27:18 -0700 (PDT) In-Reply-To: <20160420181511.GB21072@hmsreliant.think-freely.org> References: <1453120248-28274-1-git-send-email-david.marchand@6wind.com> <1461156236-25349-1-git-send-email-david.marchand@6wind.com> <1461156236-25349-2-git-send-email-david.marchand@6wind.com> <20160420132908.GA21072@hmsreliant.think-freely.org> <20160420181511.GB21072@hmsreliant.think-freely.org> From: David Marchand Date: Thu, 21 Apr 2016 09:27:18 +0200 Message-ID: To: Neil Horman Cc: "dev@dpdk.org" , Thomas Monjalon , Stephen Hemminger , "Richardson, Bruce" , Panu Matilainen , Christian Ehrhardt , Wenzhuo Lu , "Zhang, Helin" Content-Type: text/plain; charset=UTF-8 Subject: Re: [dpdk-dev] [PATCH v3 01/13] e1000: move pci device ids to driver 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: Thu, 21 Apr 2016 07:27:38 -0000 On Wed, Apr 20, 2016 at 8:15 PM, Neil Horman wrote: > On Wed, Apr 20, 2016 at 03:39:59PM +0200, David Marchand wrote: >> On Wed, Apr 20, 2016 at 3:29 PM, Neil Horman wrote: >> >> +#ifndef RTE_PCI_DEV_ID_DECL_EM >> >> +#define RTE_PCI_DEV_ID_DECL_EM(vend, dev) >> >> +#endif >> >> + >> >> +#ifndef PCI_VENDOR_ID_INTEL >> >> +/** Vendor ID used by Intel devices */ >> >> +#define PCI_VENDOR_ID_INTEL 0x8086 >> >> +#endif >> >> + >> > This is broken, PCI_VENDOR_ID_INTEL should be defined in a central location for >> > all pci drivers, not redefined in every compilation unit. And you can likely >> >> Well we can keep the vendors in a common header, but I don't see the benefit. >> > ? > The fact that you won't have to do this > #ifndef PCI_VENDOR_ID_INTEL > #define PCI_VENDOR_ID_INTEL ... > #endif > in every pmd Ok, so you would keep the rte_pci_dev_ids.h with just the vendors in it ? Or, we could rely on linux kernel headers (pci_ids.h), rather than maintain a header in dpdk. But this would add a dependency build on dpdk and I am not sure there is something equivalent on freebsd (afaics all drivers seem to duplicate the pci vendor id). Any freebsd gourou reading this ? >> > just remvoe the RTE_PCI_DEV_ID_DECL_* macros from each patch and use the >> > RTE_PCI_DEV macro, as all the DECL macros just evaluate to that anyway. >> >> app/test/test_pci.c:#define RTE_PCI_DEV_ID_DECL_IGB(vend, dev) >> {RTE_PCI_DEVICE(vend, dev)}, >> lib/librte_eal/linuxapp/kni/kni_misc.c: #define >> RTE_PCI_DEV_ID_DECL_IGB(vend, dev) case (dev): >> >> All this stuff is because of pci tests and kni code. >> > You're going to have to explain a bit further than that. Why does the kni code > and pci testing code require that each driver have their own macro that just > maps to the same macro underneath? Looking at the test_pci code, it appears its > done because we used to contain all our pci device ids in a single file, and the > device specific macros were used to selectively enable devices that were there > for testing. But the point of this series is in part to separate out the device > ids to their own locations, so it seems like you will have to fix up the pci > tests anyway (and additionally it would be nice to include more than just EM, > IGB, and IXGBE in thsoe tests anyway, though I understand thats outside the > scope of this series) - test_pci.c should be about testing pci infrastructure. Relying on igb / ixgbe (or whatever pci device if we extend the list to all pci ids) being present on the system to run successfully is flawed but I have no better idea. - kni implements specific ethtool stuff based on pci ids. kni contains duplicated code from the kernel and it uses those ids to drive to the right ops. The solutions I have imagined so far : * use a shared header for the devices that it supports * drop the use of pci ids between kni library and kni kernel driver, instead use the pmd name that would be resolved internally by the kni library at RTE_KNI_IOCTL_CREATE time, and use this in the kni kernel driver * drop kni :-) I don't mind doing trivial changes, but I don't have time for more on this series. -- David Marchand