[dpdk-dev] [RFC] pdump: remove deprecated APIs

[dpdk-dev] [RFC] pdump: remove deprecated APIs

[Tiwei Bie]

We already changed to use generic IPC in pdump since below commit:

commit 660098d61f57 ("pdump: use generic multi-process channel")

The `rte_pdump_set_socket_dir()`, the `path` parameter of
`rte_pdump_init()` and the `enum rte_pdump_socktype` have been
deprecated since then. This commit removes these deprecated
APIs and also bumps the pdump ABI.

Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
---
This patch is marked as RFC because the API and ABI changes should
also be documented by this patch in the `release_19_02.rst` which
doesn't exist currently. I will send a new version once we have it.

 app/test-pmd/testpmd.c                 |  2 +-
 doc/guides/prog_guide/pdump_lib.rst    | 14 ++------------
 doc/guides/rel_notes/deprecation.rst   |  7 -------
 lib/librte_pdump/Makefile              |  2 +-
 lib/librte_pdump/meson.build           |  2 +-
 lib/librte_pdump/rte_pdump.c           |  9 +--------
 lib/librte_pdump/rte_pdump.h           | 34 +---------------------------------
 lib/librte_pdump/rte_pdump_version.map |  1 -
 8 files changed, 7 insertions(+), 64 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 4c75587d0..a10bc40bb 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -3104,7 +3104,7 @@ main(int argc, char** argv)
 
 #ifdef RTE_LIBRTE_PDUMP
 	/* initialize packet capture framework */
-	rte_pdump_init(NULL);
+	rte_pdump_init();
 #endif
 
 	count = 0;
diff --git a/doc/guides/prog_guide/pdump_lib.rst b/doc/guides/prog_guide/pdump_lib.rst
index ed3c15e58..afb5b3411 100644
--- a/doc/guides/prog_guide/pdump_lib.rst
+++ b/doc/guides/prog_guide/pdump_lib.rst
@@ -34,10 +34,6 @@ or disable the packet capture, and to uninitialize it:
 * ``rte_pdump_uninit()``:
   This API uninitializes the packet capture framework.
 
-* ``rte_pdump_set_socket_dir()``:
-  This API sets the server and client socket paths.
-  Note: This API is not thread-safe.
-
 
 Operation
 ---------
@@ -60,8 +56,8 @@ enabling or disabling the packet capture.
 Implementation Details
 ----------------------
 
-The library API ``rte_pdump_init()``, initializes the packet capture framework by creating the pthread and the server
-socket. The server socket in the pthread context will be listening to the client requests to enable or disable the
+The library API ``rte_pdump_init()``, initializes the packet capture framework by creating the pdump server by calling
+``rte_mp_action_register()`` function. The server will listen to the client requests to enable or disable the
 packet capture.
 
 The library APIs ``rte_pdump_enable()`` and ``rte_pdump_enable_by_deviceid()`` enables the packet capture.
@@ -82,12 +78,6 @@ received from the server, the client socket is closed.
 The library API ``rte_pdump_uninit()``, uninitializes the packet capture framework by closing the pthread and the
 server socket.
 
-The library API ``rte_pdump_set_socket_dir()``, sets the given path as either server socket path
-or client socket path based on the ``type`` argument of the API.
-If the given path is ``NULL``, default path will be selected, i.e. either ``/var/run/.dpdk`` for root user or ``~/.dpdk``
-for non root user. Clients also need to call this API to set their server socket path if the server socket
-path is different from default path.
-
 
 Use Case: Packet Capturing
 --------------------------
diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 34b28234c..586bf98c5 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -48,10 +48,3 @@ Deprecation Notices
   PMDs that implement the latter.
   Target release for removal of the legacy API will be defined once most
   PMDs have switched to rte_flow.
-
-* pdump: As we changed to use generic IPC, some changes in APIs and structure
-  are expected in subsequent release.
-
-  - ``rte_pdump_set_socket_dir`` will be removed;
-  - The parameter, ``path``, of ``rte_pdump_init`` will be removed;
-  - The enum ``rte_pdump_socktype`` will be removed.
diff --git a/lib/librte_pdump/Makefile b/lib/librte_pdump/Makefile
index b241151dc..89593689a 100644
--- a/lib/librte_pdump/Makefile
+++ b/lib/librte_pdump/Makefile
@@ -12,7 +12,7 @@ LDLIBS += -lrte_eal -lrte_mempool -lrte_mbuf -lrte_ethdev
 
 EXPORT_MAP := rte_pdump_version.map
 
