[1/2] app/testpmd: add test for remote PD and CTX

Message ID 20220301202615.4103972-2-michaelba@nvidia.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series app/testpmd: external RxQ tests |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Michael Baum March 1, 2022, 8:26 p.m. UTC
  Add mlx5 internal option in testpmd run-time function "port attach" to
add another parameter named "mlx5_socket" for attaching port and add 2
devargs before.

The arguments are "cmd_fd" and "pd_handle" using to import device
created out of PMD. Testpmd application import it using IPC, and updates
the devargs list before attaching.

The syntax is:

  testpmd > port attach (identifier) mlx5_socket=(path)

Where "path" is the IPC socket path agreed on the remote process.

Signed-off-by: Michael Baum <michaelba@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
---
 app/test-pmd/cmdline.c                      |  14 +-
 app/test-pmd/meson.build                    |   3 +
 app/test-pmd/testpmd.c                      | 153 ++++++++++++++++++++
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  40 +++++
 4 files changed, 208 insertions(+), 2 deletions(-)
  

Comments

Ferruh Yigit March 3, 2022, 12:57 p.m. UTC | #1
On 3/1/2022 8:26 PM, Michael Baum wrote:
> Add mlx5 internal option in testpmd run-time function "port attach" to
> add another parameter named "mlx5_socket" for attaching port and add 2
> devargs before.
> 
> The arguments are "cmd_fd" and "pd_handle" using to import device
> created out of PMD. Testpmd application import it using IPC, and updates
> the devargs list before attaching.
> 
> The syntax is:
> 
>    testpmd > port attach (identifier) mlx5_socket=(path)
> 
> Where "path" is the IPC socket path agreed on the remote process.
> 
> Signed-off-by: Michael Baum <michaelba@nvidia.com>
> Acked-by: Matan Azrad <matan@nvidia.com>

<...>

> diff --git a/app/test-pmd/meson.build b/app/test-pmd/meson.build
> index 43130c8856..c4fd379e67 100644
> --- a/app/test-pmd/meson.build
> +++ b/app/test-pmd/meson.build
> @@ -73,3 +73,6 @@ endif
>   if dpdk_conf.has('RTE_NET_DPAA')
>       deps += ['bus_dpaa', 'mempool_dpaa', 'net_dpaa']
>   endif
> +if dpdk_conf.has('RTE_NET_MLX5')
> +    deps += 'net_mlx5'
> +endif

Is this patch introduce any build time dependency to mlx5
driver? If not this chunk should go to next patch, which
uses mlx5 PMD specific API.

<...>

> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> index 1083c6d538..d6490947c4 100644
> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> @@ -2127,6 +2127,46 @@ the mode and slave parameters must be given.
>      Done
>   
>   
> +port attach with mlx5 socket path
> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +
> +MLX5 internal option to attach a port specified by pci address or virtual device
> +args and add extra devargs to it, which is imported from external process::
> +
> +   testpmd> port attach (identifier) mlx5_socket=(path)
> +
> +where:
> +
> +* ``identifier``: pci address or virtual device args.
> +* ``path``: socket path to import arguments agreed by the external process.
> +
> +The mlx5 PMD enables to import CTX and PD created outside the PMD.
> +It gets as devargs the device's ``cmd_fd`` and ``pd_handle``,
> +then using those arguments to import objects.
> +See :ref:`mlx5 driver options <mlx5_common_driver_options>` for more information.
> +
> +When ``cmd_fd`` and ``pd_handle`` arguments are coming from another process,
> +the FD must be dup'd before being passed.
> +In this function, testpmd initializes IPC socket to get FD using SCM_RIGHTS.
> +It gets the external process socket path, then import the ``cmd_fd`` and
> +``pd_handle`` arguments and add them to devargs list.
> +After updating this, it calls the regular ``port attach`` function
> +with extended idevtifier.
> +
> +For example, to attach a port whose pci address is ``0000:0a:00.0`` and its
> +socket path is ``/var/run/import_ipc_socket``.
> +
> +.. code-block:: console
> +
> +   testpmd> port attach 0000:0a:00.0 mlx5_socket=/var/run/import_ipc_socket
> +   Attaching a new port...
> +   testpmd: MLX5 socket path is /var/run/import_ipc_socket
> +   testpmd: Attach port with extra devargs 0000:0a:00.0,cmd_fd=40,pd_handle=1
> +   EAL: Probe PCI driver: mlx5_pci (15b3:101d) device: 0000:03:00.0 (socket 0)
> +   Port 0 is attached. Now total ports is 1
> +   Done
> +
> +


