From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id B6ACF1CB4F for ; Thu, 5 Apr 2018 13:52:33 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 05 Apr 2018 04:52:32 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.48,411,1517904000"; d="scan'208";a="34525888" Received: from dpdk06.sh.intel.com ([10.67.110.196]) by fmsmga002.fm.intel.com with ESMTP; 05 Apr 2018 04:52:31 -0700 From: Jianfeng Tan To: dev@dpdk.org Cc: Jianfeng Tan , reshma.pattan@intel.com Date: Thu, 5 Apr 2018 11:54:24 +0000 Message-Id: <1522929264-145845-1-git-send-email-jianfeng.tan@intel.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1520175844-55443-1-git-send-email-jianfeng.tan@intel.com> References: <1520175844-55443-1-git-send-email-jianfeng.tan@intel.com> Subject: [dpdk-dev] [PATCH v3] pdump: change to use generic multi-process channel X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 05 Apr 2018 11:52:34 -0000 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.pattan@intel.com Signed-off-by: Jianfeng Tan --- v3: - Deprecate the enum as suggested by Reshma. - Add __rte_deprecate flag for rte_pdump_set_socket_dir. - Delete rte_pdump_set_socket_dir in pdump example. 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 | 22 +- doc/guides/rel_notes/deprecation.rst | 7 + 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 | 3 +- 6 files changed, 88 insertions(+), 377 deletions(-) diff --git a/app/pdump/main.c b/app/pdump/main.c index d29de03..2d0879c 100644 --- a/app/pdump/main.c +++ b/app/pdump/main.c @@ -156,9 +156,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); } @@ -744,22 +746,6 @@ 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) { diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst index ec70b5f..857450a 100644 --- a/doc/guides/rel_notes/deprecation.rst +++ b/doc/guides/rel_notes/deprecation.rst @@ -150,3 +150,10 @@ 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, 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_18_05.rst b/doc/guides/rel_notes/release_18_05.rst index e5fac1c..966bd36 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 ad6efc6..f7cfaec 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 @@ -21,16 +12,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 @@ -40,11 +28,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 { uint16_t ver; uint16_t op; @@ -308,7 +291,7 @@ pdump_register_tx_callbacks(uint16_t end_q, uint16_t port, uint16_t queue, } static int -set_pdump_rxtx_cbs(struct pdump_request *p) +set_pdump_rxtx_cbs(const struct pdump_request *p) { uint16_t nb_rx_q = 0, nb_tx_q = 0, end_q, queue; uint16_t port; @@ -392,313 +375,49 @@ set_pdump_rxtx_cbs(struct pdump_request *p) return ret; } -/* get socket path (/var/run if root, $HOME otherwise) */ static int -pdump_get_socket_path(char *buffer, int bufsz, enum rte_pdump_socktype type) +pdump_server(const struct rte_mp_msg *mp_msg, const void *peer) { - 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) - strlcpy(dir, server_socket_dir, sizeof(dir)); - else if (type == RTE_PDUMP_SOCKET_CLIENT && client_socket_dir[0] != 0) - strlcpy(dir, client_socket_dir, sizeof(dir)); - else { - if (getuid() != 0) { - dir_home = getenv(SOCKET_PATH_HOME); - if (!dir_home) { - RTE_LOG(ERR, PDUMP, - "Failed to get environment variable" - " value for %s, %s:%d\n", - SOCKET_PATH_HOME, __func__, __LINE__); - return -1; - } - snprintf(dpdk_dir, sizeof(dpdk_dir), "%s%s", - dir_home, DPDK_DIR); - } else - snprintf(dpdk_dir, sizeof(dpdk_dir), "%s%s", - SOCKET_PATH_VAR_RUN, DPDK_DIR); - - mkdir(dpdk_dir, 0700); - snprintf(dir, sizeof(dir), "%s%s", - dpdk_dir, SOCKET_DIR); - } - - ret = mkdir(dir, 0700); - /* 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 - snprintf(buffer, bufsz, CLIENT_SOCKET, dir, getpid(), - rte_sys_gettid()); - - return 0; -} - -static int -pdump_create_server_socket(void) -{ - int ret, socket_fd; - struct sockaddr_un addr; - socklen_t addr_len; - - ret = pdump_get_socket_path(addr.sun_path, sizeof(addr.sun_path), - RTE_PDUMP_SOCKET_SERVER); - if (ret != 0) { - RTE_LOG(ERR, PDUMP, - "Failed to get server socket path: %s:%d\n", - __func__, __LINE__); - return -1; - } - addr.sun_family = AF_UNIX; - - /* remove if file already exists */ - unlink(addr.sun_path); - - /* set up a server socket */ - socket_fd = socket(AF_UNIX, SOCK_DGRAM, 0); - if (socket_fd < 0) { - RTE_LOG(ERR, PDUMP, - "Failed to create server socket: %s, %s:%d\n", - strerror(errno), __func__, __LINE__); - return -1; - } - - addr_len = sizeof(struct sockaddr_un); - ret = bind(socket_fd, (struct sockaddr *) &addr, addr_len); - if (ret) { - RTE_LOG(ERR, PDUMP, - "Failed to bind to server socket: %s, %s:%d\n", - strerror(errno), __func__, __LINE__); - close(socket_fd); - return -1; + struct rte_mp_msg mp_resp; + const struct pdump_request *cli_req; + struct pdump_response *resp = (struct pdump_response *)&mp_resp.param; + + /* 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; + } else { + cli_req = (const struct pdump_request *)mp_msg->param; + resp->ver = cli_req->ver; + resp->res_op = cli_req->op; + resp->err_value = set_pdump_rxtx_cbs(cli_req); } - /* save the socket in local configuration */ - pdump_socket_fd = socket_fd; + strlcpy(mp_resp.name, PDUMP_MP, RTE_MP_MAX_NAME_LEN); + 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__); return 0; } -static __attribute__((noreturn)) void * -pdump_thread_main(__rte_unused void *arg) -{ - struct sockaddr_un cli_addr; - socklen_t cli_len; - struct pdump_request cli_req; - struct pdump_response resp; - int n; - int ret = 0; - - /* host thread, never break out */ - for (;;) { - /* recv client requests */ - cli_len = sizeof(cli_addr); - n = recvfrom(pdump_socket_fd, &cli_req, - sizeof(struct pdump_request), 0, - (struct sockaddr *)&cli_addr, &cli_len); - if (n < 0) { - RTE_LOG(ERR, PDUMP, - "failed to recv from client:%s, %s:%d\n", - strerror(errno), __func__, __LINE__); - continue; - } - - ret = set_pdump_rxtx_cbs(&cli_req); - - resp.ver = cli_req.ver; - resp.res_op = cli_req.op; - resp.err_value = ret; - n = sendto(pdump_socket_fd, &resp, - sizeof(struct pdump_response), - 0, (struct sockaddr *)&cli_addr, cli_len); - if (n < 0) { - RTE_LOG(ERR, PDUMP, - "failed to send to client:%s, %s:%d\n", - strerror(errno), __func__, __LINE__); - } - } -} - int -rte_pdump_init(const char *path) +rte_pdump_init(const char *path __rte_unused) { - int ret = 0; - char thread_name[RTE_MAX_THREAD_NAME_LEN]; - - ret = rte_pdump_set_socket_dir(path, RTE_PDUMP_SOCKET_SERVER); - if (ret != 0) - return -1; - - ret = pdump_create_server_socket(); - if (ret != 0) { - RTE_LOG(ERR, PDUMP, - "Failed to create server socket:%s:%d\n", - __func__, __LINE__); - return -1; - } - - /* create the host thread to wait/handle pdump requests */ - ret = pthread_create(&pdump_thread, NULL, pdump_thread_main, NULL); - if (ret != 0) { - RTE_LOG(ERR, PDUMP, - "Failed to create the pdump thread:%s, %s:%d\n", - strerror(ret), __func__, __LINE__); - return -1; - } - /* Set thread_name for aid in debugging. */ - snprintf(thread_name, RTE_MAX_THREAD_NAME_LEN, "pdump-thread"); - ret = rte_thread_setname(pdump_thread, thread_name); - if (ret != 0) { - RTE_LOG(DEBUG, PDUMP, - "Failed to set thread name for pdump handling\n"); - } - - return 0; + return rte_mp_action_register(PDUMP_MP, pdump_server); } int rte_pdump_uninit(void) { - int ret; - - ret = pthread_cancel(pdump_thread); - if (ret != 0) { - RTE_LOG(ERR, PDUMP, - "Failed to cancel the pdump thread:%s, %s:%d\n", - strerror(ret), __func__, __LINE__); - return -1; - } - - ret = close(pdump_socket_fd); - if (ret != 0) { - RTE_LOG(ERR, PDUMP, - "Failed to close server socket: %s, %s:%d\n", - strerror(errno), __func__, __LINE__); - return -1; - } - - struct sockaddr_un addr; - - ret = pdump_get_socket_path(addr.sun_path, sizeof(addr.sun_path), - RTE_PDUMP_SOCKET_SERVER); - if (ret != 0) { - RTE_LOG(ERR, PDUMP, - "Failed to get server socket path: %s:%d\n", - __func__, __LINE__); - return -1; - } - ret = unlink(addr.sun_path); - if (ret != 0) { - RTE_LOG(ERR, PDUMP, - "Failed to remove server socket addr: %s, %s:%d\n", - strerror(errno), __func__, __LINE__); - return -1; - } + rte_mp_action_unregister(PDUMP_MP); return 0; } static int -pdump_create_client_socket(struct pdump_request *p) -{ - int ret, socket_fd; - int pid; - int n; - struct pdump_response server_resp; - struct sockaddr_un addr, serv_addr, from; - socklen_t addr_len, serv_len; - - pid = getpid(); - - socket_fd = socket(AF_UNIX, SOCK_DGRAM, 0); - if (socket_fd < 0) { - RTE_LOG(ERR, PDUMP, - "client socket(): %s:pid(%d):tid(%u), %s:%d\n", - strerror(errno), pid, rte_sys_gettid(), - __func__, __LINE__); - rte_errno = errno; - return -1; - } - - ret = pdump_get_socket_path(addr.sun_path, sizeof(addr.sun_path), - RTE_PDUMP_SOCKET_CLIENT); - if (ret != 0) { - 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; - addr_len = sizeof(struct sockaddr_un); - - do { - ret = bind(socket_fd, (struct sockaddr *) &addr, addr_len); - if (ret) { - RTE_LOG(ERR, PDUMP, - "client bind(): %s, %s:%d\n", - strerror(errno), __func__, __LINE__); - rte_errno = errno; - break; - } - - serv_len = sizeof(struct sockaddr_un); - memset(&serv_addr, 0, sizeof(serv_addr)); - ret = pdump_get_socket_path(serv_addr.sun_path, - sizeof(serv_addr.sun_path), - RTE_PDUMP_SOCKET_SERVER); - if (ret != 0) { - 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; - - n = sendto(socket_fd, p, sizeof(struct pdump_request), 0, - (struct sockaddr *)&serv_addr, serv_len); - if (n < 0) { - RTE_LOG(ERR, PDUMP, - "failed to send to server:%s, %s:%d\n", - strerror(errno), __func__, __LINE__); - rte_errno = errno; - ret = -1; - break; - } - - n = recvfrom(socket_fd, &server_resp, - sizeof(struct pdump_response), 0, - (struct sockaddr *)&from, &serv_len); - if (n < 0) { - RTE_LOG(ERR, PDUMP, - "failed to recv from server:%s, %s:%d\n", - strerror(errno), __func__, __LINE__); - rte_errno = errno; - ret = -1; - break; - } - ret = server_resp.err_value; - } while (0); - -exit: - close(socket_fd); - unlink(addr.sun_path); - return ret; -} - -static int pdump_validate_ring_mp(struct rte_ring *ring, struct rte_mempool *mp) { if (ring == NULL || mp == NULL) { @@ -769,36 +488,48 @@ pdump_prepare_client_request(char *device, uint16_t queue, struct rte_mempool *mp, void *filter) { - int ret; - struct pdump_request req = {.ver = 1,}; - - req.flags = flags; - req.op = operation; + int ret = -1; + struct rte_mp_msg mp_req, *mp_rep; + struct rte_mp_reply mp_reply; + struct timespec ts = {.tv_sec = 5, .tv_nsec = 0}; + struct pdump_request *req = (struct pdump_request *)mp_req.param; + struct pdump_response *resp; + + req->ver = 1; + req->flags = flags; + req->op = operation; if ((operation & ENABLE) != 0) { - snprintf(req.data.en_v1.device, sizeof(req.data.en_v1.device), - "%s", device); - req.data.en_v1.queue = queue; - req.data.en_v1.ring = ring; - req.data.en_v1.mp = mp; - req.data.en_v1.filter = filter; + snprintf(req->data.en_v1.device, + sizeof(req->data.en_v1.device), "%s", device); + req->data.en_v1.queue = queue; + req->data.en_v1.ring = ring; + req->data.en_v1.mp = mp; + req->data.en_v1.filter = filter; } else { - snprintf(req.data.dis_v1.device, sizeof(req.data.dis_v1.device), - "%s", device); - req.data.dis_v1.queue = queue; - req.data.dis_v1.ring = NULL; - req.data.dis_v1.mp = NULL; - req.data.dis_v1.filter = NULL; + snprintf(req->data.dis_v1.device, + sizeof(req->data.dis_v1.device), "%s", device); + req->data.dis_v1.queue = queue; + req->data.dis_v1.ring = NULL; + req->data.dis_v1.mp = NULL; + req->data.dis_v1.filter = NULL; + } + + strlcpy(mp_req.name, PDUMP_MP, RTE_MP_MAX_NAME_LEN); + mp_req.len_param = sizeof(*req); + mp_req.num_fds = 0; + if (rte_mp_request_sync(&mp_req, &mp_reply, &ts) == 0) { + mp_rep = &mp_reply.msgs[0]; + resp = (struct pdump_response *)mp_rep->param; + rte_errno = resp->err_value; + if (!resp->err_value) + ret = 0; + free(mp_reply.msgs); } - ret = pdump_create_client_socket(&req); - if (ret < 0) { + if (ret < 0) RTE_LOG(ERR, PDUMP, "client request for pdump enable/disable failed\n"); - rte_errno = ret; - return -1; - } - - return 0; + return ret; } int @@ -885,30 +616,8 @@ rte_pdump_disable_by_deviceid(char *device_id, uint16_t queue, } 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) { - int ret, count; - - if (path != NULL) { - if (type == RTE_PDUMP_SOCKET_SERVER) { - count = sizeof(server_socket_dir); - ret = strlcpy(server_socket_dir, path, count); - } else { - count = sizeof(client_socket_dir); - ret = strlcpy(client_socket_dir, path, count); - } - - if (ret < 0 || ret >= count) { - RTE_LOG(ERR, PDUMP, - "Invalid socket path:%s:%d\n", - __func__, __LINE__); - if (type == RTE_PDUMP_SOCKET_SERVER) - server_socket_dir[0] = 0; - else - client_socket_dir[0] = 0; - return -EINVAL; - } - } - return 0; } diff --git a/lib/librte_pdump/rte_pdump.h b/lib/librte_pdump/rte_pdump.h index a7e8372..c72f72f 100644 --- a/lib/librte_pdump/rte_pdump.h +++ b/lib/librte_pdump/rte_pdump.h @@ -163,6 +163,7 @@ 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. @@ -181,7 +182,7 @@ rte_pdump_disable_by_deviceid(char *device_id, uint16_t queue, * 0 on success, -EINVAL on error * */ -int +__rte_deprecated int rte_pdump_set_socket_dir(const char *path, enum rte_pdump_socktype type); #ifdef __cplusplus -- 2.7.4