Re: [dpdk-dev] [PATCH v2] pdump: change to use generic multi-process channel

2018-04-05 Thread Tan, Jianfeng 

Sorry, it seems that I missed this email.


On 4/5/2018 5:45 PM, Pattan, Reshma wrote:

Hi


Signed-off-by: Jianfeng Tan 
---
Note this patch needs this patch set:
http://dpdk.org/dev/patchwork/patch/36814/
v2:
   - Update doc for deprecation of API, rte_pdump_set_socket_dir,
 and API change for rte_pdump_init.
   - Add notice for known incompatibility issue in doc.
  app/pdump/main.c   |   6 +-
  doc/guides/rel_notes/deprecation.rst   |   4 +
  doc/guides/rel_notes/release_18_05.rst |   7 +
  lib/librte_pdump/Makefile  |   3 +-
  lib/librte_pdump/rte_pdump.c   | 423 +
  lib/librte_pdump/rte_pdump.h   |   1 +
  6 files changed, 84 insertions(+), 360 deletions(-)
diff --git a/lib/librte_pdump/rte_pdump.c b/lib/librte_pdump/rte_pdump.c
+




+
+   /* recv client requests */
+   if (mp_msg->len_param != sizeof(*cli_req)) {
+   RTE_LOG(ERR, PDUMP, "failed to recv from client\n");
+   resp->err_value = EINVAL;

resp->err_value = -EINVAL


OK, this keeps consistent with other places in this file.




-   /* save the socket in local configuration */
-   pdump_socket_fd = socket_fd;
+   snprintf(mp_resp.name, RTE_MP_MAX_NAME_LEN, PDUMP_MP);
+   mp_resp.len_param = sizeof(*resp);
+   mp_resp.num_fds = 0;
+   if (rte_mp_reply(&mp_resp, peer) < 0)
+   RTE_LOG(ERR, PDUMP, "failed to send to client:%s, %s:%d\n",
+   strerror(rte_errno), __func__, __LINE__);


If failed to send the reply should'nt we return -1?


Yes, will fix it.




return 0;
  }

  int
