DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] net/i40e: fix vlan packets drop
@ 2019-10-14  7:53 Xiao Zhang
  2019-10-14 17:41 ` Kevin Traynor
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Xiao Zhang @ 2019-10-14  7:53 UTC (permalink / raw)
  To: dev; +Cc: beilei.xing, qi.z.zhang, Xiao Zhang, stable

vlan packets with ip length bigger then 1496 will not be received by
i40e due to wrong packets size checking. This patch fixes the issue by
correcting the maximum frame size during checking.

Fixes: 35b2d13fd6fd ("net: add rte prefix to ether defines")
Cc: stable@dpdk.org

Signed-off-by: Xiao Zhang <xiao.zhang@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 2ca14da..156d67b 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -12103,7 +12103,7 @@ i40e_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
 		return -EBUSY;
 	}
 
-	if (frame_size > RTE_ETHER_MAX_LEN)
+	if (frame_size > RTE_ETHER_MAX_LEN + I40E_VLAN_TAG_SIZE * 2)
 		dev_data->dev_conf.rxmode.offloads |=
 			DEV_RX_OFFLOAD_JUMBO_FRAME;
 	else
-- 
2.7.4


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [dpdk-dev] net/i40e: fix vlan packets drop
  2019-10-14  7:53 [dpdk-dev] net/i40e: fix vlan packets drop Xiao Zhang
@ 2019-10-14 17:41 ` Kevin Traynor
  2019-10-15  1:41   ` Zhang, Xiao
  2019-10-21  2:44 ` [dpdk-dev] [v2] " Xiao Zhang
  2019-10-29  5:12 ` [dpdk-dev] [v3] " Xiao Zhang
  2 siblings, 1 reply; 10+ messages in thread
From: Kevin Traynor @ 2019-10-14 17:41 UTC (permalink / raw)
  To: Xiao Zhang, dev; +Cc: beilei.xing, qi.z.zhang, stable, Stokes, Ian

On 14/10/2019 08:53, Xiao Zhang wrote:
> vlan packets with ip length bigger then 1496 will not be received by
> i40e due to wrong packets size checking. This patch fixes the issue by
> correcting the maximum frame size during checking.
> 
> Fixes: 35b2d13fd6fd ("net: add rte prefix to ether defines")

To make sure it is backported to the correct stable branches, please tag
the commit that introduced the bug, not the last commit to touch the line.

> Cc: stable@dpdk.org
> 
> Signed-off-by: Xiao Zhang <xiao.zhang@intel.com>
> ---
>  drivers/net/i40e/i40e_ethdev.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 2ca14da..156d67b 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c

What about vf?

> @@ -12103,7 +12103,7 @@ i40e_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
>  		return -EBUSY;
>  	}
>  
> -	if (frame_size > RTE_ETHER_MAX_LEN)
> +	if (frame_size > RTE_ETHER_MAX_LEN + I40E_VLAN_TAG_SIZE * 2)
>  		dev_data->dev_conf.rxmode.offloads |=
>  			DEV_RX_OFFLOAD_JUMBO_FRAME;
>  	else
> 

+cc Ian, who looked into MTU for i40e a while back.

MTU code changing makes me nervous. You would need to look through
everywhere there is something related to pkt len to check it is still ok.

E.g. if I got it right (maybe I miss something), this means a 1500 mtu
will set frame_size to 1526, which will turn off jumbo and set
dev_data->dev_conf.rxmode.max_rx_pkt_len = 1526

Then in i40e_rx_queue_config()

rxq->max_pkt_len = RTE_MIN(len, data->dev_conf.rxmode.max_rx_pkt_len);
                   ^^^ lets say max_rx_pkt_len (1526) is the min

if (data->dev_conf.rxmode.offloads & DEV_RX_OFFLOAD_JUMBO_FRAME) {

[snip jumbo on branch]

} else {
	if (rxq->max_pkt_len < RTE_ETHER_MIN_LEN ||
		rxq->max_pkt_len > RTE_ETHER_MAX_LEN) {
                ^^^ 1526           ^^^ 1518

		PMD_DRV_LOG(ERR, "maximum packet length must be "
			    "larger than %u and smaller than %u, "
			    "as jumbo frame is disabled",
			    (uint32_t)RTE_ETHER_MIN_LEN,
			    (uint32_t)RTE_ETHER_MAX_LEN);
		return I40E_ERR_CONFIG;
                ^^^ Error returned ???
	}
}

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [dpdk-dev] net/i40e: fix vlan packets drop
  2019-10-14 17:41 ` Kevin Traynor
