DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH 0/2] add missing local symbols catch-all
@ 2022-03-06  9:20 Thomas Monjalon
  2022-03-06  9:20 ` [PATCH 1/2] regexdev: fix section attribute of symbols Thomas Monjalon
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Thomas Monjalon @ 2022-03-06  9:20 UTC (permalink / raw)
  To: dev

The libraries (and driver) regexdev, gpudev and auxiliary bus
were missing "local:*" in their version.map linker script
which catch all non-listed functions to be hidden in shared libraries.

Thomas Monjalon (2):
  regexdev: fix section attribute of symbols
  build: hide local symbols in shared libraries

 devtools/libabigail.abignore       | 12 ++++++++++++
 drivers/bus/auxiliary/version.map  |  2 ++
 lib/gpudev/version.map             |  2 ++
 lib/regexdev/rte_regexdev.h        |  4 ++++
 lib/regexdev/rte_regexdev_driver.h |  3 +++
 lib/regexdev/version.map           | 11 +++++++++++
 6 files changed, 34 insertions(+)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/2] regexdev: fix section attribute of symbols
  2022-03-06  9:20 [PATCH 0/2] add missing local symbols catch-all Thomas Monjalon
@ 2022-03-06  9:20 ` Thomas Monjalon
  2022-03-07 10:15   ` Ori Kam
  2022-03-06  9:20 ` [PATCH 2/2] build: hide local symbols in shared libraries Thomas Monjalon
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: Thomas Monjalon @ 2022-03-06  9:20 UTC (permalink / raw)
  To: dev; +Cc: stable, Ray Kinsella, Ori Kam, Jerin Jacob, Pavan Nikhilesh

The functions used by the drivers must be internal,
while the function and variables used in inline functions
must be experimental.

These are the changes done in the shared libraries:
- DF .text  Base          rte_regexdev_get_device_by_name
+ DF .text  INTERNAL      rte_regexdev_get_device_by_name
- DF .text  Base          rte_regexdev_register
+ DF .text  INTERNAL      rte_regexdev_register
- DF .text  Base          rte_regexdev_unregister
+ DF .text  INTERNAL      rte_regexdev_unregister
- DF .text  Base          rte_regexdev_is_valid_dev
+ DF .text  EXPERIMENTAL  rte_regexdev_is_valid_dev
- DO .bss   Base          rte_regex_devices
+ DO .bss   EXPERIMENTAL  rte_regex_devices
- DO .bss   Base          rte_regexdev_logtype
+ DO .bss   EXPERIMENTAL  rte_regexdev_logtype

Because these symbols were exported in the default section in DPDK 21.11,
any change in these functions would be seen as incompatible
by the ABI compatibility check.
An exception rule is added for this experimental library,
so the ABI check will skip it until the next ABI version.

Fixes: bab9497ef78b ("regexdev: introduce API")
Cc: stable@dpdk.org

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 devtools/libabigail.abignore       | 4 ++++
 lib/regexdev/rte_regexdev.h        | 4 ++++
 lib/regexdev/rte_regexdev_driver.h | 3 +++
 lib/regexdev/version.map           | 9 +++++++++
 4 files changed, 20 insertions(+)

diff --git a/devtools/libabigail.abignore b/devtools/libabigail.abignore
index 301b3dacb8..cff7a293ae 100644
--- a/devtools/libabigail.abignore
+++ b/devtools/libabigail.abignore
@@ -21,6 +21,10 @@
 [suppress_type]
         name = rte_crypto_asym_op
 
+; Ignore section attribute fixes in experimental regexdev library
+[suppress_file]
+        soname_regexp = ^librte_regexdev\.
+
 ; Ignore changes in common mlx5 driver, should be all internal
 [suppress_file]
         soname_regexp = ^librte_common_mlx5\.
diff --git a/lib/regexdev/rte_regexdev.h b/lib/regexdev/rte_regexdev.h
index 4ba67b0c25..3bce8090f6 100644
--- a/lib/regexdev/rte_regexdev.h
+++ b/lib/regexdev/rte_regexdev.h
@@ -225,6 +225,9 @@ extern int rte_regexdev_logtype;
 } while (0)
 
 /**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
  * Check if dev_id is ready.
  *
  * @param dev_id
@@ -234,6 +237,7 @@ extern int rte_regexdev_logtype;
  *   - 0 if device state is not in ready state.
  *   - 1 if device state is ready state.
  */
+__rte_experimental
 int rte_regexdev_is_valid_dev(uint16_t dev_id);
 
 /**
diff --git a/lib/regexdev/rte_regexdev_driver.h b/lib/regexdev/rte_regexdev_driver.h
index 64742016c0..6246b144a6 100644
--- a/lib/regexdev/rte_regexdev_driver.h
+++ b/lib/regexdev/rte_regexdev_driver.h
@@ -32,6 +32,7 @@ extern "C" {
  *   A pointer to the RegEx device slot case of success,
  *   NULL otherwise.
  */
+__rte_internal
 struct rte_regexdev *rte_regexdev_register(const char *name);
 
 /**
@@ -41,6 +42,7 @@ struct rte_regexdev *rte_regexdev_register(const char *name);
  * @param dev
  *   Device to be released.
  */
+__rte_internal
 void rte_regexdev_unregister(struct rte_regexdev *dev);
 
 /**
@@ -50,6 +52,7 @@ void rte_regexdev_unregister(struct rte_regexdev *dev);
  * @param name
  *   The device name.
  */
+__rte_internal
 struct rte_regexdev *rte_regexdev_get_device_by_name(const char *name);
 
 #ifdef __cplusplus