Hi Michael,

This is too much mlx5 specific addition, and I don't think it is
good to extend testpmd with PMD specific code.
If we enable it, sure there will be other vendors willing to do
the same, making testpmd even messier.

I don't know what those ``cmd_fd`` and ``pd_handle`` (that read
from provided socket), but can they be read from some other
script and feed to testpmd, like a python wrapper etc...
  
Michael Baum March 7, 2022, 4:07 p.m. UTC | #2
On 3/3/2022 2:57 PM, Ferruh Yigit wrote: 
> On 3/1/2022 8:26 PM, Michael Baum wrote:
> > Add mlx5 internal option in testpmd run-time function "port attach" to
> > add another parameter named "mlx5_socket" for attaching port and add 2
> > devargs before.
> >
> > The arguments are "cmd_fd" and "pd_handle" using to import device
> > created out of PMD. Testpmd application import it using IPC, and
> > updates the devargs list before attaching.
> >
> > The syntax is:
> >
> >    testpmd > port attach (identifier) mlx5_socket=(path)
> >
> > Where "path" is the IPC socket path agreed on the remote process.
> >
> > Signed-off-by: Michael Baum <michaelba@nvidia.com>
> > Acked-by: Matan Azrad <matan@nvidia.com>
> 
> <...>
> 
> > diff --git a/app/test-pmd/meson.build b/app/test-pmd/meson.build index
> > 43130c8856..c4fd379e67 100644
> > --- a/app/test-pmd/meson.build
> > +++ b/app/test-pmd/meson.build
> > @@ -73,3 +73,6 @@ endif
> >   if dpdk_conf.has('RTE_NET_DPAA')
> >       deps += ['bus_dpaa', 'mempool_dpaa', 'net_dpaa']
> >   endif
> > +if dpdk_conf.has('RTE_NET_MLX5')
> > +    deps += 'net_mlx5'
> > +endif
> 
> Is this patch introduce any build time dependency to mlx5 driver? If not this
> chunk should go to next patch, which uses mlx5 PMD specific API.

OK

> 
> <...>
> 
> > diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> > b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> > index 1083c6d538..d6490947c4 100644
> > --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> > +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> > @@ -2127,6 +2127,46 @@ the mode and slave parameters must be given.
> >      Done
> >
> >
> > +port attach with mlx5 socket path
> > +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > +
> > +MLX5 internal option to attach a port specified by pci address or
> > +virtual device args and add extra devargs to it, which is imported from external process::
> > +
> > +   testpmd> port attach (identifier) mlx5_socket=(path)
> > +
> > +where:
> > +
> > +* ``identifier``: pci address or virtual device args.
> > +* ``path``: socket path to import arguments agreed by the external process.
> > +
> > +The mlx5 PMD enables to import CTX and PD created outside the PMD.
> > +It gets as devargs the device's ``cmd_fd`` and ``pd_handle``, then
> > +using those arguments to import objects.
> > +See :ref:`mlx5 driver options <mlx5_common_driver_options>` for more information.
> > +
> > +When ``cmd_fd`` and ``pd_handle`` arguments are coming from another
> > +process, the FD must be dup'd before being passed.
> > +In this function, testpmd initializes IPC socket to get FD using SCM_RIGHTS.
> > +It gets the external process socket path, then import the ``cmd_fd``
> > +and ``pd_handle`` arguments and add them to devargs list.
> > +After updating this, it calls the regular ``port attach`` function
> > +with extended idevtifier.
> > +
> > +For example, to attach a port whose pci address is ``0000:0a:00.0``
> > +and its socket path is ``/var/run/import_ipc_socket``.
> > +
> > +.. code-block:: console
> > +
> > +   testpmd> port attach 0000:0a:00.0 mlx5_socket=/var/run/import_ipc_socket
> > +   Attaching a new port...
> > +   testpmd: MLX5 socket path is /var/run/import_ipc_socket
> > +   testpmd: Attach port with extra devargs 0000:0a:00.0,cmd_fd=40,pd_handle=1
> > +   EAL: Probe PCI driver: mlx5_pci (15b3:101d) device: 0000:03:00.0 (socket 0)
> > +   Port 0 is attached. Now total ports is 1
> > +   Done
> > +
> 
> 
> Hi Michael,
> 
> This is too much mlx5 specific addition, and I don't think it is good to extend
> testpmd with PMD specific code.
> If we enable it, sure there will be other vendors willing to do the same,
> making testpmd even messier.