@ 2019-10-15  1:41   ` Zhang, Xiao
  0 siblings, 0 replies; 10+ messages in thread
From: Zhang, Xiao @ 2019-10-15  1:41 UTC (permalink / raw)
  To: Kevin Traynor, dev; +Cc: Xing, Beilei, Zhang, Qi Z, stable, Stokes, Ian

Hi Kevin,

> -----Original Message-----
> From: Kevin Traynor [mailto:ktraynor@redhat.com]
> Sent: Tuesday, October 15, 2019 1:42 AM
> To: Zhang, Xiao <xiao.zhang@intel.com>; dev@dpdk.org
> Cc: Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
> stable@dpdk.org; Stokes, Ian <ian.stokes@intel.com>
> Subject: Re: [dpdk-dev] net/i40e: fix vlan packets drop
> 
> On 14/10/2019 08:53, Xiao Zhang wrote:
> > vlan packets with ip length bigger then 1496 will not be received by
> > i40e due to wrong packets size checking. This patch fixes the issue by
> > correcting the maximum frame size during checking.
> >
> > Fixes: 35b2d13fd6fd ("net: add rte prefix to ether defines")
> 
> To make sure it is backported to the correct stable branches, please tag the
> commit that introduced the bug, not the last commit to touch the line.

Got it, the right fixline should be Fixes: 43e5488c0ac6 ("net/i40e: support MTU configuration"), will correct it.

> 
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Xiao Zhang <xiao.zhang@intel.com>
> > ---
> >  drivers/net/i40e/i40e_ethdev.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > b/drivers/net/i40e/i40e_ethdev.c index 2ca14da..156d67b 100644
> > --- a/drivers/net/i40e/i40e_ethdev.c
> > +++ b/drivers/net/i40e/i40e_ethdev.c
> 
> What about vf?

vf also need to be fixed, will add it.

> 
> > @@ -12103,7 +12103,7 @@ i40e_dev_mtu_set(struct rte_eth_dev *dev,
> uint16_t mtu)
> >  		return -EBUSY;
> >  	}
> >
> > -	if (frame_size > RTE_ETHER_MAX_LEN)
> > +	if (frame_size > RTE_ETHER_MAX_LEN + I40E_VLAN_TAG_SIZE * 2)
> >  		dev_data->dev_conf.rxmode.offloads |=
> >  			DEV_RX_OFFLOAD_JUMBO_FRAME;
> >  	else
> >
> 
> +cc Ian, who looked into MTU for i40e a while back.
> 
> MTU code changing makes me nervous. You would need to look through
> everywhere there is something related to pkt len to check it is still ok.
> 
> E.g. if I got it right (maybe I miss something), this means a 1500 mtu will set
> frame_size to 1526, which will turn off jumbo and set dev_data-
> >dev_conf.rxmode.max_rx_pkt_len = 1526
> 
> Then in i40e_rx_queue_config()
> 
> rxq->max_pkt_len = RTE_MIN(len, data->dev_conf.rxmode.max_rx_pkt_len);
>                    ^^^ lets say max_rx_pkt_len (1526) is the min
> 
> if (data->dev_conf.rxmode.offloads & DEV_RX_OFFLOAD_JUMBO_FRAME) {
> 
> [snip jumbo on branch]
> 
> } else {
> 	if (rxq->max_pkt_len < RTE_ETHER_MIN_LEN ||
> 		rxq->max_pkt_len > RTE_ETHER_MAX_LEN) {
>                 ^^^ 1526           ^^^ 1518
> 
> 		PMD_DRV_LOG(ERR, "maximum packet length must be "
> 			    "larger than %u and smaller than %u, "
> 			    "as jumbo frame is disabled",
> 			    (uint32_t)RTE_ETHER_MIN_LEN,
> 			    (uint32_t)RTE_ETHER_MAX_LEN);
> 		return I40E_ERR_CONFIG;
>                 ^^^ Error returned ???
> 	}
> }

Thanks for point out. Will go through the code to check if related pkt length still ok.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [dpdk-dev] [v2] net/i40e: fix vlan packets drop
  2019-10-14  7:53 [dpdk-dev] net/i40e: fix vlan packets drop Xiao Zhang
  2019-10-14 17:41 ` Kevin Traynor
@ 2019-10-21  2:44 ` Xiao Zhang
  2019-10-21 20:15   ` Kevin Traynor
  2019-10-29  5:12 ` [dpdk-dev] [v3] " Xiao Zhang
  2 siblings, 1 reply; 10+ messages in thread
From: Xiao Zhang @ 2019-10-21  2:44 UTC (permalink / raw)
  To: dev; +Cc: ktraynor, ian.stokes, beilei.xing, qi.z.zhang, Xiao Zhang, stable

Vlan packets with ip length bigger then 1496 will not be received by
i40e/i40evf due to wrong packets size checking. This patch fixes the issue
by correcting the maximum frame size during checking.

Fixes: 43e5488c0ac6 ("net/i40e: support MTU configuration")
Cc: stable@dpdk.org

Signed-off-by: Xiao Zhang <xiao.zhang@intel.com>
---
v2
add fix for i40evf and correct the checking when using the max_pkt_len.
---
 drivers/net/i40e/i40e_ethdev.c    | 2 +-
 drivers/net/i40e/i40e_ethdev_vf.c | 8 +++++---
 drivers/net/i40e/i40e_rxtx.c      | 6 ++++--
 3 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 2ca14da..156d67b 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -12103,7 +12103,7 @@ i40e_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
 		return -EBUSY;
 	}
 
-	if (frame_size > RTE_ETHER_MAX_LEN)
+	if (frame_size > RTE_ETHER_MAX_LEN + I40E_VLAN_TAG_SIZE * 2)
 		dev_data->dev_conf.rxmode.offloads |=
 			DEV_RX_OFFLOAD_JUMBO_FRAME;
 	else
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index 5dba092..bc825e2 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -1782,12 +1782,14 @@ i40evf_rxq_init(struct rte_eth_dev *dev, struct i40e_rx_queue *rxq)
 		}
 	} else {
 		if (rxq->max_pkt_len < RTE_ETHER_MIN_LEN ||
-		    rxq->max_pkt_len > RTE_ETHER_MAX_LEN) {
+		    rxq->max_pkt_len > RTE_ETHER_MAX_LEN +
+				I40E_VLAN_TAG_SIZE * 2) {
 			PMD_DRV_LOG(ERR, "maximum packet length must be "
 				"larger than %u and smaller than %u, as jumbo "
 				"frame is disabled",
 				(uint32_t)RTE_ETHER_MIN_LEN,
-				(uint32_t)RTE_ETHER_MAX_LEN);
+				(uint32_t)RTE_ETHER_MAX_LEN +
+					I40E_VLAN_TAG_SIZE * 2);
 			return I40E_ERR_CONFIG;
 		}
 	}
@@ -2747,7 +2749,7 @@ i40evf_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
 		return -EBUSY;
 	}
 
-	if (frame_size > RTE_ETHER_MAX_LEN)
+	if (frame_size > RTE_ETHER_MAX_LEN + I40E_VLAN_TAG_SIZE * 2)
 		dev_data->dev_conf.rxmode.offloads |=
 			DEV_RX_OFFLOAD_JUMBO_FRAME;
 	else
diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
index ff6eb4a..0c178e3 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -2634,12 +2634,14 @@ i40e_rx_queue_config(struct i40e_rx_queue *rxq)
 		}
 	} else {
 		if (rxq->max_pkt_len < RTE_ETHER_MIN_LEN ||
-			rxq->max_pkt_len > RTE_ETHER_MAX_LEN) {
+			rxq->max_pkt_len > RTE_ETHER_MAX_LEN +
+				I40E_VLAN_TAG_SIZE * 2) {
 			PMD_DRV_LOG(ERR, "maximum packet length must be "
 				    "larger than %u and smaller than %u, "
 				    "as jumbo frame is disabled",
 				    (uint32_t)RTE_ETHER_MIN_LEN,
-				    (uint32_t)RTE_ETHER_MAX_LEN);
+				    (uint32_t)RTE_ETHER_MAX_LEN +
+						I40E_VLAN_TAG_SIZE * 2);
 			return I40E_ERR_CONFIG;
 		}
 	}
-- 
2.7.4


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [dpdk-dev] [v2] net/i40e: fix vlan packets drop
  2019-10-21  2:44 ` [dpdk-dev] [v2] " Xiao Zhang
