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"