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.y...@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.

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.

>       /*
>        * If LRO is enabled, check that the maximum aggregated packet
>        * size is supported by the configured device.
> 

Reply via email to