[dpdk-dev] [PATCH 0/3] add new command line options and error handling in pdump

[dpdk-dev] [PATCH 0/3] add new command line options and error handling in pdump

[Reshma Pattan]

This patch set contains
1)Error handling fixes in pdump library.
2)Support of server and client socket path command line options in pdump tool.
3)Default socket path name fixes in pdump library doc.

Reshma Pattan (3):
  pdump: fix error handlings
  app/pdump: add new command line options for socket paths
  doc: fix default socket path names

 app/pdump/main.c                    | 57 +++++++++++++++++++++++++++++++------
 doc/guides/prog_guide/pdump_lib.rst | 12 ++++----
 doc/guides/sample_app_ug/pdump.rst  | 31 ++++++++++++++------
 lib/librte_pdump/rte_pdump.c        | 26 +++++++++++++----
 4 files changed, 97 insertions(+), 29 deletions(-)

-- 
2.5.0

[dpdk-dev] [PATCH 1/3] pdump: fix error handlings

[Reshma Pattan]

The changes include
1)If mkdir fails for user passed socket paths log error and return.

2)At some places return value was set to errno and that non-negative number
was returned, but the intention was to return negative value.
So now rte_errno was set to errno and returning the actual negative value
that the APIs has returned.

Fixes: 278f945402c5 ("pdump: add new library for packet capture")

Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
---
 lib/librte_pdump/rte_pdump.c | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/lib/librte_pdump/rte_pdump.c b/lib/librte_pdump/rte_pdump.c
index 22ed476..9b921ce 100644
--- a/lib/librte_pdump/rte_pdump.c
+++ b/lib/librte_pdump/rte_pdump.c
@@ -449,6 +449,7 @@ pdump_get_socket_path(char *buffer, int bufsz, enum rte_pdump_socktype type)
 	char dpdk_dir[PATH_MAX] = {0};
 	char dir[PATH_MAX] = {0};
 	char *dir_home = NULL;
+	int ret = 0;
 
 	if (type == RTE_PDUMP_SOCKET_SERVER && server_socket_dir[0] != 0)
 		snprintf(dir, sizeof(dir), "%s", server_socket_dir);
@@ -475,7 +476,16 @@ pdump_get_socket_path(char *buffer, int bufsz, enum rte_pdump_socktype type)
 					dpdk_dir, SOCKET_DIR);
 	}
 
-	mkdir(dir, 700);
+	ret =  mkdir(dir, 700);
+	/* if user passed socket path is invalid, return immediately */
+	if (ret < 0 && errno != EEXIST) {
+		RTE_LOG(ERR, PDUMP,
+			"Failed to create dir:%s:%s\n", dir,
+			strerror(errno));
+		rte_errno = errno;
+		return -1;
+	}
+
 	if (type == RTE_PDUMP_SOCKET_SERVER)
 		snprintf(buffer, bufsz, SERVER_SOCKET, dir);
 	else
@@ -667,8 +677,8 @@ pdump_create_client_socket(struct pdump_request *p)
 			"client socket(): %s:pid(%d):tid(%u), %s:%d\n",
 			strerror(errno), pid, rte_sys_gettid(),
 			__func__, __LINE__);
-		ret = errno;
-		return ret;
+		rte_errno = errno;
+		return -1;
 	}
 
 	ret = pdump_get_socket_path(addr.sun_path, sizeof(addr.sun_path),
@@ -677,6 +687,7 @@ pdump_create_client_socket(struct pdump_request *p)
 		RTE_LOG(ERR, PDUMP,
 			"Failed to get client socket path: %s:%d\n",
 			__func__, __LINE__);
+		rte_errno = errno;
 		goto exit;
 	}
 	addr.sun_family = AF_UNIX;
@@ -688,7 +699,7 @@ pdump_create_client_socket(struct pdump_request *p)
 			RTE_LOG(ERR, PDUMP,
 				"client bind(): %s, %s:%d\n",
 				strerror(errno), __func__, __LINE__);
-			ret = errno;
+			rte_errno = errno;
 			break;
 		}
 
@@ -701,6 +712,7 @@ pdump_create_client_socket(struct pdump_request *p)
 			RTE_LOG(ERR, PDUMP,
 				"Failed to get server socket path: %s:%d\n",
 				__func__, __LINE__);
+			rte_errno = errno;
 			break;
 		}
 		serv_addr.sun_family = AF_UNIX;
