diff mbox

[RFC,net-next,07/14] Fix intel/ixgbe

Message ID 1340118848-30978-8-git-send-email-yuvalmin@broadcom.com
State RFC, archived
Delegated to: David Miller
Headers show

Commit Message

Yuval Mintz June 19, 2012, 3:14 p.m. UTC
Signed-off-by: Yuval Mintz <yuvalmin@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>

Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

Comments

Duyck, Alexander H June 19, 2012, 3:54 p.m. UTC | #1
On 06/19/2012 08:14 AM, Yuval Mintz wrote:
> Signed-off-by: Yuval Mintz <yuvalmin@broadcom.com>
> Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
>
> Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> ---
>  drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
> index af1a531..21e4513 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
> @@ -802,7 +802,8 @@ static int ixgbe_set_interrupt_capability(struct ixgbe_adapter *adapter)
>  	 * The default is to use pairs of vectors.
>  	 */
>  	v_budget = max(adapter->num_rx_queues, adapter->num_tx_queues);
> -	v_budget = min_t(int, v_budget, num_online_cpus());
> +	v_budget = min_t(int, v_budget, min_t(int, num_online_cpus(),
> +					      DEFAULT_MAX_NUM_RSS_QUEUES));
>  	v_budget += NON_Q_VECTORS;
>  
>  	/*
This patch doesn't limit the number of queues.  It is limiting the
number of interrupts.  The two are not directly related as we can
support multiple queues per interrupt.

Also this change assumes we are only using receive side scaling.  We
have other features such as DCB, FCoE, and Flow Director which require
additional queues.

Thanks,

Alex
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eilon Greenstein June 19, 2012, 4:11 p.m. UTC | #2
On Tue, 2012-06-19 at 08:54 -0700, Alexander Duyck wrote:
> On 06/19/2012 08:14 AM, Yuval Mintz wrote:
> > Signed-off-by: Yuval Mintz <yuvalmin@broadcom.com>
> > Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
> >
> > Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> > ---
> >  drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c |    3 ++-
> >  1 files changed, 2 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
> > index af1a531..21e4513 100644
> > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
> > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
> > @@ -802,7 +802,8 @@ static int ixgbe_set_interrupt_capability(struct ixgbe_adapter *adapter)
> >  	 * The default is to use pairs of vectors.
> >  	 */
> >  	v_budget = max(adapter->num_rx_queues, adapter->num_tx_queues);
> > -	v_budget = min_t(int, v_budget, num_online_cpus());
> > +	v_budget = min_t(int, v_budget, min_t(int, num_online_cpus(),
> > +					      DEFAULT_MAX_NUM_RSS_QUEUES));
> >  	v_budget += NON_Q_VECTORS;
> >  
> >  	/*
> This patch doesn't limit the number of queues.  It is limiting the
> number of interrupts.  The two are not directly related as we can
> support multiple queues per interrupt.
> 
> Also this change assumes we are only using receive side scaling.  We
> have other features such as DCB, FCoE, and Flow Director which require
> additional queues.

You are right - but DEFAULT_MAX_NUM_RSS_QUEUES is there to limit the RSS
and not everything else. It is harder to determine what else should be
set to a lower value and the two goals were to limit the memory waste in
correlation to the number of CPUs and to have some unification between
the drivers - both goals are applicable mostly to the RSS and not so
much to DCB, FCoE and similar features.

Thanks,
Eilon



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric Dumazet June 20, 2012, 8:55 a.m. UTC | #3
On Tue, 2012-06-19 at 08:54 -0700, Alexander Duyck wrote:

> This patch doesn't limit the number of queues.  It is limiting the
> number of interrupts.  The two are not directly related as we can
> support multiple queues per interrupt.
> 
> Also this change assumes we are only using receive side scaling.  We
> have other features such as DCB, FCoE, and Flow Director which require
> additional queues.
> 

Yet, it would be good if ixgbe doesnt allocate 36 queues on a 4 cpu
machine.

"tc -s class show dev eth0" output is full of not used classes.



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
John Fastabend June 20, 2012, 3:30 p.m. UTC | #4
On 6/20/2012 1:55 AM, Eric Dumazet wrote:
> On Tue, 2012-06-19 at 08:54 -0700, Alexander Duyck wrote:
>
>> This patch doesn't limit the number of queues.  It is limiting the
>> number of interrupts.  The two are not directly related as we can
>> support multiple queues per interrupt.
>>
>> Also this change assumes we are only using receive side scaling.  We
>> have other features such as DCB, FCoE, and Flow Director which require
>> additional queues.
>>
>
> Yet, it would be good if ixgbe doesnt allocate 36 queues on a 4 cpu
> machine.
>
> "tc -s class show dev eth0" output is full of not used classes.
>
>
>

We do this for the DCB/FCoE/RSS/Flow Director case where we want to
use multiple queues per traffic class (802.1Qaz). As it is now we
have to set the max queues at alloc_etherdev_mq() time so we use a
max of
	(num_cpu * max traffic classes) + num_cpu

The last num_cpu is in error and I have a patch in JeffK's tree to
remove this. In many cases it seems excessive but sometimes it is
helpful.

.John
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
index af1a531..21e4513 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
@@ -802,7 +802,8 @@  static int ixgbe_set_interrupt_capability(struct ixgbe_adapter *adapter)
 	 * The default is to use pairs of vectors.
 	 */
 	v_budget = max(adapter->num_rx_queues, adapter->num_tx_queues);
-	v_budget = min_t(int, v_budget, num_online_cpus());
+	v_budget = min_t(int, v_budget, min_t(int, num_online_cpus(),
+					      DEFAULT_MAX_NUM_RSS_QUEUES));
 	v_budget += NON_Q_VECTORS;
 
 	/*