On 1/14/2021 5:29 PM, Andrew Boyer wrote:
On Jan 14, 2021, at 12:13 PM, Ferruh Yigit <ferruh.yi...@intel.com
<mailto: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 <mailto: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?
Hi Andrew,
Technically JUMBO_FRAME flag is an user config and application should set it. It
is application's responsibility to check the capability and set the flag when
necessary.
But, after above said, many PMDs set it based on provided MTU value, if the
explicitly requested MTU is bigger than the RTE_ETHER_MTU, this means user
implied the JUMBO_FRAME support, for this case PMDs set the flag implicitly
instead of failing.
In another thread Andrew R. & Konstantin suggested to remove the JUMBO_FRAME
flag, since it is redundant and causing this kind of confusion, instead driver
can decide based on requested MTU value, and driver reported 'max_mtu' value can
be used by application to detect the capability. We will probably do this
change, but it can be done only in the ABI break release, v21.11.
For now, PMD can set the flag itself if requested MTU > RTE_ETHER_MTU and driver
supports jumbo frames.
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?