@@ -711,7 +723,8 @@ pdump_create_client_socket(struct pdump_request *p)
 			RTE_LOG(ERR, PDUMP,
 				"failed to send to server:%s, %s:%d\n",
 				strerror(errno), __func__, __LINE__);
-			ret =  errno;
+			rte_errno = errno;
+			ret = -1;
 			break;
 		}
 
@@ -722,7 +735,8 @@ pdump_create_client_socket(struct pdump_request *p)
 			RTE_LOG(ERR, PDUMP,
 				"failed to recv from server:%s, %s:%d\n",
 				strerror(errno), __func__, __LINE__);
-			ret = errno;
+			rte_errno = errno;
+			ret = -1;
 			break;
 		}
 		ret = server_resp.err_value;
-- 
2.5.0

[dpdk-dev] [PATCH 2/3] app/pdump: add new command line options for socket paths

[Reshma Pattan]

Since users of the pdump library and tool can chose to have their own
server and client paths, it is must for the pdump tool to use the same
server socket path that was used by primary application while
initializing packet capture framework by rte_pdump_init() or
rte_pdump_set_socket_dir() APIs.

To pass the socket path info to pdump tool a new optional command
line options "server-socket-path" and "client-socket-path" are added.

"client-socket-path" is also added, if the users want to have client
sockets in their own defined paths.

Updated pdump tool guide with the new changes.

Fixes: caa7028276b8 ("app/pdump: add tool for packet capturing")

Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
---
 app/pdump/main.c                   | 57 ++++++++++++++++++++++++++++++++------
 doc/guides/sample_app_ug/pdump.rst | 31 +++++++++++++++------
 2 files changed, 71 insertions(+), 17 deletions(-)

diff --git a/app/pdump/main.c b/app/pdump/main.c
index 2087c15..e0ff8be 100644
--- a/app/pdump/main.c
+++ b/app/pdump/main.c
@@ -55,6 +55,7 @@
 #include <rte_ring.h>
 #include <rte_pdump.h>
 
+#define CMD_LINE_OPT_PDUMP "pdump"
 #define PDUMP_PORT_ARG "port"
 #define PDUMP_PCI_ARG "device_id"
 #define PDUMP_QUEUE_ARG "queue"
@@ -64,6 +65,8 @@
 #define PDUMP_RING_SIZE_ARG "ring-size"
 #define PDUMP_MSIZE_ARG "mbuf-size"
 #define PDUMP_NUM_MBUFS_ARG "total-num-mbufs"
+#define CMD_LINE_OPT_SER_SOCK_PATH "server-socket-path"
+#define CMD_LINE_OPT_CLI_SOCK_PATH "client-socket-path"
 
 #define VDEV_PCAP "eth_pcap_%s_%d,tx_pcap=%s"
 #define VDEV_IFACE "eth_pcap_%s_%d,tx_iface=%s"
@@ -166,6 +169,8 @@ struct parse_val {
 int num_tuples;
 static struct rte_eth_conf port_conf_default;
 volatile uint8_t quit_signal;
+static char server_socket_path[PATH_MAX];
+static char client_socket_path[PATH_MAX];
 
 /**< display usage */
 static void
@@ -178,8 +183,11 @@ pdump_usage(const char *prgname)
 			" tx-dev=<iface or pcap file>,"
 			"[ring-size=<ring size>default:16384],"
 			"[mbuf-size=<mbuf data size>default:2176],"
-			"[total-num-mbufs=<number of mbufs>default:65535]"
-			"'\n",
+			"[total-num-mbufs=<number of mbufs>default:65535]'\n"
+			"[--server-socket-path=<server socket dir>"
+				"default:/var/run/.dpdk/ (or) ~/.dpdk/]\n"
+			"[--client-socket-path=<client socket dir>"
+				"default:/var/run/.dpdk/ (or) ~/.dpdk/]\n",
 			prgname);
 }
 
@@ -226,9 +234,6 @@ parse_rxtxdev(const char *key, const char *value, void *extra_args)
 		/* identify the tx stream type for pcap vdev */
 		if (if_nametoindex(pt->tx_dev))
 			pt->tx_vdev_stream_type = IFACE;
-	} else {
-		printf("invalid dev type %s, must be rx or tx\n", value);
-		return -1;
 	}
 
 	return 0;
@@ -407,6 +412,8 @@ launch_args_parse(int argc, char **argv, char *prgname)
 	int option_index;
 	static struct option long_option[] = {
 		{"pdump", 1, 0, 0},
+		{"server-socket-path", 1, 0, 0},
+		{"client-socket-path", 1, 0, 0},
 		{NULL, 0, 0, 0}
 	};
 
