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 dpdk.space (Postfix) with ESMTP id 0D6E8A0096
	for <public@inbox.dpdk.org>; Thu,  6 Jun 2019 16:02:15 +0200 (CEST)
Received: from [92.243.14.124] (localhost [127.0.0.1])
	by dpdk.org (Postfix) with ESMTP id 90CC81B95A;
	Thu,  6 Jun 2019 16:02:14 +0200 (CEST)
Received: from mx0b-0016f401.pphosted.com (mx0b-0016f401.pphosted.com
 [67.231.156.173]) by dpdk.org (Postfix) with ESMTP id DF4551B959
 for <dev@dpdk.org>; Thu,  6 Jun 2019 16:02:12 +0200 (CEST)
Received: from pps.filterd (m0045851.ppops.net [127.0.0.1])
 by mx0b-0016f401.pphosted.com (8.16.0.27/8.16.0.27) with SMTP id
 x56DtFXV025026; Thu, 6 Jun 2019 07:02:12 -0700
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=marvell.com;
 h=from : to : cc :
 subject : date : message-id : references : in-reply-to : content-type :
 content-transfer-encoding : mime-version; s=pfpt0818;
 bh=LBnFB30OC37sXMFoEeni7MEL5DJ4ttSCorBSCeN2He4=;
 b=sU7sSeM/MbJWuIlaATqRddjSH5rPzowVsd3HXqAXa0obwze2Jh4VpUDiNiAoBTiAnH0k
 sGby+GzLt+UT7aOLaCJf4oVE32XiH7ziPKUfOhym/Fza+r2rS1bDIVAnG1BNLcUr0IJI
 QZFYTuxQBMnl5FLALxurgQcEvReneM9vUZo6fRMwFQE03ZxHakkZKeHoS6+DecsI65iV
 VWU4pfWPUFoVwdNnRVwe1I2/E5tIWXhSVYZB2uWy+tD6wW5e7NmI+wA6UUw9xOdkxegE
 +mobNPd4qusnoJxwrPzyWm0giQ7oVCG/kHlTHQnhyTGPWkwNl8VwfNOTE7XWQesX2mmD JA== 
Received: from sc-exch02.marvell.com ([199.233.58.182])
 by mx0b-0016f401.pphosted.com with ESMTP id 2sxthej3gb-2
 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-SHA384 bits=256 verify=NOT);
 Thu, 06 Jun 2019 07:02:11 -0700
Received: from SC-EXCH02.marvell.com (10.93.176.82) by SC-EXCH02.marvell.com
 (10.93.176.82) with Microsoft SMTP Server (TLS) id 15.0.1367.3; Thu, 6 Jun
 2019 07:02:09 -0700
Received: from NAM02-SN1-obe.outbound.protection.outlook.com (104.47.36.59) by
 SC-EXCH02.marvell.com (10.93.176.82) with Microsoft SMTP Server
 (TLS) id
 15.0.1367.3 via Frontend Transport; Thu, 6 Jun 2019 07:02:09 -0700
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=marvell.onmicrosoft.com; s=selector2-marvell-onmicrosoft-com;
 h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck;
 bh=LBnFB30OC37sXMFoEeni7MEL5DJ4ttSCorBSCeN2He4=;
 b=P0nDsN4Zn7il8Vrnv5rOAY+QZhML1wkQVPUeDzerxR4W1eJD+inSUFo/6yAb+LR53z20BVoRNO126Ecgv7hIj6O72AKEIOKA50AhUGwUkafFtkwg3Y86I9n+jwY+PU5jSH0TBCarjkRcQ34jMbzgtSu38+0O1vbT5LmzI9ovJ0A=
Received: from BYAPR18MB2424.namprd18.prod.outlook.com (20.179.91.149) by
 BYAPR18MB2581.namprd18.prod.outlook.com (20.179.93.210) with Microsoft SMTP
 Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id
 15.20.1965.14; Thu, 6 Jun 2019 14:02:03 +0000