-LIBABIVER := 2
+LIBABIVER := 3
 
 # all source are stored in SRCS-y
 SRCS-$(CONFIG_RTE_LIBRTE_PDUMP) := rte_pdump.c
diff --git a/lib/librte_pdump/meson.build b/lib/librte_pdump/meson.build
index be80904b9..b4b4f26c5 100644
--- a/lib/librte_pdump/meson.build
+++ b/lib/librte_pdump/meson.build
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: BSD-3-Clause
 # Copyright(c) 2017 Intel Corporation
 
-version = 2
+version = 3
 sources = files('rte_pdump.c')
 headers = files('rte_pdump.h')
 allow_experimental_apis = true
diff --git a/lib/librte_pdump/rte_pdump.c b/lib/librte_pdump/rte_pdump.c
index 6c3a88581..4f38ac58b 100644
--- a/lib/librte_pdump/rte_pdump.c
+++ b/lib/librte_pdump/rte_pdump.c
@@ -406,7 +406,7 @@ pdump_server(const struct rte_mp_msg *mp_msg, const void *peer)
 }
 
 int
-rte_pdump_init(const char *path __rte_unused)
+rte_pdump_init(void)
 {
 	return rte_mp_action_register(PDUMP_MP, pdump_server);
 }
@@ -616,10 +616,3 @@ rte_pdump_disable_by_deviceid(char *device_id, uint16_t queue,
 
 	return ret;
 }
-
-int
-rte_pdump_set_socket_dir(const char *path __rte_unused,
-			 enum rte_pdump_socktype type __rte_unused)
-{
-	return 0;
-}
diff --git a/lib/librte_pdump/rte_pdump.h b/lib/librte_pdump/rte_pdump.h
index 673a2b070..6b00fc17a 100644
--- a/lib/librte_pdump/rte_pdump.h
+++ b/lib/librte_pdump/rte_pdump.h
@@ -29,25 +29,16 @@ enum {
 	RTE_PDUMP_FLAG_RXTX = (RTE_PDUMP_FLAG_RX|RTE_PDUMP_FLAG_TX)
 };
 
-enum rte_pdump_socktype {
-	RTE_PDUMP_SOCKET_SERVER = 1,
-	RTE_PDUMP_SOCKET_CLIENT = 2
-};
-
 /**
  * Initialize packet capturing handling
  *
  * Register the IPC action for communication with target (primary) process.
  *
- * @param path
- * This parameter is going to be deprecated; it was used for specifying the
- * directory path for server socket.
- *
  * @return
  *    0 on success, -1 on error
  */
 int