-rte_pdump_set_socket_dir(const char *path, enum rte_pdump_socktype
type)
+rte_pdump_set_socket_dir(const char *path __rte_unused,
+enum rte_pdump_socktype type __rte_unused)
  {

What about enum rte_pdump_socktype in header file? When to delete them?


Will deprecate it.



We need to update doxygen comments in header file for rte_pdump_init and 
rte_pdump_uninit()? What do you say.


I'll try to find a flag for deprecating a param. If you know how to do 
that, please let me know.


Thanks,
Jianfeng



Thanks,
Reshma

Re: [dpdk-dev] [PATCH v2] pdump: change to use generic multi-process channel

2018-04-05 Thread Tan, Jianfeng 



On 4/5/2018 6:37 PM, Pattan, Reshma wrote:

Hi,


-Original Message-
From: Tan, Jianfeng
Sent: Wednesday, April 4, 2018 4:08 PM
To: dev@dpdk.org
Cc: Tan, Jianfeng ; Pattan, Reshma

Subject: [PATCH v2] pdump: change to use generic multi-process channel

The original code replies on the private channel for primary and secondary
communication. Change to use the generic multi-process channel.

Note with this change, dpdk-pdump will be not compatible with old version
DPDK applications.

Cc: reshma.pat...@intel.com

Signed-off-by: Jianfeng Tan 
---
diff --git a/doc/guides/rel_notes/deprecation.rst
b/doc/guides/rel_notes/deprecation.rst
index 0c696f7..d55fd05 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -153,3 +153,7 @@ Deprecation Notices
be added between the producer and consumer structures. The size of the
structure and the offset of the fields will remain the same on
platforms with 64B cache line, but will change on other platforms.
+
+* pdump: As we changed to use generic IPC, ``rte_pdump_set_socket_dir``
+will be
+  deprecated and removed in subsequent release; the parameter, path, of
+  ``rte_pdump_init`` will also be removed.

Do we need to mention about deprecation of below enums too?
enum rte_pdump_socktype {
 RTE_PDUMP_SOCKET_SERVER = 1,
 RTE_PDUMP_SOCKET_CLIENT = 2
};


Nice catch, will add in next version.

Thanks,
Jianfeng



Thanks,
Reshma

Re: [dpdk-dev] [PATCH v2] pdump: change to use generic multi-process channel

2018-04-05 Thread Pattan, Reshma 
Hi,

> -Original Message-
> From: Tan, Jianfeng
> Sent: Wednesday, April 4, 2018 4:08 PM
> To: dev@dpdk.org
> Cc: Tan, Jianfeng ; Pattan, Reshma
> 
> Subject: [PATCH v2] pdump: change to use generic multi-process channel
> 
> The original code replies on the private channel for primary and secondary
> communication. Change to use the generic multi-process channel.
> 
> Note with this change, dpdk-pdump will be not compatible with old version
> DPDK applications.
> 
> Cc: reshma.pat...@intel.com
> 
> Signed-off-by: Jianfeng Tan 
> ---
> diff --git a/doc/guides/rel_notes/deprecation.rst
> b/doc/guides/rel_notes/deprecation.rst
> index 0c696f7..d55fd05 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -153,3 +153,7 @@ Deprecation Notices
>be added between the producer and consumer structures. The size of the
>structure and the offset of the fields will remain the same on
>platforms with 64B cache line, but will change on other platforms.
> +
> +* pdump: As we changed to use generic IPC, ``rte_pdump_set_socket_dir``
> +will be
> +  deprecated and removed in subsequent release; the parameter, path, of
> +  ``rte_pdump_init`` will also be removed.

Do we need to mention about deprecation of below enums too?
enum rte_pdump_socktype {
RTE_PDUMP_SOCKET_SERVER = 1,
RTE_PDUMP_SOCKET_CLIENT = 2
};

Thanks,
Reshma

Re: [dpdk-dev] [PATCH v2] pdump: change to use generic multi-process channel

2018-04-05 Thread Pattan, Reshma 
Hi

> 
> Signed-off-by: Jianfeng Tan 
> ---
> Note this patch needs this patch set:
> http://dpdk.org/dev/patchwork/patch/36814/
> v2:
>   - Update doc for deprecation of API, rte_pdump_set_socket_dir,
> and API change for rte_pdump_init.
>   - Add notice for known incompatibility issue in doc.
>  app/pdump/main.c   |   6 +-
>  doc/guides/rel_notes/deprecation.rst   |   4 +
>  doc/guides/rel_notes/release_18_05.rst |   7 +
>  lib/librte_pdump/Makefile  |   3 +-
>  lib/librte_pdump/rte_pdump.c   | 423 
> +
>  lib/librte_pdump/rte_pdump.h   |   1 +
>  6 files changed, 84 insertions(+), 360 deletions(-)

>diff --git a/lib/librte_pdump/rte_pdump.c b/lib/librte_pdump/rte_pdump.c
> +
> 


> +
> + /* recv client requests */
> + if (mp_msg->len_param != sizeof(*cli_req)) {
> + RTE_LOG(ERR, PDUMP, "failed to recv from client\n");
> + resp->err_value = EINVAL;

resp->err_value = -EINVAL

> - /* save the socket in local configuration */
> - pdump_socket_fd = socket_fd;
> + snprintf(mp_resp.name, RTE_MP_MAX_NAME_LEN, PDUMP_MP);
> + mp_resp.len_param = sizeof(*resp);
> + mp_resp.num_fds = 0;
> + if (rte_mp_reply(&mp_resp, peer) < 0)
> + RTE_LOG(ERR, PDUMP, "failed to send to client:%s, %s:%d\n",
> + strerror(rte_errno), __func__, __LINE__);
> 

If failed to send the reply should'nt we return -1? 

>   return 0;
>  }
> 

>  int
> -rte_pdump_set_socket_dir(const char *path, enum rte_pdump_socktype
> type)
> +rte_pdump_set_socket_dir(const char *path __rte_unused,
> +  enum rte_pdump_socktype type __rte_unused)
>  {

What about enum rte_pdump_socktype in header file? When to delete them?

We need to update doxygen comments in header file for rte_pdump_init and 
rte_pdump_uninit()? What do you say.

Thanks,
Reshma

[dpdk-dev] [PATCH v2] pdump: change to use generic multi-process channel

2018-04-04 Thread Jianfeng Tan 
The original code replies on the private channel for primary and
secondary communication. Change to use the generic multi-process
channel.

Note with this change, dpdk-pdump will be not compatible with
old version DPDK applications.

Cc: reshma.pat...@intel.com

Signed-off-by: Jianfeng Tan 
---
Note this patch needs this patch set: http://dpdk.org/dev/patchwork/patch/36814/
v2:
  - Update doc for deprecation of API, rte_pdump_set_socket_dir,
and API change for rte_pdump_init.
  - Add notice for known incompatibility issue in doc.
 app/pdump/main.c   |   6 +-
 doc/guides/rel_notes/deprecation.rst   |   4 +
 doc/guides/rel_notes/release_18_05.rst |   7 +
 lib/librte_pdump/Makefile  |   3 +-
 lib/librte_pdump/rte_pdump.c   | 423 +
 lib/librte_pdump/rte_pdump.h   |   1 +
 6 files changed, 84 insertions(+), 360 deletions(-)

diff --git a/app/pdump/main.c b/app/pdump/main.c
index f6865bd..3779164 100644
--- a/app/pdump/main.c
+++ b/app/pdump/main.c
@@ -155,9 +155,11 @@ pdump_usage(const char *prgname)
"[mbuf-size=default:2176],"
"[total-num-mbufs=default:65535]'\n"
"[--server-socket-path="
-   "default:/var/run/.dpdk/ (or) ~/.dpdk/]\n"
+   " which is deprecated and will be removed soon,"
+   " default:/var/run/.dpdk/ (or) ~/.dpdk/]\n"
"[--client-socket-path="
-   "default:/var/run/.dpdk/ (or) ~/.dpdk/]\n",
+   " which is deprecated and will be removed soon,"
+   " default:/var/run/.dpdk/ (or) ~/.dpdk/]\n",
prgname);
 }
 