@ 2019-10-21 20:15   ` Kevin Traynor
  0 siblings, 0 replies; 10+ messages in thread
From: Kevin Traynor @ 2019-10-21 20:15 UTC (permalink / raw)
  To: Xiao Zhang, dev; +Cc: ian.stokes, beilei.xing, qi.z.zhang, stable

On 21/10/2019 03:44, Xiao Zhang wrote:
> Vlan packets with ip length bigger then 1496 will not be received by
> i40e/i40evf due to wrong packets size checking. This patch fixes the issue
> by correcting the maximum frame size during checking.
> 
> Fixes: 43e5488c0ac6 ("net/i40e: support MTU configuration")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Xiao Zhang <xiao.zhang@intel.com>
> ---
> v2
> add fix for i40evf and correct the checking when using the max_pkt_len.
> ---
>  drivers/net/i40e/i40e_ethdev.c    | 2 +-
>  drivers/net/i40e/i40e_ethdev_vf.c | 8 +++++---
>  drivers/net/i40e/i40e_rxtx.c      | 6 ++++--
>  3 files changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 2ca14da..156d67b 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -12103,7 +12103,7 @@ i40e_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
>  		return -EBUSY;
>  	}
>  
> -	if (frame_size > RTE_ETHER_MAX_LEN)
> +	if (frame_size > RTE_ETHER_MAX_LEN + I40E_VLAN_TAG_SIZE * 2)
>  		dev_data->dev_conf.rxmode.offloads |=
>  			DEV_RX_OFFLOAD_JUMBO_FRAME;
>  	else
> diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
> index 5dba092..bc825e2 100644
> --- a/drivers/net/i40e/i40e_ethdev_vf.c
> +++ b/drivers/net/i40e/i40e_ethdev_vf.c
> @@ -1782,12 +1782,14 @@ i40evf_rxq_init(struct rte_eth_dev *dev, struct i40e_rx_queue *rxq)
>  		}

Size check in jumbo on case needs to be updated also

>  	} else {
>  		if (rxq->max_pkt_len < RTE_ETHER_MIN_LEN ||
> -		    rxq->max_pkt_len > RTE_ETHER_MAX_LEN) {
> +		    rxq->max_pkt_len > RTE_ETHER_MAX_LEN +
> +				I40E_VLAN_TAG_SIZE * 2) {
>  			PMD_DRV_LOG(ERR, "maximum packet length must be "
>  				"larger than %u and smaller than %u, as jumbo "
>  				"frame is disabled",
>  				(uint32_t)RTE_ETHER_MIN_LEN,
> -				(uint32_t)RTE_ETHER_MAX_LEN);
> +				(uint32_t)RTE_ETHER_MAX_LEN +
> +					I40E_VLAN_TAG_SIZE * 2);
>  			return I40E_ERR_CONFIG;
>  		}
>  	}
> @@ -2747,7 +2749,7 @@ i40evf_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
>  		return -EBUSY;
>  	}
>  
> -	if (frame_size > RTE_ETHER_MAX_LEN)
> +	if (frame_size > RTE_ETHER_MAX_LEN + I40E_VLAN_TAG_SIZE * 2)
>  		dev_data->dev_conf.rxmode.offloads |=
>  			DEV_RX_OFFLOAD_JUMBO_FRAME;
>  	else
> diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
> index ff6eb4a..0c178e3 100644
> --- a/drivers/net/i40e/i40e_rxtx.c
> +++ b/drivers/net/i40e/i40e_rxtx.c
> @@ -2634,12 +2634,14 @@ i40e_rx_queue_config(struct i40e_rx_queue *rxq)
>  		}

Size check in jumbo on case needs to be updated also

>  	} else {
>  		if (rxq->max_pkt_len < RTE_ETHER_MIN_LEN ||
> -			rxq->max_pkt_len > RTE_ETHER_MAX_LEN) {
> +			rxq->max_pkt_len > RTE_ETHER_MAX_LEN +
> +				I40E_VLAN_TAG_SIZE * 2) {
>  			PMD_DRV_LOG(ERR, "maximum packet length must be "
>  				    "larger than %u and smaller than %u, "
>  				    "as jumbo frame is disabled",
>  				    (uint32_t)RTE_ETHER_MIN_LEN,
> -				    (uint32_t)RTE_ETHER_MAX_LEN);
> +				    (uint32_t)RTE_ETHER_MAX_LEN +
> +						I40E_VLAN_TAG_SIZE * 2);
>  			return I40E_ERR_CONFIG;
>  		}
>  	}
> 

Doing quick search for RTE_ETHER_MAX_LEN,

- in i40e_fdir_rx_queue_init(), is 'rx_ctx.rxmax = RTE_ETHER_MAX_LEN;'
still correct now?

- Looking at rte_eth_dev_configure()
https://git.dpdk.org/dpdk/tree/lib/librte_ethdev/rte_ethdev.c#n1242
Does it mean an MTU not allowed without jumbo frames during
configuration is allowed later with mtu_set?

Request is to review related code and see if there are any impacts with
this change.

@Beilei Xing @Qi Zhang, this patch needs careful review by i40e maintainer.

thanks,
Kevin.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [dpdk-dev] [v3] net/i40e: fix vlan packets drop
  2019-10-14  7:53 [dpdk-dev] net/i40e: fix vlan packets drop Xiao Zhang
  2019-10-14 17:41 ` Kevin Traynor
  2019-10-21  2:44 ` [dpdk-dev] [v2] " Xiao Zhang
@ 2019-10-29  5:12 ` Xiao Zhang
  2019-11-08 19:28   ` Kevin Traynor
  2 siblings, 1 reply; 10+ messages in thread
From: Xiao Zhang @ 2019-10-29  5:12 UTC (permalink / raw)
  To: dev; +Cc: beilei.xing, qi.z.zhang, ktraynor, ian.stokes, Xiao Zhang, stable

VLAN packets with ip length bigger than 1496 will not be received by
i40e/i40evf due to wrong packets size checking. This patch fixes the
issue by correcting the maximum frame size during checking.

Fixes: 43e5488c0ac6 ("net/i40e: support MTU configuration")
Cc: stable@dpdk.org

Signed-off-by: Xiao Zhang <xiao.zhang@intel.com>
---
v3
Checking more places using max packet len.
v2
Add fix for i40evf and correct the checking when using the max_pkt_len.
---
 drivers/net/i40e/i40e_ethdev.c    |  2 +-
 drivers/net/i40e/i40e_ethdev_vf.c | 11 +++++++----
 drivers/net/i40e/i40e_fdir.c      |  2 +-
 drivers/net/i40e/i40e_rxtx.c      |  9 ++++++---
 lib/librte_ethdev/rte_ethdev.c    | 10 ++++++++--
 lib/librte_net/rte_ether.h        |  1 +
 6 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 77a4683..b0e23c7 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -12105,7 +12105,7 @@ i40e_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
 		return -EBUSY;
 	}
 
-	if (frame_size > RTE_ETHER_MAX_LEN)
+	if (frame_size > RTE_ETHER_MAX_LEN + I40E_VLAN_TAG_SIZE * 2)
 		dev_data->dev_conf.rxmode.offloads |=
 			DEV_RX_OFFLOAD_JUMBO_FRAME;
 	else
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index 5dba092..1e0b50f 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -1772,7 +1772,8 @@ i40evf_rxq_init(struct rte_eth_dev *dev, struct i40e_rx_queue *rxq)
 	 * Check if the jumbo frame and maximum packet length are set correctly
 	 */
 	if (dev_data->dev_conf.rxmode.offloads & DEV_RX_OFFLOAD_JUMBO_FRAME) {
-		if (rxq->max_pkt_len <= RTE_ETHER_MAX_LEN ||
+		if (rxq->max_pkt_len <= RTE_ETHER_MAX_LEN +
+				I40E_VLAN_TAG_SIZE * 2 ||
 		    rxq->max_pkt_len > I40E_FRAME_SIZE_MAX) {
 			PMD_DRV_LOG(ERR, "maximum packet length must be "
 				"larger than %u and smaller than %u, as jumbo "
@@ -1782,12 +1783,14 @@ i40evf_rxq_init(struct rte_eth_dev *dev, struct i40e_rx_queue *rxq)
 		}
 	} else {
 		if (rxq->max_pkt_len < RTE_ETHER_MIN_LEN ||
-		    rxq->max_pkt_len > RTE_ETHER_MAX_LEN) {
+		    rxq->max_pkt_len > RTE_ETHER_MAX_LEN +
+				I40E_VLAN_TAG_SIZE * 2) {
 			PMD_DRV_LOG(ERR, "maximum packet length must be "
 				"larger than %u and smaller than %u, as jumbo "
 				"frame is disabled",
 				(uint32_t)RTE_ETHER_MIN_LEN,
-				(uint32_t)RTE_ETHER_MAX_LEN);
+				(uint32_t)RTE_ETHER_MAX_LEN +
+					I40E_VLAN_TAG_SIZE * 2);
 			return I40E_ERR_CONFIG;
 		}
 	}
@@ -2747,7 +2750,7 @@ i40evf_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
 		return -EBUSY;
 	}
 
-	if (frame_size > RTE_ETHER_MAX_LEN)
+	if (frame_size > RTE_ETHER_MAX_LEN + I40E_VLAN_TAG_SIZE * 2)
 		dev_data->dev_conf.rxmode.offloads |=
 			DEV_RX_OFFLOAD_JUMBO_FRAME;
 	else
diff --git a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c
index dee007d..8b813cb 100644
--- a/drivers/net/i40e/i40e_fdir.c
+++ b/drivers/net/i40e/i40e_fdir.c
@@ -113,7 +113,7 @@ i40e_fdir_rx_queue_init(struct i40e_rx_queue *rxq)
 #endif
 	rx_ctx.dtype = i40e_header_split_none;
 	rx_ctx.hsplit_0 = I40E_HEADER_SPLIT_NONE;
-	rx_ctx.rxmax = RTE_ETHER_MAX_LEN;
+	rx_ctx.rxmax = RTE_ETHER_MAX_LEN + I40E_VLAN_TAG_SIZE * 2;
 	rx_ctx.tphrdesc_ena = 1;
 	rx_ctx.tphwdesc_ena = 1;
 	rx_ctx.tphdata_ena = 1;
diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
index 6a66cec..a93b11f 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -2617,7 +2617,8 @@ i40e_rx_queue_config(struct i40e_rx_queue *rxq)
 		RTE_MIN((uint32_t)(hw->func_caps.rx_buf_chain_len *
 			rxq->rx_buf_len), data->dev_conf.rxmode.max_rx_pkt_len);
 	if (data->dev_conf.rxmode.offloads & DEV_RX_OFFLOAD_JUMBO_FRAME) {
-		if (rxq->max_pkt_len <= RTE_ETHER_MAX_LEN ||
+		if (rxq->max_pkt_len <= RTE_ETHER_MAX_LEN +
+				I40E_VLAN_TAG_SIZE * 2 ||
 			rxq->max_pkt_len > I40E_FRAME_SIZE_MAX) {
 			PMD_DRV_LOG(ERR, "maximum packet length must "
 				    "be larger than %u and smaller than %u,"
@@ -2628,12 +2629,14 @@ i40e_rx_queue_config(struct i40e_rx_queue *rxq)
 		}
 	} else {
 		if (rxq->max_pkt_len < RTE_ETHER_MIN_LEN ||
-			rxq->max_pkt_len > RTE_ETHER_MAX_LEN) {
+			rxq->max_pkt_len > RTE_ETHER_MAX_LEN +
+				I40E_VLAN_TAG_SIZE * 2) {
 			PMD_DRV_LOG(ERR, "maximum packet length must be "
 				    "larger than %u and smaller than %u, "
 				    "as jumbo frame is disabled",
 				    (uint32_t)RTE_ETHER_MIN_LEN,
-				    (uint32_t)RTE_ETHER_MAX_LEN);
+				    (uint32_t)RTE_ETHER_MAX_LEN +
+						I40E_VLAN_TAG_SIZE * 2);
 			return I40E_ERR_CONFIG;
 		}
 	}
diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 7743205..f6722be 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -1257,11 +1257,17 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
 			goto rollback;
 		}
 	} else {
+		/**
+		 * The overhead from MTU to max frame size.
+		 * Considering VLAN and QinQ packet, the VLAN tag size
+		 * needs to be added to RTE_ETHER_MAX_LEN.
+		 */
 		if (dev_conf->rxmode.max_rx_pkt_len < RTE_ETHER_MIN_LEN ||
-			dev_conf->rxmode.max_rx_pkt_len > RTE_ETHER_MAX_LEN)
+			dev_conf->rxmode.max_rx_pkt_len > RTE_ETHER_MAX_LEN
+				+ RTE_ETHER_VLAN_LEN * 2)
 			/* Use default value */
 			dev->data->dev_conf.rxmode.max_rx_pkt_len =
-							RTE_ETHER_MAX_LEN;
+				RTE_ETHER_MAX_LEN + RTE_ETHER_VLAN_LEN * 2;
 	}
 
 	/* Any requested offloading must be within its device capabilities */
diff --git a/lib/librte_net/rte_ether.h b/lib/librte_net/rte_ether.h
index e069dc7..9c5eee7 100644
--- a/lib/librte_net/rte_ether.h
+++ b/lib/librte_net/rte_ether.h
@@ -26,6 +26,7 @@ extern "C" {
 #define RTE_ETHER_ADDR_LEN  6 /**< Length of Ethernet address. */
 #define RTE_ETHER_TYPE_LEN  2 /**< Length of Ethernet type field. */
 #define RTE_ETHER_CRC_LEN   4 /**< Length of Ethernet CRC. */
+#define RTE_ETHER_VLAN_LEN  4 /**< Length of Ethernet VLAN tag. */
 #define RTE_ETHER_HDR_LEN   \
 	(RTE_ETHER_ADDR_LEN * 2 + \
 		RTE_ETHER_TYPE_LEN) /**< Length of Ethernet header. */
-- 
2.7.4


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [dpdk-dev] [v3] net/i40e: fix vlan packets drop
  2019-10-29  5:12 ` [dpdk-dev] [v3] " Xiao Zhang