-rte_pdump_init(const char *path);
+rte_pdump_init(void);
 
 /**
  * Un initialize packet capturing handling
@@ -162,29 +153,6 @@ int
 rte_pdump_disable_by_deviceid(char *device_id, uint16_t queue,
 				uint32_t flags);
 
-/**
- * @deprecated
- * Allows applications to set server and client socket paths.
- * If specified path is null default path will be selected, i.e.
- *"/var/run/" for root user and "$HOME" for non root user.
- * Clients also need to call this API to set their server path if the
- * server path is different from default path.
- * This API is not thread-safe.
- *
- * @param path
- * directory path for server or client socket.
- * @param type
- * specifies RTE_PDUMP_SOCKET_SERVER if socket path is for server.
- * (or)
- * specifies RTE_PDUMP_SOCKET_CLIENT if socket path is for client.
- *
- * @return
- * 0 on success, -EINVAL on error
- *
- */
-__rte_deprecated int
-rte_pdump_set_socket_dir(const char *path, enum rte_pdump_socktype type);
-
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/librte_pdump/rte_pdump_version.map b/lib/librte_pdump/rte_pdump_version.map
index edec99a41..3e744f301 100644
--- a/lib/librte_pdump/rte_pdump_version.map
+++ b/lib/librte_pdump/rte_pdump_version.map
@@ -6,7 +6,6 @@ DPDK_16.07 {
 	rte_pdump_enable;
 	rte_pdump_enable_by_deviceid;
 	rte_pdump_init;
-	rte_pdump_set_socket_dir;
 	rte_pdump_uninit;
 
 	local: *;
-- 
2.14.5

[dpdk-dev] [PATCH] pdump: remove deprecated APIs

[Tiwei Bie]

We already changed to use generic IPC in pdump since below commit:

commit 660098d61f57 ("pdump: use generic multi-process channel")

The `rte_pdump_set_socket_dir()`, the `path` parameter of
`rte_pdump_init()` and the `enum rte_pdump_socktype` have been
deprecated since then. This commit removes these deprecated
APIs and also bumps the pdump ABI.

Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
---
changes:
- update release notes;
- minor updates in pdump_lib.rst;

 app/test-pmd/testpmd.c                 |  2 +-
 doc/guides/prog_guide/pdump_lib.rst    | 18 ++++--------------
 doc/guides/rel_notes/deprecation.rst   |  7 -------
 doc/guides/rel_notes/release_19_02.rst |  6 +++++-
 lib/librte_pdump/Makefile              |  2 +-
 lib/librte_pdump/meson.build           |  2 +-
 lib/librte_pdump/rte_pdump.c           |  9 +--------
 lib/librte_pdump/rte_pdump.h           | 34 +---------------------------------
 lib/librte_pdump/rte_pdump_version.map |  1 -
 9 files changed, 14 insertions(+), 67 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 4c75587d0..a10bc40bb 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -3104,7 +3104,7 @@ main(int argc, char** argv)
 
 #ifdef RTE_LIBRTE_PDUMP
 	/* initialize packet capture framework */
-	rte_pdump_init(NULL);
+	rte_pdump_init();
 #endif
 
 	count = 0;
diff --git a/doc/guides/prog_guide/pdump_lib.rst b/doc/guides/prog_guide/pdump_lib.rst
index ed3c15e58..2a0f1f397 100644
--- a/doc/guides/prog_guide/pdump_lib.rst
+++ b/doc/guides/prog_guide/pdump_lib.rst
@@ -34,10 +34,6 @@ or disable the packet capture, and to uninitialize it:
 * ``rte_pdump_uninit()``:
   This API uninitializes the packet capture framework.
 
-* ``rte_pdump_set_socket_dir()``:
-  This API sets the server and client socket paths.
-  Note: This API is not thread-safe.
-
 
 Operation
 ---------
@@ -60,8 +56,8 @@ enabling or disabling the packet capture.
 Implementation Details
 ----------------------
 
-The library API ``rte_pdump_init()``, initializes the packet capture framework by creating the pthread and the server
-socket. The server socket in the pthread context will be listening to the client requests to enable or disable the
+The library API ``rte_pdump_init()``, initializes the packet capture framework by creating the pdump server by calling
+``rte_mp_action_register()`` function. The server will listen to the client requests to enable or disable the
 packet capture.
 
 The library APIs ``rte_pdump_enable()`` and ``rte_pdump_enable_by_deviceid()`` enables the packet capture.
@@ -79,14 +75,8 @@ capture by removing the Ethernet RX and TX callbacks for the given port or devic
 also sends the response back to the client about the status of the request that was processed. After the response is
 received from the server, the client socket is closed.
 
-The library API ``rte_pdump_uninit()``, uninitializes the packet capture framework by closing the pthread and the
-server socket.
-
-The library API ``rte_pdump_set_socket_dir()``, sets the given path as either server socket path
-or client socket path based on the ``type`` argument of the API.
-If the given path is ``NULL``, default path will be selected, i.e. either ``/var/run/.dpdk`` for root user or ``~/.dpdk``
-for non root user. Clients also need to call this API to set their server socket path if the server socket
-path is different from default path.
+The library API ``rte_pdump_uninit()``, uninitializes the packet capture framework by calling ``rte_mp_action_unregister()``
+function.
 
 
 Use Case: Packet Capturing
diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index b48486d36..ac7fb29a7 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -98,10 +98,3 @@ Deprecation Notices
   - The size and layout of ``rte_cryptodev_qp_conf`` and syntax of
     ``rte_cryptodev_queue_pair_setup`` will change to to allow to use
     two different mempools for crypto and device private sessions.
-
-* pdump: As we changed to use generic IPC, some changes in APIs and structure
-  are expected in subsequent release.
-
-  - ``rte_pdump_set_socket_dir`` will be removed;
-  - The parameter, ``path``, of ``rte_pdump_init`` will be removed;
-  - The enum ``rte_pdump_socktype`` will be removed.
diff --git a/doc/guides/rel_notes/release_19_02.rst b/doc/guides/rel_notes/release_19_02.rst
index a94fa86a7..4710fb7b8 100644
--- a/doc/guides/rel_notes/release_19_02.rst
+++ b/doc/guides/rel_notes/release_19_02.rst
@@ -83,6 +83,10 @@ API Changes
    Also, make sure to start the actual text at the margin.
    =========================================================
 
+* pdump: The ``rte_pdump_set_socket_dir()``, the parameter ``path`` of
+  ``rte_pdump_init()`` and enum ``rte_pdump_socktype`` were deprecated
+  since 18.05 and are removed in this release.
+
 
 ABI Changes
 -----------
@@ -153,7 +157,7 @@ The libraries prepended with a plus sign were incremented in this version.
      librte_metrics.so.1
      librte_net.so.1
      librte_pci.so.1
-     librte_pdump.so.2
+   + librte_pdump.so.3
      librte_pipeline.so.3
      librte_pmd_bnxt.so.2
      librte_pmd_bond.so.2
diff --git a/lib/librte_pdump/Makefile b/lib/librte_pdump/Makefile
index b241151dc..89593689a 100644
--- a/lib/librte_pdump/Makefile
+++ b/lib/librte_pdump/Makefile
@@ -12,7 +12,7 @@ LDLIBS += -lrte_eal -lrte_mempool -lrte_mbuf -lrte_ethdev
 
 EXPORT_MAP := rte_pdump_version.map
 
-LIBABIVER := 2
+LIBABIVER := 3
 
 # all source are stored in SRCS-y
 SRCS-$(CONFIG_RTE_LIBRTE_PDUMP) := rte_pdump.c
diff --git a/lib/librte_pdump/meson.build b/lib/librte_pdump/meson.build
index be80904b9..b4b4f26c5 100644
--- a/lib/librte_pdump/meson.build
+++ b/lib/librte_pdump/meson.build
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: BSD-3-Clause
 # Copyright(c) 2017 Intel Corporation
 
-version = 2
+version = 3
 sources = files('rte_pdump.c')
 headers = files('rte_pdump.h')
 allow_experimental_apis = true
diff --git a/lib/librte_pdump/rte_pdump.c b/lib/librte_pdump/rte_pdump.c
index 6c3a88581..4f38ac58b 100644
--- a/lib/librte_pdump/rte_pdump.c
+++ b/lib/librte_pdump/rte_pdump.c
@@ -406,7 +406,7 @@ pdump_server(const struct rte_mp_msg *mp_msg, const void *peer)
 }
 
 int