Received: from BYAPR18MB2424.namprd18.prod.outlook.com
 ([fe80::1ce4:557d:eeb8:843c]) by BYAPR18MB2424.namprd18.prod.outlook.com
 ([fe80::1ce4:557d:eeb8:843c%7]) with mapi id 15.20.1965.011; Thu, 6 Jun 2019
 14:02:03 +0000
From: Jerin Jacob Kollanukkaran <jerinj@marvell.com>
To: Neil Horman <nhorman@tuxdriver.com>
CC: Bruce Richardson <bruce.richardson@intel.com>, "dev@dpdk.org"
 <dev@dpdk.org>, Thomas Monjalon <thomas@monjalon.net>
Thread-Topic: [EXT] [RFC PATCH 0/2] introduce __rte_internal tag
Thread-Index: AQHVEynnIvy0R3R0lkaWrQuIVgPnUKaNTWtQgAAIeYCAABfgAIABAvyAgAAgfgCAAAaRkIAAGyeAgAAFlPA=
Date: Thu, 6 Jun 2019 14:02:03 +0000
Message-ID: <BYAPR18MB2424C6DED1CC2A05377D342EC8170@BYAPR18MB2424.namprd18.prod.outlook.com>
References: <20190525184346.27932-1-nhorman@tuxdriver.com>
 <BYAPR18MB24240660A3CDEB3E0B0347AAC8160@BYAPR18MB2424.namprd18.prod.outlook.com>
 <20190605164541.GH1550@bricha3-MOBL.ger.corp.intel.com>
 <20190605181108.GC554@hmswarspite.think-freely.org>
 <BYAPR18MB24240EF19CFA1ADFE6B52AD1C8170@BYAPR18MB2424.namprd18.prod.outlook.com>
 <20190606113422.GA29521@hmswarspite.think-freely.org>
 <BYAPR18MB242407B0A2A21E187843917FC8170@BYAPR18MB2424.namprd18.prod.outlook.com>
 <20190606133503.GB29521@hmswarspite.think-freely.org>
In-Reply-To: <20190606133503.GB29521@hmswarspite.think-freely.org>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach: 
X-MS-TNEF-Correlator: 
x-originating-ip: [122.178.234.223]
x-ms-publictraffictype: Email
x-ms-office365-filtering-correlation-id: 0779ada5-298f-48eb-f0be-08d6ea878d7f
x-microsoft-antispam: BCL:0; PCL:0;
 RULEID:(2390118)(7020095)(4652040)(8989299)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(5600148)(711020)(4605104)(1401327)(2017052603328)(7193020);
 SRVR:BYAPR18MB2581; 
x-ms-traffictypediagnostic: BYAPR18MB2581:
x-ms-exchange-purlcount: 1
x-microsoft-antispam-prvs: <BYAPR18MB258100488FB75BFB91004BEFC8170@BYAPR18MB2581.namprd18.prod.outlook.com>
x-ms-oob-tlc-oobclassifiers: OLM:8882;
x-forefront-prvs: 00603B7EEF
x-forefront-antispam-report: SFV:NSPM;
 SFS:(10009020)(136003)(396003)(376002)(39860400002)(346002)(366004)(199004)(189003)(174874002)(13464003)(43544003)(68736007)(11346002)(73956011)(66946007)(26005)(86362001)(64756008)(76116006)(66446008)(66556008)(66476007)(76176011)(74316002)(55016002)(446003)(25786009)(478600001)(316002)(66066001)(186003)(6116002)(8936002)(3846002)(52536014)(5660300002)(33656002)(966005)(71200400001)(71190400001)(99286004)(9686003)(53936002)(81166006)(81156014)(7696005)(14444005)(256004)(53546011)(14454004)(6916009)(486006)(8676002)(305945005)(4326008)(54906003)(6506007)(6246003)(6306002)(229853002)(6436002)(102836004)(476003)(2906002)(561944003)(7736002);
 DIR:OUT; SFP:1101; SCL:1; SRVR:BYAPR18MB2581;
 H:BYAPR18MB2424.namprd18.prod.outlook.com; FPR:; SPF:None; LANG:en;
 PTR:InfoNoRecords; A:1; MX:1; 
