Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] igb: count Rx FIFO errors correctly
From: David Miller @ 2010-03-24  6:20 UTC (permalink / raw)
  To: jeffrey.t.kirsher; +Cc: netdev, gospo, mitch.a.williams
In-Reply-To: <20100324043437.32036.44229.stgit@localhost.localdomain>

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Tue, 23 Mar 2010 21:34:38 -0700

> From: Mitch Williams <mitch.a.williams@intel.com>
> 
> Don't aggregate rx_no_buffer_count into rx_fifo_errors. RNBC counts
> packets that get queued temporarily in the adapter's FIFO. These
> packets are not dropped and are not errors. The correct counter
> is rx_missed_errors (MPC).
> 
> Signed-off-by: Mitch Williams <mitch.a.williams@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

Applied.

^ permalink raw reply

* Re: [PATCH 2/2] bnx2: Use proper handler during netpoll.
From: David Miller @ 2010-03-24  6:20 UTC (permalink / raw)
  To: mchan; +Cc: netdev
In-Reply-To: <1269385992-18525-2-git-send-email-mchan@broadcom.com>

From: "Michael Chan" <mchan@broadcom.com>
Date: Tue, 23 Mar 2010 15:13:12 -0800

> Netpoll needs to call the proper handler depending on the IRQ mode
> and the vector.
> 
> Signed-off-by: Michael Chan <mchan@broadcom.com>

Applied.

^ permalink raw reply

* Re: [PATCH 1/2] bnx2: Fix netpoll crash.
From: David Miller @ 2010-03-24  6:20 UTC (permalink / raw)
  To: mchan; +Cc: netdev
In-Reply-To: <1269385992-18525-1-git-send-email-mchan@broadcom.com>

From: "Michael Chan" <mchan@broadcom.com>
Date: Tue, 23 Mar 2010 15:13:11 -0800

> From: Benjamin Li <benli@broadcom.com>
> 
> The bnx2 driver calls netif_napi_add() for all the NAPI structs during
> ->probe() time but not all of them will be used if we're not in MSI-X
> mode.  This creates a problem for netpoll since it will poll all the
> NAPI structs in the dev_list whether or not they are scheduled, resulting
> in a crash when we access structure fields not initialized for that vector.
> 
> We fix it by moving the netif_napi_add() call to ->open() after the number
> of IRQ vectors has been determined.
> 
> Signed-off-by: Benjamin Li <benli@broadcom.com>
> Signed-off-by: Michael Chan <mchan@broadcom.com>

Applied.

^ permalink raw reply

* Re: [PATCH v2] Fix locking in flush_backlog
From: David Miller @ 2010-03-24  6:17 UTC (permalink / raw)
  To: eric.dumazet; +Cc: therbert, netdev
In-Reply-To: <1269408882.2915.42.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 24 Mar 2010 06:34:42 +0100

> Le mardi 23 mars 2010 à 16:39 -0700, Tom Herbert a écrit :
>> Need to take spinlocks when dequeuing from input_pkt_queue in flush_backlog.
>> Also, flush_backlog can now be called directly from netdev_run_todo.
>> 
>> Signed-off-by: Tom Herbert <therbert@google.com>
> 
> Acked-by: Eric Dumazet <eric.dumazet@gmail.com>

Applied, thanks everyone.

^ permalink raw reply

* Re: [RFC PATCH] net: add additional lock to qdisc to increase enqueue/dequeue fairness
From: David Miller @ 2010-03-24  6:10 UTC (permalink / raw)
  To: eric.dumazet; +Cc: alexander.h.duyck, netdev
In-Reply-To: <1269409324.2915.44.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 24 Mar 2010 06:42:04 +0100

> Do we have a list of such network related documents ?

Not really, just scan http://vger.kernel.org/netconf200${N}.html
for the papers :-)

^ permalink raw reply

* Re: [PATCH v2 1/2] vlan: adds vlan_dev_select_queue
From: Eric Dumazet @ 2010-03-24  5:53 UTC (permalink / raw)
  To: Jeff Kirsher; +Cc: davem, netdev, gospo, Vasu Dev
In-Reply-To: <20100324004116.31609.73183.stgit@localhost.localdomain>

Le mardi 23 mars 2010 à 17:41 -0700, Jeff Kirsher a écrit :
> From: Vasu Dev <vasu.dev@intel.com>
> 
> This is required to correctly select vlan tx queue for a driver
> supporting multi tx queue with ndo_select_queue implemented since
> currently selected vlan tx queue is unaligned to selected queue by
> real net_devce ndo_select_queue.
> 
> Unaligned vlan tx queue selection causes thrash with higher vlan
> tx lock contention for least fcoe traffic and wrong socket tx
> queue_mapping for ixgbe having ndo_select_queue implemented.
> 
> -v2
> 
> As per Eric Dumazet<eric.dumazet@gmail.com> comments, mirrored
> vlan net_device_ops to have them with and without vlan_dev_select_queue
> and then select according to real dev ndo_select_queue present or not
> for a vlan net_device. This is to completely skip vlan_dev_select_queue
> calling for real net_device not supporting ndo_select_queue.
> 
> Signed-off-by: Vasu Dev <vasu.dev@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> ---
> 

Acked-by: Eric Dumazet <eric.dumazet@gmail.com>

Thanks Jeff




^ permalink raw reply

* Re: [PATCH] igb: only use vlan_gro_receive if vlans are registered
From: Eric Dumazet @ 2010-03-24  5:50 UTC (permalink / raw)
  To: Jeff Kirsher; +Cc: davem, netdev, gospo, Alexander Duyck
In-Reply-To: <20100324043517.32036.81274.stgit@localhost.localdomain>

