[dpdk-dev] [PATCH v2 01/22] ethdev: fix MTU size exceeds max rx packet length
Ferruh Yigit
ferruh.yigit at intel.com
Wed Jan 13 11:32:12 CET 2021
On 12/28/2020 2:51 PM, Andrew Rybchenko wrote:
> On 12/17/20 12:22 PM, Steve Yang wrote:
>> If max rx packet length is smaller then MTU + Ether overhead, that will
>> drop all MTU size packets.
>>
>> Update the MTU size according to the max rx packet and Ether overhead.
>>
>> Fixes: 59d0ecdbf0e1 ("ethdev: MTU accessors")
>>
>> Signed-off-by: Steve Yang <stevex.yang at intel.com>
>> ---
>> lib/librte_ethdev/rte_ethdev.c | 21 ++++++++++++++++++---
>> 1 file changed, 18 insertions(+), 3 deletions(-)
>>
>> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
>> index 17ddacc78d..ff6a1e675f 100644
>> --- a/lib/librte_ethdev/rte_ethdev.c
>> +++ b/lib/librte_ethdev/rte_ethdev.c
>> @@ -1292,6 +1292,7 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
>> struct rte_eth_dev *dev;
>> struct rte_eth_dev_info dev_info;
>> struct rte_eth_conf orig_conf;
>> + uint16_t overhead_len;
>> int diag;
>> int ret;
>>
>> @@ -1323,6 +1324,15 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
>> if (ret != 0)
>> goto rollback;
>>
>> + /* Get the real Ethernet overhead length */
>> + if (dev_info.max_mtu &&
>
> First of all I'm not sure that we need to handle 0 value
> separately. Or it should be checked separately and trigger
> an error since it is a driver mis-behaviour.
>
Agree. Most probably we can drop it, "dev_info.max_mtu != UINT16_MAX" covers the
case driver doesn't provide any value.
> If kept, it should be compared vs 0 explicitly in accordance
> with DPDK coding style.
>
>> + dev_info.max_mtu != UINT16_MAX &&
>> + dev_info.max_rx_pktlen &&
>
> It should be compared vs 0 explicitly in accordance
> with DPDK coding style.
>
>> + dev_info.max_rx_pktlen > dev_info.max_mtu)
>> + overhead_len = dev_info.max_rx_pktlen - dev_info.max_mtu;
>> + else
>> + overhead_len = RTE_ETHER_HDR_LEN + RTE_ETHER_CRC_LEN;
>> +
>> /* If number of queues specified by application for both Rx and Tx is
>> * zero, use driver preferred values. This cannot be done individually
>> * as it is valid for either Tx or Rx (but not both) to be zero.
>> @@ -1410,13 +1420,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)
>
> Alignment looks misleading. Either two tabs or just 4 spaces.
>
>> /* Use default value */
>> dev->data->dev_conf.rxmode.max_rx_pkt_len =
>> - RTE_ETHER_MAX_LEN;
>> + RTE_ETHER_MTU + overhead_len;
>> }
>>
>> + /* Scale the MTU size to adapt max_rx_pkt_len */
>> + dev->data->mtu = dev->data->dev_conf.rxmode.max_rx_pkt_len -
>> + overhead_len;
>> +
>
> Is it expected side effect that re-configure always resets
> previously set MTU. I.e.:
> configure -> set_mtu -> start -> stop -> re-configure
> and set MTU is lost.
>
This is the problem of two APIs updating same/related values, when device
re-configure with a given 'max_rx_pkt_len', can we know if the intentions is
update to the value to new provided 'max_rx_pkt_len' or not?
For this case if user want to keep the MTU value, can read the MTU from device
first and set 'max_rx_pkt_len' according it.
And we can reduce to updating the MTU in the configure() only when JUMBO frame
offload is requested, that should be when the 'max_rx_pkt_len' is valid only.
>> /*
>> * If LRO is enabled, check that the maximum aggregated packet
>> * size is supported by the configured device.
>>
>
More information about the dev
mailing list