[dpdk-dev] [PATCH v3 01/22] ethdev: fix MTU size exceeds max rx packet length

[Ferruh Yigit]

On 1/14/2021 4:36 PM, Ferruh Yigit wrote:
> On 1/14/2021 9:45 AM, Steve Yang wrote:
>> Ethdev is using default Ethernet overhead to decide if provided
>> 'max_rx_pkt_len' value is bigger than max (non jumbo) MTU value,
>> and limits it to MAX if it is.
>>
>> Since the application/driver used Ethernet overhead is different than
>> the ethdev one, check result is wrong.
>>
>> If the driver is using Ethernet overhead bigger than the default one,
>> the provided 'max_rx_pkt_len' is trimmed down, and in the driver when
>> correct Ethernet overhead is used to convert back, the resulting MTU is
>> less than the intended one, causing some packets to be dropped.
>>
>> Like,
>> app     -> max_rx_pkt_len = 1500/*mtu*/ + 22/*overhead*/ = 1522
>> ethdev  -> 1522 > 1518/*MAX*/; max_rx_pkt_len = 1518
>> driver  -> MTU = 1518 - 22 = 1496
>> Packets with size 1497-1500 are dropped although intention is to be able
>> to send/receive them.
>>
>> The fix is to make ethdev use the correct Ethernet overhead for port,
>> instead of default one.
>>
>> Fixes: 59d0ecdbf0e1 ("ethdev: MTU accessors")
>>
>> Signed-off-by: Steve Yang <stevex.yang at intel.com>
> 
> <...>
> 
>> @@ -1410,11 +1422,18 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t 
>> nb_rx_q, uint16_t nb_tx_q,
>>               goto rollback;
>>           }
>>       } else {
>> -        if (dev_conf->rxmode.max_rx_pkt_len < RTE_ETHER_MIN_LEN ||
>> -            dev_conf->rxmode.max_rx_pkt_len > RTE_ETHER_MAX_LEN)
>> +        uint16_t pktlen = dev_conf->rxmode.max_rx_pkt_len;
>> +        if (pktlen < RTE_ETHER_MIN_MTU + overhead_len ||
>> +            pktlen > RTE_ETHER_MTU + overhead_len)
>>               /* Use default value */
>>               dev->data->dev_conf.rxmode.max_rx_pkt_len =
>> -                            RTE_ETHER_MAX_LEN;
>> +                        RTE_ETHER_MTU + overhead_len;
> 
> What do you think removing the above check, the else block, completely?
> Since the 'max_rx_pkt_len' should not be used when jumbo frame is not set.
> 

As I tested removing this check is causing problem because some PMDs are using 
the 'max_rx_pkt_len' even jumbo frame is not set.

Perhaps better to keep it, and make a separate patch later to remove this check, 
after PMDs fixed.

>> +    }
>> +
>> +    /* Scale the MTU size to adapt max_rx_pkt_len */
>> +    if (dev_conf->rxmode.offloads & DEV_RX_OFFLOAD_JUMBO_FRAME) {
>> +        dev->data->mtu = dev->data->dev_conf.rxmode.max_rx_pkt_len -
>> +                overhead_len;
>>       }
> 
> Above if block has exact same check, why not move it above block?

Can you still send a new version for above change please?