Le mardi 23 mars 2010 à 21:35 -0700, Jeff Kirsher a écrit :
> From: Alexander Duyck <alexander.h.duyck@intel.com>
> 
> This change makes it so that vlan_gro_receive is only used if vlans have been
> registered to the adapter structure.  Previously we were just sending all vlan
> tagged frames in via this function but this results in a null pointer
> dereference when vlans are not registered.
> 

This patch fixes bugzilla entry 15582

See http://bugzilla.kernel.org/show_bug.cgi?id=15582

> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> ---
> 

Acked-by: Eric Dumazet <eric.dumazet@gmail.com>

>  drivers/net/igb/igb_main.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/net/igb/igb_main.c b/drivers/net/igb/igb_main.c
> index e72760c..01c65c7 100644
> --- a/drivers/net/igb/igb_main.c
> +++ b/drivers/net/igb/igb_main.c
> @@ -5102,7 +5102,7 @@ static void igb_receive_skb(struct igb_q_vector *q_vector,
>  {
>  	struct igb_adapter *adapter = q_vector->adapter;
>  
> -	if (vlan_tag)
> +	if (vlan_tag && adapter->vlgrp)
>  		vlan_gro_receive(&q_vector->napi, adapter->vlgrp,
>  		                 vlan_tag, skb);
>  	else
> 
> --




^ permalink raw reply

* Re: [RFC PATCH] net: add additional lock to qdisc to increase enqueue/dequeue fairness
From: Eric Dumazet @ 2010-03-24  5:42 UTC (permalink / raw)
  To: David Miller; +Cc: alexander.h.duyck, netdev
In-Reply-To: <20100323.195855.00471034.davem@davemloft.net>

Le mardi 23 mars 2010 à 19:58 -0700, David Miller a écrit :
> From: "Duyck, Alexander H" <alexander.h.duyck@intel.com>
> Date: Tue, 23 Mar 2010 15:08:12 -0700
> 
> > I'll do some digging on this to see what I can find out.
> 
> Please see:  http://vger.kernel.org/jamal_netconf2006.sxi
> --

Thanks David, this is very informative !

Do we have a list of such network related documents ?



^ permalink raw reply

* Re: [PATCH v2] Fix locking in flush_backlog
From: Eric Dumazet @ 2010-03-24  5:34 UTC (permalink / raw)
  To: Tom Herbert; +Cc: davem, netdev
In-Reply-To: <alpine.DEB.1.00.1003231635190.360@pokey.mtv.corp.google.com>

Le mardi 23 mars 2010 à 16:39 -0700, Tom Herbert a écrit :
> Need to take spinlocks when dequeuing from input_pkt_queue in flush_backlog.
> Also, flush_backlog can now be called directly from netdev_run_todo.
> 
> Signed-off-by: Tom Herbert <therbert@google.com>

Acked-by: Eric Dumazet <eric.dumazet@gmail.com>

Thanks Tom

> ---
> diff --git a/net/core/dev.c b/net/core/dev.c
> diff --git a/net/core/dev.c b/net/core/dev.c
> index a03aab4..5e3dc28 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -2766,17 +2766,19 @@ int netif_receive_skb(struct sk_buff *skb)
>  EXPORT_SYMBOL(netif_receive_skb);
>  
>  /* Network device is going away, flush any packets still pending  */
> -static void flush_backlog(void *arg)
> +static void flush_backlog(struct net_device *dev, int cpu)
>  {
> -	struct net_device *dev = arg;
> -	struct softnet_data *queue = &__get_cpu_var(softnet_data);
> +	struct softnet_data *queue = &per_cpu(softnet_data, cpu);
>  	struct sk_buff *skb, *tmp;
> +	unsigned long flags;
>  
> +	spin_lock_irqsave(&queue->input_pkt_queue.lock, flags);
>  	skb_queue_walk_safe(&queue->input_pkt_queue, skb, tmp)
>  		if (skb->dev == dev) {
>  			__skb_unlink(skb, &queue->input_pkt_queue);
>  			kfree_skb(skb);
>  		}
> +	spin_unlock_irqrestore(&queue->input_pkt_queue.lock, flags);
>  }
>  
>  static int napi_gro_complete(struct sk_buff *skb)
> @@ -5545,6 +5547,7 @@ void netdev_run_todo(void)
>  	while (!list_empty(&list)) {
>  		struct net_device *dev
>  			= list_first_entry(&list, struct net_device, todo_list);
> +		int i;
>  		list_del(&dev->todo_list);
>  
>  		if (unlikely(dev->reg_state != NETREG_UNREGISTERING)) {
> @@ -5556,7 +5559,8 @@ void netdev_run_todo(void)
>  
>  		dev->reg_state = NETREG_UNREGISTERED;
>  
> -		on_each_cpu(flush_backlog, dev, 1);
> +		for_each_online_cpu(i)
> +			flush_backlog(dev, i);
>  
>  		netdev_wait_allrefs(dev);
>  


^ permalink raw reply

* [PATCH] igb: only use vlan_gro_receive if vlans are registered
From: Jeff Kirsher @ 2010-03-24  4:35 UTC (permalink / raw)
  To: davem; +Cc: netdev, gospo, Alexander Duyck, Jeff Kirsher

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

This change makes it so that vlan_gro_receive is only used if vlans have been
registered to the adapter structure.  Previously we were just sending all vlan
tagged frames in via this function but this results in a null pointer
dereference when vlans are not registered.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---

 drivers/net/igb/igb_main.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/igb/igb_main.c b/drivers/net/igb/igb_main.c
index e72760c..01c65c7 100644
--- a/drivers/net/igb/igb_main.c
+++ b/drivers/net/igb/igb_main.c
@@ -5102,7 +5102,7 @@ static void igb_receive_skb(struct igb_q_vector *q_vector,
 {
 	struct igb_adapter *adapter = q_vector->adapter;
 
-	if (vlan_tag)
+	if (vlan_tag && adapter->vlgrp)
 		vlan_gro_receive(&q_vector->napi, adapter->vlgrp,
 		                 vlan_tag, skb);
 	else


^ permalink raw reply related

* [PATCH] igb: do not modify tx_queue_len on link speed change
From: Jeff Kirsher @ 2010-03-24  4:34 UTC (permalink / raw)
  To: davem; +Cc: netdev, gospo, Emil Tantilov, Jeff Kirsher

From: Emil Tantilov <emil.s.tantilov@intel.com>

Previously the driver tweaked txqueuelen to avoid false Tx hang reports seen at half duplex.
This had the effect of overriding user set values on link change/reset. Testing shows that
adjusting only the timeout factor is sufficient to prevent Tx hang reports at half duplex.

Based on e1000e patch by Franco Fichtner <franco@lastsummer.de>

Signed-off-by: Emil Tantilov <emil.s.tantilov@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---

 drivers/net/igb/igb.h      |    1 -
 drivers/net/igb/igb_main.c |   10 +---------
 2 files changed, 1 insertions(+), 10 deletions(-)

diff --git a/drivers/net/igb/igb.h b/drivers/net/igb/igb.h
index a177570..3b772b8 100644
--- a/drivers/net/igb/igb.h
+++ b/drivers/net/igb/igb.h
@@ -267,7 +267,6 @@ struct igb_adapter {
 
 	/* TX */
 	struct igb_ring *tx_ring[16];
-	unsigned long tx_queue_len;
 	u32 tx_timeout_count;
 
 	/* RX */
diff --git a/drivers/net/igb/igb_main.c b/drivers/net/igb/igb_main.c
index 70dc03b..e72760c 100644
--- a/drivers/net/igb/igb_main.c
+++ b/drivers/net/igb/igb_main.c
@@ -1105,9 +1105,6 @@ static void igb_configure(struct igb_adapter *adapter)
 		struct igb_ring *ring = adapter->rx_ring[i];
 		igb_alloc_rx_buffers_adv(ring, igb_desc_unused(ring));
 	}
-
-
-	adapter->tx_queue_len = netdev->tx_queue_len;
 }
 
 /**
@@ -1213,7 +1210,6 @@ void igb_down(struct igb_adapter *adapter)
 	del_timer_sync(&adapter->watchdog_timer);
 	del_timer_sync(&adapter->phy_info_timer);
 
-	netdev->tx_queue_len = adapter->tx_queue_len;
 	netif_carrier_off(netdev);
 
 	/* record the stats before reset*/
@@ -3106,17 +3102,13 @@ static void igb_watchdog_task(struct work_struct *work)
 			       ((ctrl & E1000_CTRL_RFCE) ?  "RX" :
 			       ((ctrl & E1000_CTRL_TFCE) ?  "TX" : "None")));
 
-			/* tweak tx_queue_len according to speed/duplex and
-			 * adjust the timeout factor */
-			netdev->tx_queue_len = adapter->tx_queue_len;
+			/* adjust timeout factor according to speed/duplex */
 			adapter->tx_timeout_factor = 1;
 			switch (adapter->link_speed) {
 			case SPEED_10:
-				netdev->tx_queue_len = 10;
 				adapter->tx_timeout_factor = 14;
 				break;
 			case SPEED_100:
-				netdev->tx_queue_len = 100;
 				/* maybe add some timeout factor ? */
 				break;
 			}


^ permalink raw reply related

* [PATCH] igb: count Rx FIFO errors correctly
From: Jeff Kirsher @ 2010-03-24  4:34 UTC (permalink / raw)
  To: davem; +Cc: netdev, gospo, Mitch Williams, Jeff Kirsher

From: Mitch Williams <mitch.a.williams@intel.com>

Don't aggregate rx_no_buffer_count into rx_fifo_errors. RNBC counts
packets that get queued temporarily in the adapter's FIFO. These
packets are not dropped and are not errors. The correct counter
is rx_missed_errors (MPC).

Signed-off-by: Mitch Williams <mitch.a.williams@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---

 drivers/net/igb/igb_main.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/igb/igb_main.c b/drivers/net/igb/igb_main.c
index 45a0e4f..70dc03b 100644
--- a/drivers/net/igb/igb_main.c
+++ b/drivers/net/igb/igb_main.c
@@ -3963,7 +3963,7 @@ void igb_update_stats(struct igb_adapter *adapter)
 	struct net_device_stats *net_stats = igb_get_stats(adapter->netdev);
 	struct e1000_hw *hw = &adapter->hw;
 	struct pci_dev *pdev = adapter->pdev;
-	u32 rnbc, reg;
+	u32 reg, mpc;
 	u16 phy_tmp;
 	int i;
 	u64 bytes, packets;
@@ -4021,7 +4021,9 @@ void igb_update_stats(struct igb_adapter *adapter)
 	adapter->stats.symerrs += rd32(E1000_SYMERRS);
 	adapter->stats.sec += rd32(E1000_SEC);
 
-	adapter->stats.mpc += rd32(E1000_MPC);
+	mpc = rd32(E1000_MPC);
+	adapter->stats.mpc += mpc;
+	net_stats->rx_fifo_errors += mpc;
 	adapter->stats.scc += rd32(E1000_SCC);
 	adapter->stats.ecol += rd32(E1000_ECOL);
 	adapter->stats.mcc += rd32(E1000_MCC);
@@ -4036,9 +4038,7 @@ void igb_update_stats(struct igb_adapter *adapter)
 	adapter->stats.gptc += rd32(E1000_GPTC);
 	adapter->stats.gotc += rd32(E1000_GOTCL);
 	rd32(E1000_GOTCH); /* clear GOTCL */
-	rnbc = rd32(E1000_RNBC);
-	adapter->stats.rnbc += rnbc;
-	net_stats->rx_fifo_errors += rnbc;
+	adapter->stats.rnbc += rd32(E1000_RNBC);
 	adapter->stats.ruc += rd32(E1000_RUC);
 	adapter->stats.rfc += rd32(E1000_RFC);
 	adapter->stats.rjc += rd32(E1000_RJC);


^ permalink raw reply related

* Re: [RFC PATCH] net: add additional lock to qdisc to increase enqueue/dequeue fairness
From: David Miller @ 2010-03-24  2:58 UTC (permalink / raw)
  To: alexander.h.duyck; +Cc: eric.dumazet, netdev
In-Reply-To: <80769D7B14936844A23C0C43D9FBCF0F254E49AC69@orsmsx501.amr.corp.intel.com>

From: "Duyck, Alexander H" <alexander.h.duyck@intel.com>
Date: Tue, 23 Mar 2010 15:08:12 -0700

> I'll do some digging on this to see what I can find out.

Please see:  http://vger.kernel.org/jamal_netconf2006.sxi

^ permalink raw reply

* Re: [RFC Patch 1/3] netpoll: add generic support for bridge and bonding devices
From: Cong Wang @ 2010-03-24  2:29 UTC (permalink / raw)
  To: Jeff Moyer
  Cc: Matt Mackall, linux-kernel, netdev, bridge, Andy Gospodarek,
	Neil Horman, Stephen Hemminger, bonding-devel, Jay Vosburgh,
	David Miller
In-Reply-To: <x49zl1zxmou.fsf@segfault.boston.devel.redhat.com>

Jeff Moyer wrote:
> Matt Mackall <mpm@selenic.com> writes:
> 
>> On Mon, 2010-03-22 at 04:17 -0400, Amerigo Wang wrote:
>>> This whole patchset is for adding netpoll support to bridge and bonding
>>> devices. I already tested it for bridge, bonding, bridge over bonding,
>>> and bonding over bridge. It looks fine now.
>> Ages ago, Jeff Moyer took a run at this, added him to the cc: on the off
>> chance he still cares.
> 
> I'll take a look at it in a bit.  For now, here is the link to my
> original post on this for Amerigo's reading pleasure:
> 
>   http://lkml.indiana.edu/hypermail/linux/kernel/0507.0/0206.html
> 

Thanks, Jeff! I will take a look at it.

^ permalink raw reply

* Re: [RFC PATCH] net: add additional lock to qdisc to increase enqueue/dequeue fairness
From: Andi Kleen @ 2010-03-24  2:12 UTC (permalink / raw)
  To: David Miller; +Cc: alexander.h.duyck, netdev
In-Reply-To: <20100323.133124.139557595.davem@davemloft.net>

David Miller <davem@davemloft.net> writes:

> From: Alexander Duyck <alexander.h.duyck@intel.com>
> Date: Tue, 23 Mar 2010 13:25:53 -0700
>
>> The qdisc layer shows a significant issue when you start transmitting from
>> multiple CPUs.
>
> Why are you hitting any central qdisc lock on a multiqueue
> configuration?

One thing to remember is that systems can have a lot more CPUs than a NIC queues
(e.g. 64 vs 8)

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.

^ permalink raw reply

* [PATCH v2 2/2] vlan: updates vlan real_num_tx_queues
From: Jeff Kirsher @ 2010-03-24  0:42 UTC (permalink / raw)
  To: davem; +Cc: netdev, gospo, Vasu Dev, Jeff Kirsher
In-Reply-To: <20100324004116.31609.73183.stgit@localhost.localdomain>

From: Vasu Dev <vasu.dev@intel.com>

Updates real_num_tx_queues in case underlying real device
has changed real_num_tx_queues.

-v2
 As per Eric Dumazet<eric.dumazet@gmail.com> comment:-
   -- adds BUG_ON to catch case of real_num_tx_queues exceeding num_tx_queues.
   -- created this self contained patch to just update real_num_tx_queues.

Signed-off-by: Vasu Dev <vasu.dev@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---

 net/8021q/vlan.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index 4535122..db783d7 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -378,6 +378,8 @@ static void vlan_transfer_features(struct net_device *dev,
 #if defined(CONFIG_FCOE) || defined(CONFIG_FCOE_MODULE)
 	vlandev->fcoe_ddp_xid = dev->fcoe_ddp_xid;
 #endif
+	vlandev->real_num_tx_queues = dev->real_num_tx_queues;
+	BUG_ON(vlandev->real_num_tx_queues > vlandev->num_tx_queues);
 
 	if (old_features != vlandev->features)
 		netdev_features_change(vlandev);


^ permalink raw reply related

* [PATCH v2 1/2] vlan: adds vlan_dev_select_queue
From: Jeff Kirsher @ 2010-03-24  0:41 UTC (permalink / raw)
  To: davem; +Cc: netdev, gospo, Vasu Dev, Jeff Kirsher

From: Vasu Dev <vasu.dev@intel.com>

This is required to correctly select vlan tx queue for a driver
supporting multi tx queue with ndo_select_queue implemented since
currently selected vlan tx queue is unaligned to selected queue by
real net_devce ndo_select_queue.

Unaligned vlan tx queue selection causes thrash with higher vlan
tx lock contention for least fcoe traffic and wrong socket tx
queue_mapping for ixgbe having ndo_select_queue implemented.

-v2

As per Eric Dumazet<eric.dumazet@gmail.com> comments, mirrored
vlan net_device_ops to have them with and without vlan_dev_select_queue
and then select according to real dev ndo_select_queue present or not
for a vlan net_device. This is to completely skip vlan_dev_select_queue
calling for real net_device not supporting ndo_select_queue.

Signed-off-by: Vasu Dev <vasu.dev@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---

 net/8021q/vlan_dev.c |   71 ++++++++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 68 insertions(+), 3 deletions(-)

diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index 9e83272..2fd057c 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -361,6 +361,14 @@ static netdev_tx_t vlan_dev_hwaccel_hard_start_xmit(struct sk_buff *skb,
 	return ret;
 }
 
+static u16 vlan_dev_select_queue(struct net_device *dev, struct sk_buff *skb)
+{
+	struct net_device *rdev = vlan_dev_info(dev)->real_dev;
+	const struct net_device_ops *ops = rdev->netdev_ops;
+
+	return ops->ndo_select_queue(rdev, skb);
+}
+
 static int vlan_dev_change_mtu(struct net_device *dev, int new_mtu)
 {
 	/* TODO: gotta make sure the underlying layer can handle it,
@@ -688,7 +696,8 @@ static const struct header_ops vlan_header_ops = {
 	.parse	 = eth_header_parse,
 };
 
-static const struct net_device_ops vlan_netdev_ops, vlan_netdev_accel_ops;
+static const struct net_device_ops vlan_netdev_ops, vlan_netdev_accel_ops,
+		    vlan_netdev_ops_sq, vlan_netdev_accel_ops_sq;
 
 static int vlan_dev_init(struct net_device *dev)
 {
@@ -722,11 +731,17 @@ static int vlan_dev_init(struct net_device *dev)
 	if (real_dev->features & NETIF_F_HW_VLAN_TX) {
 		dev->header_ops      = real_dev->header_ops;
 		dev->hard_header_len = real_dev->hard_header_len;
-		dev->netdev_ops         = &vlan_netdev_accel_ops;
+		if (real_dev->netdev_ops->ndo_select_queue)
+			dev->netdev_ops = &vlan_netdev_accel_ops_sq;
+		else
+			dev->netdev_ops = &vlan_netdev_accel_ops;
 	} else {
 		dev->header_ops      = &vlan_header_ops;
 		dev->hard_header_len = real_dev->hard_header_len + VLAN_HLEN;
-		dev->netdev_ops         = &vlan_netdev_ops;
+		if (real_dev->netdev_ops->ndo_select_queue)
+			dev->netdev_ops = &vlan_netdev_ops_sq;
+		else
+			dev->netdev_ops = &vlan_netdev_ops;
 	}
 
 	if (is_vlan_dev(real_dev))
@@ -865,6 +880,56 @@ static const struct net_device_ops vlan_netdev_accel_ops = {
 #endif
 };
 
+static const struct net_device_ops vlan_netdev_ops_sq = {
+	.ndo_select_queue	= vlan_dev_select_queue,
+	.ndo_change_mtu		= vlan_dev_change_mtu,
+	.ndo_init		= vlan_dev_init,
+	.ndo_uninit		= vlan_dev_uninit,
+	.ndo_open		= vlan_dev_open,
+	.ndo_stop		= vlan_dev_stop,
+	.ndo_start_xmit =  vlan_dev_hard_start_xmit,
+	.ndo_validate_addr	= eth_validate_addr,
+	.ndo_set_mac_address	= vlan_dev_set_mac_address,
+	.ndo_set_rx_mode	= vlan_dev_set_rx_mode,
+	.ndo_set_multicast_list	= vlan_dev_set_rx_mode,
+	.ndo_change_rx_flags	= vlan_dev_change_rx_flags,
+	.ndo_do_ioctl		= vlan_dev_ioctl,
+	.ndo_neigh_setup	= vlan_dev_neigh_setup,
+	.ndo_get_stats		= vlan_dev_get_stats,
+#if defined(CONFIG_FCOE) || defined(CONFIG_FCOE_MODULE)
+	.ndo_fcoe_ddp_setup	= vlan_dev_fcoe_ddp_setup,
+	.ndo_fcoe_ddp_done	= vlan_dev_fcoe_ddp_done,
+	.ndo_fcoe_enable	= vlan_dev_fcoe_enable,
+	.ndo_fcoe_disable	= vlan_dev_fcoe_disable,
+	.ndo_fcoe_get_wwn	= vlan_dev_fcoe_get_wwn,
+#endif
+};
+
+static const struct net_device_ops vlan_netdev_accel_ops_sq = {
+	.ndo_select_queue	= vlan_dev_select_queue,
+	.ndo_change_mtu		= vlan_dev_change_mtu,
+	.ndo_init		= vlan_dev_init,
+	.ndo_uninit		= vlan_dev_uninit,
+	.ndo_open		= vlan_dev_open,
+	.ndo_stop		= vlan_dev_stop,
+	.ndo_start_xmit =  vlan_dev_hwaccel_hard_start_xmit,
+	.ndo_validate_addr	= eth_validate_addr,
+	.ndo_set_mac_address	= vlan_dev_set_mac_address,
+	.ndo_set_rx_mode	= vlan_dev_set_rx_mode,
+	.ndo_set_multicast_list	= vlan_dev_set_rx_mode,
+	.ndo_change_rx_flags	= vlan_dev_change_rx_flags,
+	.ndo_do_ioctl		= vlan_dev_ioctl,
+	.ndo_neigh_setup	= vlan_dev_neigh_setup,
+	.ndo_get_stats		= vlan_dev_get_stats,
+#if defined(CONFIG_FCOE) || defined(CONFIG_FCOE_MODULE)
+	.ndo_fcoe_ddp_setup	= vlan_dev_fcoe_ddp_setup,
+	.ndo_fcoe_ddp_done	= vlan_dev_fcoe_ddp_done,
+	.ndo_fcoe_enable	= vlan_dev_fcoe_enable,
+	.ndo_fcoe_disable	= vlan_dev_fcoe_disable,
+	.ndo_fcoe_get_wwn	= vlan_dev_fcoe_get_wwn,
+#endif
+};
+
 void vlan_setup(struct net_device *dev)
 {
 	ether_setup(dev);


^ permalink raw reply related

* Re: ixgbe: Set IXGBE_RSC_CB(skb)->DMA field to zero after unmapping the address
From: Benjamin Herrenschmidt @ 2010-03-23 23:40 UTC (permalink / raw)
  To: Malli
  Cc: Mallikarjuna R Chilakala, Linux Kernel Mailing List, David Miller,
	netdev
In-Reply-To: <7991c5481003231040u3479778dpcb27d9578383afc0@mail.gmail.com>

On Tue, 2010-03-23 at 10:40 -0700, Malli wrote:
> 
> Yes. I just realized that i can't assign a zero magic "bad" value. It
> is only
> valid in x86/arm/m68k/alpha architecures and not in spark & PowerPC
> arch,
> (it should be ~0). In some other architecutres it throws a BUG() on
> with
> dma_mapping_error() checks. So the patch is not a total bogus in some
> architectures :(.
> 
> May be it is best to create an internal FLAG in IXGBE_RSC_CB(skb)
> which can be used to avoid this double freeing. 

I suppose if Linus reads that he'll just come back and say "you idiots,
just make 0 invalid everywhere" like he did for interrupts tho :-) It
wouldn't be too hard for us to make the IOMMU allocator always skip "0".

But for now, I'd suggest you add a flag.

Cheers,
Ben.

^ permalink raw reply

* [PATCH v2] Fix locking in flush_backlog
From: Tom Herbert @ 2010-03-23 23:39 UTC (permalink / raw)
  To: davem, netdev

Need to take spinlocks when dequeuing from input_pkt_queue in flush_backlog.
Also, flush_backlog can now be called directly from netdev_run_todo.

Signed-off-by: Tom Herbert <therbert@google.com>
---
diff --git a/net/core/dev.c b/net/core/dev.c
diff --git a/net/core/dev.c b/net/core/dev.c
index a03aab4..5e3dc28 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2766,17 +2766,19 @@ int netif_receive_skb(struct sk_buff *skb)
 EXPORT_SYMBOL(netif_receive_skb);
 
 /* Network device is going away, flush any packets still pending  */
-static void flush_backlog(void *arg)
+static void flush_backlog(struct net_device *dev, int cpu)
 {
-	struct net_device *dev = arg;
-	struct softnet_data *queue = &__get_cpu_var(softnet_data);
+	struct softnet_data *queue = &per_cpu(softnet_data, cpu);
 	struct sk_buff *skb, *tmp;
+	unsigned long flags;
 
+	spin_lock_irqsave(&queue->input_pkt_queue.lock, flags);
 	skb_queue_walk_safe(&queue->input_pkt_queue, skb, tmp)
 		if (skb->dev == dev) {
 			__skb_unlink(skb, &queue->input_pkt_queue);
 			kfree_skb(skb);
 		}
+	spin_unlock_irqrestore(&queue->input_pkt_queue.lock, flags);
 }
 
 static int napi_gro_complete(struct sk_buff *skb)
@@ -5545,6 +5547,7 @@ void netdev_run_todo(void)
 	while (!list_empty(&list)) {
 		struct net_device *dev
 			= list_first_entry(&list, struct net_device, todo_list);
+		int i;
 		list_del(&dev->todo_list);
 
 		if (unlikely(dev->reg_state != NETREG_UNREGISTERING)) {
@@ -5556,7 +5559,8 @@ void netdev_run_todo(void)
 
 		dev->reg_state = NETREG_UNREGISTERED;
 
-		on_each_cpu(flush_backlog, dev, 1);
+		for_each_online_cpu(i)
+			flush_backlog(dev, i);
 
 		netdev_wait_allrefs(dev);
 

^ permalink raw reply related

* [PATCH 2/2] bnx2: Use proper handler during netpoll.
From: Michael Chan @ 2010-03-23 23:13 UTC (permalink / raw)
  To: davem; +Cc: netdev
In-Reply-To: <1269385992-18525-1-git-send-email-mchan@broadcom.com>

Netpoll needs to call the proper handler depending on the IRQ mode
and the vector.

Signed-off-by: Michael Chan <mchan@broadcom.com>
---
 drivers/net/bnx2.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c
index 417de1c..a257bab 100644
--- a/drivers/net/bnx2.c
+++ b/drivers/net/bnx2.c
@@ -7646,9 +7646,11 @@ poll_bnx2(struct net_device *dev)
 	int i;
 
 	for (i = 0; i < bp->irq_nvecs; i++) {
-		disable_irq(bp->irq_tbl[i].vector);
-		bnx2_interrupt(bp->irq_tbl[i].vector, &bp->bnx2_napi[i]);
-		enable_irq(bp->irq_tbl[i].vector);
+		struct bnx2_irq *irq = &bp->irq_tbl[i];
+
+		disable_irq(irq->vector);
+		irq->handler(irq->vector, &bp->bnx2_napi[i]);
+		enable_irq(irq->vector);
 	}
 }
 #endif
-- 
1.6.4.GIT



^ permalink raw reply related

* [PATCH 1/2] bnx2: Fix netpoll crash.
From: Michael Chan @ 2010-03-23 23:13 UTC (permalink / raw)
  To: davem; +Cc: netdev

From: Benjamin Li <benli@broadcom.com>

The bnx2 driver calls netif_napi_add() for all the NAPI structs during
->probe() time but not all of them will be used if we're not in MSI-X
mode.  This creates a problem for netpoll since it will poll all the
NAPI structs in the dev_list whether or not they are scheduled, resulting
in a crash when we access structure fields not initialized for that vector.

We fix it by moving the netif_napi_add() call to ->open() after the number
of IRQ vectors has been determined.

Signed-off-by: Benjamin Li <benli@broadcom.com>
Signed-off-by: Michael Chan <mchan@broadcom.com>
---
 drivers/net/bnx2.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c
index 381887b..417de1c 100644
--- a/drivers/net/bnx2.c
+++ b/drivers/net/bnx2.c
@@ -246,6 +246,8 @@ static const struct flash_spec flash_5709 = {
 
 MODULE_DEVICE_TABLE(pci, bnx2_pci_tbl);
 
+static void bnx2_init_napi(struct bnx2 *bp);
+
 static inline u32 bnx2_tx_avail(struct bnx2 *bp, struct bnx2_tx_ring_info *txr)
 {
 	u32 diff;
@@ -6197,6 +6199,7 @@ bnx2_open(struct net_device *dev)
 	bnx2_disable_int(bp);
 
 	bnx2_setup_int_mode(bp, disable_msi);
+	bnx2_init_napi(bp);
 	bnx2_napi_enable(bp);
 	rc = bnx2_alloc_mem(bp);
 	if (rc)
@@ -8207,7 +8210,7 @@ bnx2_init_napi(struct bnx2 *bp)
 {
 	int i;
 
-	for (i = 0; i < BNX2_MAX_MSIX_VEC; i++) {
+	for (i = 0; i < bp->irq_nvecs; i++) {
 		struct bnx2_napi *bnapi = &bp->bnx2_napi[i];
 		int (*poll)(struct napi_struct *, int);
 
@@ -8276,7 +8279,6 @@ bnx2_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	dev->ethtool_ops = &bnx2_ethtool_ops;
 
 	bp = netdev_priv(dev);
-	bnx2_init_napi(bp);
 
 	pci_set_drvdata(pdev, dev);
 
-- 
1.6.4.GIT



^ permalink raw reply related

* inet6_ifa_finish_destroy() blurt
From: Andrew Morton @ 2010-03-23 22:35 UTC (permalink / raw)
  To: netdev; +Cc: Stephen Hemminger


Got this during boot using today's linux-next.  The
WARN_ON(!hlist_unhashed(&ifp->addr_lst)) triggered. 

c2e21293c054817c42eb5fa9c613d2ad51954136 ("ipv6: convert addrconf list
to hlist") added it.



[   53.677562] calling  init_autofs4_fs+0x0/0x23 [autofs4] @ 3535
[   53.677931] initcall init_autofs4_fs+0x0/0x23 [autofs4] returned 0 after 57 usecs
[  247.122112] ------------[ cut here ]------------
[  247.122187] WARNING: at net/ipv6/addrconf.c:544 inet6_ifa_finish_destroy+0x24/0x9b [ipv6]()
[  247.122244] Modules linked in: autofs4 sunrpc ipv6 dm_mirror dm_region_hash dm_log dm_multipath dm_mod video output sbs sbshc battery ac lp parport sg ide_cd_mod cdrom serio_raw floppy snd_hda_intel snd_hda_codec snd_seq_oss snd_seq_midi_event snd_seq snd_seq_device button snd_pcm_oss snd_mixer_oss snd_pcm i2c_i801 i2c_core pcspkr snd_timer snd soundcore snd_page_alloc shpchp ehci_hcd ohci_hcd uhci_hcd
[  247.124152] Pid: 0, comm: swapper Not tainted 2.6.34-rc2-mm1+ #1
[  247.124208] Call Trace:
[  247.124255]  <IRQ>  [<ffffffffa01941d3>] ? inet6_ifa_finish_destroy+0x24/0x9b [ipv6]
[  247.124369]  [<ffffffff81035489>] warn_slowpath_common+0x7e/0x97
[  247.124425]  [<ffffffff810354b7>] warn_slowpath_null+0x15/0x17
[  247.124487]  [<ffffffffa01941d3>] inet6_ifa_finish_destroy+0x24/0x9b [ipv6]
[  247.124550]  [<ffffffffa019666b>] addrconf_prefix_rcv+0x546/0x713 [ipv6]
[  247.124609]  [<ffffffff812d7625>] ? neigh_update+0x3cc/0x3ec
[  247.124669]  [<ffffffffa01a01ac>] ndisc_rcv+0xa45/0xf2c [ipv6]
[  247.124723]  [<ffffffff812d80d1>] ? rtnl_notify+0x2b/0x2d
[  247.124785]  [<ffffffffa0199962>] ? inet6_rt_notify+0x104/0x129 [ipv6]
[  247.124851]  [<ffffffffa01a5dd5>] icmpv6_rcv+0x5cb/0x66b [ipv6]
[  247.124919]  [<ffffffffa01986b4>] ? __ip6_ins_rt+0x3d/0x48 [ipv6]
[  247.124976]  [<ffffffff8135b0fd>] ? _raw_read_unlock_bh+0x12/0x14
[  247.125041]  [<ffffffffa01a83e7>] ? ipv6_chk_mcast_addr+0x129/0x14f [ipv6]
[  247.125104]  [<ffffffffa0192184>] ip6_input_finish+0x19f/0x28b [ipv6]
[  247.125167]  [<ffffffffa0191b62>] ip6_input+0x4a/0x4e [ipv6]
[  247.125227]  [<ffffffffa0191c4d>] ip6_mc_input+0xc3/0xd4 [ipv6]
[  247.125288]  [<ffffffffa0191b86>] ip6_rcv_finish+0x20/0x24 [ipv6]
[  247.125351]  [<ffffffffa0191fac>] ipv6_rcv+0x34e/0x387 [ipv6]
[  247.125407]  [<ffffffff812d03fd>] __netif_receive_skb+0x4bf/0x4e4
[  247.125460]  [<ffffffff812d0515>] netif_receive_skb+0x24/0x32
[  247.125515]  [<ffffffff812d0bbf>] napi_skb_finish+0x21/0x37
[  247.125569]  [<ffffffff812d0bff>] napi_gro_receive+0x2a/0x2f
[  247.125625]  [<ffffffff8125e7e2>] e1000_receive_skb+0x60/0x69
[  247.125678]  [<ffffffff812606f6>] e1000_clean_rx_irq+0x240/0x2da
[  247.125734]  [<ffffffff8126241c>] e1000_clean+0x79/0x224
[  247.125788]  [<ffffffff812d06e4>] net_rx_action+0xca/0x1da
[  247.125841]  [<ffffffff8103a502>] __do_softirq+0x9e/0x11f
[  247.125895]  [<ffffffff8100390c>] call_softirq+0x1c/0x28
[  247.125950]  [<ffffffff81004ff5>] do_softirq+0x33/0x6a
[  247.126002]  [<ffffffff8103a42e>] irq_exit+0x38/0x3a
[  247.126055]  [<ffffffff81004ebc>] do_IRQ+0xaa/0xc1
[  247.126108]  [<ffffffff8135b393>] ret_from_intr+0x0/0xa
[  247.126159]  <EOI>  [<ffffffff81009d13>] ? mwait_idle+0x59/0x5e
[  247.126260]  [<ffffffff8100104a>] ? enter_idle+0x20/0x22
[  247.126425]  [<ffffffff810014c5>] cpu_idle+0x43/0x61
[  247.126591]  [<ffffffff81355389>] start_secondary+0x1aa/0x1ae
[  247.126757]  [<ffffffff813551df>] ? start_secondary+0x0/0x1ae
[  247.126922] ---[ end trace ea6028b997f8cf89 ]---
[  247.127085] Freeing alive inet6 address ffff88025d872a80

^ permalink raw reply

* RE: [RFC PATCH] net: add additional lock to qdisc to increase enqueue/dequeue fairness
From: Duyck, Alexander H @ 2010-03-23 22:08 UTC (permalink / raw)
  To: David Miller, eric.dumazet@gmail.com; +Cc: netdev@vger.kernel.org
In-Reply-To: <20100323.144512.140757007.davem@davemloft.net>

David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Tue, 23 Mar 2010 21:54:27 +0100
> 
>> Quite frankly, the real problem in this case is not the reduced
>> throughput, but fact that one cpu can stay a long time doing the
>> xmits to device, of skb queued by other cpus. This can hurt
>> latencies a lot, for real time threads for example...
>> 
>> I wonder if ticket spinlocks are not the problem. Maybe we want a
>> variant of spinlocks, so that cpu doing transmits can get the lock
>> before other cpus...
> 
> I want to note that things operate the way they do now
> intentionally.
> 
> Herbert Xu and Jamal Hadi Salim were active in this area
> about 4 years ago.

I'll do some digging on this to see what I can find out.  I'm not sure why we would want to do things this way since it seems like the CPU that has qdisc_running is sitting idle due to the fact that the ticket spinlocks have us waiting for all of the other CPU enqueues before we can get around to another dequeue.  It seems like it would be in our best interest to keep qdisc_running going while we are still enqueuing on the other CPUs.

I'll also look into the idea of possibly coming up with a more portable solution such as a priority based spinlock solution.

Thanks,

Alex


^ permalink raw reply

* Re: [RFC PATCH] net: add additional lock to qdisc to increase enqueue/dequeue fairness
From: Eric Dumazet @ 2010-03-23 22:13 UTC (permalink / raw)
  To: David Miller; +Cc: alexander.h.duyck, netdev
In-Reply-To: <20100323.144512.140757007.davem@davemloft.net>

Le mardi 23 mars 2010 à 14:45 -0700, David Miller a écrit :
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Tue, 23 Mar 2010 21:54:27 +0100
> 
> > Quite frankly, the real problem in this case is not the reduced
> > throughput, but fact that one cpu can stay a long time doing the xmits
> > to device, of skb queued by other cpus. This can hurt latencies a lot,
> > for real time threads for example...
> > 
> > I wonder if ticket spinlocks are not the problem. Maybe we want a
> > variant of spinlocks, so that cpu doing transmits can get the lock
> > before other cpus...
> 
> I want to note that things operate the way they do now
> intentionally.
> 
> Herbert Xu and Jamal Hadi Salim were active in this area
> about 4 years ago.

Yes, but ticket spinlocks were added after their work (in 2008 - 2.6.25
if I remember well) and change things.

We want cpu owning __QDISC_STATE_RUNNING being able to re-get the lock
as fast as possible. Alexander results can show the possible speedup.




^ permalink raw reply

* Re: [RFC PATCH] net: add additional lock to qdisc to increase enqueue/dequeue fairness
From: David Miller @ 2010-03-23 21:45 UTC (permalink / raw)
  To: eric.dumazet; +Cc: alexander.h.duyck, netdev
In-Reply-To: <1269377667.2915.25.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 23 Mar 2010 21:54:27 +0100

> Quite frankly, the real problem in this case is not the reduced
> throughput, but fact that one cpu can stay a long time doing the xmits
> to device, of skb queued by other cpus. This can hurt latencies a lot,
> for real time threads for example...
> 
> I wonder if ticket spinlocks are not the problem. Maybe we want a
> variant of spinlocks, so that cpu doing transmits can get the lock
> before other cpus...

I want to note that things operate the way they do now
intentionally.

Herbert Xu and Jamal Hadi Salim were active in this area
about 4 years ago.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox