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

[Huisong Li <lihuisong@huawei.com>]

在 2021/4/27 19:19, Li, Xiaoyun 写道:
>
>> -----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.
ok
> 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.
>    */

Thank you😁 I'm going to fix it as follows:

     /*
      * The fwd_config_setup() is called when the port is RTE_PORT_STARTED
      * or RTE_PORT_STOPPED.
      *
      * Re-configure ports to get updated mapping between tc and queue when
      * the queue number of the port is changed. Skip for started ports 
since
      * modifying queue number and calling dev_configure need 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
> .