-rte_pdump_init(const char *path __rte_unused)
+rte_pdump_init(void)
 {
 	return rte_mp_action_register(PDUMP_MP, pdump_server);
 }
@@ -616,10 +616,3 @@ rte_pdump_disable_by_deviceid(char *device_id, uint16_t queue,
 
 	return ret;
 }
-
-int
-rte_pdump_set_socket_dir(const char *path __rte_unused,
-			 enum rte_pdump_socktype type __rte_unused)
-{
-	return 0;
-}
diff --git a/lib/librte_pdump/rte_pdump.h b/lib/librte_pdump/rte_pdump.h
index 673a2b070..6b00fc17a 100644
--- a/lib/librte_pdump/rte_pdump.h
+++ b/lib/librte_pdump/rte_pdump.h
@@ -29,25 +29,16 @@ enum {
 	RTE_PDUMP_FLAG_RXTX = (RTE_PDUMP_FLAG_RX|RTE_PDUMP_FLAG_TX)
 };
 
-enum rte_pdump_socktype {
-	RTE_PDUMP_SOCKET_SERVER = 1,
-	RTE_PDUMP_SOCKET_CLIENT = 2
-};
-
 /**
  * Initialize packet capturing handling
  *
  * Register the IPC action for communication with target (primary) process.
  *
- * @param path
- * This parameter is going to be deprecated; it was used for specifying the
- * directory path for server socket.
- *
  * @return
  *    0 on success, -1 on error
  */
 int
