From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id 4342EA0096 for ; Thu, 6 Jun 2019 14:05:06 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 8664D1B9BB; Thu, 6 Jun 2019 14:05:05 +0200 (CEST) Received: from mx0b-0016f401.pphosted.com (mx0a-0016f401.pphosted.com [67.231.148.174]) by dpdk.org (Postfix) with ESMTP id 2524A1B9B9 for ; Thu, 6 Jun 2019 14:05:03 +0200 (CEST) Received: from pps.filterd (m0045849.ppops.net [127.0.0.1]) by mx0a-0016f401.pphosted.com (8.16.0.27/8.16.0.27) with SMTP id x56BuG5m021407; Thu, 6 Jun 2019 05:05:03 -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=cs0encbbpO6VyvHmooHfoc8WemIjHsF08IbAYsnimSU=; b=v2/rBizQCfZJrppoOfKr8euwY4rFfUIg9/+2jfVSq8z/eyqf0fTaGjHAeHBpL2N/SXWB g11FsZxK7aH3/UCp3E3heDvhsnBB1if6JW7HsBF7SsPFSsKyaZCgopzP/fTUdI1f39ZE Y9ptK+M9udNqVefnSj67Ylu6ifgdhvO0LJyrTVp5yfZLEyTl65ehieiN7H9LwaCVrzOY Gtb21JHaZax48YL/7/0GFJbyuSJb2GW04D2Jmnc0jORW5RdzFS5cVMzEbSbIYDyiUMd6 p93cG9H23ZKHoXgUfFEoX1iXze2/7hOoyoUYlZ42wuycUV4iHKLNdpPoFIWWmZOYpurV mw== Received: from sc-exch03.marvell.com ([199.233.58.183]) by mx0a-0016f401.pphosted.com with ESMTP id 2sxwgns5xv-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-SHA384 bits=256 verify=NOT); Thu, 06 Jun 2019 05:05:02 -0700 Received: from SC-EXCH03.marvell.com (10.93.176.83) by SC-EXCH03.marvell.com (10.93.176.83) with Microsoft SMTP Server (TLS) id 15.0.1367.3; Thu, 6 Jun 2019 05:05:02 -0700 Received: from NAM04-SN1-obe.outbound.protection.outlook.com (104.47.44.59) by SC-EXCH03.marvell.com (10.93.176.83) with Microsoft SMTP Server (TLS) id 15.0.1367.3 via Frontend Transport; Thu, 6 Jun 2019 05:05:02 -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=cs0encbbpO6VyvHmooHfoc8WemIjHsF08IbAYsnimSU=; b=Ff07Fjrbsm5HGurF8JE8K0H7eQWmWnnUIhyyD0DWFfYzqph4GkVqpvPJJnXWuQjkvNU8r2AsKAQkqySlN6pXHzxCuXZomK4WFUVKpZTLoW+ukTf/Ea8HuFij4sDc9sQC/a6l5PEtl5fNABF+aXwZp+/UzXaby/DUy/RMUMV31dU= Received: from BYAPR18MB2424.namprd18.prod.outlook.com (20.179.91.149) by BYAPR18MB2630.namprd18.prod.outlook.com (20.179.94.155) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1943.22; Thu, 6 Jun 2019 12:04:57 +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 12:04:57 +0000 From: Jerin Jacob Kollanukkaran To: Neil Horman CC: Bruce Richardson , "dev@dpdk.org" , Thomas Monjalon Thread-Topic: [EXT] [RFC PATCH 0/2] introduce __rte_internal tag Thread-Index: AQHVEynnIvy0R3R0lkaWrQuIVgPnUKaNTWtQgAAIeYCAABfgAIABAvyAgAAgfgCAAAaRkA== Date: Thu, 6 Jun 2019 12:04:57 +0000 Message-ID: References: <20190525184346.27932-1-nhorman@tuxdriver.com> <20190605164541.GH1550@bricha3-MOBL.ger.corp.intel.com> <20190605181108.GC554@hmswarspite.think-freely.org> <20190606113422.GA29521@hmswarspite.think-freely.org> In-Reply-To: <20190606113422.GA29521@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: 4d55e92c-3272-43fc-fcdd-08d6ea7731be x-microsoft-antispam: BCL:0; PCL:0; RULEID:(2390118)(7020095)(4652040)(8989299)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(5600148)(711020)(4605104)(1401327)(2017052603328)(7193020); SRVR:BYAPR18MB2630; x-ms-traffictypediagnostic: BYAPR18MB2630: x-ms-exchange-purlcount: 1 x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:8882; x-forefront-prvs: 00603B7EEF x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(346002)(376002)(39860400002)(366004)(396003)(136003)(189003)(174874002)(199004)(13464003)(8936002)(76116006)(561944003)(11346002)(316002)(73956011)(6916009)(33656002)(81166006)(81156014)(6246003)(8676002)(66476007)(446003)(64756008)(66446008)(66556008)(66946007)(102836004)(68736007)(53936002)(476003)(4326008)(26005)(6506007)(486006)(71190400001)(53546011)(71200400001)(3846002)(6116002)(6436002)(6306002)(54906003)(229853002)(7736002)(76176011)(55016002)(305945005)(74316002)(52536014)(9686003)(186003)(99286004)(966005)(14454004)(86362001)(2906002)(5660300002)(478600001)(25786009)(7696005)(66066001)(14444005)(256004); DIR:OUT; SFP:1101; SCL:1; SRVR:BYAPR18MB2630; H:BYAPR18MB2424.namprd18.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; MX:1; A: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: t257VD+az+TVL3CwL+nP7M6ZthbxtiKxppn+he1l3NdSkbuJlAOjUPOHs8NOqDGdPZGdPA3XYsajRKuPMItFw8QOmyZI0//DwleXaKrhfV9mBS7Qr1h5bwSmznWJDeQFiEPmbKccKmXxY1wLlUxr7Ys/MRz9vOuODyJBpWtyvp89nCT+puffA0/1kaO+9WkuFujd877u1uKkS3dsRISUU+sL6ivaz5zhRDgH6dXLSrEoNmOCrCmcNsDxNqxH7YBdVNm7V2puwh/r9eMgwaTLW/OQm9QJOxWgamLgrfMPVMSGOtheQwONlU8SpFwXz87d8yJ2XBI9Udjd+NPH/Rk86Ejdk1riiSgSkNE4ItvN0NJbl1mdmeiBwScyZwr8zH+7J+86JT4BwS/yk2mGRKTXpGmShlymPZkzvX+5+kmPKgQ= Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-MS-Exchange-CrossTenant-Network-Message-Id: 4d55e92c-3272-43fc-fcdd-08d6ea7731be X-MS-Exchange-CrossTenant-originalarrivaltime: 06 Jun 2019 12:04:57.4672 (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: BYAPR18MB2630 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" > -----Original Message----- > From: Neil Horman > Sent: Thursday, June 6, 2019 5:04 PM > To: Jerin Jacob Kollanukkaran > Cc: Bruce Richardson ; dev@dpdk.org; > Thomas Monjalon > Subject: Re: [EXT] [RFC PATCH 0/2] introduce __rte_internal tag >=20 > On Thu, Jun 06, 2019 at 09:44:52AM +0000, Jerin Jacob Kollanukkaran wrote= : > > > -----Original Message----- > > > From: Neil Horman > > > Sent: Wednesday, June 5, 2019 11:41 PM > > > To: Bruce Richardson > > > Cc: Jerin Jacob Kollanukkaran ; dev@dpdk.org; > > > Thomas Monjalon > > > 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 > > > > > > Sent: Sunday, May 26, 2019 12:14 AM > > > > > > To: dev@dpdk.org > > > > > > Cc: Neil Horman ; Jerin Jacob > > > > > > Kollanukkaran ; Bruce Richardson > > > > > > ; Thomas Monjalon > > > > > > > > > > > > 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 version 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 function= s > requires the the library to build prior to run the checkpatch. > > >=20 > 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 >=20 > 2) Adding internal documentation is also fine, but does nothing to correl= ate > the code implementing those functions to the documentation. Its valuable > to have a tag on a function identifying it as internal only. >=20 > 3) Using naming conventions to separate internal only from non-internal > functions is a vague approach, requiring future developers to be cognisce= nt > of the convention and make the appropriate naming choices. It also impli= citly > restricts the abliity for future developers to make naming changes in con= flict > with that convention Enforcing the naming convention can be achieved through tooling as well. >=20 > 4) Adding a tag like __rte_internal creates an interlock whereby, not onl= y 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 understanding during the development process= . No issues in adding __rte_internal. But, I am against current implementaion= ,=20 Ie. adding objdump dependency to checkpatch i.e developer has to build the library first so that checkpa= tch can can know, Is it belongs to internal section or not? >=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 >=20 > 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. >=20 >=20 > [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 > > > > > >