[SRU,Xenial] i40e: Limit TX descriptor count in cases where frag size is greater than 16K

Message ID 20170911230122.4063-1-dan.streetman@canonical.com
State New
Headers show
Series
  • [SRU,Xenial] i40e: Limit TX descriptor count in cases where frag size is greater than 16K
Related show

Commit Message

Dan Streetman Sept. 11, 2017, 11:01 p.m.
From: Alexander Duyck <alexander.h.duyck@intel.com>

BugLink: http://bugs.launchpad.net/bugs/1713553

The i40e driver was incorrectly assuming that we would always be pulling
no more than 1 descriptor from each fragment.  It is in fact possible for
us to end up with the case where 2 descriptors worth of data may be pulled
when a frame is larger than one of the pieces generated when aligning the
payload to either 4K or pieces smaller than 16K.

To adjust for this we just need to make certain to test all the way to the
end of the fragments as it is possible for us to span 2 descriptors in the
block before us so we need to guarantee that even the last 6 descriptors
have enough data to fill a full frame.

Change-ID: Ic2ecb4d6b745f447d334e66c14002152f50e2f99
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
(cherry-picked from 841493a3f64395b60554afbcaa17f4350f90e764 upstream)
Signed-off-by: Dan Streetman <dan.streetman@canonical.com>
---
This patch is already in Zesty/Artful kernels, and is a required
follow-on patch to what was applied for lp bug 1700834.

 drivers/net/ethernet/intel/i40e/i40e_txrx.c   | 7 ++-----
 drivers/net/ethernet/intel/i40evf/i40e_txrx.c | 7 ++-----
 2 files changed, 4 insertions(+), 10 deletions(-)

Comments

Colin King Sept. 13, 2017, 7:54 a.m. | #1
On 12/09/17 00:01, Dan Streetman wrote:
> From: Alexander Duyck <alexander.h.duyck@intel.com>
> 
> BugLink: http://bugs.launchpad.net/bugs/1713553
> 
> The i40e driver was incorrectly assuming that we would always be pulling
> no more than 1 descriptor from each fragment.  It is in fact possible for
> us to end up with the case where 2 descriptors worth of data may be pulled
> when a frame is larger than one of the pieces generated when aligning the
> payload to either 4K or pieces smaller than 16K.
> 
> To adjust for this we just need to make certain to test all the way to the
> end of the fragments as it is possible for us to span 2 descriptors in the
> block before us so we need to guarantee that even the last 6 descriptors
> have enough data to fill a full frame.
> 
> Change-ID: Ic2ecb4d6b745f447d334e66c14002152f50e2f99
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> (cherry-picked from 841493a3f64395b60554afbcaa17f4350f90e764 upstream)
> Signed-off-by: Dan Streetman <dan.streetman@canonical.com>
> ---
> This patch is already in Zesty/Artful kernels, and is a required
> follow-on patch to what was applied for lp bug 1700834.
> 
>  drivers/net/ethernet/intel/i40e/i40e_txrx.c   | 7 ++-----
>  drivers/net/ethernet/intel/i40evf/i40e_txrx.c | 7 ++-----
>  2 files changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> index c57476633913..8cfb5fe972ba 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> @@ -2617,9 +2617,7 @@ bool __i40e_chk_linearize(struct sk_buff *skb)
>  		return false;
>  
>  	/* We need to walk through the list and validate that each group
> -	 * of 6 fragments totals at least gso_size.  However we don't need
> -	 * to perform such validation on the last 6 since the last 6 cannot
> -	 * inherit any data from a descriptor after them.
> +	 * of 6 fragments totals at least gso_size.
>  	 */
>  	nr_frags -= I40E_MAX_BUFFER_TXD - 2;
>  	frag = &skb_shinfo(skb)->frags[0];
> @@ -2650,8 +2648,7 @@ bool __i40e_chk_linearize(struct sk_buff *skb)
>  		if (sum < 0)
>  			return true;
>  
> -		/* use pre-decrement to avoid processing last fragment */
> -		if (!--nr_frags)
> +		if (!nr_frags--)
>  			break;
>  
>  		sum -= skb_frag_size(stale++);
> diff --git a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
> index f0646bc26d04..50d1dd278860 100644
> --- a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
> +++ b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
> @@ -1820,9 +1820,7 @@ bool __i40evf_chk_linearize(struct sk_buff *skb)
>  		return false;
>  
>  	/* We need to walk through the list and validate that each group
> -	 * of 6 fragments totals at least gso_size.  However we don't need
> -	 * to perform such validation on the last 6 since the last 6 cannot
> -	 * inherit any data from a descriptor after them.
> +	 * of 6 fragments totals at least gso_size.
>  	 */
>  	nr_frags -= I40E_MAX_BUFFER_TXD - 2;
>  	frag = &skb_shinfo(skb)->frags[0];
> @@ -1853,8 +1851,7 @@ bool __i40evf_chk_linearize(struct sk_buff *skb)
>  		if (sum < 0)
>  			return true;
>  
> -		/* use pre-decrement to avoid processing last fragment */
> -		if (!--nr_frags)
> +		if (!nr_frags--)
>  			break;
>  
>  		sum -= skb_frag_size(stale++);
> 
Clean cherry pick. Thanks Dan