@@ -418,14 +425,32 @@ launch_args_parse(int argc, char **argv, char *prgname)
 			long_option, &option_index)) != EOF) {
 		switch (opt) {
 		case 0:
-			if (!strncmp(long_option[option_index].name, "pdump",
-					MAX_LONG_OPT_SZ)) {
+			if (!strncmp(long_option[option_index].name,
+					CMD_LINE_OPT_PDUMP,
+					sizeof(CMD_LINE_OPT_PDUMP))) {
 				ret = parse_pdump(optarg);
 				if (ret) {
 					pdump_usage(prgname);
 					return -1;
 				}
 			}
+
+			if (!strncmp(long_option[option_index].name,
+					CMD_LINE_OPT_SER_SOCK_PATH,
+					sizeof(CMD_LINE_OPT_SER_SOCK_PATH))) {
+				snprintf(server_socket_path,
+					sizeof(server_socket_path), "%s",
+					optarg);
+			}
+
+			if (!strncmp(long_option[option_index].name,
+					CMD_LINE_OPT_CLI_SOCK_PATH,
+					sizeof(CMD_LINE_OPT_CLI_SOCK_PATH))) {
+				snprintf(client_socket_path,
+					sizeof(client_socket_path), "%s",
+					optarg);
+			}
+
 			break;
 		default:
 			pdump_usage(prgname);
@@ -719,6 +744,22 @@ enable_pdump(void)
 	struct pdump_tuples *pt;
 	int ret = 0, ret1 = 0;
 
+	if (server_socket_path[0] != 0)
+		ret = rte_pdump_set_socket_dir(server_socket_path,
+				RTE_PDUMP_SOCKET_SERVER);
+	if (ret == 0 && client_socket_path[0] != 0) {
+		ret = rte_pdump_set_socket_dir(client_socket_path,
+				RTE_PDUMP_SOCKET_CLIENT);
+	}
+	if (ret < 0) {
+		cleanup_pdump_resources();
+		rte_exit(EXIT_FAILURE,
+				"failed to set socket paths of server:%s, "
+				"client:%s\n",
+				server_socket_path,
+				client_socket_path);
+	}
+
 	for (i = 0; i < num_tuples; i++) {
 		pt = &pdump_t[i];
 		if (pt->dir == RTE_PDUMP_FLAG_RXTX) {
@@ -729,7 +770,7 @@ enable_pdump(void)
 						RTE_PDUMP_FLAG_RX,
 						pt->rx_ring,
 						pt->mp, NULL);
-				ret = rte_pdump_enable_by_deviceid(
+				ret1 = rte_pdump_enable_by_deviceid(
 						pt->device_id,
 						pt->queue,
 						RTE_PDUMP_FLAG_TX,
diff --git a/doc/guides/sample_app_ug/pdump.rst b/doc/guides/sample_app_ug/pdump.rst
index 96c8709..054cfc2 100644
--- a/doc/guides/sample_app_ug/pdump.rst
+++ b/doc/guides/sample_app_ug/pdump.rst
@@ -33,14 +33,14 @@
 dpdk_pdump Application
 ======================
 
-The ``dpdk_pdump`` application is a Data Plane Development Kit (DPDK) application that runs as a DPDK secondary process and
-is capable of enabling packet capture on dpdk ports.
+The ``dpdk_pdump`` application is a Data Plane Development Kit (DPDK) application that runs as a DPDK secondary
+process and is capable of enabling packet capture on dpdk ports.
 
 
 Running the Application
 -----------------------
 
-The application has a ``--pdump`` command line option with various sub arguments:
+The application has a number of command line options:
 
 .. code-block:: console
 
@@ -52,18 +52,31 @@ The application has a ``--pdump`` command line option with various sub arguments
                                    [ring-size=<ring size>],
                                    [mbuf-size=<mbuf data size>],
                                    [total-num-mbufs=<number of mbufs>]'
+                          [--server-socket-path=<server socket dir>]
+                          [--client-socket-path=<client socket dir>]
 
-Note:
+The ``--pdump`` command line option is mandatory and it takes various sub arguments which are described in
+below section.
 
-* Parameters inside the parentheses represents mandatory parameters.
+   .. Note::
+
+      * Parameters inside the parentheses represents mandatory parameters.
+
+      * Parameters inside the square brackets represents optional parameters.
+
+      * Multiple instances of ``--pdump`` can be passed to capture packets on different port and queue combinations.
 
-* Parameters inside the square brackets represents optional parameters.
+The ``--server-socket-path`` command line option is optional. This represents the server socket directory.
+If no value is passed default values are used i.e. ``/var/run/.dpdk/`` for root users and ``~/.dpdk/``
+for non root users.
 
-Multiple instances of ``--pdump`` can be passed to capture packets on different port and queue combinations.
+The ``--client-socket-path`` command line option is optional. This represents the client socket directory.
+If no value is passed default values are used i.e. ``/var/run/.dpdk/`` for root users and ``~/.dpdk/``
+for non root users.
 
 
-Parameters
-~~~~~~~~~~
+The ``--pdump`` parameters
+~~~~~~~~~~~~~~~~~~~~~~~~~~
 
 ``port``:
 Port id of the eth device on which packets should be captured.
-- 
2.5.0

[dpdk-dev] [PATCH 3/3] doc: fix default socket path names

[Reshma Pattan]

Fixed default socket path name "/var/run" to "/var/run/.dpdk" and
"$HOME" to "~/.dpdk".

Fixes: 278f945402c5 ("pdump: add new library for packet capture")

Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
---
 doc/guides/prog_guide/pdump_lib.rst | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/doc/guides/prog_guide/pdump_lib.rst b/doc/guides/prog_guide/pdump_lib.rst
index 580ffcb..0136781 100644
--- a/doc/guides/prog_guide/pdump_lib.rst
+++ b/doc/guides/prog_guide/pdump_lib.rst
@@ -75,13 +75,13 @@ the packet capture.
 
 The packet capture framework, as part of its initialization, creates the pthread and the server socket in
 the pthread. The application that calls the framework initialization will have the server socket created,
-either under the path that the application has passed or under the default path i.e. either ``/var/run`` for
-root user or ``$HOME`` for non root user.
+either under the path that the application has passed or under the default path i.e. either ``/var/run/.dpdk`` for
+root user or ``~/.dpdk`` for non root user.
 
 Applications that request enabling or disabling of the packet capture will have the client socket created either under
-the path that the application has passed or under the default path i.e. either ``/var/run/`` for root user or ``$HOME``
-for not root user to send the requests to the server.
-The server socket will listen for client requests for enabling or disabling the packet capture.
+the path that the application has passed or under the default path i.e. either ``/var/run/.dpdk`` for root user or
+``~/.dpdk`` for not root user to send the requests to the server. The server socket will listen for client requests for
+enabling or disabling the packet capture.
 
 
 Implementation Details
@@ -111,7 +111,7 @@ 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/`` for root user or ``$HOME``
+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.
 
-- 
2.5.0

Re: [dpdk-dev] [PATCH 0/3] add new command line options and error handling in pdump

[Ananyev, Konstantin]

This patch set contains
1)Error handling fixes in pdump library.
2)Support of server and client socket path command line options in pdump tool.
3)Default socket path name fixes in pdump library doc.

Reshma Pattan (3):
  pdump: fix error handlings
  app/pdump: add new command line options for socket paths
  doc: fix default socket path names

 app/pdump/main.c                    | 57 +++++++++++++++++++++++++++++++------
 doc/guides/prog_guide/pdump_lib.rst | 12 ++++----  doc/guides/sample_app_ug/pdump.rst  | 31 ++++++++++++++------
 lib/librte_pdump/rte_pdump.c        | 26 +++++++++++++----
 4 files changed, 97 insertions(+), 29 deletions(-)

Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

--
2.5.0

Re: [dpdk-dev] [PATCH 0/3] add new command line options and error handling in pdump

[Thomas Monjalon]

2016-07-14 09:59, Ananyev, Konstantin:
> 
> This patch set contains
> 1)Error handling fixes in pdump library.
> 2)Support of server and client socket path command line options in pdump tool.
> 3)Default socket path name fixes in pdump library doc.
> 
> Reshma Pattan (3):
>   pdump: fix error handlings
>   app/pdump: add new command line options for socket paths
>   doc: fix default socket path names
> 
>  app/pdump/main.c                    | 57 +++++++++++++++++++++++++++++++------
>  doc/guides/prog_guide/pdump_lib.rst | 12 ++++----  doc/guides/sample_app_ug/pdump.rst  | 31 ++++++++++++++------
>  lib/librte_pdump/rte_pdump.c        | 26 +++++++++++++----
>  4 files changed, 97 insertions(+), 29 deletions(-)
> 
> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

Applied, thanks