netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [net-next-2.6 PATCH] e1000: allow option to limit number of descriptors down to 48 per ring
@ 2010-07-19 23:43 Jeff Kirsher
  2010-07-20  3:24 ` David Miller
  2010-07-20  4:40 ` Eric Dumazet
  0 siblings, 2 replies; 4+ messages in thread
From: Jeff Kirsher @ 2010-07-19 23:43 UTC (permalink / raw)
  To: davem; +Cc: netdev, gospo, bphilips, Alexander Duyck, Jeff Kirsher

From: Alexander Duyck <alexander.h.duyck@intel.com>

This change makes it possible to limit the number of descriptors down to 48
per ring.  The reason for this change is to address a variation on hardware
errata 10 for 82546GB in which descriptors will be lost if more than 32
descriptors are fetched and the PCI-X MRBC is 512.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Tested-by: Emil Tantilov <emil.s.tantilov@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---

 drivers/net/e1000/e1000.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/e1000/e1000.h b/drivers/net/e1000/e1000.h
index 40b62b4..65298a6 100644
--- a/drivers/net/e1000/e1000.h
+++ b/drivers/net/e1000/e1000.h
@@ -86,12 +86,12 @@ struct e1000_adapter;
 /* TX/RX descriptor defines */
 #define E1000_DEFAULT_TXD                  256
 #define E1000_MAX_TXD                      256
-#define E1000_MIN_TXD                       80
+#define E1000_MIN_TXD                       48
 #define E1000_MAX_82544_TXD               4096
 
 #define E1000_DEFAULT_RXD                  256
 #define E1000_MAX_RXD                      256
-#define E1000_MIN_RXD                       80
+#define E1000_MIN_RXD                       48
 #define E1000_MAX_82544_RXD               4096
 
 #define E1000_MIN_ITR_USECS		10 /* 100000 irq/sec */


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

* Re: [net-next-2.6 PATCH] e1000: allow option to limit number of descriptors down to 48 per ring
  2010-07-19 23:43 [net-next-2.6 PATCH] e1000: allow option to limit number of descriptors down to 48 per ring Jeff Kirsher
@ 2010-07-20  3:24 ` David Miller
  2010-07-20  4:40 ` Eric Dumazet
  1 sibling, 0 replies; 4+ messages in thread
From: David Miller @ 2010-07-20  3:24 UTC (permalink / raw)
  To: jeffrey.t.kirsher; +Cc: netdev, gospo, bphilips, alexander.h.duyck

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Mon, 19 Jul 2010 16:43:47 -0700

> From: Alexander Duyck <alexander.h.duyck@intel.com>
> 
> This change makes it possible to limit the number of descriptors down to 48
> per ring.  The reason for this change is to address a variation on hardware
> errata 10 for 82546GB in which descriptors will be lost if more than 32
> descriptors are fetched and the PCI-X MRBC is 512.
> 
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> Tested-by: Emil Tantilov <emil.s.tantilov@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

Applied.

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

