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 2B536A00C2; Fri, 24 Apr 2020 16:53:08 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 9F1081D163; Fri, 24 Apr 2020 16:53:07 +0200 (CEST) Received: from us-smtp-1.mimecast.com (us-smtp-delivery-1.mimecast.com [205.139.110.120]) by dpdk.org (Postfix) with ESMTP id B74371C433 for ; Fri, 24 Apr 2020 16:53:06 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1587739985; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=MenNoLCkiRIngzy1mPP3ysUyHX2zVXAZj9Ov2SH8Rvg=; b=CjH5PcBIRnKh1gVDXp8lxhiAjFG1MOEQi9cnH4I4syNjFkAoF2jPKF3DATmJdMSAdN61q8 peo/274vVEJOIv5aT3aOC6ryqC+tdRcxv8PYgVNegCoi67ZfST8GyjbErX4NU9L3rNgZLp e4NToyAeWcPtryAa5Z+TC2sDmd3xRyc= Received: from mail-vk1-f200.google.com (mail-vk1-f200.google.com [209.85.221.200]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-356-6ir3LejLPUWzdlDxqGKyYg-1; Fri, 24 Apr 2020 10:53:02 -0400 X-MC-Unique: 6ir3LejLPUWzdlDxqGKyYg-1 Received: by mail-vk1-f200.google.com with SMTP id o68so2646206vkc.19 for ; Fri, 24 Apr 2020 07:53:02 -0700 (PDT) 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=NFtzlHPWd9eAoWCcjQ/fWfeCvVr8z2P0sWjNrTjYxhw=; b=sFdmEqRXiZTCcDRbxnlqcqiJVCUWEXwfOw90ucXfwvnRH9KbXbhwa+yvnOmQOUzlfm 0Xhvt/4ISvhCyzP54uMthcTl9rFDV/ifCarcexJZo+PmWOfcy01soHZhC12NNVkj/zxp pSiaDrGEia53JmNe3xXmQWrUf1ImYEYY3HA5nuTVwf/EP9NaZpyZ3YDjsxhu5jBGrPAd J3UT8lsqh9S7rdggaxvEHKJAlEDL//XyX0SVW8nf56WjVqmhqWQ2wIP2hiuhjQp2ogqf m+/TlVFQ9q7tMVmBTNLtQJhDVhnh3KRBhMWbBE68ABKHCpOxKM2RybSAKBT+2k40eCyc yWTw== X-Gm-Message-State: AGi0PuY8eCDV+jRpaA2MslDWzGP26dYrAO5/zH6Ry3RM5t1sAXJIobL4 665Op3De9qpCgF3qZ6wViXRt5pyckYz0BXyJZzg4FXUVcuzyQ80m84bAEafuWfTBRMhAxDwwC0L f8p6+3fkQSWFw31velow= X-Received: by 2002:a67:d817:: with SMTP id e23mr3840754vsj.39.1587739981487; Fri, 24 Apr 2020 07:53:01 -0700 (PDT) X-Google-Smtp-Source: APiQypIIEnQjR4NMYbBRe9VLWorNPCzQZayi8dMXtfht8SKRTpqYMWKaKsxBU0HYe2MGlVcemb15dJg6RdeNPaB+rCg= X-Received: by 2002:a67:d817:: with SMTP id e23mr3840727vsj.39.1587739981147; Fri, 24 Apr 2020 07:53:01 -0700 (PDT) MIME-Version: 1.0 References: <20190613142344.9188-1-nhorman@tuxdriver.com> <20200423031947.4477-1-haiyue.wang@intel.com> <20200423031947.4477-2-haiyue.wang@intel.com> In-Reply-To: <20200423031947.4477-2-haiyue.wang@intel.com> From: David Marchand Date: Fri, 24 Apr 2020 16:52:49 +0200 Message-ID: To: Haiyue Wang Cc: dev , Thomas Monjalon , Bruce Richardson , "Yigit, Ferruh" , Neil Horman , Ray Kinsella X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Subject: Re: [dpdk-dev] [PATCH v5 1/1] eal: add internal ABI marking support 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 Thu, Apr 23, 2020 at 5:25 AM Haiyue Wang wrote: > > Introduce __rte_internal tag to mark internal ABI function which is used > by the drivers or other libraries. > > Signed-off-by: Haiyue Wang > --- > buildtools/check-internal-syms.sh | 57 +++++++++++++++++++++++++++++ > devtools/check-symbol-change.sh | 7 ++++ > devtools/checkpatches.sh | 38 +++++++++++++++++++ > devtools/libabigail.abignore | 5 +++ > drivers/meson.build | 2 +- > lib/librte_eal/include/rte_compat.h | 13 +++++++ > lib/meson.build | 2 +- > mk/internal/rte.compile-pre.mk | 3 ++ > mk/target/generic/rte.vars.mk | 1 + > 9 files changed, 126 insertions(+), 2 deletions(-) > create mode 100755 buildtools/check-internal-syms.sh We can have a single script (let's say devtools/check-symbols.sh) that contains both experimental and internal symbols. > diff --git a/devtools/check-symbol-change.sh b/devtools/check-symbol-chan= ge.sh > index ed2178e36..978979077 100755 > --- a/devtools/check-symbol-change.sh > +++ b/devtools/check-symbol-change.sh > @@ -91,6 +91,13 @@ check_for_rule_violations() > if [ "$ar" =3D "add" ] > then > > + if [ "$secname" =3D "INTERNAL" ] > + then > + # these are absolved from any further che= cking > + echo "Skipping symbol $symname in INTERNA= L" > + continue > + fi > + > if [ "$secname" =3D "unknown" ] > then > # Just inform the user of this occurrence= , but This only handles symbol additions to the INTERNAL section. Other cases like moving or removing a INTERNAL symbol are not handled. > diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh > index c30ce64cc..2df8d7f2c 100755 > --- a/devtools/checkpatches.sh > +++ b/devtools/checkpatches.sh > @@ -111,6 +111,36 @@ check_experimental_tags() { # > return $res > } > > +check_internal_tags() { # > + res=3D0 > + > + cat "$1" |awk ' > + BEGIN { > + current_file =3D ""; > + ret =3D 0; > + } > + /^+++ b\// { > + current_file =3D $2; > + } > + /^+.*__rte_internal/ { > + if (current_file ~ ".c$" ) { > + print "Please only put __rte_internal tags in " \ > + "headers ("current_file")"; > + ret =3D 1; > + } > + if ($1 !=3D "+__rte_internal" || $2 !=3D "") { > + print "__rte_internal must appear alone on the li= ne" \ > + " immediately preceding the return type o= f a function." > + ret =3D 1; > + } > + } > + END { > + exit ret; > + }' || res=3D1 > + > + return $res > +} > + > number=3D0 > range=3D'origin/master..' > quiet=3Dfalse > @@ -194,6 +224,14 @@ check () { # > ret=3D1 > fi > > + ! $verbose || printf '\nChecking __rte_internal tags:\n' > + report=3D$(check_internal_tags "$tmpinput") > + if [ $? -ne 0 ] ; then > + $headline_printed || print_headline "$3" > + printf '%s\n' "$report" > + ret=3D1 > + fi > + > if [ "$tmpinput" !=3D "$1" ]; then > rm -f "$tmpinput" > trap - INT > diff --git a/devtools/libabigail.abignore b/devtools/libabigail.abignore > index cd86d89ca..3e8e2ea74 100644 > --- a/devtools/libabigail.abignore > +++ b/devtools/libabigail.abignore > @@ -3,6 +3,11 @@ > [suppress_variable] > symbol_version =3D EXPERIMENTAL > > +[suppress_function] > + symbol_version =3D INTERNAL > +[suppress_variable] > + symbol_version =3D INTERNAL > + > ; Explicit ignore for driver-only ABI > [suppress_type] > name =3D rte_cryptodev_ops > diff --git a/drivers/meson.build b/drivers/meson.build > index 4d8f842ab..cac07161f 100644 > --- a/drivers/meson.build > +++ b/drivers/meson.build > @@ -20,7 +20,7 @@ dpdk_driver_classes =3D ['common', > disabled_drivers =3D run_command(list_dir_globs, get_option('disable_dri= vers'), > ).stdout().split() > > -default_cflags =3D machine_args + ['-DALLOW_EXPERIMENTAL_API'] > +default_cflags =3D machine_args + ['-DALLOW_EXPERIMENTAL_API'] + ['-DALL= OW_INTERNAL_API'] Nit: default_cflags =3D machine_args default_cflags +=3D ['-DALLOW_EXPERIMENTAL_API'] default_cflags +=3D ['-DALLOW_INTERNAL_API'] > if cc.has_argument('-Wno-format-truncation') > default_cflags +=3D '-Wno-format-truncation' > endif More importantly on this file: - drivers/meson.build is not updated to check for internal symbols, see: https://git.dpdk.org/dpdk/tree/drivers/meson.build#n166 - For fully experimental libraries, we have a special so version: https://git.dpdk.org/dpdk/tree/drivers/meson.build#n131 This will apply to common drivers that will be 100% internal. Not sure if this is an issue. > diff --git a/lib/librte_eal/include/rte_compat.h b/lib/librte_eal/include= /rte_compat.h > index 3eb33784b..4cd8f68d6 100644 > --- a/lib/librte_eal/include/rte_compat.h > +++ b/lib/librte_eal/include/rte_compat.h > @@ -19,4 +19,17 @@ __attribute__((section(".text.experimental"))) > > #endif > > +#ifndef ALLOW_INTERNAL_API > + > +#define __rte_internal \ > +__attribute__((error("Symbol is not public ABI"), \ > +section(".text.internal"))) > + > +#else > + > +#define __rte_internal \ > +__attribute__((section(".text.internal"))) > + > +#endif > + > #endif /* _RTE_COMPAT_H_ */ > diff --git a/lib/meson.build b/lib/meson.build > index c28b8df83..4d2f90d6a 100644 > --- a/lib/meson.build > +++ b/lib/meson.build > @@ -38,7 +38,7 @@ if is_windows > libraries =3D ['kvargs','eal'] # only supported libraries for win= dows > endif > > -default_cflags =3D machine_args + ['-DALLOW_EXPERIMENTAL_API'] > +default_cflags =3D machine_args + ['-DALLOW_EXPERIMENTAL_API'] + ['-DALL= OW_INTERNAL_API'] Same comments as for drivers/meson.build. > diff --git a/mk/internal/rte.compile-pre.mk b/mk/internal/rte.compile-pre= .mk > index 82fe098f7..0369786a5 100644 > --- a/mk/internal/rte.compile-pre.mk > +++ b/mk/internal/rte.compile-pre.mk > @@ -58,6 +58,8 @@ C_TO_O_DISP =3D $(if $(V),"$(C_TO_O_STR)"," CC $(@)") > endif > EXPERIMENTAL_CHECK =3D $(RTE_SDK)/buildtools/check-experimental-syms.sh > CHECK_EXPERIMENTAL =3D $(EXPERIMENTAL_CHECK) $(SRCDIR)/$(EXPORT_MAP) $@ > +INTERNAL_CHECK =3D $(RTE_SDK)/buildtools/check-internal-syms.sh > +CHECK_INTERNAL =3D $(INTERNAL_CHECK) $(SRCDIR)/$(EXPORT_MAP) $@ With a single script, we can go with: CHECK_SYMBOLS_SCRIPT =3D $(RTE_SDK)/buildtools/check-symbols.sh CHECK_SYMBOLS =3D $(CHECK_SYMBOLS_SCRIPT) $(SRCDIR)/$(EXPORT_MAP) $@ > > PMDINFO_GEN =3D $(RTE_SDK_BIN)/app/dpdk-pmdinfogen $@ $@.pmd.c > PMDINFO_CC =3D $(CC) $(CPPFLAGS) $(CFLAGS) $(EXTRA_CFLAGS) -c -o $@.pmd.= o $@.pmd.c > @@ -76,6 +78,7 @@ C_TO_O_DO =3D @set -e; \ > $(C_TO_O) && \ > $(PMDINFO_TO_O) && \ > $(CHECK_EXPERIMENTAL) && \ > + $(CHECK_INTERNAL) && \ + $(CHECK_SYMBOLS) && \ > echo $(C_TO_O_CMD) > $(call obj2cmd,$(@)) && \ > sed 's,'$@':,dep_'$@' =3D,' $(call obj2dep,$(@)).tmp > $(call obj= 2dep,$(@)) && \ > rm -f $(call obj2dep,$(@)).tmp > diff --git a/mk/target/generic/rte.vars.mk b/mk/target/generic/rte.vars.m= k > index ec2672897..11b0418e5 100644 > --- a/mk/target/generic/rte.vars.mk > +++ b/mk/target/generic/rte.vars.mk > @@ -106,6 +106,7 @@ ifeq ($(BUILDING_RTE_SDK),1) > # building sdk > CFLAGS +=3D -include $(RTE_OUTPUT)/include/rte_config.h > CFLAGS +=3D -DALLOW_EXPERIMENTAL_API > +CFLAGS +=3D -DALLOW_INTERNAL_API > else > # if we are building an external application, include SDK's lib and > # includes too > -- > 2.26.2 > We are missing updates on devtools/check-abi-version.sh and devtools/update_version_map_abi.py. --=20 David Marchand