From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (xvm-189-124.dc0.ghst.net [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 495EFA0524; Fri, 8 Jan 2021 11:29:00 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id BD603140DF9; Fri, 8 Jan 2021 11:28:59 +0100 (CET) Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by mails.dpdk.org (Postfix) with ESMTP id 0F0E6140DF2 for ; Fri, 8 Jan 2021 11:28:57 +0100 (CET) IronPort-SDR: 8SmVyX3B/3vnRNZwY/jDRafPRrCW7waTpnnZHjR8fWQNZIJxlH2TMFKHbwe7sQaHZAS4sF7fYn 6XCWNgpvDSbg== X-IronPort-AV: E=McAfee;i="6000,8403,9857"; a="156759743" X-IronPort-AV: E=Sophos;i="5.79,330,1602572400"; d="scan'208";a="156759743" Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 08 Jan 2021 02:28:57 -0800 IronPort-SDR: 22KEO2ZAmdpyUpsiJp1CBd6ncPqSB6Em59rMpkoA/aEuVRtV3QHWTlCYirpDOpOuTYQEX2AlgG v3lJIpGSqBPQ== X-IronPort-AV: E=Sophos;i="5.79,330,1602572400"; d="scan'208";a="398946628" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.213.237.192]) ([10.213.237.192]) by fmsmga002-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 08 Jan 2021 02:28:55 -0800 To: Lijun Ou , wenzhuo.lu@intel.com, beilei.xing@intel.com, bernard.iremonger@intel.com Cc: dev@dpdk.org References: <1610099184-37328-1-git-send-email-oulijun@huawei.com> From: Ferruh Yigit Message-ID: Date: Fri, 8 Jan 2021 10:28:51 +0000 MIME-Version: 1.0 In-Reply-To: <1610099184-37328-1-git-send-email-oulijun@huawei.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [RFC] app/testpmd: support multi-process X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 1/8/2021 9:46 AM, Lijun Ou wrote: > This patch adds multi-process support for testpmd. > The test cmd example as follows: > the primary cmd: > ./testpmd -w xxx --file-prefix=xx -l 0-1 -n 2 -- -i\ > --rxq=16 --txq=16 --num-procs=2 --proc-id=0 > the secondary cmd: > ./testpmd -w xxx --file-prefix=xx -l 2-3 -n 2 -- -i\ > --rxq=16 --txq=16 --num-procs=2 --proc-id=1 > +1 to add multi-process support to testpmd, missing it reduces multi-process testing a lot, but I guess concern is it may cause lots of changes and create some corner cases in testpmd. Can you please explain a little why new 'proc-id' testpmd argument is needed, why the regular way of using eal '--proc-type' command and 'RTE_PROC_SECONDARY' checks is not enough? Also why 'MULTIPLE_PROCESS_HANDLE' define is required? Thanks, ferruh > Signed-off-by: Min Hu (Connor) > Signed-off-by: Lijun Ou > --- > app/test-pmd/cmdline.c | 6 ++- > app/test-pmd/config.c | 9 +++- > app/test-pmd/parameters.c | 9 ++++ > app/test-pmd/testpmd.c | 133 ++++++++++++++++++++++++++++++++-------------- > app/test-pmd/testpmd.h | 7 +++ > 5 files changed, 121 insertions(+), 43 deletions(-) > > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c > index 2ccbaa0..2237c9f 100644 > --- a/app/test-pmd/cmdline.c > +++ b/app/test-pmd/cmdline.c > @@ -71,8 +71,6 @@ > #include "cmdline_tm.h" > #include "bpf_cmd.h" > > -static struct cmdline *testpmd_cl; > - > static void cmd_reconfig_device_queue(portid_t id, uint8_t dev, uint8_t queue); > > /* *** Help command with introduction. *** */ > @@ -17115,6 +17113,10 @@ prompt(void) > if (testpmd_cl == NULL) > return; > cmdline_interact(testpmd_cl); > + if (unlikely(f_quit == 1)) { > + dup2(testpmd_fd_copy, testpmd_cl->s_in); > + close(testpmd_fd_copy); > + } > cmdline_stdin_exit(testpmd_cl); > } > > diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c > index 3f6c864..1702e0d 100644 > --- a/app/test-pmd/config.c > +++ b/app/test-pmd/config.c > @@ -3100,6 +3100,8 @@ rss_fwd_config_setup(void) > queueid_t rxq; > queueid_t nb_q; > streamid_t sm_id; > + int start; > + int end; > > nb_q = nb_rxq; > if (nb_q > nb_txq) > @@ -3117,7 +3119,10 @@ rss_fwd_config_setup(void) > init_fwd_streams(); > > setup_fwd_config_of_each_lcore(&cur_fwd_config); > - rxp = 0; rxq = 0; > + start = proc_id * nb_q / num_procs; > + end = start + nb_q / num_procs; > + rxp = 0; > + rxq = start; > for (sm_id = 0; sm_id < cur_fwd_config.nb_fwd_streams; sm_id++) { > struct fwd_stream *fs; > > @@ -3134,6 +3139,8 @@ rss_fwd_config_setup(void) > continue; > rxp = 0; > rxq++; > + if (rxq >= end) > + rxq=start; > } > } > > diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c > index 414a006..7807afa 100644 > --- a/app/test-pmd/parameters.c > +++ b/app/test-pmd/parameters.c > @@ -45,6 +45,8 @@ > #include > > #include "testpmd.h" > +#define PARAM_PROC_ID "proc-id" > +#define PARAM_NUM_PROCS "num-procs" > > static void > usage(char* progname) > @@ -603,6 +605,8 @@ launch_args_parse(int argc, char** argv) > { "rx-mq-mode", 1, 0, 0 }, > { "record-core-cycles", 0, 0, 0 }, > { "record-burst-stats", 0, 0, 0 }, > + { PARAM_NUM_PROCS, 1, 0, 0 }, > + { PARAM_PROC_ID, 1, 0, 0 }, > { 0, 0, 0, 0 }, > }; > > @@ -1359,6 +1363,11 @@ launch_args_parse(int argc, char** argv) > record_core_cycles = 1; > if (!strcmp(lgopts[opt_idx].name, "record-burst-stats")) > record_burst_stats = 1; > + > + if (strncmp(lgopts[opt_idx].name, PARAM_NUM_PROCS, 8) == 0) > + num_procs = atoi(optarg); > + if (strncmp(lgopts[opt_idx].name, PARAM_PROC_ID, 7) == 0) > + proc_id = atoi(optarg); > break; > case 'h': > usage(argv[0]); > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c > index 2b60f6c..7ab5f48 100644 > --- a/app/test-pmd/testpmd.c > +++ b/app/test-pmd/testpmd.c > @@ -63,6 +63,8 @@ > > #include "testpmd.h" > > +int testpmd_fd_copy = 500; /* the copy of STDIN_FILENO */ > + > #ifndef MAP_HUGETLB > /* FreeBSD may not have MAP_HUGETLB (in fact, it probably doesn't) */ > #define HUGE_FLAG (0x40000) > @@ -79,6 +81,7 @@ > > #define EXTMEM_HEAP_NAME "extmem" > #define EXTBUF_ZONE_SIZE RTE_PGSIZE_2M > +#define MULTIPLE_PROCESS_HANDLE > > uint16_t verbose_level = 0; /**< Silent by default. */ > int testpmd_logtype; /**< Log type for testpmd logs */ > @@ -125,6 +128,9 @@ uint8_t port_numa[RTE_MAX_ETHPORTS]; > */ > uint8_t rxring_numa[RTE_MAX_ETHPORTS]; > > +int proc_id = 0; > +unsigned num_procs = 1; > + > /* > * Store specified sockets on which TX ring to be used by ports > * is allocated. > @@ -978,16 +984,36 @@ mbuf_pool_create(uint16_t mbuf_seg_size, unsigned nb_mbuf, > /* wrapper to rte_mempool_create() */ > TESTPMD_LOG(INFO, "preferred mempool ops selected: %s\n", > rte_mbuf_best_mempool_ops()); > +#ifdef MULTIPLE_PROCESS_HANDLE > + if (rte_eal_process_type() == RTE_PROC_PRIMARY) > + rte_mp = rte_pktmbuf_pool_create(pool_name, > + nb_mbuf, mb_mempool_cache, 0, > + mbuf_seg_size, socket_id); > + else > + rte_mp = rte_mempool_lookup(pool_name); > +#else > rte_mp = rte_pktmbuf_pool_create(pool_name, nb_mbuf, > mb_mempool_cache, 0, mbuf_seg_size, socket_id); > +#endif > break; > } > case MP_ALLOC_ANON: > { > +#ifdef MULTIPLE_PROCESS_HANDLE > + if (rte_eal_process_type() == RTE_PROC_PRIMARY) > + rte_mp = rte_mempool_create_empty(pool_name, > + nb_mbuf, mb_size, > + (unsigned int)mb_mempool_cache, > + sizeof(struct rte_pktmbuf_pool_private), > + socket_id, 0); > + else > + rte_mp = rte_mempool_lookup(pool_name); > +#else > rte_mp = rte_mempool_create_empty(pool_name, nb_mbuf, > mb_size, (unsigned int) mb_mempool_cache, > sizeof(struct rte_pktmbuf_pool_private), > socket_id, mempool_flags); > +#endif > if (rte_mp == NULL) > goto err; > > @@ -1017,9 +1043,18 @@ mbuf_pool_create(uint16_t mbuf_seg_size, unsigned nb_mbuf, > > TESTPMD_LOG(INFO, "preferred mempool ops selected: %s\n", > rte_mbuf_best_mempool_ops()); > +#ifdef MULTIPLE_PROCESS_HANDLE > + if (rte_eal_process_type() == RTE_PROC_PRIMARY) > + rte_mp = rte_pktmbuf_pool_create(pool_name, > + nb_mbuf, mb_mempool_cache, 0, > + mbuf_seg_size, heap_socket); > + else > + rte_mp = rte_mempool_lookup(pool_name); > +#else > rte_mp = rte_pktmbuf_pool_create(pool_name, nb_mbuf, > mb_mempool_cache, 0, mbuf_seg_size, > heap_socket); > +#endif > break; > } > case MP_ALLOC_XBUF: > @@ -2485,21 +2520,28 @@ start_port(portid_t pid) > return -1; > } > /* configure port */ > - diag = rte_eth_dev_configure(pi, nb_rxq + nb_hairpinq, > + if (rte_eal_process_type() == RTE_PROC_PRIMARY) { > + diag = rte_eth_dev_configure(pi, > + nb_rxq + nb_hairpinq, > nb_txq + nb_hairpinq, > &(port->dev_conf)); > - if (diag != 0) { > - if (rte_atomic16_cmpset(&(port->port_status), > - RTE_PORT_HANDLING, RTE_PORT_STOPPED) == 0) > - printf("Port %d can not be set back " > - "to stopped\n", pi); > - printf("Fail to configure port %d\n", pi); > - /* try to reconfigure port next time */ > - port->need_reconfig = 1; > - return -1; > + if (diag != 0) { > + if (rte_atomic16_cmpset( > + &(port->port_status), > + RTE_PORT_HANDLING, > + RTE_PORT_STOPPED) == 0) > + printf("Port %d can not be set " > + "back to stopped\n", pi); > + printf("Fail to configure port %d\n", > + pi); > + /* try to reconfigure port next time */ > + port->need_reconfig = 1; > + return -1; > + } > } > } > - if (port->need_reconfig_queues > 0) { > + if (port->need_reconfig_queues > 0 && > + rte_eal_process_type() == RTE_PROC_PRIMARY) { > port->need_reconfig_queues = 0; > /* setup tx queues */ > for (qi = 0; qi < nb_txq; qi++) { > @@ -2600,15 +2642,18 @@ start_port(portid_t pid) > cnt_pi++; > > /* start port */ > - if (rte_eth_dev_start(pi) < 0) { > - printf("Fail to start port %d\n", pi); > - > - /* Fail to setup rx queue, return */ > - if (rte_atomic16_cmpset(&(port->port_status), > - RTE_PORT_HANDLING, RTE_PORT_STOPPED) == 0) > - printf("Port %d can not be set back to " > - "stopped\n", pi); > - continue; > + if (rte_eal_process_type() == RTE_PROC_PRIMARY) { > + diag = rte_eth_dev_start(pi); > + if (diag < 0) { > + printf("Fail to start port %d\n", pi); > + > + /* Fail to setup rx queue, return */ > + if (rte_atomic16_cmpset(&(port->port_status), > + RTE_PORT_HANDLING, RTE_PORT_STOPPED) == 0) > + printf("Port %d can not be set back to " > + "stopped\n", pi); > + continue; > + } > } > > if (rte_atomic16_cmpset(&(port->port_status), > @@ -2737,7 +2782,7 @@ stop_port(portid_t pid) > if (port->flow_list) > port_flow_flush(pi); > > - if (rte_eth_dev_stop(pi) != 0) > + if (rte_eal_process_type() == RTE_PROC_PRIMARY && rte_eth_dev_stop(pi) != 0) > RTE_LOG(ERR, EAL, "rte_eth_dev_stop failed for port %u\n", > pi); > > @@ -2806,8 +2851,10 @@ close_port(portid_t pid) > continue; > } > > - port_flow_flush(pi); > - rte_eth_dev_close(pi); > + if (rte_eal_process_type() == RTE_PROC_PRIMARY) > + port_flow_flush(pi); > + if (rte_eal_process_type() == RTE_PROC_PRIMARY) > + rte_eth_dev_close(pi); > } > > remove_invalid_ports(); > @@ -3071,7 +3118,7 @@ pmd_test_exit(void) > } > } > for (i = 0 ; i < RTE_DIM(mempools) ; i++) { > - if (mempools[i]) > + if (rte_eal_process_type() == RTE_PROC_PRIMARY && mempools[i]) > rte_mempool_free(mempools[i]); > } > > @@ -3519,6 +3566,10 @@ init_port_dcb_config(portid_t pid, > int retval; > uint16_t i; > > + if (num_procs > 1) { > + printf("The multi-process feature doesn't support dcb.\n"); > + return -ENOTSUP; > + } > rte_port = &ports[pid]; > > memset(&port_conf, 0, sizeof(struct rte_eth_conf)); > @@ -3617,13 +3668,6 @@ init_port(void) > } > > static void > -force_quit(void) > -{ > - pmd_test_exit(); > - prompt_exit(); > -} > - > -static void > print_stats(void) > { > uint8_t i; > @@ -3654,12 +3698,16 @@ signal_handler(int signum) > if (latencystats_enabled != 0) > rte_latencystats_uninit(); > #endif > - force_quit(); > /* Set flag to indicate the force termination. */ > f_quit = 1; > - /* exit with the expected status */ > - signal(signum, SIG_DFL); > - kill(getpid(), signum); > + if (interactive == 1) { > + dup2(testpmd_cl->s_in, testpmd_fd_copy); > + close(testpmd_cl->s_in); > + } else { > + dup2(0, testpmd_fd_copy); > + close(0); > + } > + > } > } > > @@ -3684,10 +3732,6 @@ main(int argc, char** argv) > rte_exit(EXIT_FAILURE, "Cannot init EAL: %s\n", > rte_strerror(rte_errno)); > > - if (rte_eal_process_type() == RTE_PROC_SECONDARY) > - rte_exit(EXIT_FAILURE, > - "Secondary process type not supported.\n"); > - > ret = register_eth_event_callback(); > if (ret != 0) > rte_exit(EXIT_FAILURE, "Cannot register for ethdev events"); > @@ -3783,8 +3827,10 @@ main(int argc, char** argv) > } > } > > - if (!no_device_start && start_port(RTE_PORT_ALL) != 0) > + if (!no_device_start && start_port(RTE_PORT_ALL) != 0) { > + pmd_test_exit(); > rte_exit(EXIT_FAILURE, "Start ports failed\n"); > + } > > /* set all ports to promiscuous mode by default */ > RTE_ETH_FOREACH_DEV(port_id) { > @@ -3830,6 +3876,8 @@ main(int argc, char** argv) > } > prompt(); > pmd_test_exit(); > + if (unlikely(f_quit == 1)) > + prompt_exit(); > } else > #endif > { > @@ -3865,6 +3913,11 @@ main(int argc, char** argv) > printf("Press enter to exit\n"); > rc = read(0, &c, 1); > pmd_test_exit(); > + if (unlikely(f_quit == 1)) { > + dup2(testpmd_fd_copy, 0); > + close(testpmd_fd_copy); > + prompt_exit(); > + } > if (rc < 0) > return 1; > } > diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h > index 5f23162..8629c57 100644 > --- a/app/test-pmd/testpmd.h > +++ b/app/test-pmd/testpmd.h > @@ -13,6 +13,7 @@ > #include > #include > #include > +#include "cmdline.h" > > #define RTE_PORT_ALL (~(portid_t)0x0) > > @@ -24,6 +25,10 @@ > #define RTE_PORT_CLOSED (uint16_t)2 > #define RTE_PORT_HANDLING (uint16_t)3 > > +uint8_t f_quit; > +int testpmd_fd_copy; > +struct cmdline *testpmd_cl; > + > /* > * It is used to allocate the memory for hash key. > * The hash key size is NIC dependent. > @@ -421,6 +426,8 @@ extern uint64_t noisy_lkup_mem_sz; > extern uint64_t noisy_lkup_num_writes; > extern uint64_t noisy_lkup_num_reads; > extern uint64_t noisy_lkup_num_reads_writes; > +extern int proc_id; > +extern unsigned num_procs; > > extern uint8_t dcb_config; > extern uint8_t dcb_test; >