received-spf: None (protection.outlook.com: marvell.com does not designate
 permitted sender hosts)
x-ms-exchange-senderadcheck: 1
x-microsoft-antispam-message-info: w6GO52Ltu9aUvjJGHZDAYgu6okXFCkR7rHdgrCVm1ONuQ+IKqgxxzKeGXyjra0QmdV7gIaMkdtoB1ybO64SlYf9gGjmzRrNUDbuz2Lufsp/BPmUxjnMbFTUGzAUUf+9HbGa1bXs3/YPBUN/+dkwTVw5tdcBC7F/T7A7Duan/4/mBw24njtpw6NpSLdwzWy27LC9vfn+Os4WYF6/qefYmLjfOtz9oRaW2yp63LoY+xKVpA1MldrmInrkV0hMN/jO/UDAXNNS+7Mn9qkzYrVPXI7GYQ/XfjvYxZiL4gUprE6UOxwAHqxN5obbH3oQHjXOoAfF3Y+Qgt/FPUL1ZbTnkE+WzfB/xyjGH8d7WBz8QQ1oVyEXAJqiw//eLtE67tHJxCNXpMrWqHlAUKa1bhI4gTmG/ts0NzNXA6ZwcXdFDK4U=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 0779ada5-298f-48eb-f0be-08d6ea878d7f
X-MS-Exchange-CrossTenant-originalarrivaltime: 06 Jun 2019 14:02:03.2891 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 70e1fb47-1155-421d-87fc-2e58f638b6e0
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: jerinj@marvell.com
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR18MB2581
X-OriginatorOrg: marvell.com
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:, ,
 definitions=2019-06-06_10:, , signatures=0
Subject: Re: [dpdk-dev] [EXT] [RFC PATCH 0/2] introduce __rte_internal tag
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>

