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.

+       }
+
+       /* 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?

Reply via email to