-rte_pdump_init(const char *path);
+rte_pdump_init(void);
 
 /**
  * Un initialize packet capturing handling
@@ -162,29 +153,6 @@ int
 rte_pdump_disable_by_deviceid(char *device_id, uint16_t queue,
 				uint32_t flags);
 
-/**
- * @deprecated
- * Allows applications to set server and client socket paths.
- * If specified path is null default path will be selected, i.e.
- *"/var/run/" for root user and "$HOME" for non root user.
- * Clients also need to call this API to set their server path if the
- * server path is different from default path.
- * This API is not thread-safe.
- *
- * @param path
- * directory path for server or client socket.
- * @param type
- * specifies RTE_PDUMP_SOCKET_SERVER if socket path is for server.
- * (or)
- * specifies RTE_PDUMP_SOCKET_CLIENT if socket path is for client.
- *
- * @return
- * 0 on success, -EINVAL on error
- *
- */
-__rte_deprecated int
-rte_pdump_set_socket_dir(const char *path, enum rte_pdump_socktype type);
-
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/librte_pdump/rte_pdump_version.map b/lib/librte_pdump/rte_pdump_version.map
index edec99a41..3e744f301 100644
--- a/lib/librte_pdump/rte_pdump_version.map
+++ b/lib/librte_pdump/rte_pdump_version.map
@@ -6,7 +6,6 @@ DPDK_16.07 {
 	rte_pdump_enable;
 	rte_pdump_enable_by_deviceid;
 	rte_pdump_init;
-	rte_pdump_set_socket_dir;
 	rte_pdump_uninit;
 
 	local: *;
-- 
2.14.5

Re: [dpdk-dev] [PATCH] pdump: remove deprecated APIs

[Pattan, Reshma]

Hi,

> -----Original Message-----
> From: Bie, Tiwei
> Sent: Monday, December 3, 2018 2:59 AM
> To: dev@dpdk.org
> Cc: Pattan, Reshma <reshma.pattan@intel.com>; Mcnamara, John
> <john.mcnamara@intel.com>; Kovacevic, Marko <marko.kovacevic@intel.com>
> Subject: [PATCH] pdump: remove deprecated APIs
> 
> We already changed to use generic IPC in pdump since below commit:
> 
> commit 660098d61f57 ("pdump: use generic multi-process channel")
> 
> The `rte_pdump_set_socket_dir()`, the `path` parameter of `rte_pdump_init()`
> and the `enum rte_pdump_socktype` have been deprecated since then. This
> commit removes these deprecated APIs and also bumps the pdump ABI.
> 
> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> ---
> changes:
> - update release notes;
> - minor updates in pdump_lib.rst;
> 
>  app/test-pmd/testpmd.c                 |  2 +-
>  doc/guides/prog_guide/pdump_lib.rst    | 18 ++++--------------
>  doc/guides/rel_notes/deprecation.rst   |  7 -------
>  doc/guides/rel_notes/release_19_02.rst |  6 +++++-
>  lib/librte_pdump/Makefile              |  2 +-
>  lib/librte_pdump/meson.build           |  2 +-
>  lib/librte_pdump/rte_pdump.c           |  9 +--------
>  lib/librte_pdump/rte_pdump.h           | 34 +---------------------------------
>  lib/librte_pdump/rte_pdump_version.map |  1 -
>  9 files changed, 14 insertions(+), 67 deletions(-)
> 
Reviewed-by: Reshma Pattan <reshma.pattan@intel.com>
Acked-by: Reshma Pattan <reshma.pattan@intel.com>

Re: [dpdk-dev] [PATCH] pdump: remove deprecated APIs

[Thomas Monjalon]

13/12/2018 18:15, Pattan, Reshma:
> Hi,
> 
> From: Bie, Tiwei
> > 
> > We already changed to use generic IPC in pdump since below commit:
> > 
> > commit 660098d61f57 ("pdump: use generic multi-process channel")
> > 
> > The `rte_pdump_set_socket_dir()`, the `path` parameter of `rte_pdump_init()`
> > and the `enum rte_pdump_socktype` have been deprecated since then. This
> > commit removes these deprecated APIs and also bumps the pdump ABI.
> > 
> > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> > 
> Reviewed-by: Reshma Pattan <reshma.pattan@intel.com>
> Acked-by: Reshma Pattan <reshma.pattan@intel.com>

Applied, thanks