@ 2019-11-08 19:28   ` Kevin Traynor
  2019-11-08 19:49     ` Thomas Monjalon
  0 siblings, 1 reply; 10+ messages in thread
From: Kevin Traynor @ 2019-11-08 19:28 UTC (permalink / raw)
  To: Xiao Zhang, dev
  Cc: beilei.xing, qi.z.zhang, ian.stokes, stable, Andrew Rybchenko,
	Yigit, Ferruh, thomas, Xiaolong Ye

Hi Xiao,

On 29/10/2019 05:12, Xiao Zhang wrote:
> VLAN packets with ip length bigger than 1496 will not be received by
> i40e/i40evf due to wrong packets size checking. This patch fixes the
> issue by correcting the maximum frame size during checking.
> 
> Fixes: 43e5488c0ac6 ("net/i40e: support MTU configuration")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Xiao Zhang <xiao.zhang@intel.com>
> ---
> v3
> Checking more places using max packet len.
> v2
> Add fix for i40evf and correct the checking when using the max_pkt_len.
> ---
>  drivers/net/i40e/i40e_ethdev.c    |  2 +-
>  drivers/net/i40e/i40e_ethdev_vf.c | 11 +++++++----
>  drivers/net/i40e/i40e_fdir.c      |  2 +-
>  drivers/net/i40e/i40e_rxtx.c      |  9 ++++++---
>  lib/librte_ethdev/rte_ethdev.c    | 10 ++++++++--
>  lib/librte_net/rte_ether.h        |  1 +
>  6 files changed, 24 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 77a4683..b0e23c7 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -12105,7 +12105,7 @@ i40e_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
>  		return -EBUSY;
>  	}
>  
> -	if (frame_size > RTE_ETHER_MAX_LEN)
> +	if (frame_size > RTE_ETHER_MAX_LEN + I40E_VLAN_TAG_SIZE * 2)
>  		dev_data->dev_conf.rxmode.offloads |=
>  			DEV_RX_OFFLOAD_JUMBO_FRAME;
>  	else
> diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
> index 5dba092..1e0b50f 100644
> --- a/drivers/net/i40e/i40e_ethdev_vf.c
> +++ b/drivers/net/i40e/i40e_ethdev_vf.c
> @@ -1772,7 +1772,8 @@ i40evf_rxq_init(struct rte_eth_dev *dev, struct i40e_rx_queue *rxq)
>  	 * Check if the jumbo frame and maximum packet length are set correctly
>  	 */
>  	if (dev_data->dev_conf.rxmode.offloads & DEV_RX_OFFLOAD_JUMBO_FRAME) {
> -		if (rxq->max_pkt_len <= RTE_ETHER_MAX_LEN ||
> +		if (rxq->max_pkt_len <= RTE_ETHER_MAX_LEN +
> +				I40E_VLAN_TAG_SIZE * 2 ||
>  		    rxq->max_pkt_len > I40E_FRAME_SIZE_MAX) {
>  			PMD_DRV_LOG(ERR, "maximum packet length must be "
>  				"larger than %u and smaller than %u, as jumbo "

The log needs to be match the check

> @@ -1782,12 +1783,14 @@ i40evf_rxq_init(struct rte_eth_dev *dev, struct i40e_rx_queue *rxq)
>  		}
>  	} else {
>  		if (rxq->max_pkt_len < RTE_ETHER_MIN_LEN ||
> -		    rxq->max_pkt_len > RTE_ETHER_MAX_LEN) {
> +		    rxq->max_pkt_len > RTE_ETHER_MAX_LEN +
> +				I40E_VLAN_TAG_SIZE * 2) {
>  			PMD_DRV_LOG(ERR, "maximum packet length must be "
>  				"larger than %u and smaller than %u, as jumbo "
>  				"frame is disabled",
>  				(uint32_t)RTE_ETHER_MIN_LEN,
> -				(uint32_t)RTE_ETHER_MAX_LEN);
> +				(uint32_t)RTE_ETHER_MAX_LEN +
> +					I40E_VLAN_TAG_SIZE * 2);
>  			return I40E_ERR_CONFIG;
>  		}
>  	}
> @@ -2747,7 +2750,7 @@ i40evf_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
>  		return -EBUSY;
>  	}
>  
> -	if (frame_size > RTE_ETHER_MAX_LEN)
> +	if (frame_size > RTE_ETHER_MAX_LEN + I40E_VLAN_TAG_SIZE * 2)
>  		dev_data->dev_conf.rxmode.offloads |=
>  			DEV_RX_OFFLOAD_JUMBO_FRAME;
>  	else
> diff --git a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c
> index dee007d..8b813cb 100644
> --- a/drivers/net/i40e/i40e_fdir.c
> +++ b/drivers/net/i40e/i40e_fdir.c
> @@ -113,7 +113,7 @@ i40e_fdir_rx_queue_init(struct i40e_rx_queue *rxq)
>  #endif
>  	rx_ctx.dtype = i40e_header_split_none;
>  	rx_ctx.hsplit_0 = I40E_HEADER_SPLIT_NONE;
> -	rx_ctx.rxmax = RTE_ETHER_MAX_LEN;
> +	rx_ctx.rxmax = RTE_ETHER_MAX_LEN + I40E_VLAN_TAG_SIZE * 2;
>  	rx_ctx.tphrdesc_ena = 1;
>  	rx_ctx.tphwdesc_ena = 1;
>  	rx_ctx.tphdata_ena = 1;
> diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
> index 6a66cec..a93b11f 100644
> --- a/drivers/net/i40e/i40e_rxtx.c
> +++ b/drivers/net/i40e/i40e_rxtx.c
> @@ -2617,7 +2617,8 @@ i40e_rx_queue_config(struct i40e_rx_queue *rxq)
>  		RTE_MIN((uint32_t)(hw->func_caps.rx_buf_chain_len *
>  			rxq->rx_buf_len), data->dev_conf.rxmode.max_rx_pkt_len);
>  	if (data->dev_conf.rxmode.offloads & DEV_RX_OFFLOAD_JUMBO_FRAME) {
> -		if (rxq->max_pkt_len <= RTE_ETHER_MAX_LEN ||
> +		if (rxq->max_pkt_len <= RTE_ETHER_MAX_LEN +
> +				I40E_VLAN_TAG_SIZE * 2 ||
>  			rxq->max_pkt_len > I40E_FRAME_SIZE_MAX) {
>  			PMD_DRV_LOG(ERR, "maximum packet length must "
>  				    "be larger than %u and smaller than %u,"

The log needs to be match the check

> @@ -2628,12 +2629,14 @@ i40e_rx_queue_config(struct i40e_rx_queue *rxq)
>  		}
>  	} else {
>  		if (rxq->max_pkt_len < RTE_ETHER_MIN_LEN ||
> -			rxq->max_pkt_len > RTE_ETHER_MAX_LEN) {
> +			rxq->max_pkt_len > RTE_ETHER_MAX_LEN +
> +				I40E_VLAN_TAG_SIZE * 2) {
>  			PMD_DRV_LOG(ERR, "maximum packet length must be "
>  				    "larger than %u and smaller than %u, "
>  				    "as jumbo frame is disabled",
>  				    (uint32_t)RTE_ETHER_MIN_LEN,
> -				    (uint32_t)RTE_ETHER_MAX_LEN);
> +				    (uint32_t)RTE_ETHER_MAX_LEN +
> +						I40E_VLAN_TAG_SIZE * 2);
>  			return I40E_ERR_CONFIG;
>  		}
>  	}
> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> index 7743205..f6722be 100644
> --- a/lib/librte_ethdev/rte_ethdev.c
> +++ b/lib/librte_ethdev/rte_ethdev.c
> @@ -1257,11 +1257,17 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
>  			goto rollback;
>  		}
>  	} else {
> +		/**
> +		 * The overhead from MTU to max frame size.
> +		 * Considering VLAN and QinQ packet, the VLAN tag size
> +		 * needs to be added to RTE_ETHER_MAX_LEN.
> +		 */
>  		if (dev_conf->rxmode.max_rx_pkt_len < RTE_ETHER_MIN_LEN ||
> -			dev_conf->rxmode.max_rx_pkt_len > RTE_ETHER_MAX_LEN)
> +			dev_conf->rxmode.max_rx_pkt_len > RTE_ETHER_MAX_LEN
> +				+ RTE_ETHER_VLAN_LEN * 2)
>  			/* Use default value */
>  			dev->data->dev_conf.rxmode.max_rx_pkt_len =
> -							RTE_ETHER_MAX_LEN;
> +				RTE_ETHER_MAX_LEN + RTE_ETHER_VLAN_LEN * 2;

