Re: [dpdk-dev] [PATCH] app/testpmd: configure rxd and txd number correctly

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 4/18/2020 3:30 AM, oulijun wrote:
> 
> 
> 在 2020/4/18 8:42, Ferruh Yigit 写道:
>> On 4/17/2020 11:59 AM, Lijun Ou wrote:
>>> When users configure rxds and txds by used port config cmd based
>>> on testpmd application, it will not be able to configure rxd and
>>> txd according to the max capability range supported by the actual
>>> NIC hardware. Due testpmd defects, it can only configure a fixed
>>> range to 0 to 2048.
>>> The final result is that an incorrect printing prompt appears and
>>> cannot be applied using rxd && txd according to the actual
>>> capabilities supported by the device.
>>> In order to solve the above problems, we modify the testpmd. First
>>> by calling the rte_eth_dev_info_get api to obtain the max and min
>>> rx/tx capability supported by the hns3, and then use this range
>>> to compare with the actual value by users configured and make
>>> reasonable limitation.
>>>
>>> Signed-off-by: Lijun Ou <oulijun@huawei.com>
>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>>> Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
>>
>> <...>
>>
>>> @@ -1212,6 +1383,8 @@ init_config(void)
>>>   	lcoreid_t  lc_id;
>>>   	uint8_t port_per_socket[RTE_MAX_NUMA_NODES];
>>>   	struct rte_gro_param gro_param;
>>> +	uint16_t allowed_max_rxd;
>>> +	uint16_t allowed_max_txd;
>>>   	uint32_t gso_types;
>>>   	uint16_t data_size;
>>>   	bool warning = 0;
>>> @@ -1239,6 +1412,9 @@ init_config(void)
>>>   		fwd_lcores[lc_id]->cpuid_idx = lc_id;
>>>   	}
>>>   
>>> +	allowed_max_rxd = RTE_TEST_RX_DESC_MAX;
>>> +	allowed_max_txd = RTE_TEST_RX_DESC_MAX;
>>> +
>>>   	RTE_ETH_FOREACH_DEV(pid) {
>>>   		port = &ports[pid];
>>>   		/* Apply default TxRx configuration for all ports */
>>> @@ -1299,6 +1475,13 @@ init_config(void)
>>>   				warning = 1;
>>>   			}
>>>   		}
>>> +
>>> +		/* Get the maximum number of txd and rxd per queue. */
>>> +		if (port->dev_info.tx_desc_lim.nb_max > allowed_max_rxd)
>>> +			allowed_max_txd = port->dev_info.tx_desc_lim.nb_max;
>>> +
>>> +		if (port->dev_info.rx_desc_lim.nb_max > allowed_max_txd)
>>> +			allowed_max_rxd = port->dev_info.rx_desc_lim.nb_max;
>>>   	}
>>>   
>>>   	if (warning)
>>> @@ -1317,9 +1500,9 @@ init_config(void)
>>>   	if (param_total_num_mbufs)
>>>   		nb_mbuf_per_pool = param_total_num_mbufs;
>>>   	else {
>>> -		nb_mbuf_per_pool = RTE_TEST_RX_DESC_MAX +
>>> +		nb_mbuf_per_pool = allowed_max_rxd +
>>>   			(nb_lcores * mb_mempool_cache) +
>>> -			RTE_TEST_TX_DESC_MAX + MAX_PKT_BURST;
>>> +			allowed_max_txd + MAX_PKT_BURST;
>>
>> Overall patch looks good, but with above change, for the PMDs that doesn't
>> explicitly set 'dev_info.tx_desc_lim.nb_max' gets the default value
>> 'UINT16_MAX', like virtual PMDs, and this increases the memmory requirement a lot.
>>
> Hi，Ferruh
>    Thanks. if some PMDs are not configured according to the 
> specifications that are actually supported, does the PMDs driver require 
> such a large mbuf by default.
>> What do you think to keep "port config all rxd|txd <value>" the fix, but remove
>> above nb_mbuf change?
> Actually, I agree with your suggestion. But at the same time worry about 
> whether mbuf is not enough, when rxd/txd is greater that 
> RTE_TEST_TX_DESC_MAX

That is valid concern I think, but user can override the number of mbufs with
"--total-num-mbufs" parameter, and if device has more than 2048 descriptor the
user can provide bigger numbers with this param.