* Re: [net-next-2.6 PATCH] e1000: allow option to limit number of descriptors down to 48 per ring
  2010-07-19 23:43 [net-next-2.6 PATCH] e1000: allow option to limit number of descriptors down to 48 per ring Jeff Kirsher
  2010-07-20  3:24 ` David Miller
@ 2010-07-20  4:40 ` Eric Dumazet
  2010-07-20 17:12   ` Brandeburg, Jesse
  1 sibling, 1 reply; 4+ messages in thread
From: Eric Dumazet @ 2010-07-20  4:40 UTC (permalink / raw)
  To: Jeff Kirsher; +Cc: davem, netdev, gospo, bphilips, Alexander Duyck

Le lundi 19 juillet 2010 à 16:43 -0700, Jeff Kirsher a écrit :
> From: Alexander Duyck <alexander.h.duyck@intel.com>
> 
> This change makes it possible to limit the number of descriptors down to 48
> per ring.  The reason for this change is to address a variation on hardware
> errata 10 for 82546GB in which descriptors will be lost if more than 32
> descriptors are fetched and the PCI-X MRBC is 512.
> 
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> Tested-by: Emil Tantilov <emil.s.tantilov@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> ---
> 
>  drivers/net/e1000/e1000.h |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/e1000/e1000.h b/drivers/net/e1000/e1000.h
> index 40b62b4..65298a6 100644
> --- a/drivers/net/e1000/e1000.h
> +++ b/drivers/net/e1000/e1000.h
> @@ -86,12 +86,12 @@ struct e1000_adapter;
>  /* TX/RX descriptor defines */
>  #define E1000_DEFAULT_TXD                  256
>  #define E1000_MAX_TXD                      256
> -#define E1000_MIN_TXD                       80
> +#define E1000_MIN_TXD                       48
>  #define E1000_MAX_82544_TXD               4096
>  
>  #define E1000_DEFAULT_RXD                  256
>  #define E1000_MAX_RXD                      256
> -#define E1000_MIN_RXD                       80
> +#define E1000_MIN_RXD                       48
>  #define E1000_MAX_82544_RXD               4096
>  
>  #define E1000_MIN_ITR_USECS		10 /* 100000 irq/sec */
> 

So this limit is a pure software one ?

Why not let an admin chose a lower limit if he wants to ?

I am asking because big ring sizes can be a latency source in some
workloads.

Thanks



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

* Re: [net-next-2.6 PATCH] e1000: allow option to limit number of descriptors down to 48 per ring
  2010-07-20  4:40 ` Eric Dumazet
@ 2010-07-20 17:12   ` Brandeburg, Jesse
  0 siblings, 0 replies; 4+ messages in thread
From: Brandeburg, Jesse @ 2010-07-20 17:12 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Kirsher, Jeffrey T, davem@davemloft.net, netdev@vger.kernel.org,
	gospo@redhat.com, bphilips@novell.com, Duyck, Alexander H



On Mon, 19 Jul 2010, Eric Dumazet wrote:

> Le lundi 19 juillet 2010 à 16:43 -0700, Jeff Kirsher a écrit :
> > From: Alexander Duyck <alexander.h.duyck@intel.com>
> > 
> > This change makes it possible to limit the number of descriptors down to 48
> > per ring.  The reason for this change is to address a variation on hardware
> > errata 10 for 82546GB in which descriptors will be lost if more than 32
> > descriptors are fetched and the PCI-X MRBC is 512.
> > -#define E1000_MIN_TXD                       80
> > +#define E1000_MIN_TXD                       48
> >  #define E1000_MAX_82544_TXD               4096
> >  
> >  #define E1000_DEFAULT_RXD                  256
> >  #define E1000_MAX_RXD                      256
> > -#define E1000_MIN_RXD                       80
> > +#define E1000_MIN_RXD                       48
> >  #define E1000_MAX_82544_RXD               4096

> So this limit is a pure software one ?

Yes, the hardware will work fine (with the exception of some limits when 
certain performance registers are configured like [TR]XDCTL) all the way 
down to 1 descriptor, that said, the practical limit is probably more like 
8 for RX descriptors, and is 2 * (MAX_SKB_FRAGS + 1) for transmit if TSO 
is enabled.

if all offloads are disabled you could run with only 8 tx descriptors, and 
I believe it would work fine.

> Why not let an admin chose a lower limit if he wants to ?

I think in this case just to prevent odd interaction bugs with low values 
(there would have to be several driver changes and the testing work would 
increase to support very low values)

> I am asking because big ring sizes can be a latency source in some
> workloads.

yes, if you are trying to pace traffic in software.  That said, at least 
on transmit, most frames exit immediately from the hardware unless some 
external event like flow control is slowing down transmit.

Jesse

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

end of thread, other threads:[~2010-07-20 17:12 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-07-19 23:43 [net-next-2.6 PATCH] e1000: allow option to limit number of descriptors down to 48 per ring Jeff Kirsher
2010-07-20  3:24 ` David Miller
2010-07-20  4:40 ` Eric Dumazet
2010-07-20 17:12   ` Brandeburg, Jesse

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).