+cc ethdev maintainers

This looks ok to me for i40e case, but I don't know if there is a
consequence for other PMDs. It seems late to change this, so maybe you
can live without this part for now.

Even on the i40e parts, there can be some subtle bug and I requested
i40e maintainers to review carefully but it has not happened, so for me
it shouldn't be merged at present.

>  	}
>  
>  	/* Any requested offloading must be within its device capabilities */
> diff --git a/lib/librte_net/rte_ether.h b/lib/librte_net/rte_ether.h
> index e069dc7..9c5eee7 100644
> --- a/lib/librte_net/rte_ether.h
> +++ b/lib/librte_net/rte_ether.h
> @@ -26,6 +26,7 @@ extern "C" {
>  #define RTE_ETHER_ADDR_LEN  6 /**< Length of Ethernet address. */
>  #define RTE_ETHER_TYPE_LEN  2 /**< Length of Ethernet type field. */
>  #define RTE_ETHER_CRC_LEN   4 /**< Length of Ethernet CRC. */
> +#define RTE_ETHER_VLAN_LEN  4 /**< Length of Ethernet VLAN tag. */
>  #define RTE_ETHER_HDR_LEN   \
>  	(RTE_ETHER_ADDR_LEN * 2 + \
>  		RTE_ETHER_TYPE_LEN) /**< Length of Ethernet header. */
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [dpdk-dev] [v3] net/i40e: fix vlan packets drop
  2019-11-08 19:28   ` Kevin Traynor
@ 2019-11-08 19:49     ` Thomas Monjalon
  2019-11-09  3:01       ` Zhang, Qi Z
  0 siblings, 1 reply; 10+ messages in thread
From: Thomas Monjalon @ 2019-11-08 19:49 UTC (permalink / raw)
  To: Kevin Traynor
  Cc: Xiao Zhang, dev, beilei.xing, qi.z.zhang, ian.stokes, stable,
	Andrew Rybchenko, Yigit, Ferruh, Xiaolong Ye

08/11/2019 20:28, Kevin Traynor:
> Hi Xiao,
> 
> On 29/10/2019 05:12, Xiao Zhang wrote:
> > VLAN packets with ip length bigger than 1496 will not be received by
> > i40e/i40evf due to wrong packets size checking. This patch fixes the
> > issue by correcting the maximum frame size during checking.
> > 
> > Fixes: 43e5488c0ac6 ("net/i40e: support MTU configuration")
> > Cc: stable@dpdk.org
> > 
> > Signed-off-by: Xiao Zhang <xiao.zhang@intel.com>
> > ---
> > v3
> > Checking more places using max packet len.
> > v2
> > Add fix for i40evf and correct the checking when using the max_pkt_len.
> > ---
> >  drivers/net/i40e/i40e_ethdev.c    |  2 +-
> >  drivers/net/i40e/i40e_ethdev_vf.c | 11 +++++++----
> >  drivers/net/i40e/i40e_fdir.c      |  2 +-
> >  drivers/net/i40e/i40e_rxtx.c      |  9 ++++++---
> >  lib/librte_ethdev/rte_ethdev.c    | 10 ++++++++--
> >  lib/librte_net/rte_ether.h        |  1 +
> >  6 files changed, 24 insertions(+), 11 deletions(-)
> > 
> > --- a/lib/librte_ethdev/rte_ethdev.c
> > +++ b/lib/librte_ethdev/rte_ethdev.c
> > @@ -1257,11 +1257,17 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
> >  			goto rollback;
> >  		}
> >  	} else {
> > +		/**
> > +		 * The overhead from MTU to max frame size.
> > +		 * Considering VLAN and QinQ packet, the VLAN tag size
> > +		 * needs to be added to RTE_ETHER_MAX_LEN.
> > +		 */
> >  		if (dev_conf->rxmode.max_rx_pkt_len < RTE_ETHER_MIN_LEN ||
> > -			dev_conf->rxmode.max_rx_pkt_len > RTE_ETHER_MAX_LEN)
> > +			dev_conf->rxmode.max_rx_pkt_len > RTE_ETHER_MAX_LEN
> > +				+ RTE_ETHER_VLAN_LEN * 2)
> >  			/* Use default value */
> >  			dev->data->dev_conf.rxmode.max_rx_pkt_len =
> > -							RTE_ETHER_MAX_LEN;
> > +				RTE_ETHER_MAX_LEN + RTE_ETHER_VLAN_LEN * 2;
> 
> +cc ethdev maintainers
> 
> This looks ok to me for i40e case, but I don't know if there is a
> consequence for other PMDs. It seems late to change this, so maybe you
> can live without this part for now.
> 
> Even on the i40e parts, there can be some subtle bug and I requested
> i40e maintainers to review carefully but it has not happened, so for me
> it shouldn't be merged at present.

I would nack for another, simpler, reason:
No ethdev behaviour change should be submitted if title does not start
with "ethdev:" and if ethdev maintainers are not Cc'ed.




^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [dpdk-dev] [v3] net/i40e: fix vlan packets drop
  2019-11-08 19:49     ` Thomas Monjalon
@ 2019-11-09  3:01       ` Zhang, Qi Z
  2020-02-10 13:48         ` Mattias Rönnblom
  0 siblings, 1 reply; 10+ messages in thread
From: Zhang, Qi Z @ 2019-11-09  3:01 UTC (permalink / raw)
  To: Thomas Monjalon, Kevin Traynor
  Cc: Zhang, Xiao, dev, Xing, Beilei, Stokes, Ian, stable,
	Andrew Rybchenko, Yigit, Ferruh, Ye, Xiaolong



> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Saturday, November 9, 2019 3:49 AM
> To: Kevin Traynor <ktraynor@redhat.com>
> Cc: Zhang, Xiao <xiao.zhang@intel.com>; dev@dpdk.org; Xing, Beilei
> <beilei.xing@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Stokes, Ian
> <ian.stokes@intel.com>; stable@dpdk.org; Andrew Rybchenko
> <arybchenko@solarflare.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Ye,
> Xiaolong <xiaolong.ye@intel.com>
> Subject: Re: [v3] net/i40e: fix vlan packets drop
> 
> 08/11/2019 20:28, Kevin Traynor:
> > Hi Xiao,
> >
> > On 29/10/2019 05:12, Xiao Zhang wrote:
> > > VLAN packets with ip length bigger than 1496 will not be received by
> > > i40e/i40evf due to wrong packets size checking. This patch fixes the
> > > issue by correcting the maximum frame size during checking.
> > >
> > > Fixes: 43e5488c0ac6 ("net/i40e: support MTU configuration")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Xiao Zhang <xiao.zhang@intel.com>
> > > ---
> > > v3
> > > Checking more places using max packet len.
> > > v2
> > > Add fix for i40evf and correct the checking when using the max_pkt_len.
> > > ---
> > >  drivers/net/i40e/i40e_ethdev.c    |  2 +-
> > >  drivers/net/i40e/i40e_ethdev_vf.c | 11 +++++++----
> > >  drivers/net/i40e/i40e_fdir.c      |  2 +-
> > >  drivers/net/i40e/i40e_rxtx.c      |  9 ++++++---
> > >  lib/librte_ethdev/rte_ethdev.c    | 10 ++++++++--
> > >  lib/librte_net/rte_ether.h        |  1 +
> > >  6 files changed, 24 insertions(+), 11 deletions(-)
> > >
> > > --- a/lib/librte_ethdev/rte_ethdev.c
> > > +++ b/lib/librte_ethdev/rte_ethdev.c
> > > @@ -1257,11 +1257,17 @@ rte_eth_dev_configure(uint16_t port_id,
> uint16_t nb_rx_q, uint16_t nb_tx_q,
> > >  			goto rollback;
> > >  		}
> > >  	} else {
> > > +		/**
> > > +		 * The overhead from MTU to max frame size.
> > > +		 * Considering VLAN and QinQ packet, the VLAN tag size
> > > +		 * needs to be added to RTE_ETHER_MAX_LEN.
> > > +		 */
> > >  		if (dev_conf->rxmode.max_rx_pkt_len < RTE_ETHER_MIN_LEN
> ||
> > > -			dev_conf->rxmode.max_rx_pkt_len > RTE_ETHER_MAX_LEN)
> > > +			dev_conf->rxmode.max_rx_pkt_len > RTE_ETHER_MAX_LEN
> > > +				+ RTE_ETHER_VLAN_LEN * 2)
> > >  			/* Use default value */
> > >  			dev->data->dev_conf.rxmode.max_rx_pkt_len =
> > > -							RTE_ETHER_MAX_LEN;
> > > +				RTE_ETHER_MAX_LEN + RTE_ETHER_VLAN_LEN * 2;
> >
> > +cc ethdev maintainers
> >
> > This looks ok to me for i40e case, but I don't know if there is a
> > consequence for other PMDs. It seems late to change this, so maybe you
> > can live without this part for now.
> >
> > Even on the i40e parts, there can be some subtle bug and I requested
> > i40e maintainers to review carefully but it has not happened, so for
> > me it shouldn't be merged at present.
> 
> I would nack for another, simpler, reason:
> No ethdev behaviour change should be submitted if title does not start with
> "ethdev:" and if ethdev maintainers are not Cc'ed.
> 
> 
Yes, the patch should be dropped even without the ethdev part change, the fix for i40e need more refine. 
Sorry for review this late.



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [dpdk-dev] [v3] net/i40e: fix vlan packets drop
  2019-11-09  3:01       ` Zhang, Qi Z
@ 2020-02-10 13:48         ` Mattias Rönnblom
  0 siblings, 0 replies; 10+ messages in thread
