From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <dev-bounces@dpdk.org> Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id EB708A2EEB for <public@inbox.dpdk.org>; Sat, 14 Sep 2019 12:34:42 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 2E1501C2A0; Sat, 14 Sep 2019 12:34:41 +0200 (CEST) Received: from mail-io1-f66.google.com (mail-io1-f66.google.com [209.85.166.66]) by dpdk.org (Postfix) with ESMTP id C37D31C12D for <dev@dpdk.org>; Sat, 14 Sep 2019 12:34:38 +0200 (CEST) Received: by mail-io1-f66.google.com with SMTP id h144so68131210iof.7 for <dev@dpdk.org>; Sat, 14 Sep 2019 03:34:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=TbXweADyvLLp06JqG9o2eR6Vz18IjtD+2ILOet1hNx0=; b=Nblfw481KM00vhaz3u0ZwYo714hdzmD6jciluA44oJzRIuuc2QE8pYsdTk8qO/6suP Teb6xHEqhNdDKhasJqOQfAiBnISWr6e4V5xCSSexlmUeXh+CTXv3Vff32N1gyFTdHJHC XCSzFHHGZd4ko//OVyV1rO/LKlwN6QKcLJ+S4DtY+xRNqJmcqqMNaNUuvBVCWSUOZ157 fSwOREKQFEggvrfqB2nRDCTIqVapBHbHl+nVp2RJjzzkCJT7vWmx9jhgAOj8DjQYcUIe FarwQSptHOAHEfU5lrG8dDxX/6Ro49X6hAn5qi5AmlNhRkp3MULHrh6q1tcsMkhC65om oavg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=TbXweADyvLLp06JqG9o2eR6Vz18IjtD+2ILOet1hNx0=; b=E4lE5MK8Zcl83TQH/oOFsklfRM1/Ln1GldA7Kn7EPJHPC83WUVXvW2goFh+BxkEtuD g9Bi24wWky4tdwS9U7/N8bafk0HIzqYfhlIZTimXN+rEqzr9960n67Xljtn5plBv6D35 G/q+PLvvrfBBhM1jVzcNLJgPz8zjlamZz/EIRvUC+9Qdw4PsgPpxLKKai8ddW2whxLAd 6dPgRhKyrwFxODhDqkVMj16Cxn1C8E37qoGeUZA5wR7s37XIrtChVhSXhI6WFzGHDOMy FkLQaMCmrZXKDdWX1UagH57VZ0RWewlHqcY0HPSL2xh5ac85lMurig3ONTk188RVvAKM Iasw== X-Gm-Message-State: APjAAAW0qsPiIBhbujXdtcwi/G4I229TES/GDAKmjwdNJDbQi/KoViiB GZEGY4/tC76XpJ9KB4WlqkcGSuMLgBU9KscvWVYVrco8hHMAXg== X-Google-Smtp-Source: APXvYqzWiCWfZyA73uErGtM1L98rRw10E2pY/+e8dmvArBjAz7MMjIsNNH79d/TtF2aWFtgvZFZicNyq4nCzsQVB4pQ= X-Received: by 2002:a5d:9714:: with SMTP id h20mr3515876iol.294.1568457277910; Sat, 14 Sep 2019 03:34:37 -0700 (PDT) MIME-Version: 1.0 References: <20190730124950.1293-1-marcinx.a.zapolski@intel.com> <20190906131813.1343-2-marcinx.a.zapolski@intel.com> <693a2150-3c05-c2ad-bb67-f3a510f10f7e@intel.com> <51FEE37A1339864DB0A4E34597F561E30D5D50FD@HASMSX113.ger.corp.intel.com> <a301e121-7fe5-9d55-51e5-f0051d444e22@intel.com> <51FEE37A1339864DB0A4E34597F561E30D5D6AB2@HASMSX113.ger.corp.intel.com> <3a8e2412-12a0-7f18-1380-a50e5807373c@intel.com> In-Reply-To: <3a8e2412-12a0-7f18-1380-a50e5807373c@intel.com> From: Jerin Jacob <jerinjacobk@gmail.com> Date: Sat, 14 Sep 2019 16:04:26 +0530 Message-ID: <CALBAE1OFLgpJaXb43X0zPChY1MYBpUnBf=WV092d+6_z+rm4LA@mail.gmail.com> To: Ferruh Yigit <ferruh.yigit@intel.com> Cc: "Zapolski, MarcinX A" <marcinx.a.zapolski@intel.com>, "dev@dpdk.org" <dev@dpdk.org>, Jerin Jacob <jerin.jacob@caviumnetworks.com>, "Laatz, Kevin" <kevin.laatz@intel.com> Content-Type: text/plain; charset="UTF-8" Subject: Re: [dpdk-dev] [RFC 19.11 v2 1/3] ethdev: hide key ethdev structures from public API X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions <dev.dpdk.org> List-Unsubscribe: <https://mails.dpdk.org/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://mails.dpdk.org/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <https://mails.dpdk.org/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> On Mon, Sep 9, 2019 at 3:54 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote: > > On 9/9/2019 11:02 AM, Zapolski, MarcinX A wrote: > >> -----Original Message----- > >> From: Yigit, Ferruh > >> Sent: Monday, September 9, 2019 12:00 PM > >> To: Zapolski, MarcinX A <marcinx.a.zapolski@intel.com>; dev@dpdk.org > >> Subject: Re: [dpdk-dev] [RFC 19.11 v2 1/3] ethdev: hide key ethdev > >> structures from public API > >> > >> On 9/9/2019 9:07 AM, Zapolski, MarcinX A wrote: > >>>> -----Original Message----- > >>>> From: Yigit, Ferruh > >>>> Sent: Friday, September 6, 2019 4:38 PM > >>>> To: Zapolski, MarcinX A <marcinx.a.zapolski@intel.com>; dev@dpdk.org > >>>> Subject: Re: [dpdk-dev] [RFC 19.11 v2 1/3] ethdev: hide key ethdev > >>>> structures from public API > >>>> > >>>> On 9/6/2019 2:18 PM, Marcin Zapolski wrote: > >>>>> Split rte_eth_dev structure to two parts: head that is available for > >>>>> user applications, and rest which is DPDK internal. > >>>>> Make an array of pointers to rte_eth_dev structures available for > >>>>> user applications. > >>>>> > >>>>> Signed-off-by: Marcin Zapolski <marcinx.a.zapolski@intel.com> > >>>> > >>>> <...> > >>>> > >>>>> diff --git a/lib/librte_bitratestats/rte_bitrate.c > >>>>> b/lib/librte_bitratestats/rte_bitrate.c > >>>>> index 639e47547..82d469514 100644 > >>>>> --- a/lib/librte_bitratestats/rte_bitrate.c > >>>>> +++ b/lib/librte_bitratestats/rte_bitrate.c > >>>>> @@ -3,7 +3,7 @@ > >>>>> */ > >>>>> > >>>>> #include <rte_common.h> > >>>>> -#include <rte_ethdev.h> > >>>>> +#include <rte_ethdev_driver.h> > >>>> > >>>> This is in the library, not sure if libraries should include the > >>>> header file for the drivers, can you please explain why this change is > >> needed? > >>>> > >>> It is needed to make rte_eth_dev structure available. But yes, I agree that > >> it will be more appropriate to include rte_ethdev.h and rte_ethdev_core.h > >> separately. I probably wanted less includes, silly me. > >>>> <...> > >>>> > >>>>> @@ -6,6 +6,7 @@ > >>>>> #define _RTE_ETHDEV_PROFILE_H_ > >>>>> > >>>>> #include "rte_ethdev.h" > >>>>> +#include "rte_ethdev_core.h" > >>>>> > >>>>> /** > >>>>> * Initialization of the Ethernet device profiling. > >>>>> diff --git a/lib/librte_ethdev/rte_ethdev.c > >>>>> b/lib/librte_ethdev/rte_ethdev.c index 17d183e1f..5c6cc640a 100644 > >>>>> --- a/lib/librte_ethdev/rte_ethdev.c > >>>>> +++ b/lib/librte_ethdev/rte_ethdev.c > >>>>> @@ -40,6 +40,7 @@ > >>>>> > >>>>> #include "rte_ether.h" > >>>>> #include "rte_ethdev.h" > >>>>> +#include "rte_ethdev_core.h" > >>>>> #include "rte_ethdev_driver.h" > >>>>> #include "ethdev_profile.h" > >>>>> #include "ethdev_private.h" > >>>> > >>>> I was hoping "rte_ethdev_core.h" can be removed completely by > >>>> distributing its content to "ethdev_private.h", "rte_ethdev_driver.h" > >>>> and perhaps even to "rte_ethdev.h". > >>>> > >>>> Can you please explain what prevents removing "rte_ethdev_core.h"? > >>> I could rename it to rte_ethdev_private. There is just rte_eth_dev and > >> rte_eth_dev_data left in it. > >>> > >> > >> I think drivers access to both 'rte_eth_dev' and 'rte_eth_dev_data' so can't > >> move them to 'ethdev_private.h' why not move it to 'rte_ethdev_driver.h'? > > > > Because the libraries use them as well. > > > > These are 'librte_ethdev' library internal data, I think other libraries > shouldn't access them directly. > > As far as I can see, > librte_eventdev => rte_eth_dev_data > librte_eventdev => rte_eth_dev > librte_telemetry => rte_eth_dev > > Can you see any other library accessing 'rte_eth_dev' and 'rte_eth_dev_data' ? > > > I am not sure about 'eventdev', specially because of the Rx/Tx adapters of it, > perhaps they can include the "rte_ethdev_driver.h", cc'ed Jerin. Yes. Overall this patch theme looks good to me. I will wait for v3 to change PMDs I maintains. My only comment is, We should make rte_ethdev_driver.h as private i.e it should not be included by applications. Currently. On dpdk install, the private header file is copied to prefix ie make install T=x86_64-native-linux-gcc DESTDIR=install [master][dpdk.org] $ ls install/include/dpdk/rte_ethdev_driver.h install/include/dpdk/rte_ethdev_driver.h The internal libraries can access the header file through the following scheme without exposing it as public. CFLAGS += -I$(RTE_SDK)/lib/librte_ethdev/ in .c file #include "ethdev_driver.h"