From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id C0DCCA0588; Fri, 17 Apr 2020 04:38:31 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id C97551DDEE; Fri, 17 Apr 2020 04:38:30 +0200 (CEST) Received: from smtp.tuxdriver.com (charlotte.tuxdriver.com [70.61.120.58]) by dpdk.org (Postfix) with ESMTP id A55121DDCF for ; Fri, 17 Apr 2020 04:38:27 +0200 (CEST) Received: from [107.15.85.130] (helo=localhost) by smtp.tuxdriver.com with esmtpsa (TLSv1:AES256-SHA:256) (Exim 4.63) (envelope-from ) id 1jPGtV-00060x-NA; Thu, 16 Apr 2020 22:38:21 -0400 Date: Thu, 16 Apr 2020 22:38:12 -0400 From: Neil Horman To: "Wang, Haiyue" Cc: "dev@dpdk.org" , Jerin Jacob Kollanukkaran , "Richardson, Bruce" , Thomas Monjalon , David Marchand , "Yigit, Ferruh" Message-ID: <20200417023812.rrwe5t7rwfsk6j3p@penguin.lxd> References: <20190525184346.27932-1-nhorman@tuxdriver.com> <20190613142344.9188-1-nhorman@tuxdriver.com> <20190613142344.9188-2-nhorman@tuxdriver.com> <018d2553c44b437387a39d040553cbb6@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <018d2553c44b437387a39d040553cbb6@intel.com> User-Agent: NeoMutt/20170113 (1.7.2) X-Spam-Score: -2.9 (--) X-Spam-Status: No Subject: Re: [dpdk-dev] [PATCH v2 01/10] Add __rte_internal tag for functions and version target 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" On Fri, Apr 17, 2020 at 02:04:30AM +0000, Wang, Haiyue wrote: > Hi Neil, > > > -----Original Message----- > > From: dev On Behalf Of Neil Horman > > Sent: Thursday, June 13, 2019 22:24 > > To: dev@dpdk.org > > Cc: Neil Horman ; Jerin Jacob Kollanukkaran ; Richardson, > > Bruce ; Thomas Monjalon > > Subject: [dpdk-dev] [PATCH v2 01/10] Add __rte_internal tag for functions and version target > > > > This tag is meant to be used on function prototypes to identify > > functions that are only meant to be used by internal DPDK libraries > > (i.e. libraries that are built while building the SDK itself, as > > identified by the defining of the BUILDING_RTE_SDK macro). When that > > flag is not set, it will resolve to an error function attribute, causing > > build breakage for any compilation unit attempting to build it > > > > Validate the use of this tag in much the same way we validate > > __rte_experimental. By adding an INTERNAL version to library map files, > > we can exempt internal-only functions from ABI checking, and handle them > > to ensure that symbols we wish to only be for internal use between dpdk > > libraries are properly tagged with __rte_experimental > > > > Note this patch updates the check-experimental-syms.sh script, which > > normally only check the EXPERIMENTAL section to also check the INTERNAL > > section now. As such its been renamed to the now more appropriate > > check-special-syms.sh > > > > Signed-off-by: Neil Horman > > CC: Jerin Jacob Kollanukkaran > > CC: Bruce Richardson > > CC: Thomas Monjalon > > --- > > ...rimental-syms.sh => check-special-syms.sh} | 24 ++++++++++++++++++- > > lib/librte_eal/common/include/rte_compat.h | 12 ++++++++++ > > mk/internal/rte.compile-pre.mk | 6 ++--- > > mk/target/generic/rte.vars.mk | 2 +- > > 4 files changed, 39 insertions(+), 5 deletions(-) > > rename buildtools/{check-experimental-syms.sh => check-special-syms.sh} (53%) > > > .... > > > diff --git a/lib/librte_eal/common/include/rte_compat.h b/lib/librte_eal/common/include/rte_compat.h > > index 92ff28faf..739e8485c 100644 > > --- a/lib/librte_eal/common/include/rte_compat.h > > +++ b/lib/librte_eal/common/include/rte_compat.h > > @@ -89,4 +89,16 @@ __attribute__((section(".text.experimental"))) > > > > #endif > > > > +/* > > + * __rte_internal tags mark functions as internal only, If specified in public > > + * header files, this tag will resolve to an error directive, preventing > > + * external applications from attempting to make calls to functions not meant > > + * for consumption outside the dpdk library > > + */ > > +#ifdef BUILDING_RTE_SDK > > +#define __rte_internal __attribute__((section(".text.internal"))) > > +#else > > +#define __rte_internal __attribute__((error("This function cannot be used outside of the core DPDK > > library"), \ > > + section(".text.internal"))) > > +#endif > > #endif /* _RTE_COMPAT_H_ */ > > Since struct definition is also a kind of ABI (am I right ? ;-) ), like: > Yes, thats correct, which is why I've advocated for making structs opaque as part of the abi, but I suppose thats not where we are. :) > drivers/bus/pci/rte_bus_pci.h > struct rte_pci_device { > ... > struct rte_intr_handle vfio_req_intr_handle; > /**< Handler of VFIO request interrupt */ > } __rte_internal; > > Then will capture the errors anyway by using one of __rte_internal definition. > error: 'section' attribute does not apply to types [-Werror=attributes] > error: 'error' attribute does not apply to types > As it is currently written, the __rte_internal macro is only written to work on functions. If you don't want a struct to be part of the ABI, we would need to either: a) make a simmilar macro (say __rte_internal_data) which uses a simmilar gcc attibute to catch external usage. or b) just move the strucute definition to a location that isn't exposed as part of the external ABI Neil > > 2.20.1 > >