Hi Ferruh,

It is mlx5 PMD specific API, which enables to import device from remote process.
This extension is the way to test this API, you can see a lot of PMD specific APIs along testpmd files. 

If one day, other vendors want to import devargs from remote process, they will remove the mlx5 build time dependency and use it.

> 
> I don't know what those ``cmd_fd`` and ``pd_handle`` (that read from
> provided socket), but can they be read from some other script and feed to
> testpmd, like a python wrapper etc...
  
Thomas Monjalon March 8, 2022, 9:40 a.m. UTC | #3
07/03/2022 17:07, Michael Baum:
> On 3/3/2022 2:57 PM, Ferruh Yigit wrote: 
> > Hi Michael,
> > 
> > This is too much mlx5 specific addition, and I don't think it is good to extend
> > testpmd with PMD specific code.
> > If we enable it, sure there will be other vendors willing to do the same,
> > making testpmd even messier.
> 
> Hi Ferruh,
> 
> It is mlx5 PMD specific API, which enables to import device from remote process.
> This extension is the way to test this API, you can see a lot of PMD specific APIs along testpmd files. 
> 
> If one day, other vendors want to import devargs from remote process, they will remove the mlx5 build time dependency and use it.
> 
> > I don't know what those ``cmd_fd`` and ``pd_handle`` (that read from
> > provided socket), but can they be read from some other script and feed to
> > testpmd, like a python wrapper etc...

I agree with Ferruh that it's a lot of code only for mlx5.
Yes we are already calling other PMD-specific API in testpmd
but we should try to keep it as small as possible.
I propose to try a rework to make it easier to digest.
As a consequence, we won't have this testpmd feature in 22.03,
and we can work together for the next release.
  

