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 8C96FA0524; Sat, 9 Jan 2021 10:54:35 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 0C31A140E85; Sat, 9 Jan 2021 10:54:35 +0100 (CET) Received: from szxga06-in.huawei.com (szxga06-in.huawei.com [45.249.212.32]) by mails.dpdk.org (Postfix) with ESMTP id B450B140E82 for ; Sat, 9 Jan 2021 10:54:33 +0100 (CET) Received: from DGGEMS411-HUB.china.huawei.com (unknown [172.30.72.60]) by szxga06-in.huawei.com (SkyGuard) with ESMTP id 4DCZzs00xhzhtbw for ; Sat, 9 Jan 2021 17:53:44 +0800 (CST) Received: from [10.67.103.119] (10.67.103.119) by DGGEMS411-HUB.china.huawei.com (10.3.19.211) with Microsoft SMTP Server id 14.3.498.0; Sat, 9 Jan 2021 17:54:26 +0800 To: Ferruh Yigit , , , CC: References: <1610099184-37328-1-git-send-email-oulijun@huawei.com> From: oulijun Message-ID: Date: Sat, 9 Jan 2021 17:54:26 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.1.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.67.103.119] X-CFilter-Loop: Reflected 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" 在 2021/1/8 18:28, Ferruh Yigit 写道: > 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. > Yes, We have not found any other problems. > 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? We refer to the definition of symmetric_mp,We need him to identify exactly which one from the process. > > Also why 'MULTIPLE_PROCESS_HANDLE' define is required? > Yes, I think it is unncessary. I will delete it. > 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; >> > > . >