Re: [dpdk-dev] [PATCH V3 3/7] app/testpmd: fix a segment fault when DCB test

["Li, Xiaoyun" <xiaoyun.li@intel.com>]

> -----Original Message-----
> From: Huisong Li <lihuisong@huawei.com>
> Sent: Tuesday, April 20, 2021 17:01
> To: dev@dpdk.org
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Li, Xiaoyun <xiaoyun.li@intel.com>;
> linuxarm@openeuler.org; lihuisong@huawei.com
> Subject: [PATCH V3 3/7] app/testpmd: fix a segment fault when DCB test
> 
> 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

When the port is RTE_PORT_STARTED or RTE_PORT_STARTED? Missing an 'or' here.
Maybe something like the following will be better? (just a reference, you can put it in a better way)
/*
  * Re-configure ports to get updated mapping between tc and queue in case the queue number of the port is changed.
  * Skip for started ports since configuring queue number needs to stop ports first.
  */

> +	 * 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