Currently, 'nb_fwd_lcores' value are both adjusted based on 'nb_fwd_streams' in rss/simple/icmp_echo_fwd_config_setup. But the operation is missing in dcb_fwd_config_setup, which may lead to a bad behavior in which multiple polling threads operate on the same queue. In this case, the device sends and receives packets, causing unexpected results. The procedure is as follows: 1/ run testpmd with "--rxq=8 --txq=8" 2/ port stop all 3/ set nbcore 8 4/ port config 0 dcb vt off 4 pfc on 5/ port config all rxq 16 6/ port config all txq 16 7/ port start all 8/ set fwd mac 9/ start For the DCB forwarding test, each core is assigned to each traffic class and each core is assigned a multi-stream. Therefore, 'nb_fwd_lcores' value needs to be adjusted based on 'total_tc_num' in all forwarding ports. Fixes: 900550de04a7 ("app/testpmd: add dcb support") Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings") Cc: stable@dpdk.org Signed-off-by: Huisong Li <lihuisong@huawei.com> --- app/test-pmd/config.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index ccb9bd3..03ee40c 100644 --- a/app/test-pmd/config.c +++ b/app/test-pmd/config.c @@ -2961,6 +2961,21 @@ rss_fwd_config_setup(void) } } +static uint16_t +get_fwd_port_total_tc_num(void) +{ + struct rte_eth_dcb_info dcb_info; + uint16_t total_tc_num = 0; + unsigned int i; + + for (i = 0; i < nb_fwd_ports; i++) { + (void)rte_eth_dev_get_dcb_info(fwd_ports_ids[i], &dcb_info); + total_tc_num += dcb_info.nb_tcs; + } + + return total_tc_num; +} + /** * For the DCB forwarding test, each core is assigned on each traffic class. * @@ -2980,12 +2995,16 @@ dcb_fwd_config_setup(void) lcoreid_t lc_id; uint16_t nb_rx_queue, nb_tx_queue; uint16_t i, j, k, sm_id = 0; + uint16_t total_tc_num; uint8_t tc = 0; cur_fwd_config.nb_fwd_lcores = (lcoreid_t) nb_fwd_lcores; cur_fwd_config.nb_fwd_ports = nb_fwd_ports; cur_fwd_config.nb_fwd_streams = (streamid_t) (nb_rxq * cur_fwd_config.nb_fwd_ports); + total_tc_num = get_fwd_port_total_tc_num(); + if (cur_fwd_config.nb_fwd_lcores > total_tc_num) + cur_fwd_config.nb_fwd_lcores = total_tc_num; /* reinitialize forwarding streams */ init_fwd_streams(); -- 2.7.4
Currently, 'nb_fwd_lcores' value are both adjusted based on 'nb_fwd_streams' in rss/simple/icmp_echo_fwd_config_setup. But the operation is missing in dcb_fwd_config_setup, which may lead to a bad behavior in which multiple polling threads operate on the same queue. In this case, the device sends and receives packets, causing unexpected results. The procedure is as follows: 1/ run testpmd with "--rxq=8 --txq=8" 2/ port stop all 3/ set nbcore 8 4/ port config 0 dcb vt off 4 pfc on 5/ port config all rxq 16 6/ port config all txq 16 7/ port start all 8/ set fwd mac 9/ start For the DCB forwarding test, each core is assigned to each traffic class and each core is assigned a multi-stream. Therefore, 'nb_fwd_lcores' value needs to be adjusted based on 'total_tc_num' in all forwarding ports. Fixes: 900550de04a7 ("app/testpmd: add dcb support") Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings") Cc: stable@dpdk.org Signed-off-by: Huisong Li <lihuisong@huawei.com> --- app/test-pmd/config.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index ccb9bd3..03ee40c 100644 --- a/app/test-pmd/config.c +++ b/app/test-pmd/config.c @@ -2961,6 +2961,21 @@ rss_fwd_config_setup(void) } } +static uint16_t +get_fwd_port_total_tc_num(void) +{ + struct rte_eth_dcb_info dcb_info; + uint16_t total_tc_num = 0; + unsigned int i; + + for (i = 0; i < nb_fwd_ports; i++) { + (void)rte_eth_dev_get_dcb_info(fwd_ports_ids[i], &dcb_info); + total_tc_num += dcb_info.nb_tcs; + } + + return total_tc_num; +} + /** * For the DCB forwarding test, each core is assigned on each traffic class. * @@ -2980,12 +2995,16 @@ dcb_fwd_config_setup(void) lcoreid_t lc_id; uint16_t nb_rx_queue, nb_tx_queue; uint16_t i, j, k, sm_id = 0; + uint16_t total_tc_num; uint8_t tc = 0; cur_fwd_config.nb_fwd_lcores = (lcoreid_t) nb_fwd_lcores; cur_fwd_config.nb_fwd_ports = nb_fwd_ports; cur_fwd_config.nb_fwd_streams = (streamid_t) (nb_rxq * cur_fwd_config.nb_fwd_ports); + total_tc_num = get_fwd_port_total_tc_num(); + if (cur_fwd_config.nb_fwd_lcores > total_tc_num) + cur_fwd_config.nb_fwd_lcores = total_tc_num; /* reinitialize forwarding streams */ init_fwd_streams(); -- 2.7.4
Currently, 'nb_fwd_lcores' value are both adjusted based on 'nb_fwd_streams' in rss/simple/icmp_echo_fwd_config_setup. But the operation is missing in dcb_fwd_config_setup, which may lead to a bad behavior in which multiple polling threads operate on the same queue. In this case, the device sends and receives packets, causing unexpected results. The procedure is as follows: 1/ run testpmd with "--rxq=8 --txq=8" 2/ port stop all 3/ set nbcore 8 4/ port config 0 dcb vt off 4 pfc on 5/ port config all rxq 16 6/ port config all txq 16 7/ port start all 8/ set fwd mac 9/ start For the DCB forwarding test, each core is assigned to each traffic class and each core is assigned a multi-stream. Therefore, 'nb_fwd_lcores' value needs to be adjusted based on 'total_tc_num' in all forwarding ports. Fixes: 900550de04a7 ("app/testpmd: add dcb support") Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings") Cc: stable@dpdk.org Signed-off-by: Huisong Li <lihuisong@huawei.com> Signed-off-by: Lijun Ou <oulijun@huawei.com> --- app/test-pmd/config.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index ccb9bd3..03ee40c 100644 --- a/app/test-pmd/config.c +++ b/app/test-pmd/config.c @@ -2961,6 +2961,21 @@ rss_fwd_config_setup(void) } } +static uint16_t +get_fwd_port_total_tc_num(void) +{ + struct rte_eth_dcb_info dcb_info; + uint16_t total_tc_num = 0; + unsigned int i; + + for (i = 0; i < nb_fwd_ports; i++) { + (void)rte_eth_dev_get_dcb_info(fwd_ports_ids[i], &dcb_info); + total_tc_num += dcb_info.nb_tcs; + } + + return total_tc_num; +} + /** * For the DCB forwarding test, each core is assigned on each traffic class. * @@ -2980,12 +2995,16 @@ dcb_fwd_config_setup(void) lcoreid_t lc_id; uint16_t nb_rx_queue, nb_tx_queue; uint16_t i, j, k, sm_id = 0; + uint16_t total_tc_num; uint8_t tc = 0; cur_fwd_config.nb_fwd_lcores = (lcoreid_t) nb_fwd_lcores; cur_fwd_config.nb_fwd_ports = nb_fwd_ports; cur_fwd_config.nb_fwd_streams = (streamid_t) (nb_rxq * cur_fwd_config.nb_fwd_ports); + total_tc_num = get_fwd_port_total_tc_num(); + if (cur_fwd_config.nb_fwd_lcores > total_tc_num) + cur_fwd_config.nb_fwd_lcores = total_tc_num; /* reinitialize forwarding streams */ init_fwd_streams(); -- 2.7.4
After DCB mode is configured, the operations of port stop and port start change the value of the global variable "dcb_test", As a result, the forwarding configuration from DCB to RSS mode, namely, “dcb_fwd_config_setup()” to "rss_fwd_config_setup()". Currently, the 'dcb_flag' field in struct 'rte_port' indicates whether the port is configured with DCB. And it is sufficient to have 'dcb_config' as a global variable to control the DCB test status. So this patch deletes the "dcb_test". In addition, the 'dcb_config' is first set to 1 first in init_port_dcb_config(), but the function may fail. So it should be moved to the end. Fixes: 900550de04a7 ("app/testpmd: add dcb support") Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings") Fixes: 7741e4cf16c0 ("app/testpmd: VMDq and DCB updates") Cc: stable@dpdk.org Signed-off-by: Huisong Li <lihuisong@huawei.com> Signed-off-by: Lijun Ou <oulijun@huawei.com> --- app/test-pmd/testpmd.c | 18 ++++-------------- app/test-pmd/testpmd.h | 1 - 2 files changed, 4 insertions(+), 15 deletions(-) diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index d4be23f..a076b1d 100644 --- a/app/test-pmd/testpmd.c +++ b/app/test-pmd/testpmd.c @@ -246,9 +246,6 @@ uint16_t mb_mempool_cache = DEF_MBUF_CACHE; /**< Size of mbuf mempool cache. */ /* current configuration is in DCB or not,0 means it is not in DCB mode */ uint8_t dcb_config = 0; -/* Whether the dcb is in testing status */ -uint8_t dcb_test = 0; - /* * Configurable number of RX/TX queues. */ @@ -2167,8 +2164,7 @@ start_packet_forwarding(int with_tx_first) return; } - - if(dcb_test) { + if (dcb_config) { for (i = 0; i < nb_fwd_ports; i++) { pt_id = fwd_ports_ids[i]; port = &ports[pt_id]; @@ -2476,8 +2472,6 @@ start_port(portid_t pid) if (port_id_is_invalid(pid, ENABLED_WARN)) return 0; - if(dcb_config) - dcb_test = 1; RTE_ETH_FOREACH_DEV(pi) { if (pid != pi && pid != (portid_t)RTE_PORT_ALL) continue; @@ -2717,11 +2711,6 @@ stop_port(portid_t pid) portid_t peer_pl[RTE_MAX_ETHPORTS]; int peer_pi; - if (dcb_test) { - dcb_test = 0; - dcb_config = 0; - } - if (port_id_is_invalid(pid, ENABLED_WARN)) return; @@ -3625,8 +3614,6 @@ init_port_dcb_config(portid_t pid, rte_port = &ports[pid]; memset(&port_conf, 0, sizeof(struct rte_eth_conf)); - /* Enter DCB configuration status */ - dcb_config = 1; port_conf.rxmode = rte_port->dev_conf.rxmode; port_conf.txmode = rte_port->dev_conf.txmode; @@ -3694,6 +3681,9 @@ init_port_dcb_config(portid_t pid, rte_port->dcb_flag = 1; + /* Enter DCB configuration status */ + dcb_config = 1; + return 0; } diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h index 9530ec5..432c66d 100644 --- a/app/test-pmd/testpmd.h +++ b/app/test-pmd/testpmd.h @@ -425,7 +425,6 @@ extern uint64_t noisy_lkup_num_reads; extern uint64_t noisy_lkup_num_reads_writes; extern uint8_t dcb_config; -extern uint8_t dcb_test; extern uint32_t mbuf_data_size_n; extern uint16_t mbuf_data_size[MAX_SEGS_BUFFER_SPLIT]; -- 2.7.4
After DCB mode is configured, if we decrease the number of RX and TX queues, fwd_config_setup() will be called to setup the DCB forwarding configuration. And forwarding streams are updated based on new queue numbers in fwd_config_setup(), but the mapping between the TC and queues obtained by rte_eth_dev_get_dcb_info() is still old queue numbers (old queue numbers are greater than new queue numbers). In this case, the segment fault happens. So rte_eth_dev_configure() should be called again to update the mapping between the TC and queues before rte_eth_dev_get_dcb_info(). Like: set nbcore 4 port stop all port config 0 dcb vt off 4 pfc on port start all port stop all port config all rxq 8 port config all txq 8 Fixes: 900550de04a7 ("app/testpmd: add dcb support") Cc: stable@dpdk.org Signed-off-by: Huisong Li <lihuisong@huawei.com> Signed-off-by: Lijun Ou <oulijun@huawei.com> --- app/test-pmd/config.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index 03ee40c..18b197b 100644 --- a/app/test-pmd/config.c +++ b/app/test-pmd/config.c @@ -2996,7 +2996,33 @@ dcb_fwd_config_setup(void) uint16_t nb_rx_queue, nb_tx_queue; uint16_t i, j, k, sm_id = 0; uint16_t total_tc_num; + struct rte_port *port; uint8_t tc = 0; + portid_t pid; + int ret; + + /* + * The fwd_config_setup() is called when the port is RTE_PORT_STARTED + * RTE_PORT_STOPPED. When a port is RTE_PORT_STARTED, dev_configure + * cannot be called. + * + * re-configure the device after changing queue numbers of stopped + * ports, so that the updated mapping between tc and queue can be + * obtained. + */ + for (pid = 0; pid < nb_fwd_ports; pid++) { + if (port_is_started(pid) == 1) + continue; + + port = &ports[pid]; + ret = rte_eth_dev_configure(pid, nb_rxq, nb_txq, + &port->dev_conf); + if (ret < 0) { + printf("Failed to re-configure port %d, ret = %d.\n", + pid, ret); + return; + } + } cur_fwd_config.nb_fwd_lcores = (lcoreid_t) nb_fwd_lcores; cur_fwd_config.nb_fwd_ports = nb_fwd_ports; -- 2.7.4