Patch

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 7ab0575e64..479e0290c4 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -773,6 +773,12 @@  static void cmd_help_long_parsed(void *parsed_result,
 			"port attach (ident)\n"
 			"    Attach physical or virtual dev by pci address or virtual device name\n\n"
 
+#ifdef RTE_NET_MLX5
+			"port attach (ident) mlx5_socket=(path)\n"
+			"    Attach physical or virtual dev by pci address or virtual device name "
+			"and add \"cmd_fd\" and \"pd_handle\" devargs before attaching\n\n"
+#endif
+
 			"port detach (port_id)\n"
 			"    Detach physical or virtual dev by port_id\n\n"
 
@@ -1379,8 +1385,12 @@  cmdline_parse_token_string_t cmd_operate_attach_port_identifier =
 cmdline_parse_inst_t cmd_operate_attach_port = {
 	.f = cmd_operate_attach_port_parsed,
 	.data = NULL,
-	.help_str = "port attach <identifier>: "
-		"(identifier: pci address or virtual dev name)",
+	.help_str = "port attach <identifier> mlx5_socket=<path>: "
+		"(identifier: pci address or virtual dev name"
+#ifdef RTE_NET_MLX5
+		", path (optional): socket path to get cmd FD and PD handle"
+#endif
+		")",
 	.tokens = {
 		(void *)&cmd_operate_attach_port_port,
 		(void *)&cmd_operate_attach_port_keyword,
diff --git a/app/test-pmd/meson.build b/app/test-pmd/meson.build
index 43130c8856..c4fd379e67 100644
--- a/app/test-pmd/meson.build
+++ b/app/test-pmd/meson.build
@@ -73,3 +73,6 @@  endif
 if dpdk_conf.has('RTE_NET_DPAA')
     deps += ['bus_dpaa', 'mempool_dpaa', 'net_dpaa']
 endif
+if dpdk_conf.has('RTE_NET_MLX5')
+    deps += 'net_mlx5'
+endif
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index fe2ce19f99..7ec95e5ae4 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -11,6 +11,10 @@ 
 #include <fcntl.h>
 #ifndef RTE_EXEC_ENV_WINDOWS
 #include <sys/mman.h>
+#ifdef RTE_NET_MLX5
+#include <sys/socket.h>
+#include <sys/un.h>
+#endif
 #endif
 #include <sys/types.h>
 #include <errno.h>
@@ -3200,11 +3204,150 @@  reset_port(portid_t pid)
 	printf("Done\n");
 }
 
+#if defined(RTE_NET_MLX5) && !defined(RTE_EXEC_ENV_WINDOWS)
+static const char*
+get_socket_path(char *extend)
+{
+	if (strstr(extend, "mlx5_socket=") == extend) {
+		const char *socket_path = strchr(extend, '=') + 1;
+
+		TESTPMD_LOG(DEBUG, "MLX5 socket path is %s\n", socket_path);
+		return socket_path;
+	}
+
+	TESTPMD_LOG(ERR, "Failed to extract a valid socket path from %s\n",
+		    extend);
+	return NULL;
+}
+
+static int
+attach_port_extend_devargs(char *identifier, char *extend)
+{
+	struct sockaddr_un un = {
+		.sun_family = AF_UNIX,
+	};
+	struct sockaddr_un dst = {
+		.sun_family = AF_UNIX,
+	};
+	int cmd_fd;
+	int pd_handle;
+	struct iovec iov = {
+		.iov_base = &pd_handle,
+		.iov_len = sizeof(int),
+	};
+	union {
+		char buf[CMSG_SPACE(sizeof(int))];
+		struct cmsghdr align;
+	} control;
+	struct msghdr msgh = {
+		.msg_name = &dst,
+		.msg_namelen = sizeof(dst),
+		.msg_iov = NULL,
+		.msg_iovlen = 0,
+	};
+	struct cmsghdr *cmsg;
+	const char *path = get_socket_path(extend + 1);
+	size_t length = 1;
+	int socket_fd;
+	int ret;
+
+	if (path == NULL) {
+		TESTPMD_LOG(ERR, "Invalid devargs extension is specified\n");
+		return -1;
+	}
+
+	/* Initialize IPC channel. */
+	socket_fd = socket(AF_UNIX, SOCK_DGRAM, 0);
+	if (socket_fd < 0) {
+		TESTPMD_LOG(ERR, "Failed to create unix socket: %s\n",
+			    strerror(errno));
+		return -1;
+	}
+	snprintf(un.sun_path, sizeof(un.sun_path), "%s_%d", path, getpid());
+	unlink(un.sun_path); /* May still exist since last run */
+	if (bind(socket_fd, (struct sockaddr *)&un, sizeof(un)) < 0) {
+		TESTPMD_LOG(ERR, "Failed to bind %s: %s\n", un.sun_path,
+			    strerror(errno));
+		close(socket_fd);
+		return -1;
+	}
+
+	strlcpy(dst.sun_path, path, sizeof(dst.sun_path));
+	/* Send the request message. */
+	do {
+		ret = sendmsg(socket_fd, &msgh, 0);
+	} while (ret < 0 && errno == EINTR);
+	if (ret < 0) {
+		TESTPMD_LOG(ERR, "Failed to send request to (%s): %s\n", path,
+			    strerror(errno));
+		close(socket_fd);
+		unlink(un.sun_path);
+		return -1;
+	}
+
+	msgh.msg_iov = &iov;
+	msgh.msg_iovlen = 1;
+	msgh.msg_control = control.buf;
+	msgh.msg_controllen = sizeof(control.buf);
+	do {
+		ret = recvmsg(socket_fd, &msgh, 0);
+	} while (ret < 0);
+	if (ret != sizeof(int) || (msgh.msg_flags & (MSG_TRUNC | MSG_CTRUNC))) {
+		TESTPMD_LOG(ERR, "truncated msg");
+		close(socket_fd);
+		unlink(un.sun_path);
+		return -1;
+	}
+
+	/* Translate the FD. */
+	cmsg = CMSG_FIRSTHDR(&msgh);
+	if (cmsg == NULL || cmsg->cmsg_len != CMSG_LEN(sizeof(int)) ||
+	    cmsg->cmsg_level != SOL_SOCKET || cmsg->cmsg_type != SCM_RIGHTS) {
+		TESTPMD_LOG(ERR, "Fail to get FD using SCM_RIGHTS mechanism\n");
+		close(socket_fd);
+		unlink(un.sun_path);
+		return -1;
+	}
+	memcpy(&cmd_fd, CMSG_DATA(cmsg), sizeof(int));
+
+	TESTPMD_LOG(DEBUG, "Command FD (%d) and PD handle (%d) "
+		    "are successfully imported from remote process\n",
+		    cmd_fd, pd_handle);
+
+	/* Cleanup IPC channel. */
+	close(socket_fd);
+	unlink(un.sun_path);
+
+	/* Calculate the new length of devargs string. */
+	length += snprintf(NULL, 0, ",cmd_fd=%d,pd_handle=%d",
+			   cmd_fd, pd_handle);
+	/* Extend the devargs string. */
+	snprintf(extend, length, ",cmd_fd=%d,pd_handle=%d", cmd_fd, pd_handle);
+
+	TESTPMD_LOG(DEBUG, "Attach port with extra devargs %s\n", identifier);
+	return 0;
+}
+
+static bool
+is_delimiter_path_spaces(char *extend)
+{
+	while (*extend != '\0') {
+		if (*extend != ' ')
+			return true;
+		extend++;
+	}
+	return false;
+}
+#endif
+
 void
 attach_port(char *identifier)
 {
 	portid_t pi;
 	struct rte_dev_iterator iterator;
+#if defined(RTE_NET_MLX5) && !defined(RTE_EXEC_ENV_WINDOWS)
+	char *extend;
+#endif
 
 	printf("Attaching a new port...\n");
 
@@ -3213,6 +3356,16 @@  attach_port(char *identifier)
 		return;
 	}
 
+#if defined(RTE_NET_MLX5) && !defined(RTE_EXEC_ENV_WINDOWS)
+	extend = strchr(identifier, ' ');
+	if (extend != NULL && is_delimiter_path_spaces(extend) &&
+	    attach_port_extend_devargs(identifier, extend) < 0) {
+		TESTPMD_LOG(ERR, "Failed to extend devargs for port %s\n",
+			    identifier);
+		return;
+	}
+#endif
+
 	if (rte_dev_probe(identifier) < 0) {
 		TESTPMD_LOG(ERR, "Failed to attach port %s\n", identifier);
 		return;
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 1083c6d538..d6490947c4 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -2127,6 +2127,46 @@  the mode and slave parameters must be given.
    Done
 
 
+port attach with mlx5 socket path
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+MLX5 internal option to attach a port specified by pci address or virtual device
+args and add extra devargs to it, which is imported from external process::
+
+   testpmd> port attach (identifier) mlx5_socket=(path)
+
+where:
+
+* ``identifier``: pci address or virtual device args.
+* ``path``: socket path to import arguments agreed by the external process.
+
+The mlx5 PMD enables to import CTX and PD created outside the PMD.
+It gets as devargs the device's ``cmd_fd`` and ``pd_handle``,
+then using those arguments to import objects.
+See :ref:`mlx5 driver options <mlx5_common_driver_options>` for more information.
+
+When ``cmd_fd`` and ``pd_handle`` arguments are coming from another process,
+the FD must be dup'd before being passed.
+In this function, testpmd initializes IPC socket to get FD using SCM_RIGHTS.
+It gets the external process socket path, then import the ``cmd_fd`` and
+``pd_handle`` arguments and add them to devargs list.
+After updating this, it calls the regular ``port attach`` function
+with extended idevtifier.
+
+For example, to attach a port whose pci address is ``0000:0a:00.0`` and its
+socket path is ``/var/run/import_ipc_socket``.
+
+.. code-block:: console
+
+   testpmd> port attach 0000:0a:00.0 mlx5_socket=/var/run/import_ipc_socket
+   Attaching a new port...
+   testpmd: MLX5 socket path is /var/run/import_ipc_socket
+   testpmd: Attach port with extra devargs 0000:0a:00.0,cmd_fd=40,pd_handle=1
+   EAL: Probe PCI driver: mlx5_pci (15b3:101d) device: 0000:03:00.0 (socket 0)
+   Port 0 is attached. Now total ports is 1
+   Done
+
+
 port detach
 ~~~~~~~~~~~