Acked-by: Colin Ian King <colin.king@canonical.com>
Stefan Bader Sept. 15, 2017, 10:07 a.m. | #2
On 12.09.2017 01:01, Dan Streetman wrote:
> From: Alexander Duyck <alexander.h.duyck@intel.com>
> 
> BugLink: http://bugs.launchpad.net/bugs/1713553
> 
> The i40e driver was incorrectly assuming that we would always be pulling
> no more than 1 descriptor from each fragment.  It is in fact possible for
> us to end up with the case where 2 descriptors worth of data may be pulled
> when a frame is larger than one of the pieces generated when aligning the
> payload to either 4K or pieces smaller than 16K.
> 
> To adjust for this we just need to make certain to test all the way to the
> end of the fragments as it is possible for us to span 2 descriptors in the
> block before us so we need to guarantee that even the last 6 descriptors
> have enough data to fill a full frame.
> 
> Change-ID: Ic2ecb4d6b745f447d334e66c14002152f50e2f99
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> (cherry-picked from 841493a3f64395b60554afbcaa17f4350f90e764 upstream)
> Signed-off-by: Dan Streetman <dan.streetman@canonical.com>
Acked-by: Stefan Bader <stefan.bader@canonical.com>

> ---
> This patch is already in Zesty/Artful kernels, and is a required
> follow-on patch to what was applied for lp bug 1700834.
> 
>  drivers/net/ethernet/intel/i40e/i40e_txrx.c   | 7 ++-----
>  drivers/net/ethernet/intel/i40evf/i40e_txrx.c | 7 ++-----
>  2 files changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> index c57476633913..8cfb5fe972ba 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> @@ -2617,9 +2617,7 @@ bool __i40e_chk_linearize(struct sk_buff *skb)
>  		return false;
>  
>  	/* We need to walk through the list and validate that each group
> -	 * of 6 fragments totals at least gso_size.  However we don't need
> -	 * to perform such validation on the last 6 since the last 6 cannot
> -	 * inherit any data from a descriptor after them.
> +	 * of 6 fragments totals at least gso_size.
>  	 */
>  	nr_frags -= I40E_MAX_BUFFER_TXD - 2;
>  	frag = &skb_shinfo(skb)->frags[0];
> @@ -2650,8 +2648,7 @@ bool __i40e_chk_linearize(struct sk_buff *skb)
>  		if (sum < 0)
>  			return true;
>  
> -		/* use pre-decrement to avoid processing last fragment */
> -		if (!--nr_frags)
> +		if (!nr_frags--)
>  			break;
>  
>  		sum -= skb_frag_size(stale++);
> diff --git a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
> index f0646bc26d04..50d1dd278860 100644
> --- a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
> +++ b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
> @@ -1820,9 +1820,7 @@ bool __i40evf_chk_linearize(struct sk_buff *skb)
>  		return false;
>  
>  	/* We need to walk through the list and validate that each group
> -	 * of 6 fragments totals at least gso_size.  However we don't need
> -	 * to perform such validation on the last 6 since the last 6 cannot
> -	 * inherit any data from a descriptor after them.
> +	 * of 6 fragments totals at least gso_size.
>  	 */
>  	nr_frags -= I40E_MAX_BUFFER_TXD - 2;
>  	frag = &skb_shinfo(skb)->frags[0];
> @@ -1853,8 +1851,7 @@ bool __i40evf_chk_linearize(struct sk_buff *skb)
>  		if (sum < 0)
>  			return true;
>  
> -		/* use pre-decrement to avoid processing last fragment */
> -		if (!--nr_frags)
> +		if (!nr_frags--)
>  			break;
>  
>  		sum -= skb_frag_size(stale++);
>
Stefan Bader Sept. 15, 2017, 1:56 p.m. | #3
On 12.09.2017 01:01, Dan Streetman wrote:
> From: Alexander Duyck <alexander.h.duyck@intel.com>
> 
> BugLink: http://bugs.launchpad.net/bugs/1713553
> 
> The i40e driver was incorrectly assuming that we would always be pulling
> no more than 1 descriptor from each fragment.  It is in fact possible for
> us to end up with the case where 2 descriptors worth of data may be pulled
> when a frame is larger than one of the pieces generated when aligning the
> payload to either 4K or pieces smaller than 16K.
> 
> To adjust for this we just need to make certain to test all the way to the
> end of the fragments as it is possible for us to span 2 descriptors in the
> block before us so we need to guarantee that even the last 6 descriptors
> have enough data to fill a full frame.
> 
> Change-ID: Ic2ecb4d6b745f447d334e66c14002152f50e2f99
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> (cherry-picked from 841493a3f64395b60554afbcaa17f4350f90e764 upstream)
> Signed-off-by: Dan Streetman <dan.streetman@canonical.com>
> ---
> This patch is already in Zesty/Artful kernels, and is a required
> follow-on patch to what was applied for lp bug 1700834.
> 
>  drivers/net/ethernet/intel/i40e/i40e_txrx.c   | 7 ++-----
>  drivers/net/ethernet/intel/i40evf/i40e_txrx.c | 7 ++-----
>  2 files changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> index c57476633913..8cfb5fe972ba 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> @@ -2617,9 +2617,7 @@ bool __i40e_chk_linearize(struct sk_buff *skb)
>  		return false;
>  
>  	/* We need to walk through the list and validate that each group
> -	 * of 6 fragments totals at least gso_size.  However we don't need
> -	 * to perform such validation on the last 6 since the last 6 cannot
> -	 * inherit any data from a descriptor after them.
> +	 * of 6 fragments totals at least gso_size.
>  	 */
>  	nr_frags -= I40E_MAX_BUFFER_TXD - 2;
>  	frag = &skb_shinfo(skb)->frags[0];
> @@ -2650,8 +2648,7 @@ bool __i40e_chk_linearize(struct sk_buff *skb)
>  		if (sum < 0)
>  			return true;
>  
> -		/* use pre-decrement to avoid processing last fragment */
> -		if (!--nr_frags)
> +		if (!nr_frags--)
>  			break;
>  
>  		sum -= skb_frag_size(stale++);
> diff --git a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
> index f0646bc26d04..50d1dd278860 100644
> --- a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
> +++ b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
> @@ -1820,9 +1820,7 @@ bool __i40evf_chk_linearize(struct sk_buff *skb)
>  		return false;
>  
>  	/* We need to walk through the list and validate that each group
> -	 * of 6 fragments totals at least gso_size.  However we don't need
> -	 * to perform such validation on the last 6 since the last 6 cannot
> -	 * inherit any data from a descriptor after them.
> +	 * of 6 fragments totals at least gso_size.
>  	 */
>  	nr_frags -= I40E_MAX_BUFFER_TXD - 2;
>  	frag = &skb_shinfo(skb)->frags[0];
> @@ -1853,8 +1851,7 @@ bool __i40evf_chk_linearize(struct sk_buff *skb)
>  		if (sum < 0)
>  			return true;
>  
> -		/* use pre-decrement to avoid processing last fragment */
> -		if (!--nr_frags)
> +		if (!nr_frags--)
>  			break;
>  
>  		sum -= skb_frag_size(stale++);
> 
Applied to Xenial master-next

