DPDK patches and discussions
 help / color / mirror / Atom feed
From: Neil Horman <nhorman@tuxdriver.com>
To: David Marchand <david.marchand@redhat.com>
Cc: dev@dpdk.org, thomas@monjalon.net, arybchenko@solarflare.com,
	mdr@ashroe.eu, stable@dpdk.org,
	John McNamara <john.mcnamara@intel.com>,
	Marko Kovacevic <marko.kovacevic@intel.com>,
	Qiming Yang <qiming.yang@intel.com>,
	Wenzhuo Lu <wenzhuo.lu@intel.com>,
	Nicolas Chautru <nicolas.chautru@intel.com>,
	Declan Doherty <declan.doherty@intel.com>,
	Adrien Mazarguil <adrien.mazarguil@6wind.com>,
	Ferruh Yigit <ferruh.yigit@intel.com>,
	Cristian Dumitrescu <cristian.dumitrescu@intel.com>,
	Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Subject: Re: [dpdk-dev] [PATCH] mark experimental variables
Date: Tue, 3 Dec 2019 10:26:08 -0500	[thread overview]
Message-ID: <20191203152608.GA5273@hmswarspite.think-freely.org> (raw)
In-Reply-To: <20191202152030.7800-1-david.marchand@redhat.com>

On Mon, Dec 02, 2019 at 04:20:29PM +0100, David Marchand wrote:
> So far, we did not pay attention to direct access to variables but they
> are part of the API/ABI too and should be clearly identified.
> 
> Introduce a __rte_experimental_var tag and mark existing exported
> variables.
> 
> Fixes: a4bcd61de82d ("buildtools: add script to check experimental API exports")
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
> Changelog since RFC:
> - s/resp\./or/ in documentation,
> - caught more variables by looking at the *COM* section,
> 
> ---
>  buildtools/check-experimental-syms.sh      | 17 +++++++++++++++--
>  devtools/checkpatches.sh                   | 16 ++++++++++++----
>  doc/guides/contributing/abi_policy.rst     |  7 ++++---
>  drivers/net/ice/rte_pmd_ice.h              |  8 ++++++++
>  lib/librte_bbdev/rte_bbdev.h               |  1 +
>  lib/librte_cryptodev/rte_crypto_asym.h     |  3 +++
>  lib/librte_eal/common/include/rte_compat.h |  5 +++++
>  lib/librte_ethdev/rte_flow.h               | 17 +++++++++++++++++
>  lib/librte_port/rte_port_eventdev.h        |  5 +++++
>  lib/librte_rcu/rte_rcu_qsbr.h              |  1 +
>  10 files changed, 71 insertions(+), 9 deletions(-)
> 
> diff --git a/buildtools/check-experimental-syms.sh b/buildtools/check-experimental-syms.sh
> index f3603e5ba..f3a2f92fb 100755
> --- a/buildtools/check-experimental-syms.sh
> +++ b/buildtools/check-experimental-syms.sh
> @@ -34,13 +34,26 @@ do
>  		Please add __rte_experimental to the definition of $SYM
>  		END_OF_MESSAGE
>  		ret=1
> +	elif grep -qe "\(\.data\|\*COM\*\).*[[:space:]]$SYM$" $DUMPFILE &&
> +		! grep -q "\.data\.experimental.*[[:space:]]$SYM$" $DUMPFILE
> +	then
> +		cat >&2 <<- END_OF_MESSAGE
> +		$SYM is not flagged as experimental
> +		but is listed in version map
> +		Please add __rte_experimental_var to the definition of $SYM
> +		END_OF_MESSAGE
> +		ret=1
>  	fi
>  done
>  
>  # Filter out symbols suffixed with a . for icc
>  for SYM in `awk '{
> -	if ($2 != "l" && $4 == ".text.experimental" && !($NF ~ /\.$/)) {
> -		print $NF
> +	if ($2 == "l" || $NF ~ /\.$/) {
> +		next;
> +	}
> +	if ($4 == ".text.experimental" ||
> +	    $4 == ".data.experimental") {
> +		print $NF;
>  	}
>  }' $DUMPFILE`
>  do
> diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
> index b16bace92..92b0ae40a 100755
> --- a/devtools/checkpatches.sh
> +++ b/devtools/checkpatches.sh
> @@ -90,11 +90,19 @@ check_experimental_tags() { # <patch>
>  				"headers ("current_file")";
>  			ret = 1;
>  		}
> -		if ($1 != "+__rte_experimental" || $2 != "") {
> +
> +		if (NF == 1 && ($1 == "+__rte_experimental" ||
> +				$1 == "+__rte_experimental_var")) {
> +			next;
> +		}
> +		if ($0 ~ "__rte_experimental_var") {
> +			print "__rte_experimental_var must appear alone on the line" \
> +				" immediately preceding the type of a variable.";
> +		} else {
>  			print "__rte_experimental must appear alone on the line" \
> -				" immediately preceding the return type of a function."
> -			ret = 1;
> +				" immediately preceding the return type of a function.";
>  		}
> +		ret = 1;
>  	}
>  	END {
>  		exit ret;
> @@ -178,7 +186,7 @@ check () { # <patch> <commit> <title>
>  		ret=1
>  	fi
>  
> -	! $verbose || printf '\nChecking __rte_experimental tags:\n'
> +	! $verbose || printf '\nChecking __rte_experimental* tags:\n'
>  	report=$(check_experimental_tags "$tmpinput")
>  	if [ $? -ne 0 ] ; then
>  		$headline_printed || print_headline "$3"
> diff --git a/doc/guides/contributing/abi_policy.rst b/doc/guides/contributing/abi_policy.rst
> index 05ca95980..f933234d6 100644
> --- a/doc/guides/contributing/abi_policy.rst
> +++ b/doc/guides/contributing/abi_policy.rst
> @@ -300,9 +300,10 @@ Note that marking an API as experimental is a multi step process.
>  To mark an API as experimental, the symbols which are desired to be exported
>  must be placed in an EXPERIMENTAL version block in the corresponding libraries'
>  version map script.
> -Secondly, the corresponding prototypes of those exported functions (in the
> -development header files), must be marked with the ``__rte_experimental`` tag
> -(see ``rte_compat.h``).
> +Secondly, the corresponding prototypes of those exported functions (or
> +variables) must be marked with the ``__rte_experimental`` (or
> +``__rte_experimental_var``) tag in the development header files (see
> +``rte_compat.h``).
>  The DPDK build makefiles perform a check to ensure that the map file and the
>  C code reflect the same list of symbols.
>  This check can be circumvented by defining ``ALLOW_EXPERIMENTAL_API``
> diff --git a/drivers/net/ice/rte_pmd_ice.h b/drivers/net/ice/rte_pmd_ice.h
> index e254db053..a5c80c43c 100644
> --- a/drivers/net/ice/rte_pmd_ice.h
> +++ b/drivers/net/ice/rte_pmd_ice.h
> @@ -15,6 +15,8 @@
>   */
>  
>  #include <stdio.h>
> +
> +#include <rte_compat.h>
>  #include <rte_mbuf.h>
>  #include <rte_mbuf_dyn.h>
>  
> @@ -81,13 +83,19 @@ union rte_net_ice_proto_xtr_metadata {
>  };
>  
>  /* Offset of mbuf dynamic field for protocol extraction data */
> +__rte_experimental_var
>  extern int rte_net_ice_dynfield_proto_xtr_metadata_offs;
>  
>  /* Mask of mbuf dynamic flags for protocol extraction type */
> +__rte_experimental_var
>  extern uint64_t rte_net_ice_dynflag_proto_xtr_vlan_mask;
> +__rte_experimental_var
>  extern uint64_t rte_net_ice_dynflag_proto_xtr_ipv4_mask;
> +__rte_experimental_var
>  extern uint64_t rte_net_ice_dynflag_proto_xtr_ipv6_mask;
> +__rte_experimental_var
>  extern uint64_t rte_net_ice_dynflag_proto_xtr_ipv6_flow_mask;
> +__rte_experimental_var
>  extern uint64_t rte_net_ice_dynflag_proto_xtr_tcp_mask;
>  
>  /**
> diff --git a/lib/librte_bbdev/rte_bbdev.h b/lib/librte_bbdev/rte_bbdev.h
> index 591fb7914..e044dec88 100644
> --- a/lib/librte_bbdev/rte_bbdev.h
> +++ b/lib/librte_bbdev/rte_bbdev.h
> @@ -466,6 +466,7 @@ struct __rte_cache_aligned rte_bbdev {
>  };
>  
>  /** @internal array of all devices */
> +__rte_experimental_var
>  extern struct rte_bbdev rte_bbdev_devices[];
>  
>  /**
> diff --git a/lib/librte_cryptodev/rte_crypto_asym.h b/lib/librte_cryptodev/rte_crypto_asym.h
> index 0d34ce8df..d4e84910f 100644
> --- a/lib/librte_cryptodev/rte_crypto_asym.h
> +++ b/lib/librte_cryptodev/rte_crypto_asym.h
> @@ -24,6 +24,7 @@ extern "C" {
>  #include <rte_memory.h>
>  #include <rte_mempool.h>
>  #include <rte_common.h>
> +#include <rte_compat.h>
>  
>  #include "rte_crypto_sym.h"
>  
> @@ -37,10 +38,12 @@ typedef struct rte_crypto_param_t {
>  } rte_crypto_param;
>  
>  /** asym xform type name strings */
> +__rte_experimental_var
>  extern const char *
>  rte_crypto_asym_xform_strings[];
>  
>  /** asym operations type name strings */
> +__rte_experimental_var
>  extern const char *
>  rte_crypto_asym_op_strings[];
>  
> diff --git a/lib/librte_eal/common/include/rte_compat.h b/lib/librte_eal/common/include/rte_compat.h
> index 3eb33784b..3fd05179f 100644
> --- a/lib/librte_eal/common/include/rte_compat.h
> +++ b/lib/librte_eal/common/include/rte_compat.h
> @@ -11,11 +11,16 @@
>  #define __rte_experimental \
>  __attribute__((deprecated("Symbol is not yet part of stable ABI"), \
>  section(".text.experimental")))
> +#define __rte_experimental_var \
> +__attribute__((deprecated("Symbol is not yet part of stable ABI"), \
> +section(".data.experimental")))
>  
>  #else
>  
>  #define __rte_experimental \
>  __attribute__((section(".text.experimental")))
> +#define __rte_experimental_var \
> +__attribute__((section(".data.experimental")))
>  
>  #endif
>  
> diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> index 452d359a1..c8ea71acc 100644
> --- a/lib/librte_ethdev/rte_flow.h
> +++ b/lib/librte_ethdev/rte_flow.h
> @@ -19,6 +19,7 @@
>  
>  #include <rte_arp.h>
>  #include <rte_common.h>
> +#include <rte_compat.h>
>  #include <rte_ether.h>
>  #include <rte_icmp.h>
>  #include <rte_ip.h>
> @@ -2531,9 +2532,11 @@ struct rte_flow_action_set_meta {
>  };
>  
>  /* Mbuf dynamic field offset for metadata. */
> +__rte_experimental_var
>  extern int rte_flow_dynf_metadata_offs;
>  
>  /* Mbuf dynamic field flag mask for metadata. */
> +__rte_experimental_var
>  extern uint64_t rte_flow_dynf_metadata_mask;
>  
>  /* Mbuf dynamic field pointer for metadata. */
> @@ -2548,14 +2551,24 @@ __rte_experimental
>  static inline uint32_t
>  rte_flow_dynf_metadata_get(struct rte_mbuf *m)
>  {
> +#ifdef ALLOW_EXPERIMENTAL_API
>  	return *RTE_FLOW_DYNF_METADATA(m);
> +#else
> +	RTE_SET_USED(m);
> +	return 0;
> +#endif
>  }
>  
>  __rte_experimental
>  static inline void
>  rte_flow_dynf_metadata_set(struct rte_mbuf *m, uint32_t v)
>  {
> +#ifdef ALLOW_EXPERIMENTAL_API
>  	*RTE_FLOW_DYNF_METADATA(m) = v;
> +#else
> +	RTE_SET_USED(m);
> +	RTE_SET_USED(v);
> +#endif
>  }
>  
>  /*
> @@ -2800,7 +2813,11 @@ __rte_experimental
>  static inline int
>  rte_flow_dynf_metadata_avail(void)
>  {
> +#ifdef ALLOW_EXPERIMENTAL_API
>  	return !!rte_flow_dynf_metadata_mask;
> +#else
> +	return 0;
> +#endif
>  }
>  
>  /**
> diff --git a/lib/librte_port/rte_port_eventdev.h b/lib/librte_port/rte_port_eventdev.h
> index acf88f4e9..59246e204 100644
> --- a/lib/librte_port/rte_port_eventdev.h
> +++ b/lib/librte_port/rte_port_eventdev.h
> @@ -25,6 +25,8 @@ extern "C" {
>  **/
>  
>  #include <stdint.h>
> +
> +#include <rte_compat.h>
>  #include <rte_eventdev.h>
>  
>  #include "rte_port.h"
> @@ -39,6 +41,7 @@ struct rte_port_eventdev_reader_params {
>  };
>  
>  /** Eventdev_reader port operations. */
> +__rte_experimental_var
>  extern struct rte_port_in_ops rte_port_eventdev_reader_ops;
>  
>  /** Eventdev_writer port parameters. */
> @@ -63,6 +66,7 @@ struct rte_port_eventdev_writer_params {
>  };
>  
>  /** Eventdev_writer port operations. */
> +__rte_experimental_var
>  extern struct rte_port_out_ops rte_port_eventdev_writer_ops;
>  
>  /** Event_writer_nodrop port parameters. */
> @@ -90,6 +94,7 @@ struct rte_port_eventdev_writer_nodrop_params {
>  };
>  
>  /** Eventdev_writer_nodrop port operations. */
> +__rte_experimental_var
>  extern struct rte_port_out_ops rte_port_eventdev_writer_nodrop_ops;
>  
>  #ifdef __cplusplus
> diff --git a/lib/librte_rcu/rte_rcu_qsbr.h b/lib/librte_rcu/rte_rcu_qsbr.h
> index 0b5585925..55c100673 100644
> --- a/lib/librte_rcu/rte_rcu_qsbr.h
> +++ b/lib/librte_rcu/rte_rcu_qsbr.h
> @@ -35,6 +35,7 @@ extern "C" {
>  #include <rte_debug.h>
>  #include <rte_atomic.h>
>  
> +__rte_experimental_var
>  extern int rte_rcu_log_type;
>  
>  #if RTE_LOG_DP_LEVEL >= RTE_LOG_DEBUG
> -- 
> 2.23.0
> 
> 
Acked-by: Neil Horman <nhorman@tuxdriver.com>

  parent reply	other threads:[~2019-12-03 15:26 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-25 16:13 [dpdk-dev] [RFC PATCH] " David Marchand
2019-11-26  9:25 ` Ray Kinsella
2019-11-26  9:50   ` David Marchand
2019-11-26 14:15   ` Neil Horman
2019-11-26 14:22 ` Neil Horman
2019-11-27 20:45   ` David Marchand
2019-11-29 11:43     ` Neil Horman
2019-11-29 12:03       ` David Marchand
2019-12-02 15:20 ` [dpdk-dev] [PATCH] " David Marchand
2019-12-03  8:33   ` Andrew Rybchenko
2019-12-03 15:26   ` Neil Horman [this message]
2020-01-09 14:13   ` [dpdk-dev] [dpdk-stable] " Thomas Monjalon
2020-01-09 16:49     ` David Marchand
2023-06-12  2:49 ` [dpdk-dev] [RFC PATCH] " Stephen Hemminger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191203152608.GA5273@hmswarspite.think-freely.org \
    --to=nhorman@tuxdriver.com \
    --cc=adrien.mazarguil@6wind.com \
    --cc=arybchenko@solarflare.com \
    --cc=cristian.dumitrescu@intel.com \
    --cc=david.marchand@redhat.com \
    --cc=declan.doherty@intel.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=honnappa.nagarahalli@arm.com \
    --cc=john.mcnamara@intel.com \
    --cc=marko.kovacevic@intel.com \
    --cc=mdr@ashroe.eu \
    --cc=nicolas.chautru@intel.com \
    --cc=qiming.yang@intel.com \
    --cc=stable@dpdk.org \
    --cc=thomas@monjalon.net \
    --cc=wenzhuo.lu@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).