> -----Original Message-----
> From: Neil Horman <nhorman@tuxdriver.com>
> Sent: Thursday, June 6, 2019 7:05 PM
> To: Jerin Jacob Kollanukkaran <jerinj@marvell.com>
> Cc: Bruce Richardson <bruce.richardson@intel.com>; dev@dpdk.org;
> Thomas Monjalon <thomas@monjalon.net>
> Subject: Re: [EXT] [RFC PATCH 0/2] introduce __rte_internal tag
>=20
> On Thu, Jun 06, 2019 at 12:04:57PM +0000, Jerin Jacob Kollanukkaran wrote=
:
> > > -----Original Message-----
> > > From: Neil Horman <nhorman@tuxdriver.com>
> > > Sent: Thursday, June 6, 2019 5:04 PM
> > > To: Jerin Jacob Kollanukkaran <jerinj@marvell.com>
> > > Cc: Bruce Richardson <bruce.richardson@intel.com>; dev@dpdk.org;
> > > Thomas Monjalon <thomas@monjalon.net>
> > > Subject: Re: [EXT] [RFC PATCH 0/2] introduce __rte_internal tag
> > >
> > > On Thu, Jun 06, 2019 at 09:44:52AM +0000, Jerin Jacob Kollanukkaran
> wrote:
> > > > > -----Original Message-----
> > > > > From: Neil Horman <nhorman@tuxdriver.com>
> > > > > Sent: Wednesday, June 5, 2019 11:41 PM
> > > > > To: Bruce Richardson <bruce.richardson@intel.com>
> > > > > Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
> > > > > dev@dpdk.org; Thomas Monjalon <thomas@monjalon.net>
> > > > > Subject: Re: [EXT] [RFC PATCH 0/2] introduce __rte_internal tag
> > > > >
> > > > > On Wed, Jun 05, 2019 at 05:45:41PM +0100, Bruce Richardson wrote:
> > > > > > On Wed, Jun 05, 2019 at 04:24:09PM +0000, Jerin Jacob
> > > > > > Kollanukkaran
> > > > > wrote:
> > > > > > > > -----Original Message-----
> > > > > > > > From: Neil Horman <nhorman@tuxdriver.com>
> > > > > > > > Sent: Sunday, May 26, 2019 12:14 AM
> > > > > > > > To: dev@dpdk.org
> > > > > > > > Cc: Neil Horman <nhorman@tuxdriver.com>; Jerin Jacob
> > > > > > > > Kollanukkaran <jerinj@marvell.com>; Bruce Richardson
> > > > > > > > <bruce.richardson@intel.com>; Thomas Monjalon
> > > > > > > > <thomas@monjalon.net>
> > > > > > > > Subject: [EXT] [RFC PATCH 0/2] introduce __rte_internal
> > > > > > > > tag
> > > > > > > >
> > > > > > > > Hey-
> > > > > > > > 	Based on our recent conversations regarding the use of
> > > > > > > > symbols only meant for internal dpdk consumption (between
> > > > > > > > dpdk libraries), this is an idea that I've come up with
> > > > > > > > that I'd like to get some feedback on
> > > > > > > >
> > > > > > > > Summary:
> > > > > > > > 1) We have symbols in the DPDK that are meant to be used
> > > > > > > > between DPDK libraries, but not by applications linking to
> > > > > > > > them
> > > > > > > > 2) We would like to document those symbols in the code, so
> > > > > > > > as to note them clearly as for being meant for internal
> > > > > > > > use only
> > > > > > > > 3) Linker symbol visibility is a very coarse grained tool,
> > > > > > > > and so there is no good way in a single library to mark
> > > > > > > > items as being meant for use only by other DPDK libraries,
> > > > > > > > at least not without some extensive runtime checking
> > > > > > > >
> > > > > > > >
> > > > > > > > Proposal:
> > > > > > > > I'm proposing that we introduce the __rte_internal tag.
> > > > > > > > From a coding standpoint it works a great deal like the
> > > > > > > > __rte_experimental tag in that it expempts the tagged
> > > > > > > > symbol from ABI constraints (as the only users should be
> > > > > > > > represented in the DPDK build environment).  Additionally,
> > > > > > > > the __rte_internal macro resolves differently based on the
> > > > > > > > definition of the BUILDING_RTE_SDK flag (working under the
> > > > > > > > assumption that said flag should only ever be set if we
> > > > > > > > are actually building DPDK libraries which will make use
> > > > > > > > of internal calls).  If the BUILDING_RTE_SDK flag is set
> > > > > > > > __rte_internal resolves to __attribute__((section
> > > > > > > > "text.internal)), placing it in a special text section
> > > > > > > > which is then used to validate that the the symbol appears
> > > > > > > > in the INTERNAL section of the corresponding library versio=
n
> map).
> > > > > > > > If BUILDING_RTE_SDK is not set, then __rte_internal
> > > > > > > > resolves to
> > > > > __attribute__((error("..."))), which causes any caller of the
> > > > > tagged function to throw an error at compile time, indicating
> > > > > that the symbol is not available for external use.
> > > > > > > >
> > > > > > > > This isn't a perfect solution, as applications can still
> > > > > > > > hack around it of course,
> > > > > > >
> > > > > > > I think, one way to, avoid, hack around could be to,
> > > > > > >
> > > > > > > 1) at config stage, create  a random number for the build
> > > > > > > 2) introduce RTE_CALL_INTERNAL macro for calling internal
> > > > > > > function, compare the generated random number for allowing
> > > > > > > the calls to make within the library. i.e leverage the fact
> > > > > > > that external library would never know the random number
> > > > > > > generated for the DPDK build
> > > > > and internal driver code does.
> > > > > > >
> > > > > > Do we really need to care about this. If have some determined
> > > > > > enough to hack around our limitations, then they surely know
> > > > > > that they have an unsupported configuration. We just need to
> > > > > > protect against inadvertent use of internals, IMHO.
> > > > > >
> > > > > I agree, I too had thought about doing some sort of internal
> > > > > runtime checking to match internal only symbols, such that they
> > > > > were only accessable by internally approved users, but it
> > > > > started to feel like a great
> > > deal of overhead.
> > > > > Its a good idea for a general mechanism I think, but I believe
> > > > > the value here is more to internally document which apis we want
> > > > > to mark as being for internal use only, and create a lightweight
> > > > > roadblock at build time to catch users inadvertently using them.
> > > > > Determined users will get around anything, and theres not much
> > > > > we can do to stop
> > > them.
> > > >
> > > > I agree too. IMHO, Simply having following items would be enough
> > > >
> > > > 1) Avoid exposing the internal function prototype through public
> > > > header files
> > > > 2) Add @internal to API documentation
> > > > 3) Just decide the name space for internal API for tooling(i.e not
> > > > start with rte_ or so) Using objdump scheme to detect internal
> > > > functions
> > > requires the the library to build prior to run the checkpatch.
> > > >
> > >
> > > No, I'm not comfortable with that approach, and I've stated why:
> > > 1) Not exposing the functions via header files is a fine start
> > >
> > > 2) Adding internal documentation is also fine, but does nothing to
> > > correlate the code implementing those functions to the
> > > documentation.  Its valuable to have a tag on a function identifying =
it as
> internal only.
> > >
> > > 3) Using naming conventions to separate internal only from
> > > non-internal functions is a vague approach, requiring future
> > > developers to be cogniscent of the convention and make the
> > > appropriate naming choices.  It also implicitly restricts the
> > > abliity for future developers to make naming changes in conflict
> > > with that convention
> >
> > Enforcing the naming convention can be achieved through tooling as well=
.
> >
> Sure, but why enforce any function naming at all, when you don't have to.