Patch

diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index c57476633913..8cfb5fe972ba 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -2617,9 +2617,7 @@  bool __i40e_chk_linearize(struct sk_buff *skb)
 		return false;
 
 	/* We need to walk through the list and validate that each group
-	 * of 6 fragments totals at least gso_size.  However we don't need
-	 * to perform such validation on the last 6 since the last 6 cannot
-	 * inherit any data from a descriptor after them.
+	 * of 6 fragments totals at least gso_size.
 	 */
 	nr_frags -= I40E_MAX_BUFFER_TXD - 2;
 	frag = &skb_shinfo(skb)->frags[0];
@@ -2650,8 +2648,7 @@  bool __i40e_chk_linearize(struct sk_buff *skb)
 		if (sum < 0)
 			return true;
 
-		/* use pre-decrement to avoid processing last fragment */
-		if (!--nr_frags)
+		if (!nr_frags--)
 			break;
 
 		sum -= skb_frag_size(stale++);
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
index f0646bc26d04..50d1dd278860 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
@@ -1820,9 +1820,7 @@  bool __i40evf_chk_linearize(struct sk_buff *skb)
 		return false;
 
 	/* We need to walk through the list and validate that each group
-	 * of 6 fragments totals at least gso_size.  However we don't need
-	 * to perform such validation on the last 6 since the last 6 cannot
-	 * inherit any data from a descriptor after them.
+	 * of 6 fragments totals at least gso_size.
 	 */
 	nr_frags -= I40E_MAX_BUFFER_TXD - 2;
 	frag = &skb_shinfo(skb)->frags[0];
@@ -1853,8 +1851,7 @@  bool __i40evf_chk_linearize(struct sk_buff *skb)
 		if (sum < 0)
 			return true;
 
-		/* use pre-decrement to avoid processing last fragment */
-		if (!--nr_frags)
+		if (!nr_frags--)
 			break;
 
 		sum -= skb_frag_size(stale++);