diff --git a/doc/guides/rel_notes/deprecation.rst 
b/doc/guides/rel_notes/deprecation.rst
index 0c696f7..d55fd05 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -153,3 +153,7 @@ Deprecation Notices
   be added between the producer and consumer structures. The size of the
   structure and the offset of the fields will remain the same on
   platforms with 64B cache line, but will change on other platforms.
+
+* pdump: As we changed to use generic IPC, ``rte_pdump_set_socket_dir`` will be
+  deprecated and removed in subsequent release; the parameter, path, of
+  ``rte_pdump_init`` will also be removed.
diff --git a/doc/guides/rel_notes/release_18_05.rst 
b/doc/guides/rel_notes/release_18_05.rst
index 9cc77f8..eefb901 100644
--- a/doc/guides/rel_notes/release_18_05.rst
+++ b/doc/guides/rel_notes/release_18_05.rst
@@ -114,6 +114,13 @@ Known Issues
Also, make sure to start the actual text at the margin.
=
 
+* **pdump is not compatible with old applications.**
+
+  As we changed to use generic multi-process communication for pdump 
negotiation
+  instead of previous dedicated unix socket way, pdump applications, including
+  dpdk-pdump example and any other applications using librte_pdump, cannot work
+  with older version DPDK primary applications.
+
 
 Shared Library Versions
 ---
diff --git a/lib/librte_pdump/Makefile b/lib/librte_pdump/Makefile
index 98fa752..0ee0fa1 100644
--- a/lib/librte_pdump/Makefile
+++ b/lib/librte_pdump/Makefile
@@ -1,11 +1,12 @@
 # SPDX-License-Identifier: BSD-3-Clause
-# Copyright(c) 2016 Intel Corporation
+# Copyright(c) 2016-2018 Intel Corporation
 
 include $(RTE_SDK)/mk/rte.vars.mk
 
 # library name
 LIB = librte_pdump.a
 
+CFLAGS += -DALLOW_EXPERIMENTAL_API
 CFLAGS += $(WERROR_FLAGS) -I$(SRCDIR) -O3
 CFLAGS += -D_GNU_SOURCE
 LDLIBS += -lpthread
diff --git a/lib/librte_pdump/rte_pdump.c b/lib/librte_pdump/rte_pdump.c
index 4fb0b42..37fba47 100644
--- a/lib/librte_pdump/rte_pdump.c
+++ b/lib/librte_pdump/rte_pdump.c
@@ -1,16 +1,7 @@
 /* SPDX-License-Identifier: BSD-3-Clause
- * Copyright(c) 2016 Intel Corporation
+ * Copyright(c) 2016-2018 Intel Corporation
  */
 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-
 #include 
 #include 
 #include 
@@ -20,16 +11,13 @@
 
 #include "rte_pdump.h"
 
-#define SOCKET_PATH_VAR_RUN "/var/run"
-#define SOCKET_PATH_HOME "HOME"
-#define DPDK_DIR "/.dpdk"
-#define SOCKET_DIR   "/pdump_sockets"
-#define SERVER_SOCKET "%s/pdump_server_socket"
-#define CLIENT_SOCKET "%s/pdump_client_socket_%d_%u"
 #define DEVICE_ID_SIZE 64
 /* Macros for printing using RTE_LOG */
 #define RTE_LOGTYPE_PDUMP RTE_LOGTYPE_USER1
 
+/* Used for the multi-process communication */
+#define PDUMP_MP   "mp_pdump"
+
 enum pdump_operation {
DISABLE = 1,
ENABLE = 2
@@ -39,11 +27,6 @@ enum pdump_version {
V1 = 1
 };
 
-static pthread_t pdump_thread;
-static int pdump_socket_fd;
-static char server_socket_dir[PATH_MAX];
-static char client_socket_dir[PATH_MAX];
-
 struct pdump_request {