> On Jan 14, 2021, at 12:13 PM, Ferruh Yigit <ferruh.yi...@intel.com> wrote:
>
> 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.y...@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.
Hello Ferruh -
Working on fixing our PMD here. Do you want PMDs to update the JUMBO_FRAME flag
based on the mtu value in dev_set_mtu(), or do you want the application to be
solely responsible for it?
Thanks,
Andrew
>>> + }
>>> +
>>> + /* 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?