May I ask,  why to  enforce __rte_internal, when you don't have to

>=20
> > >
> > > 4) Adding a tag like __rte_internal creates an interlock whereby,
> > > not only are internal functions excused from ABI constraints, but
> > > forces developers to intentionally mark their internal functions as
> > > being internal in the code, which is beneficial to clarlity of unders=
tanding
> during the development process.
> >
> > No issues in adding __rte_internal. But, I am against current
> > implementaion, Ie. adding objdump dependency
> That dependency already exists for the __rte_external flag

Sorry, I could not see the dependency.

[master][dpdk.org] $ grep -ri "objdump" devtools/
[master][dpdk.org] $ grep -ri "objdump" usertools/
[master][dpdk.org] $ grep -ri "__rte_external" *

>=20
> > to checkpatch i.e developer has to build the library first so  that
> > checkpatch can can know, Is it belongs to internal section or not?
> >
> What developer is running checkpatch/posting patches without first buildi=
ng
> their changes?

# it is not developer, The CI/CD tools can quicky check the sanity of patch=
es
before the build itself. Why to add unnecessary dependency?
# If some PMD is not building if the requirements are not meet(say AES NI P=
MD for crypto)
then how do take care of the dependency.


>=20
>=20
> > >
> > > 5) Adding a tag like __rte_internal is explicit, and allows
> > > developers to use a single header file instead of multiple header
> > > files if they so choose
> > >
> > > We went through this with experimental symbols as well[1], and it
> > > just makes more sense to me to clearly document in the code what
> > > constitutes an internal symbol rather than relying on naming
> > > conventions and hoping that developers read the documentation before
> > > exporting a symbol publically.
> > >
> > >
> > > [1] https://mails.dpdk.org/archives/dev/2017-December/083828.html
> > > > >
> > > > > If we really wanted to go down that road, we could use a
> > > > > mechainsm simmilar to the EXPORT_SYMBOL / EXPORT_SYMBOL_GPL
> > > > > infrastructure that the kernel uses, but that would required
> > > > > building our own custom linker script, which seems like overkill =
here.
> > > > >
> > > > > Best
> > > > > Neil
> > > > >
> > > > > > /Bruce
> > > > > >
> > > >
> >