| 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 | expand |
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>
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++); >
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
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++);