diff mbox series

[v6,2/2] librte_ethdev: fix MTU size exceeds max rx packet length

Message ID 20201022084851.35134-3-stevex.yang@intel.com (mailing list archive)
State Changes Requested, archived
Delegated to: Ferruh Yigit
Headers
Series fix default max mtu size when device configured |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-broadcom-Functional fail Functional Testing issues
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-testing success Testing PASS
ci/iol-intel-Functional fail Functional Testing issues
ci/Intel-compilation success Compilation OK
ci/iol-mellanox-Performance success Performance Testing PASS
ci/travis-robot success Travis build: passed

Commit Message

Steve Yang Oct. 22, 2020, 8:48 a.m. UTC 
  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: SteveX Yang <stevex.yang@intel.com>
---
 lib/librte_ethdev/rte_ethdev.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

Comments

Ferruh Yigit Oct. 22, 2020, 4:31 p.m. UTC | #1 
On 10/22/2020 9:48 AM, SteveX 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: SteveX Yang <stevex.yang@intel.com>
> ---
>   lib/librte_ethdev/rte_ethdev.c | 14 ++++++++++++++
>   1 file changed, 14 insertions(+)
> 
> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> index b12bb3854..17f1c33ac 100644
> --- a/lib/librte_ethdev/rte_ethdev.c
> +++ b/lib/librte_ethdev/rte_ethdev.c
> @@ -1290,6 +1290,8 @@ 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;
> +	uint16_t max_rx_pktlen;
>   	int diag;
>   	int ret;
>   
> @@ -1415,6 +1417,18 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
>   							RTE_ETHER_MAX_LEN;
>   	}
>   
> +	/*
> +	 * Update MTU value if MTU + OVERHEAD exceeds the max_rx_pkt_len
> +	 */

I am not sure this conditional update is required, the target is keep 
'max_rx_pktlen' & 'mtu' in sync.

So why not just:
dev->data->mtu = max_rx_pktlen - overhead_len;


> +	max_rx_pktlen = dev->data->dev_conf.rxmode.max_rx_pkt_len;
> +	if (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 (max_rx_pktlen < dev->data->mtu + overhead_len)
> +		dev->data->mtu = max_rx_pktlen - overhead_len;
> +
>   	/*
>   	 * If LRO is enabled, check that the maximum aggregated packet
>   	 * size is supported by the configured device.
>
Ananyev, Konstantin Oct. 22, 2020, 4:52 p.m. UTC | #2 
> 
> Update the MTU size according to the max rx packet and Ether overhead.
> 
> Fixes: 59d0ecdbf0e1 ("ethdev: MTU accessors")
> 
> Signed-off-by: SteveX Yang <stevex.yang@intel.com>
> ---
>  lib/librte_ethdev/rte_ethdev.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> index b12bb3854..17f1c33ac 100644
> --- a/lib/librte_ethdev/rte_ethdev.c
> +++ b/lib/librte_ethdev/rte_ethdev.c
> @@ -1290,6 +1290,8 @@ 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;
> +	uint16_t max_rx_pktlen;
>  	int diag;
>  	int ret;
> 
> @@ -1415,6 +1417,18 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
>  							RTE_ETHER_MAX_LEN;
>  	}
> 
> +	/*
> +	 * Update MTU value if MTU + OVERHEAD exceeds the max_rx_pkt_len
> +	 */
> +	max_rx_pktlen = dev->data->dev_conf.rxmode.max_rx_pkt_len;
> +	if (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 (max_rx_pktlen < dev->data->mtu + overhead_len)

Do we need that if() here?
Might be do assignment unconditionally?

> +		dev->data->mtu = max_rx_pktlen - overhead_len;
> +
>  	/*
>  	 * If LRO is enabled, check that the maximum aggregated packet
>  	 * size is supported by the configured device.
> --
> 2.17.1
diff mbox series

Patch

diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index b12bb3854..17f1c33ac 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -1290,6 +1290,8 @@  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;
+	uint16_t max_rx_pktlen;
 	int diag;
 	int ret;
 
@@ -1415,6 +1417,18 @@  rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
 							RTE_ETHER_MAX_LEN;
 	}
 
+	/*
+	 * Update MTU value if MTU + OVERHEAD exceeds the max_rx_pkt_len
+	 */
+	max_rx_pktlen = dev->data->dev_conf.rxmode.max_rx_pkt_len;
+	if (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 (max_rx_pktlen < dev->data->mtu + overhead_len)
+		dev->data->mtu = max_rx_pktlen - overhead_len;
+
 	/*
 	 * If LRO is enabled, check that the maximum aggregated packet
 	 * size is supported by the configured device.