From: Mattias Rönnblom @ 2020-02-10 13:48 UTC (permalink / raw)
  To: Zhang, Qi Z, Thomas Monjalon, Kevin Traynor
  Cc: Zhang, Xiao, dev, Xing, Beilei, Stokes, Ian, stable,
	Andrew Rybchenko, Yigit, Ferruh, Ye, Xiaolong

On 2019-11-09 04:01, Zhang, Qi Z wrote:
>
>> -----Original Message-----
>> From: Thomas Monjalon <thomas@monjalon.net>
>> Sent: Saturday, November 9, 2019 3:49 AM
>> To: Kevin Traynor <ktraynor@redhat.com>
>> Cc: Zhang, Xiao <xiao.zhang@intel.com>; dev@dpdk.org; Xing, Beilei
>> <beilei.xing@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Stokes, Ian
>> <ian.stokes@intel.com>; stable@dpdk.org; Andrew Rybchenko
>> <arybchenko@solarflare.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Ye,
>> Xiaolong <xiaolong.ye@intel.com>
>> Subject: Re: [v3] net/i40e: fix vlan packets drop
>>
>> 08/11/2019 20:28, Kevin Traynor:
>>> Hi Xiao,
>>>
>>> On 29/10/2019 05:12, Xiao Zhang wrote:
>>>> VLAN packets with ip length bigger than 1496 will not be received by
>>>> i40e/i40evf due to wrong packets size checking. This patch fixes the
>>>> issue by correcting the maximum frame size during checking.
>>>>
>>>> Fixes: 43e5488c0ac6 ("net/i40e: support MTU configuration")
>>>> Cc: stable@dpdk.org
>>>>
>>>> Signed-off-by: Xiao Zhang <xiao.zhang@intel.com>
>>>> ---
>>>> v3
>>>> Checking more places using max packet len.
>>>> v2
>>>> Add fix for i40evf and correct the checking when using the max_pkt_len.
>>>> ---
>>>>   drivers/net/i40e/i40e_ethdev.c    |  2 +-
>>>>   drivers/net/i40e/i40e_ethdev_vf.c | 11 +++++++----
>>>>   drivers/net/i40e/i40e_fdir.c      |  2 +-
>>>>   drivers/net/i40e/i40e_rxtx.c      |  9 ++++++---
>>>>   lib/librte_ethdev/rte_ethdev.c    | 10 ++++++++--
>>>>   lib/librte_net/rte_ether.h        |  1 +
>>>>   6 files changed, 24 insertions(+), 11 deletions(-)
>>>>
>>>> --- a/lib/librte_ethdev/rte_ethdev.c
>>>> +++ b/lib/librte_ethdev/rte_ethdev.c
>>>> @@ -1257,11 +1257,17 @@ rte_eth_dev_configure(uint16_t port_id,
>> uint16_t nb_rx_q, uint16_t nb_tx_q,
>>>>   			goto rollback;
>>>>   		}
>>>>   	} else {
>>>> +		/**
>>>> +		 * The overhead from MTU to max frame size.
>>>> +		 * Considering VLAN and QinQ packet, the VLAN tag size
>>>> +		 * needs to be added to RTE_ETHER_MAX_LEN.
>>>> +		 */
>>>>   		if (dev_conf->rxmode.max_rx_pkt_len < RTE_ETHER_MIN_LEN
>> ||
>>>> -			dev_conf->rxmode.max_rx_pkt_len > RTE_ETHER_MAX_LEN)
>>>> +			dev_conf->rxmode.max_rx_pkt_len > RTE_ETHER_MAX_LEN
>>>> +				+ RTE_ETHER_VLAN_LEN * 2)
>>>>   			/* Use default value */
>>>>   			dev->data->dev_conf.rxmode.max_rx_pkt_len =
>>>> -							RTE_ETHER_MAX_LEN;
>>>> +				RTE_ETHER_MAX_LEN + RTE_ETHER_VLAN_LEN * 2;
>>> +cc ethdev maintainers
>>>
>>> This looks ok to me for i40e case, but I don't know if there is a
>>> consequence for other PMDs. It seems late to change this, so maybe you
>>> can live without this part for now.
>>>
>>> Even on the i40e parts, there can be some subtle bug and I requested
>>> i40e maintainers to review carefully but it has not happened, so for
>>> me it shouldn't be merged at present.
>> I would nack for another, simpler, reason:
>> No ethdev behaviour change should be submitted if title does not start with
>> "ethdev:" and if ethdev maintainers are not Cc'ed.
>>
>>
> Yes, the patch should be dropped even without the ethdev part change, the fix for i40e need more refine.
> Sorry for review this late.


 From what I can see, no fix for the bug this patch attempts to address 
has been merged yet. Correct?




^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-02-10 13:48 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-14  7:53 [dpdk-dev] net/i40e: fix vlan packets drop Xiao Zhang
2019-10-14 17:41 ` Kevin Traynor
2019-10-15  1:41   ` Zhang, Xiao
2019-10-21  2:44 ` [dpdk-dev] [v2] " Xiao Zhang
2019-10-21 20:15   ` Kevin Traynor
2019-10-29  5:12 ` [dpdk-dev] [v3] " Xiao Zhang
2019-11-08 19:28   ` Kevin Traynor
2019-11-08 19:49     ` Thomas Monjalon
2019-11-09  3:01       ` Zhang, Qi Z
2020-02-10 13:48         ` Mattias Rönnblom

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).