diff --git a/lib/regexdev/version.map b/lib/regexdev/version.map
index 8db9b17018..988b909638 100644
--- a/lib/regexdev/version.map
+++ b/lib/regexdev/version.map
@@ -1,6 +1,7 @@
 EXPERIMENTAL {
 	global:
 
+	rte_regex_devices;
 	rte_regexdev_attr_get;
 	rte_regexdev_attr_set;
 	rte_regexdev_close;
@@ -11,6 +12,8 @@ EXPERIMENTAL {
 	rte_regexdev_enqueue_burst;
 	rte_regexdev_get_dev_id;
 	rte_regexdev_info_get;
+	rte_regexdev_is_valid_dev;
+	rte_regexdev_logtype;
 	rte_regexdev_queue_pair_setup;
 	rte_regexdev_rule_db_compile_activate;
 	rte_regexdev_rule_db_export;
@@ -24,3 +27,9 @@ EXPERIMENTAL {
 	rte_regexdev_xstats_names_get;
 	rte_regexdev_xstats_reset;
 };
+
+INTERNAL {
+	rte_regexdev_get_device_by_name;
+	rte_regexdev_register;
+	rte_regexdev_unregister;
+};
-- 
2.34.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 2/2] build: hide local symbols in shared libraries
  2022-03-06  9:20 [PATCH 0/2] add missing local symbols catch-all Thomas Monjalon
  2022-03-06  9:20 ` [PATCH 1/2] regexdev: fix section attribute of symbols Thomas Monjalon
@ 2022-03-06  9:20 ` Thomas Monjalon
  2022-03-07 13:14 ` [PATCH 0/2] add missing local symbols catch-all David Marchand
  2022-03-08 14:24 ` [PATCH v2 " Thomas Monjalon
  3 siblings, 0 replies; 12+ messages in thread
From: Thomas Monjalon @ 2022-03-06  9:20 UTC (permalink / raw)
  To: dev
  Cc: stable, Ray Kinsella, Parav Pandit, Xueming Li, Elena Agostini,
	Ori Kam, Andrew Rybchenko

The symbols which are not listed in the version script
are exported by default.
Adding a local section with a wildcard make non-listed functions
and variables as hidden, as it should be in all version.map files.

These are the changes done in the shared libraries:
- DF .text  Base          auxiliary_add_device
- DF .text  Base          auxiliary_dev_exists
- DF .text  Base          auxiliary_dev_iterate
- DF .text  Base          auxiliary_insert_device
- DF .text  Base          auxiliary_is_ignored_device
- DF .text  Base          auxiliary_match
- DF .text  Base          auxiliary_on_scan
- DF .text  Base          auxiliary_scan
- DO .bss   Base          auxiliary_bus_logtype
- DO .data  Base          auxiliary_bus
- DO .bss   Base          gpu_logtype

There is no impact on regexdev library.

Because these local symbols were exported as non-internal
in DPDK 21.11, any change in these functions would break the ABI.
Exception rules are added for these experimental libraries,
so the ABI check will skip them until the next ABI version.

Fixes: 1afce3086cf4 ("bus/auxiliary: introduce auxiliary bus")
Fixes: 8b8036a66e3d ("gpudev: introduce GPU device class library")
Cc: stable@dpdk.org

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 devtools/libabigail.abignore      | 8 ++++++++
 drivers/bus/auxiliary/version.map | 2 ++
 lib/gpudev/version.map            | 2 ++
 lib/regexdev/version.map          | 2 ++
 4 files changed, 14 insertions(+)

diff --git a/devtools/libabigail.abignore b/devtools/libabigail.abignore
index cff7a293ae..d698d8199d 100644
--- a/devtools/libabigail.abignore
+++ b/devtools/libabigail.abignore
@@ -28,3 +28,11 @@
 ; Ignore changes in common mlx5 driver, should be all internal
 [suppress_file]
         soname_regexp = ^librte_common_mlx5\.
+
+; Ignore visibility fix of local functions in experimental auxiliary driver
+[suppress_file]
+        soname_regexp = ^librte_bus_auxiliary\.
+
+; Ignore visibility fix of local functions in experimental gpudev library
+[suppress_file]
+        soname_regexp = ^librte_gpudev\.
diff --git a/drivers/bus/auxiliary/version.map b/drivers/bus/auxiliary/version.map
index a52260657c..dc993e84ff 100644
--- a/drivers/bus/auxiliary/version.map
+++ b/drivers/bus/auxiliary/version.map
@@ -4,4 +4,6 @@ EXPERIMENTAL {
 	# added in 21.08
 	rte_auxiliary_register;
 	rte_auxiliary_unregister;
+
+	local: *;
 };
diff --git a/lib/gpudev/version.map b/lib/gpudev/version.map
index b23e3fd6eb..a2c8ce5759 100644
--- a/lib/gpudev/version.map
+++ b/lib/gpudev/version.map
@@ -39,4 +39,6 @@ INTERNAL {
 	rte_gpu_get_by_name;
 	rte_gpu_notify;
 	rte_gpu_release;
+
+	local: *;
 };
diff --git a/lib/regexdev/version.map b/lib/regexdev/version.map
index 988b909638..3c6e9fffa1 100644
--- a/lib/regexdev/version.map
+++ b/lib/regexdev/version.map
@@ -26,6 +26,8 @@ EXPERIMENTAL {
 	rte_regexdev_xstats_get;
 	rte_regexdev_xstats_names_get;
 	rte_regexdev_xstats_reset;
+
+	local: *;
 };
 
 INTERNAL {
-- 
2.34.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [PATCH 1/2] regexdev: fix section attribute of symbols
  2022-03-06  9:20 ` [PATCH 1/2] regexdev: fix section attribute of symbols Thomas Monjalon
@ 2022-03-07 10:15   ` Ori Kam
  0 siblings, 0 replies; 12+ messages in thread
From: Ori Kam @ 2022-03-07 10:15 UTC (permalink / raw)
  To: NBU-Contact-Thomas Monjalon (EXTERNAL), dev
  Cc: stable, Ray Kinsella, Jerin Jacob, Pavan Nikhilesh

Hi Thomas,

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Sunday, March 6, 2022 11:20 AM
> To: dev@dpdk.org
> Cc: stable@dpdk.org; Ray Kinsella <mdr@ashroe.eu>; Ori Kam <orika@nvidia.com>; Jerin Jacob
> <jerinj@marvell.com>; Pavan Nikhilesh <pbhagavatula@marvell.com>
> Subject: [PATCH 1/2] regexdev: fix section attribute of symbols
> 
> The functions used by the drivers must be internal,
> while the function and variables used in inline functions
> must be experimental.
> 
> These are the changes done in the shared libraries:
> - DF .text  Base          rte_regexdev_get_device_by_name
> + DF .text  INTERNAL      rte_regexdev_get_device_by_name
> - DF .text  Base          rte_regexdev_register
> + DF .text  INTERNAL      rte_regexdev_register
> - DF .text  Base          rte_regexdev_unregister
> + DF .text  INTERNAL      rte_regexdev_unregister
> - DF .text  Base          rte_regexdev_is_valid_dev
> + DF .text  EXPERIMENTAL  rte_regexdev_is_valid_dev
> - DO .bss   Base          rte_regex_devices
> + DO .bss   EXPERIMENTAL  rte_regex_devices
> - DO .bss   Base          rte_regexdev_logtype
> + DO .bss   EXPERIMENTAL  rte_regexdev_logtype
> 
> Because these symbols were exported in the default section in DPDK 21.11,
> any change in these functions would be seen as incompatible
> by the ABI compatibility check.
> An exception rule is added for this experimental library,
> so the ABI check will skip it until the next ABI version.
> 
> Fixes: bab9497ef78b ("regexdev: introduce API")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
 
Acked-by: Ori Kam <orika@nvidia.com>

Best,
Ori



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/2] add missing local symbols catch-all
  2022-03-06  9:20 [PATCH 0/2] add missing local symbols catch-all Thomas Monjalon
  2022-03-06  9:20 ` [PATCH 1/2] regexdev: fix section attribute of symbols Thomas Monjalon
  2022-03-06  9:20 ` [PATCH 2/2] build: hide local symbols in shared libraries Thomas Monjalon
@ 2022-03-07 13:14 ` David Marchand
  2022-03-08 14:24 ` [PATCH v2 " Thomas Monjalon
  3 siblings, 0 replies; 12+ messages in thread
From: David Marchand @ 2022-03-07 13:14 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

On Sun, Mar 6, 2022 at 10:20 AM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> The libraries (and driver) regexdev, gpudev and auxiliary bus
> were missing "local:*" in their version.map linker script
> which catch all non-listed functions to be hidden in shared libraries.
>
> Thomas Monjalon (2):
>   regexdev: fix section attribute of symbols
>   build: hide local symbols in shared libraries
>
>  devtools/libabigail.abignore       | 12 ++++++++++++
>  drivers/bus/auxiliary/version.map  |  2 ++
>  lib/gpudev/version.map             |  2 ++
>  lib/regexdev/rte_regexdev.h        |  4 ++++
>  lib/regexdev/rte_regexdev_driver.h |  3 +++
>  lib/regexdev/version.map           | 11 +++++++++++
>  6 files changed, 34 insertions(+)

The series lgtm.


What do you think of adding a check like:

diff --git a/devtools/check-symbol-maps.sh b/devtools/check-symbol-maps.sh
index 5bd290ac97..4b7e340833 100755
--- a/devtools/check-symbol-maps.sh
+++ b/devtools/check-symbol-maps.sh
@@ -53,4 +53,18 @@ if [ -n "$duplicate_symbols" ] ; then
     ret=1
 fi

+check_local_guard ()
+{
+    for map in $@ ; do
+        grep -L local: $map || true
+    done
+}
+
+local_missing_map=$(check_local_guard $@)
+if [ -n "$local_missing_map" ] ; then
+    echo "Following maps are missing a local: guard:"
+    echo "$local_missing_map"
+    ret=1
+fi
+
 exit $ret



$ ./devtools/check-symbol-maps.sh
Following maps are missing a local: guard:
lib/regexdev/version.map
lib/gpudev/version.map
drivers/bus/auxiliary/version.map



-- 
David Marchand


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 0/2] add missing local symbols catch-all
  2022-03-06  9:20 [PATCH 0/2] add missing local symbols catch-all Thomas Monjalon
                   ` (2 preceding siblings ...)
  2022-03-07 13:14 ` [PATCH 0/2] add missing local symbols catch-all David Marchand
@ 2022-03-08 14:24 ` Thomas Monjalon
  2022-03-08 14:24   ` [PATCH v2 1/2] regexdev: fix section attribute of symbols Thomas Monjalon
                     ` (2 more replies)
  3 siblings, 3 replies; 12+ messages in thread
From: Thomas Monjalon @ 2022-03-08 14:24 UTC (permalink / raw)
  To: dev; +Cc: david.marchand

The libraries (and driver) regexdev, gpudev and auxiliary bus
were missing "local:*" in their version.map linker script
which catch all non-listed functions to be hidden in shared libraries.

v2: add a devtools check

Thomas Monjalon (2):
  regexdev: fix section attribute of symbols
  build: hide local symbols in shared libraries

 devtools/check-symbol-maps.sh      |  7 +++++++
 devtools/libabigail.abignore       | 12 ++++++++++++
 drivers/bus/auxiliary/version.map  |  2 ++
 lib/gpudev/version.map             |  2 ++
 lib/regexdev/rte_regexdev.h        |  4 ++++
 lib/regexdev/rte_regexdev_driver.h |  3 +++
 lib/regexdev/version.map           | 11 +++++++++++
 7 files changed, 41 insertions(+)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 1/2] regexdev: fix section attribute of symbols
  2022-03-08 14:24 ` [PATCH v2 " Thomas Monjalon
@ 2022-03-08 14:24   ` Thomas Monjalon
  2022-03-08 14:24   ` [PATCH v2 2/2] build: hide local symbols in shared libraries Thomas Monjalon
  2022-03-08 14:31   ` [PATCH v2 0/2] add missing local symbols catch-all Thomas Monjalon
  2 siblings, 0 replies; 12+ messages in thread
From: Thomas Monjalon @ 2022-03-08 14:24 UTC (permalink / raw)
  To: dev
  Cc: david.marchand, stable, Ori Kam, Ray Kinsella, Pavan Nikhilesh,
	Jerin Jacob

The functions used by the drivers must be internal,
while the function and variables used in inline functions
must be experimental.

These are the changes done in the shared library:
- DF .text  Base          rte_regexdev_get_device_by_name
+ DF .text  INTERNAL      rte_regexdev_get_device_by_name
- DF .text  Base          rte_regexdev_register
+ DF .text  INTERNAL      rte_regexdev_register
- DF .text  Base          rte_regexdev_unregister
+ DF .text  INTERNAL      rte_regexdev_unregister
- DF .text  Base          rte_regexdev_is_valid_dev
+ DF .text  EXPERIMENTAL  rte_regexdev_is_valid_dev
- DO .bss   Base          rte_regex_devices
+ DO .bss   EXPERIMENTAL  rte_regex_devices
- DO .bss   Base          rte_regexdev_logtype
+ DO .bss   EXPERIMENTAL  rte_regexdev_logtype

Because these symbols were exported in the default section in DPDK 21.11,
any change in these functions would be seen as incompatible
by the ABI compatibility check.
An exception rule is added for this experimental library,
so the ABI check will skip it until the next ABI version.

Fixes: bab9497ef78b ("regexdev: introduce API")
Cc: stable@dpdk.org

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
Acked-by: Ori Kam <orika@nvidia.com>
---
 devtools/libabigail.abignore       | 4 ++++
 lib/regexdev/rte_regexdev.h        | 4 ++++
 lib/regexdev/rte_regexdev_driver.h | 3 +++
 lib/regexdev/version.map           | 9 +++++++++
 4 files changed, 20 insertions(+)

diff --git a/devtools/libabigail.abignore b/devtools/libabigail.abignore
index 9c921c47d4..18c11c80c6 100644
--- a/devtools/libabigail.abignore
+++ b/devtools/libabigail.abignore
@@ -25,6 +25,10 @@
 [suppress_type]
         name = rte_crypto_asym_op
 
+; Ignore section attribute fixes in experimental regexdev library
+[suppress_file]
+        soname_regexp = ^librte_regexdev\.
+
 ; Ignore changes in common mlx5 driver, should be all internal
 [suppress_file]
         soname_regexp = ^librte_common_mlx5\.
diff --git a/lib/regexdev/rte_regexdev.h b/lib/regexdev/rte_regexdev.h
index 4ba67b0c25..3bce8090f6 100644
--- a/lib/regexdev/rte_regexdev.h
+++ b/lib/regexdev/rte_regexdev.h
@@ -225,6 +225,9 @@ extern int rte_regexdev_logtype;
 } while (0)
 
 /**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
  * Check if dev_id is ready.
  *
  * @param dev_id
@@ -234,6 +237,7 @@ extern int rte_regexdev_logtype;
  *   - 0 if device state is not in ready state.
  *   - 1 if device state is ready state.
  */
+__rte_experimental
 int rte_regexdev_is_valid_dev(uint16_t dev_id);
 
 /**
diff --git a/lib/regexdev/rte_regexdev_driver.h b/lib/regexdev/rte_regexdev_driver.h
index 64742016c0..6246b144a6 100644
--- a/lib/regexdev/rte_regexdev_driver.h
+++ b/lib/regexdev/rte_regexdev_driver.h
@@ -32,6 +32,7 @@ extern "C" {
  *   A pointer to the RegEx device slot case of success,
  *   NULL otherwise.
  */
+__rte_internal
 struct rte_regexdev *rte_regexdev_register(const char *name);
 
 /**
@@ -41,6 +42,7 @@ struct rte_regexdev *rte_regexdev_register(const char *name);
  * @param dev
  *   Device to be released.
  */
+__rte_internal
 void rte_regexdev_unregister(struct rte_regexdev *dev);
 
 /**
@@ -50,6 +52,7 @@ void rte_regexdev_unregister(struct rte_regexdev *dev);
  * @param name
  *   The device name.
  */
+__rte_internal
 struct rte_regexdev *rte_regexdev_get_device_by_name(const char *name);
 
 #ifdef __cplusplus
diff --git a/lib/regexdev/version.map b/lib/regexdev/version.map
index 8db9b17018..988b909638 100644
--- a/lib/regexdev/version.map
+++ b/lib/regexdev/version.map
@@ -1,6 +1,7 @@
 EXPERIMENTAL {
 	global:
 
+	rte_regex_devices;
 	rte_regexdev_attr_get;
 	rte_regexdev_attr_set;
 	rte_regexdev_close;
@@ -11,6 +12,8 @@ EXPERIMENTAL {
 	rte_regexdev_enqueue_burst;
 	rte_regexdev_get_dev_id;
 	rte_regexdev_info_get;
+	rte_regexdev_is_valid_dev;
+	rte_regexdev_logtype;
 	rte_regexdev_queue_pair_setup;
 	rte_regexdev_rule_db_compile_activate;
 	rte_regexdev_rule_db_export;
@@ -24,3 +27,9 @@ EXPERIMENTAL {
 	rte_regexdev_xstats_names_get;
 	rte_regexdev_xstats_reset;
 };
+
+INTERNAL {
+	rte_regexdev_get_device_by_name;
+	rte_regexdev_register;
+	rte_regexdev_unregister;
+};
-- 
2.34.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 2/2] build: hide local symbols in shared libraries
  2022-03-08 14:24 ` [PATCH v2 " Thomas Monjalon
  2022-03-08 14:24   ` [PATCH v2 1/2] regexdev: fix section attribute of symbols Thomas Monjalon
@ 2022-03-08 14:24   ` Thomas Monjalon
  2022-03-09 10:58     ` Kevin Traynor
  2022-04-15 14:56     ` Ray Kinsella
  2022-03-08 14:31   ` [PATCH v2 0/2] add missing local symbols catch-all Thomas Monjalon
  2 siblings, 2 replies; 12+ messages in thread
From: Thomas Monjalon @ 2022-03-08 14:24 UTC (permalink / raw)
  To: dev
  Cc: david.marchand, stable, Ray Kinsella, Parav Pandit, Xueming Li,
	Elena Agostini, Ori Kam, Andrew Rybchenko

The symbols which are not listed in the version script
are exported by default.
Adding a local section with a wildcard make non-listed functions
and variables as hidden, as it should be in all version.map files.

These are the changes done in the shared libraries:
- DF .text  Base          auxiliary_add_device
- DF .text  Base          auxiliary_dev_exists
- DF .text  Base          auxiliary_dev_iterate
- DF .text  Base          auxiliary_insert_device
- DF .text  Base          auxiliary_is_ignored_device
- DF .text  Base          auxiliary_match
- DF .text  Base          auxiliary_on_scan
- DF .text  Base          auxiliary_scan
- DO .bss   Base          auxiliary_bus_logtype
- DO .data  Base          auxiliary_bus
- DO .bss   Base          gpu_logtype

There is no impact on regexdev library.

Because these local symbols were exported as non-internal
in DPDK 21.11, any change in these functions would break the ABI.
Exception rules are added for these experimental libraries,
so the ABI check will skip them until the next ABI version.

A check is added to avoid such miss in future.

Fixes: 1afce3086cf4 ("bus/auxiliary: introduce auxiliary bus")
Fixes: 8b8036a66e3d ("gpudev: introduce GPU device class library")
Cc: stable@dpdk.org

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 devtools/check-symbol-maps.sh     | 7 +++++++
 devtools/libabigail.abignore      | 8 ++++++++
 drivers/bus/auxiliary/version.map | 2 ++
 lib/gpudev/version.map            | 2 ++
 lib/regexdev/version.map          | 2 ++
 5 files changed, 21 insertions(+)

diff --git a/devtools/check-symbol-maps.sh b/devtools/check-symbol-maps.sh
index 5bd290ac97..8266fdf9ea 100755
--- a/devtools/check-symbol-maps.sh
+++ b/devtools/check-symbol-maps.sh
@@ -53,4 +53,11 @@ if [ -n "$duplicate_symbols" ] ; then
     ret=1
 fi
 
+local_miss_maps=$(grep -L 'local: \*;' $@)
+if [ -n "$local_miss_maps" ] ; then
+    echo "Found maps without local catch-all:"
+    echo "$local_miss_maps"
+    ret=1
+fi
+
 exit $ret
diff --git a/devtools/libabigail.abignore b/devtools/libabigail.abignore
index 18c11c80c6..c618f20032 100644
--- a/devtools/libabigail.abignore
+++ b/devtools/libabigail.abignore
@@ -32,3 +32,11 @@
 ; Ignore changes in common mlx5 driver, should be all internal
 [suppress_file]
         soname_regexp = ^librte_common_mlx5\.
+
+; Ignore visibility fix of local functions in experimental auxiliary driver
+[suppress_file]
+        soname_regexp = ^librte_bus_auxiliary\.
+
+; Ignore visibility fix of local functions in experimental gpudev library
+[suppress_file]
+        soname_regexp = ^librte_gpudev\.
diff --git a/drivers/bus/auxiliary/version.map b/drivers/bus/auxiliary/version.map
index a52260657c..dc993e84ff 100644
--- a/drivers/bus/auxiliary/version.map
+++ b/drivers/bus/auxiliary/version.map
@@ -4,4 +4,6 @@ EXPERIMENTAL {
 	# added in 21.08
 	rte_auxiliary_register;
 	rte_auxiliary_unregister;
+
+	local: *;
 };
diff --git a/lib/gpudev/version.map b/lib/gpudev/version.map
index b23e3fd6eb..a2c8ce5759 100644
--- a/lib/gpudev/version.map
+++ b/lib/gpudev/version.map
@@ -39,4 +39,6 @@ INTERNAL {
 	rte_gpu_get_by_name;
 	rte_gpu_notify;
 	rte_gpu_release;
+
+	local: *;
 };
diff --git a/lib/regexdev/version.map b/lib/regexdev/version.map
index 988b909638..3c6e9fffa1 100644
--- a/lib/regexdev/version.map
+++ b/lib/regexdev/version.map
@@ -26,6 +26,8 @@ EXPERIMENTAL {
 	rte_regexdev_xstats_get;
 	rte_regexdev_xstats_names_get;
 	rte_regexdev_xstats_reset;
+
+	local: *;
 };
 
 INTERNAL {
-- 
2.34.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 0/2] add missing local symbols catch-all
  2022-03-08 14:24 ` [PATCH v2 " Thomas Monjalon
  2022-03-08 14:24   ` [PATCH v2 1/2] regexdev: fix section attribute of symbols Thomas Monjalon
  2022-03-08 14:24   ` [PATCH v2 2/2] build: hide local symbols in shared libraries Thomas Monjalon
@ 2022-03-08 14:31   ` Thomas Monjalon
  2 siblings, 0 replies; 12+ messages in thread
From: Thomas Monjalon @ 2022-03-08 14:31 UTC (permalink / raw)
  To: dev; +Cc: david.marchand

08/03/2022 15:24, Thomas Monjalon:
> The libraries (and driver) regexdev, gpudev and auxiliary bus
> were missing "local:*" in their version.map linker script
> which catch all non-listed functions to be hidden in shared libraries.
> 
> v2: add a devtools check
> 
> Thomas Monjalon (2):
>   regexdev: fix section attribute of symbols
>   build: hide local symbols in shared libraries

Applied




^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 2/2] build: hide local symbols in shared libraries
  2022-03-08 14:24   ` [PATCH v2 2/2] build: hide local symbols in shared libraries Thomas Monjalon
@ 2022-03-09 10:58     ` Kevin Traynor
  2022-03-09 18:54       ` Thomas Monjalon
  2022-04-15 14:56     ` Ray Kinsella
  1 sibling, 1 reply; 12+ messages in thread
From: Kevin Traynor @ 2022-03-09 10:58 UTC (permalink / raw)
  To: Thomas Monjalon, dev
  Cc: david.marchand, stable, Ray Kinsella, Parav Pandit, Xueming Li,
	Elena Agostini, Ori Kam, Andrew Rybchenko, Michael Baum

Hi Thomas,

On 08/03/2022 14:24, Thomas Monjalon wrote:
> The symbols which are not listed in the version script
> are exported by default.
> Adding a local section with a wildcard make non-listed functions
> and variables as hidden, as it should be in all version.map files.
> 
> These are the changes done in the shared libraries:
> - DF .text  Base          auxiliary_add_device
> - DF .text  Base          auxiliary_dev_exists
> - DF .text  Base          auxiliary_dev_iterate
> - DF .text  Base          auxiliary_insert_device
> - DF .text  Base          auxiliary_is_ignored_device
> - DF .text  Base          auxiliary_match
> - DF .text  Base          auxiliary_on_scan
> - DF .text  Base          auxiliary_scan
> - DO .bss   Base          auxiliary_bus_logtype
> - DO .data  Base          auxiliary_bus
> - DO .bss   Base          gpu_logtype
> 
> There is no impact on regexdev library.
> 
> Because these local symbols were exported as non-internal
> in DPDK 21.11, any change in these functions would break the ABI.
> Exception rules are added for these experimental libraries,
> so the ABI check will skip them until the next ABI version.
> 
> A check is added to avoid such miss in future.
> 
> Fixes: 1afce3086cf4 ("bus/auxiliary: introduce auxiliary bus")
> Fixes: 8b8036a66e3d ("gpudev: introduce GPU device class library")
> Cc: stable@dpdk.org
> 

If I take this 2/2 for 21.11.1, then I also need to backport [0] so I 
won't have errors for common_mlx5.

Any problem with taking both?

[0]
commit c2e3059a10f2389b791d5d485fe71e666984c193
Author: Michael Baum <michaelba@nvidia.com>
Date:   Fri Feb 25 01:25:06 2022 +0200

     common/mlx5: consider local functions as internal


> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
>   devtools/check-symbol-maps.sh     | 7 +++++++
>   devtools/libabigail.abignore      | 8 ++++++++
>   drivers/bus/auxiliary/version.map | 2 ++
>   lib/gpudev/version.map            | 2 ++
>   lib/regexdev/version.map          | 2 ++
>   5 files changed, 21 insertions(+)
> 
> diff --git a/devtools/check-symbol-maps.sh b/devtools/check-symbol-maps.sh
> index 5bd290ac97..8266fdf9ea 100755
> --- a/devtools/check-symbol-maps.sh
> +++ b/devtools/check-symbol-maps.sh
> @@ -53,4 +53,11 @@ if [ -n "$duplicate_symbols" ] ; then
>       ret=1
>   fi
>   
> +local_miss_maps=$(grep -L 'local: \*;' $@)
> +if [ -n "$local_miss_maps" ] ; then
> +    echo "Found maps without local catch-all:"
> +    echo "$local_miss_maps"
> +    ret=1
> +fi
> +
>   exit $ret
> diff --git a/devtools/libabigail.abignore b/devtools/libabigail.abignore
> index 18c11c80c6..c618f20032 100644
> --- a/devtools/libabigail.abignore
> +++ b/devtools/libabigail.abignore
> @@ -32,3 +32,11 @@
>   ; Ignore changes in common mlx5 driver, should be all internal
>   [suppress_file]
>           soname_regexp = ^librte_common_mlx5\.
> +
> +; Ignore visibility fix of local functions in experimental auxiliary driver
> +[suppress_file]
> +        soname_regexp = ^librte_bus_auxiliary\.
> +
> +; Ignore visibility fix of local functions in experimental gpudev library
> +[suppress_file]
> +        soname_regexp = ^librte_gpudev\.
> diff --git a/drivers/bus/auxiliary/version.map b/drivers/bus/auxiliary/version.map
> index a52260657c..dc993e84ff 100644
> --- a/drivers/bus/auxiliary/version.map
> +++ b/drivers/bus/auxiliary/version.map
> @@ -4,4 +4,6 @@ EXPERIMENTAL {
>   	# added in 21.08
>   	rte_auxiliary_register;
>   	rte_auxiliary_unregister;
> +
> +	local: *;
>   };
> diff --git a/lib/gpudev/version.map b/lib/gpudev/version.map
> index b23e3fd6eb..a2c8ce5759 100644
> --- a/lib/gpudev/version.map
> +++ b/lib/gpudev/version.map
> @@ -39,4 +39,6 @@ INTERNAL {
>   	rte_gpu_get_by_name;
>   	rte_gpu_notify;
>   	rte_gpu_release;
> +
> +	local: *;
>   };
> diff --git a/lib/regexdev/version.map b/lib/regexdev/version.map
> index 988b909638..3c6e9fffa1 100644
> --- a/lib/regexdev/version.map
> +++ b/lib/regexdev/version.map
> @@ -26,6 +26,8 @@ EXPERIMENTAL {
>   	rte_regexdev_xstats_get;
>   	rte_regexdev_xstats_names_get;
>   	rte_regexdev_xstats_reset;
> +
> +	local: *;
>   };
>   
>   INTERNAL {


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 2/2] build: hide local symbols in shared libraries
  2022-03-09 10:58     ` Kevin Traynor
@ 2022-03-09 18:54       ` Thomas Monjalon
  0 siblings, 0 replies; 12+ messages in thread
From: Thomas Monjalon @ 2022-03-09 18:54 UTC (permalink / raw)
  To: Kevin Traynor
  Cc: dev, david.marchand, stable, Ray Kinsella, Parav Pandit,
	Xueming Li, Elena Agostini, Ori Kam, Andrew Rybchenko,
	Michael Baum

09/03/2022 11:58, Kevin Traynor:
> Hi Thomas,
> 
> On 08/03/2022 14:24, Thomas Monjalon wrote:
> > The symbols which are not listed in the version script
> > are exported by default.
> > Adding a local section with a wildcard make non-listed functions
> > and variables as hidden, as it should be in all version.map files.
> > 
> > These are the changes done in the shared libraries:
> > - DF .text  Base          auxiliary_add_device
> > - DF .text  Base          auxiliary_dev_exists
> > - DF .text  Base          auxiliary_dev_iterate
> > - DF .text  Base          auxiliary_insert_device
> > - DF .text  Base          auxiliary_is_ignored_device
> > - DF .text  Base          auxiliary_match
> > - DF .text  Base          auxiliary_on_scan
> > - DF .text  Base          auxiliary_scan
> > - DO .bss   Base          auxiliary_bus_logtype
> > - DO .data  Base          auxiliary_bus
> > - DO .bss   Base          gpu_logtype
> > 
> > There is no impact on regexdev library.
> > 
> > Because these local symbols were exported as non-internal
> > in DPDK 21.11, any change in these functions would break the ABI.
> > Exception rules are added for these experimental libraries,
> > so the ABI check will skip them until the next ABI version.
> > 
> > A check is added to avoid such miss in future.
> > 
> > Fixes: 1afce3086cf4 ("bus/auxiliary: introduce auxiliary bus")
> > Fixes: 8b8036a66e3d ("gpudev: introduce GPU device class library")
> > Cc: stable@dpdk.org
> > 
> 
> If I take this 2/2 for 21.11.1, then I also need to backport [0] so I 
> won't have errors for common_mlx5.
> 
> Any problem with taking both?
> 
> [0]
> commit c2e3059a10f2389b791d5d485fe71e666984c193
> Author: Michael Baum <michaelba@nvidia.com>
> Date:   Fri Feb 25 01:25:06 2022 +0200
> 
>      common/mlx5: consider local functions as internal

I think that's fine to backport this as well.





^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 2/2] build: hide local symbols in shared libraries
  2022-03-08 14:24   ` [PATCH v2 2/2] build: hide local symbols in shared libraries Thomas Monjalon
  2022-03-09 10:58     ` Kevin Traynor
@ 2022-04-15 14:56     ` Ray Kinsella
  1 sibling, 0 replies; 12+ messages in thread
From: Ray Kinsella @ 2022-04-15 14:56 UTC (permalink / raw)
  To: Thomas Monjalon
  Cc: dev, david.marchand, stable, Parav Pandit, Xueming Li,
	Elena Agostini, Ori Kam, Andrew Rybchenko


Thomas Monjalon <thomas@monjalon.net> writes:

> The symbols which are not listed in the version script
> are exported by default.
> Adding a local section with a wildcard make non-listed functions
> and variables as hidden, as it should be in all version.map files.
>
> These are the changes done in the shared libraries:
> - DF .text  Base          auxiliary_add_device
> - DF .text  Base          auxiliary_dev_exists
> - DF .text  Base          auxiliary_dev_iterate
> - DF .text  Base          auxiliary_insert_device
> - DF .text  Base          auxiliary_is_ignored_device
> - DF .text  Base          auxiliary_match
> - DF .text  Base          auxiliary_on_scan
> - DF .text  Base          auxiliary_scan
> - DO .bss   Base          auxiliary_bus_logtype
> - DO .data  Base          auxiliary_bus
> - DO .bss   Base          gpu_logtype
>
> There is no impact on regexdev library.
>
> Because these local symbols were exported as non-internal
> in DPDK 21.11, any change in these functions would break the ABI.
> Exception rules are added for these experimental libraries,
> so the ABI check will skip them until the next ABI version.
>
> A check is added to avoid such miss in future.
>
> Fixes: 1afce3086cf4 ("bus/auxiliary: introduce auxiliary bus")
> Fixes: 8b8036a66e3d ("gpudev: introduce GPU device class library")
> Cc: stable@dpdk.org

Good catch, I see you fixed a few of these recently.

>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
>  devtools/check-symbol-maps.sh     | 7 +++++++
>  devtools/libabigail.abignore      | 8 ++++++++
>  drivers/bus/auxiliary/version.map | 2 ++
>  lib/gpudev/version.map            | 2 ++
>  lib/regexdev/version.map          | 2 ++
>  5 files changed, 21 insertions(+)
>
> diff --git a/devtools/check-symbol-maps.sh b/devtools/check-symbol-maps.sh
> index 5bd290ac97..8266fdf9ea 100755
> --- a/devtools/check-symbol-maps.sh
> +++ b/devtools/check-symbol-maps.sh
> @@ -53,4 +53,11 @@ if [ -n "$duplicate_symbols" ] ; then
>      ret=1
>  fi
>  
> +local_miss_maps=$(grep -L 'local: \*;' $@)
> +if [ -n "$local_miss_maps" ] ; then
> +    echo "Found maps without local catch-all:"
> +    echo "$local_miss_maps"
> +    ret=1
> +fi
> +
>  exit $ret
> diff --git a/devtools/libabigail.abignore b/devtools/libabigail.abignore
> index 18c11c80c6..c618f20032 100644
> --- a/devtools/libabigail.abignore
> +++ b/devtools/libabigail.abignore
> @@ -32,3 +32,11 @@
>  ; Ignore changes in common mlx5 driver, should be all internal
>  [suppress_file]
>          soname_regexp = ^librte_common_mlx5\.
> +
> +; Ignore visibility fix of local functions in experimental auxiliary driver
> +[suppress_file]
> +        soname_regexp = ^librte_bus_auxiliary\.
> +
> +; Ignore visibility fix of local functions in experimental gpudev library
> +[suppress_file]
> +        soname_regexp = ^librte_gpudev\.
> diff --git a/drivers/bus/auxiliary/version.map b/drivers/bus/auxiliary/version.map
> index a52260657c..dc993e84ff 100644
> --- a/drivers/bus/auxiliary/version.map
> +++ b/drivers/bus/auxiliary/version.map
> @@ -4,4 +4,6 @@ EXPERIMENTAL {
>  	# added in 21.08
>  	rte_auxiliary_register;
>  	rte_auxiliary_unregister;
> +
> +	local: *;
>  };
> diff --git a/lib/gpudev/version.map b/lib/gpudev/version.map
> index b23e3fd6eb..a2c8ce5759 100644
> --- a/lib/gpudev/version.map
> +++ b/lib/gpudev/version.map
> @@ -39,4 +39,6 @@ INTERNAL {
>  	rte_gpu_get_by_name;
>  	rte_gpu_notify;
>  	rte_gpu_release;
> +
> +	local: *;
>  };
> diff --git a/lib/regexdev/version.map b/lib/regexdev/version.map
> index 988b909638..3c6e9fffa1 100644
> --- a/lib/regexdev/version.map
> +++ b/lib/regexdev/version.map
> @@ -26,6 +26,8 @@ EXPERIMENTAL {
>  	rte_regexdev_xstats_get;
>  	rte_regexdev_xstats_names_get;
>  	rte_regexdev_xstats_reset;
> +
> +	local: *;
>  };
>  
>  INTERNAL {


-- 
Regards, Ray K

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2022-04-15 14:56 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-06  9:20 [PATCH 0/2] add missing local symbols catch-all Thomas Monjalon
2022-03-06  9:20 ` [PATCH 1/2] regexdev: fix section attribute of symbols Thomas Monjalon
2022-03-07 10:15   ` Ori Kam
2022-03-06  9:20 ` [PATCH 2/2] build: hide local symbols in shared libraries Thomas Monjalon
2022-03-07 13:14 ` [PATCH 0/2] add missing local symbols catch-all David Marchand
2022-03-08 14:24 ` [PATCH v2 " Thomas Monjalon
2022-03-08 14:24   ` [PATCH v2 1/2] regexdev: fix section attribute of symbols Thomas Monjalon
2022-03-08 14:24   ` [PATCH v2 2/2] build: hide local symbols in shared libraries Thomas Monjalon
2022-03-09 10:58     ` Kevin Traynor
2022-03-09 18:54       ` Thomas Monjalon
2022-04-15 14:56     ` Ray Kinsella
2022-03-08 14:31   ` [PATCH v2 0/2] add missing local symbols catch-all Thomas Monjalon

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).