Re: [PATCH 2/2] app/testpmd: fix incorrect MTU verification

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

On 4/26/2022 2:38 AM, lihuisong (C) wrote:
> 
> 在 2022/4/26 0:25, Singh, Aman Deep 写道:
>>
>> On 4/6/2022 2:15 PM, Min Hu (Connor) wrote:
>>> From: Huisong Li <lihuisong@huawei.com>
>>>
>>> The macro RTE_ETHER_MIN_LEN isn't the minimum value of MTU. But testpmd
>>> used it when execute 'port config mtu 0 xx' cmd. This patch fix it.
>>>
>>> Fixes: 1bb4a528c41f ("ethdev: fix max Rx packet length")
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>

<...>

>>> @@ -1263,6 +1314,10 @@ port_mtu_set(portid_t port_id, uint16_t mtu)
>>>       if (port_id_is_invalid(port_id, ENABLED_WARN))
>>>           return;
>>>   +    diag = eth_dev_validate_mtu(port_id, mtu);
>>> +    if (diag != 0)
>>> +        return;
>>> +
>>>       if (port->need_reconfig == 0) {
>>>           diag = rte_eth_dev_set_mtu(port_id, mtu);
>>>           if (diag != 0) {
>> I just wanted to know if these added functions eth_dev_validate_mtu() 
>> &  eth_dev_get_overhead_len()
>> are copy of ethdev library API's in file "rte_ethdev.c", which get 
>> called by rte_eth_dev_set_mtu.
>> Is our intent, is to call these twice ?
> If port->need_reconfig is 1, rte_eth_dev_set_mtu doesn't be called, and
> MTU value is saved to port->dev_conf.rxmode.mtu. This dev_conf.rxmode.mtu
> will be set to driver in dev_configure(). This check is performed to
> prevent dev_configure() failure. That's what I think.
>> .

Testpmd stores the MTU value and process it during start_port(), which 
may postpone any possible error in rte_eth_dev_configure(), so I think
it is OK to duplicate the check in app level.

Acked-by: Ferruh Yigit <ferruh.yigit@xilinx.com>