All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 2.6.35-rc1] net: vmxnet3 fixes [0/5] Spare skb to avoid starvation
@ 2010-07-07  9:00 Shreyas Bhatewara
  2010-07-07  9:24 ` [PATCH 2.6.35-rc1] net: vmxnet3 fixes [2/5] Interrupt control bitmap Shreyas Bhatewara
                   ` (2 more replies)
  0 siblings, 3 replies; 32+ messages in thread
From: Shreyas Bhatewara @ 2010-07-07  9:00 UTC (permalink / raw)
  To: netdev-devel; +Cc: linux-kernel, pv-drivers, stolarchuk


From: Shreyas Bhatewara <sbhatewara@vmware.com>

skb_alloc() failure can cause the recv ring to loose all packet reception.
Avoid this by introducing a spare buffer.

Signed-off-by: Michael Stolarchuk <stolarchuk@vmware.com>
Signed-off-by: Shreyas Bhatewara <sbhatewara@vmware.com>

---

diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c
index 989b742..5a50d10 100644
--- a/drivers/net/vmxnet3/vmxnet3_drv.c
+++ b/drivers/net/vmxnet3/vmxnet3_drv.c
@@ -541,7 +541,12 @@ vmxnet3_rq_alloc_rx_buf(struct vmxnet3_rx_queue *rq, u32 ring_idx,
 							 NET_IP_ALIGN);
 				if (unlikely(rbi->skb == NULL)) {
 					rq->stats.rx_buf_alloc_failure++;
-					break;
+					/* starvation prevention */
+					if (vmxnet3_cmd_ring_desc_empty(
+							rq->rx_ring + ring_idx))
+						rbi->skb = rq->spare_skb;
+					else
+						break;
 				}
 				rbi->skb->dev = adapter->netdev;
 
@@ -611,6 +616,29 @@ vmxnet3_append_frag(struct sk_buff *skb, struct Vmxnet3_RxCompDesc *rcd,
 }
 
 
+/*
+ * Free any pages which were attached to the frags of the spare skb.  This can
+ * happen when the spare skb is attached to the rx ring to prevent starvation,
+ * but there was no issue with page allocation.
+ */
+
+static void
+vmxnet3_rx_spare_skb_free_frags(struct vmxnet3_adapter *adapter)
+{
+	struct sk_buff *skb = adapter->rx_queue.spare_skb;
+	int i;
+	for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
+		struct skb_frag_struct *frag = &skb_shinfo(skb)->frags[i];
+		BUG_ON(frag->page != 0);
+		put_page(frag->page);
+		frag->page = 0;
+		frag->size = 0;
+	}
+	skb_shinfo(skb)->nr_frags = 0;
+	skb->data_len = 0;
+}
+
+
 static void
 vmxnet3_map_pkt(struct sk_buff *skb, struct vmxnet3_tx_ctx *ctx,
 		struct vmxnet3_tx_queue *tq, struct pci_dev *pdev,
@@ -1060,8 +1088,12 @@ vmxnet3_rx_error(struct vmxnet3_rx_queue *rq, struct Vmxnet3_RxCompDesc *rcd,
 	 * ctx->skb may be NULL if this is the first and the only one
 	 * desc for the pkt
 	 */
-	if (ctx->skb)
-		dev_kfree_skb_irq(ctx->skb);
+	if (ctx->skb) {
+		if (ctx->skb == rq->spare_skb)
+			vmxnet3_rx_spare_skb_free_frags(adapter);
+		else
+			dev_kfree_skb_irq(ctx->skb);
+	}
 
 	ctx->skb = NULL;
 }
@@ -1159,6 +1191,12 @@ vmxnet3_rq_rx_complete(struct vmxnet3_rx_queue *rq,
 
 		skb = ctx->skb;
 		if (rcd->eop) {
+			if (skb == rq->spare_skb) {
+				rq->stats.drop_total++;
+				vmxnet3_rx_spare_skb_free_frags(adapter);
+				ctx->skb = NULL;
+				goto rcd_done;
+			}
 			skb->len += skb->data_len;
 			skb->truesize += skb->data_len;
 
@@ -1244,6 +1282,14 @@ vmxnet3_rq_cleanup(struct vmxnet3_rx_queue *rq,
 		rq->uncommitted[ring_idx] = 0;
 	}
 
+	/* free starvation prevention skb if allocated */
+	if (rq->spare_skb) {
+		vmxnet3_rx_spare_skb_free_frags(adapter);
+		dev_kfree_skb(rq->spare_skb);
+		rq->spare_skb = NULL;
+	}
+
+
 	rq->comp_ring.gen = VMXNET3_INIT_GEN;
 	rq->comp_ring.next2proc = 0;
 }
@@ -1325,6 +1371,15 @@ vmxnet3_rq_init(struct vmxnet3_rx_queue *rq,
 	}
 	vmxnet3_rq_alloc_rx_buf(rq, 1, rq->rx_ring[1].size - 1, adapter);
 
+	/* allocate ring starvation protection */
+	rq->spare_skb = dev_alloc_skb(PAGE_SIZE);
+	if (rq->spare_skb == NULL) {
+		vmxnet3_rq_cleanup(rq, adapter);
+		return -ENOMEM;
+	}
+
+
+
 	/* reset the comp ring */
 	rq->comp_ring.next2proc = 0;
 	memset(rq->comp_ring.base, 0, rq->comp_ring.size *
diff --git a/drivers/net/vmxnet3/vmxnet3_int.h b/drivers/net/vmxnet3/vmxnet3_int.h
index 34f392f..7985ba4 100644
--- a/drivers/net/vmxnet3/vmxnet3_int.h
+++ b/drivers/net/vmxnet3/vmxnet3_int.h
@@ -68,10 +68,10 @@
 /*
  * Version numbers
  */
-#define VMXNET3_DRIVER_VERSION_STRING   "1.0.5.0-k"
+#define VMXNET3_DRIVER_VERSION_STRING   "1.0.6.0-k"
 
 /* a 32-bit int, each byte encode a verion number in VMXNET3_DRIVER_VERSION */
-#define VMXNET3_DRIVER_VERSION_NUM      0x01000500
+#define VMXNET3_DRIVER_VERSION_NUM      0x01000600
 
 
 /*
@@ -149,6 +149,13 @@ vmxnet3_cmd_ring_desc_avail(struct vmxnet3_cmd_ring *ring)
 		ring->next2comp - ring->next2fill - 1;
 }
 
+static inline bool
+vmxnet3_cmd_ring_desc_empty(struct vmxnet3_cmd_ring *ring)
+{
+	return (ring->next2comp == ring->next2fill);
+}
+
+
 struct vmxnet3_comp_ring {
 	union Vmxnet3_GenericDesc *base;
 	u32               size;
@@ -266,9 +273,10 @@ struct vmxnet3_rx_queue {
 	u32 qid2;           /* rqID in RCD for buffer from 2nd ring */
 	u32 uncommitted[2]; /* # of buffers allocated since last RXPROD
 				* update */
-	struct vmxnet3_rx_buf_info     *buf_info[2];
-	struct Vmxnet3_RxQueueCtrl            *shared;
+	struct vmxnet3_rx_buf_info	*buf_info[2];
+	struct Vmxnet3_RxQueueCtrl	*shared;
 	struct vmxnet3_rq_driver_stats  stats;
+	struct sk_buff			*spare_skb;      /* starvation skb */
 } __attribute__((__aligned__(SMP_CACHE_BYTES)));
 
 #define VMXNET3_LINUX_MAX_MSIX_VECT     1

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

* [PATCH 2.6.35-rc1] net: vmxnet3 fixes [2/5] Interrupt control bitmap
  2010-07-07  9:00 [PATCH 2.6.35-rc1] net: vmxnet3 fixes [0/5] Spare skb to avoid starvation Shreyas Bhatewara
@ 2010-07-07  9:24 ` Shreyas Bhatewara
  2010-07-14  0:48   ` [PATCH 2.6.35-rc1] net-next: " Shreyas Bhatewara
  2010-07-07  9:28 ` [PATCH 2.6.35-rc1] net: vmxnet3 fixes [3/5] Initialize link state at probe time Shreyas Bhatewara
  2010-07-07 21:40 ` [PATCH 2.6.35-rc1] net: vmxnet3 fixes [0/5] Spare skb to avoid starvation David Miller
  2 siblings, 1 reply; 32+ messages in thread
From: Shreyas Bhatewara @ 2010-07-07  9:24 UTC (permalink / raw)
  To: netdev; +Cc: linux-kernel, pv-drivers, ronghua


A new bit map 'intrCtrl' is introduced in the DriverShared area. The driver 
should update VMXNET3_IC_DISABLE_ALL bit before writing IMR.

Signed-off-by: Ronghua Zang <ronghua@vmware.com>
Signed-off-by: Shreyas Bhatewara <sbhatewara@vmware.com>

---

diff --git a/drivers/net/vmxnet3/vmxnet3_defs.h b/drivers/net/vmxnet3/vmxnet3_defs.h
index b4889e6..ca7727b 100644
--- a/drivers/net/vmxnet3/vmxnet3_defs.h
+++ b/drivers/net/vmxnet3/vmxnet3_defs.h
@@ -464,6 +464,9 @@ enum vmxnet3_intr_type {
 /* addition 1 for events */
 #define VMXNET3_MAX_INTRS      25
 
+/* value of intrCtrl */
+#define VMXNET3_IC_DISABLE_ALL  0x1   /* bit 0 */
+
 
 struct Vmxnet3_IntrConf {
 	bool		autoMask;
@@ -471,7 +474,8 @@ struct Vmxnet3_IntrConf {
 	u8		eventIntrIdx;
 	u8		modLevels[VMXNET3_MAX_INTRS];	/* moderation level for
 							 * each intr */
-	__le32		reserved[3];
+	__le32		intrCtrl;
+	__le32		reserved[2];
 };
 
 /* one bit per VLAN ID, the size is in the units of u32	*/
diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c
index 1b2d467..29db294 100644
--- a/drivers/net/vmxnet3/vmxnet3_drv.c
+++ b/drivers/net/vmxnet3/vmxnet3_drv.c
@@ -72,6 +72,7 @@ vmxnet3_enable_all_intrs(struct vmxnet3_adapter *adapter)
 
 	for (i = 0; i < adapter->intr.num_intrs; i++)
 		vmxnet3_enable_intr(adapter, i);
+	adapter->shared->devRead.intrConf.intrCtrl &= ~VMXNET3_IC_DISABLE_ALL;
 }
 
 
@@ -80,6 +81,7 @@ vmxnet3_disable_all_intrs(struct vmxnet3_adapter *adapter)
 {
 	int i;
 
+	adapter->shared->devRead.intrConf.intrCtrl |= VMXNET3_IC_DISABLE_ALL;
 	for (i = 0; i < adapter->intr.num_intrs; i++)
 		vmxnet3_disable_intr(adapter, i);
 }
@@ -1880,6 +1882,7 @@ vmxnet3_setup_driver_shared(struct vmxnet3_adapter *adapter)
 		devRead->intrConf.modLevels[i] = adapter->intr.mod_levels[i];
 
 	devRead->intrConf.eventIntrIdx = adapter->intr.event_intr_idx;
+	devRead->intrConf.intrCtrl |= VMXNET3_IC_DISABLE_ALL;
 
 	/* rx filter settings */
 	devRead->rxFilterConf.rxMode = 0;
diff --git a/drivers/net/vmxnet3/vmxnet3_int.h b/drivers/net/vmxnet3/vmxnet3_int.h
index 7985ba4..0ddfe3c 100644
--- a/drivers/net/vmxnet3/vmxnet3_int.h
+++ b/drivers/net/vmxnet3/vmxnet3_int.h
@@ -68,10 +68,10 @@
 /*
  * Version numbers
  */
-#define VMXNET3_DRIVER_VERSION_STRING   "1.0.6.0-k"
+#define VMXNET3_DRIVER_VERSION_STRING   "1.0.9.0-k"
 
 /* a 32-bit int, each byte encode a verion number in VMXNET3_DRIVER_VERSION */
-#define VMXNET3_DRIVER_VERSION_NUM      0x01000600
+#define VMXNET3_DRIVER_VERSION_NUM      0x01000900
 
 
 /*

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

* [PATCH 2.6.35-rc1] net: vmxnet3 fixes [3/5] Initialize link state at probe time
  2010-07-07  9:00 [PATCH 2.6.35-rc1] net: vmxnet3 fixes [0/5] Spare skb to avoid starvation Shreyas Bhatewara
  2010-07-07  9:24 ` [PATCH 2.6.35-rc1] net: vmxnet3 fixes [2/5] Interrupt control bitmap Shreyas Bhatewara
@ 2010-07-07  9:28 ` Shreyas Bhatewara
  2010-07-07  9:31   ` [PATCH 2.6.35-rc1] net: vmxnet3 fixes [4/5] Do not reset when the device is not opened Shreyas Bhatewara
                     ` (2 more replies)
  2010-07-07 21:40 ` [PATCH 2.6.35-rc1] net: vmxnet3 fixes [0/5] Spare skb to avoid starvation David Miller
  2 siblings, 3 replies; 32+ messages in thread
From: Shreyas Bhatewara @ 2010-07-07  9:28 UTC (permalink / raw)
  To: netdev; +Cc: linux-kernel, pv-drivers


Author: Shreyas Bhatewara <sbhatewara@vmware.com>

Initialize vmxnet3 link state at probe time
    
This change initializes the state of link at the time when driver is
loaded. The ethtool output for 'link detected' and 'link speed'
is thus valid even before the interface is brought up.
    
Signed-off-by: Shreyas Bhatewara <sbhatewara@vmware.com>

---

diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c
index 29db294..31a838f 100644
--- a/drivers/net/vmxnet3/vmxnet3_drv.c
+++ b/drivers/net/vmxnet3/vmxnet3_drv.c
@@ -130,7 +130,7 @@ vmxnet3_tq_stop(struct vmxnet3_tx_queue *tq, struct vmxnet3_adapter *adapter)
  * Check the link state. This may start or stop the tx queue.
  */
 static void
-vmxnet3_check_link(struct vmxnet3_adapter *adapter)
+vmxnet3_check_link(struct vmxnet3_adapter *adapter, bool affectTxQueue)
 {
 	u32 ret;
 
@@ -143,14 +143,16 @@ vmxnet3_check_link(struct vmxnet3_adapter *adapter)
 		if (!netif_carrier_ok(adapter->netdev))
 			netif_carrier_on(adapter->netdev);
 
-		vmxnet3_tq_start(&adapter->tx_queue, adapter);
+		if (affectTxQueue)
+			vmxnet3_tq_start(&adapter->tx_queue, adapter);
 	} else {
 		printk(KERN_INFO "%s: NIC Link is Down\n",
 		       adapter->netdev->name);
 		if (netif_carrier_ok(adapter->netdev))
 			netif_carrier_off(adapter->netdev);
 
-		vmxnet3_tq_stop(&adapter->tx_queue, adapter);
+		if (affectTxQueue)
+			vmxnet3_tq_stop(&adapter->tx_queue, adapter);
 	}
 }
 
@@ -165,7 +167,7 @@ vmxnet3_process_events(struct vmxnet3_adapter *adapter)
 
 	/* Check if link state has changed */
 	if (events & VMXNET3_ECR_LINK)
-		vmxnet3_check_link(adapter);
+		vmxnet3_check_link(adapter, true);
 
 	/* Check if there is an error on xmit/recv queues */
 	if (events & (VMXNET3_ECR_TQERR | VMXNET3_ECR_RQERR)) {
@@ -1947,7 +1949,7 @@ vmxnet3_activate_dev(struct vmxnet3_adapter *adapter)
 	 * Check link state when first activating device. It will start the
 	 * tx queue if the link is up.
 	 */
-	vmxnet3_check_link(adapter);
+	vmxnet3_check_link(adapter, true);
 
 	napi_enable(&adapter->napi);
 	vmxnet3_enable_all_intrs(adapter);
@@ -2549,6 +2551,7 @@ vmxnet3_probe_device(struct pci_dev *pdev,
 	}
 
 	set_bit(VMXNET3_STATE_BIT_QUIESCED, &adapter->state);
+	vmxnet3_check_link(adapter, false);
 	atomic_inc(&devices_found);
 	return 0;
 
diff --git a/drivers/net/vmxnet3/vmxnet3_int.h b/drivers/net/vmxnet3/vmxnet3_int.h
index 0ddfe3c..5c94afa 100644
--- a/drivers/net/vmxnet3/vmxnet3_int.h
+++ b/drivers/net/vmxnet3/vmxnet3_int.h
@@ -68,10 +68,10 @@
 /*
  * Version numbers
  */
-#define VMXNET3_DRIVER_VERSION_STRING   "1.0.9.0-k"
+#define VMXNET3_DRIVER_VERSION_STRING   "1.0.10.0-k"
 
 /* a 32-bit int, each byte encode a verion number in VMXNET3_DRIVER_VERSION */
-#define VMXNET3_DRIVER_VERSION_NUM      0x01000900
+#define VMXNET3_DRIVER_VERSION_NUM      0x01000A00
 
 
 /*

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

* [PATCH 2.6.35-rc1] net: vmxnet3 fixes [4/5] Do not reset when the device is not opened
  2010-07-07  9:28 ` [PATCH 2.6.35-rc1] net: vmxnet3 fixes [3/5] Initialize link state at probe time Shreyas Bhatewara
@ 2010-07-07  9:31   ` Shreyas Bhatewara
  2010-07-14  0:49     ` [PATCH 2.6.35-rc1] net-next: " Shreyas Bhatewara
  2010-07-07  9:34   ` [PATCH 2.6.35-rc1] net: vmxnet3 fixes [5/5] Respect the interrupt type in VM configuration Shreyas Bhatewara
  2010-07-14  0:48   ` [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [3/5] Initialize link state at probe time Shreyas Bhatewara
  2 siblings, 1 reply; 32+ messages in thread
From: Shreyas Bhatewara @ 2010-07-07  9:31 UTC (permalink / raw)
  To: netdev; +Cc: linux-kernel, pv-drivers, ronghua, matthieu



commit bc8b3f0b3978d3c0a201926b2e2bd5c732e0352e
Author: Shreyas Bhatewara <sbhatewara@vmware.com>
Date:   Tue Jul 6 17:00:48 2010 -0700

No reset when the device is not opened
    
If a reset is scheduled, and the device goes thru close and open, it
may happen that reset and open may run in parallel.  
The reset code now bails out if the device is not opened.
 
Signed-off-by: Ronghua Zang <ronghua@vmware.com>
Signed-off-by: Matthieu Bucchianeri <matthieu@vmware.com>
Signed-off-by: Shreyas Bhatewara <sbhatewara@vmware.com>

---

diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c
index 31a838f..01a5bb7 100644
--- a/drivers/net/vmxnet3/vmxnet3_drv.c
+++ b/drivers/net/vmxnet3/vmxnet3_drv.c
@@ -2417,8 +2417,9 @@ vmxnet3_reset_work(struct work_struct *data)
 	if (test_and_set_bit(VMXNET3_STATE_BIT_RESETTING, &adapter->state))
 		return;
 
-	/* if the device is closed, we must leave it alone */
-	if (netif_running(adapter->netdev)) {
+	/* if the device is closed or is being opened, we must leave it alone */
+	if (netif_running(adapter->netdev) &&
+	    (adapter->netdev->flags & IFF_UP)) {
 		printk(KERN_INFO "%s: resetting\n", adapter->netdev->name);
 		vmxnet3_quiesce_dev(adapter);
 		vmxnet3_reset_dev(adapter);
diff --git a/drivers/net/vmxnet3/vmxnet3_int.h b/drivers/net/vmxnet3/vmxnet3_int.h
index 5c94afa..8e1f704 100644
--- a/drivers/net/vmxnet3/vmxnet3_int.h
+++ b/drivers/net/vmxnet3/vmxnet3_int.h
@@ -68,10 +68,10 @@
 /*
  * Version numbers
  */
-#define VMXNET3_DRIVER_VERSION_STRING   "1.0.10.0-k"
+#define VMXNET3_DRIVER_VERSION_STRING   "1.0.11.0-k"
 
 /* a 32-bit int, each byte encode a verion number in VMXNET3_DRIVER_VERSION */
-#define VMXNET3_DRIVER_VERSION_NUM      0x01000A00
+#define VMXNET3_DRIVER_VERSION_NUM      0x01000B00
 
 
 /*

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

* [PATCH 2.6.35-rc1] net: vmxnet3 fixes [5/5] Respect the interrupt type in VM configuration
  2010-07-07  9:28 ` [PATCH 2.6.35-rc1] net: vmxnet3 fixes [3/5] Initialize link state at probe time Shreyas Bhatewara
  2010-07-07  9:31   ` [PATCH 2.6.35-rc1] net: vmxnet3 fixes [4/5] Do not reset when the device is not opened Shreyas Bhatewara
@ 2010-07-07  9:34   ` Shreyas Bhatewara
  2010-07-14  0:51     ` [PATCH 2.6.35-rc1] net-next: " Shreyas Bhatewara
  2010-07-14  0:48   ` [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [3/5] Initialize link state at probe time Shreyas Bhatewara
  2 siblings, 1 reply; 32+ messages in thread
From: Shreyas Bhatewara @ 2010-07-07  9:34 UTC (permalink / raw)
  To: netdev; +Cc: linux-kernel, pv-drivers


Respect the interrupt type set in VM configuration.
   
When interrupt type is not auto, do not ignore the interrupt type set from
VM configuration.

Signed-off-by: Shreyas Bhatewara <sbhatewara@vmware.com>

---

diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c
index 01a5bb7..c6d756f 100644
--- a/drivers/net/vmxnet3/vmxnet3_drv.c
+++ b/drivers/net/vmxnet3/vmxnet3_drv.c
@@ -2355,9 +2355,13 @@ vmxnet3_alloc_intr_resources(struct vmxnet3_adapter *adapter)
 	adapter->intr.mask_mode = (cfg >> 2) & 0x3;
 
 	if (adapter->intr.type == VMXNET3_IT_AUTO) {
-		int err;
+		adapter->intr.type = VMXNET3_IT_MSIX;
+	}
 
 #ifdef CONFIG_PCI_MSI
+	if (adapter->intr.type == VMXNET3_IT_MSIX) {
+		int err;
+
 		adapter->intr.msix_entries[0].entry = 0;
 		err = pci_enable_msix(adapter->pdev, adapter->intr.msix_entries,
 				      VMXNET3_LINUX_MAX_MSIX_VECT);
@@ -2366,15 +2370,18 @@ vmxnet3_alloc_intr_resources(struct vmxnet3_adapter *adapter)
 			adapter->intr.type = VMXNET3_IT_MSIX;
 			return;
 		}
-#endif
+		adapter->intr.type = VMXNET3_IT_MSI;
+	}
 
+	if (adapter->intr.type == VMXNET3_IT_MSI) {
+		int err;
 		err = pci_enable_msi(adapter->pdev);
 		if (!err) {
 			adapter->intr.num_intrs = 1;
-			adapter->intr.type = VMXNET3_IT_MSI;
 			return;
 		}
 	}
+#endif /* CONFIG_PCI_MSI */
 
 	adapter->intr.type = VMXNET3_IT_INTX;
 
diff --git a/drivers/net/vmxnet3/vmxnet3_int.h b/drivers/net/vmxnet3/vmxnet3_int.h
index 8e1f704..6bab6ff 100644
--- a/drivers/net/vmxnet3/vmxnet3_int.h
+++ b/drivers/net/vmxnet3/vmxnet3_int.h
@@ -68,10 +68,10 @@
 /*
  * Version numbers
  */
-#define VMXNET3_DRIVER_VERSION_STRING   "1.0.11.0-k"
+#define VMXNET3_DRIVER_VERSION_STRING   "1.0.12.0-k"
 
 /* a 32-bit int, each byte encode a verion number in VMXNET3_DRIVER_VERSION */
-#define VMXNET3_DRIVER_VERSION_NUM      0x01000B00
+#define VMXNET3_DRIVER_VERSION_NUM      0x01000C00
 
 
 /*

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

* Re: [PATCH 2.6.35-rc1] net: vmxnet3 fixes [0/5] Spare skb to avoid starvation
  2010-07-07  9:00 [PATCH 2.6.35-rc1] net: vmxnet3 fixes [0/5] Spare skb to avoid starvation Shreyas Bhatewara
  2010-07-07  9:24 ` [PATCH 2.6.35-rc1] net: vmxnet3 fixes [2/5] Interrupt control bitmap Shreyas Bhatewara
  2010-07-07  9:28 ` [PATCH 2.6.35-rc1] net: vmxnet3 fixes [3/5] Initialize link state at probe time Shreyas Bhatewara
@ 2010-07-07 21:40 ` David Miller
  2010-07-07 22:19   ` Shreyas Bhatewara
  2 siblings, 1 reply; 32+ messages in thread
From: David Miller @ 2010-07-07 21:40 UTC (permalink / raw)
  To: sbhatewara; +Cc: netdev-devel, linux-kernel, pv-drivers, stolarchuk


A couple problems remaining in this patch set:

1) Too many bug fixes for this late in the -rc series of 2.6.35
   Pick a smaller number of fixes, the most critical ones, perhaps
   2 or 3, and re-submit those for net-2.6

   Pick the ones that produce easily triggers crashes or documented
   regressions present on the official lkml regression list.

   Send the rest in for net-next-2.6

2) Bumping the driver version every patch in a patch series is pointless.
   Do the bump once, at the end of the series.

Thanks.

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

* RE: [PATCH 2.6.35-rc1] net: vmxnet3 fixes [0/5] Spare skb to avoid starvation
  2010-07-07 21:40 ` [PATCH 2.6.35-rc1] net: vmxnet3 fixes [0/5] Spare skb to avoid starvation David Miller
@ 2010-07-07 22:19   ` Shreyas Bhatewara
  2010-07-07 22:25     ` David Miller
  0 siblings, 1 reply; 32+ messages in thread
From: Shreyas Bhatewara @ 2010-07-07 22:19 UTC (permalink / raw)
  To: David Miller; +Cc: netdev-devel, linux-kernel, pv-drivers, Michael Stolarchuk



> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Wednesday, July 07, 2010 2:41 PM
> To: Shreyas Bhatewara
> Cc: netdev-devel@vger.kernel.org; linux-kernel@vger.kernel.org; pv-
> drivers@vmware.com; Michael Stolarchuk
> Subject: Re: [PATCH 2.6.35-rc1] net: vmxnet3 fixes [0/5] Spare skb to
> avoid starvation
> 
> 
> A couple problems remaining in this patch set:
> 
> 1) Too many bug fixes for this late in the -rc series of 2.6.35
>    Pick a smaller number of fixes, the most critical ones, perhaps
>    2 or 3, and re-submit those for net-2.6
> 
>    Pick the ones that produce easily triggers crashes or documented
>    regressions present on the official lkml regression list.
> 
>    Send the rest in for net-next-2.6

Could you please apply all the patches in the series to net-next-2.6, the patches will apply cleanly. Or would you rather have them reposted them with proper subject line ?

> 
> 2) Bumping the driver version every patch in a patch series is
> pointless.
>    Do the bump once, at the end of the series.


The version numbers bumps although seemingly pointless (and harmless), make my life easy to track and maintain the bug#-to-fix mapping.


Thanks.
->Shreyas


> 
> Thanks.

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

* Re: [PATCH 2.6.35-rc1] net: vmxnet3 fixes [0/5] Spare skb to avoid starvation
  2010-07-07 22:19   ` Shreyas Bhatewara
@ 2010-07-07 22:25     ` David Miller
  0 siblings, 0 replies; 32+ messages in thread
From: David Miller @ 2010-07-07 22:25 UTC (permalink / raw)
  To: sbhatewara; +Cc: netdev-devel, linux-kernel, pv-drivers, stolarchuk

From: Shreyas Bhatewara <sbhatewara@vmware.com>
Date: Wed, 7 Jul 2010 15:19:08 -0700

> The version numbers bumps although seemingly pointless (and
> harmless), make my life easy to track and maintain the bug#-to-fix
> mapping.

But in an upstream context, they make no sense at all.

Anyone running upstream, is going to run at the last version bump you
make in the driver, so what users report to you as the driver version
will be correct.

Please fix this.

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

* [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [2/5] Interrupt control bitmap
  2010-07-07  9:24 ` [PATCH 2.6.35-rc1] net: vmxnet3 fixes [2/5] Interrupt control bitmap Shreyas Bhatewara
@ 2010-07-14  0:48   ` Shreyas Bhatewara
  2010-07-14 21:04     ` David Miller
  0 siblings, 1 reply; 32+ messages in thread
From: Shreyas Bhatewara @ 2010-07-14  0:48 UTC (permalink / raw)
  To: netdev; +Cc: linux-kernel, pv-drivers, ronghua


A new bit map 'intrCtrl' is introduced in the DriverShared area. The driver
should update VMXNET3_IC_DISABLE_ALL bit before writing IMR.

Signed-off-by: Ronghua Zang <ronghua@vmware.com>
Signed-off-by: Shreyas Bhatewara <sbhatewara@vmware.com>

---

 drivers/net/vmxnet3/vmxnet3_defs.h |    6 +++++-
 drivers/net/vmxnet3/vmxnet3_drv.c  |    3 +++
 2 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/drivers/net/vmxnet3/vmxnet3_defs.h b/drivers/net/vmxnet3/vmxnet3_defs.h
index b4889e6..ca7727b 100644
--- a/drivers/net/vmxnet3/vmxnet3_defs.h
+++ b/drivers/net/vmxnet3/vmxnet3_defs.h
@@ -464,6 +464,9 @@ enum vmxnet3_intr_type {
 /* addition 1 for events */
 #define VMXNET3_MAX_INTRS      25
 
+/* value of intrCtrl */
+#define VMXNET3_IC_DISABLE_ALL  0x1   /* bit 0 */
+
 
 struct Vmxnet3_IntrConf {
 	bool		autoMask;
@@ -471,7 +474,8 @@ struct Vmxnet3_IntrConf {
 	u8		eventIntrIdx;
 	u8		modLevels[VMXNET3_MAX_INTRS];	/* moderation level for
 							 * each intr */
-	__le32		reserved[3];
+	__le32		intrCtrl;
+	__le32		reserved[2];
 };
 
 /* one bit per VLAN ID, the size is in the units of u32	*/
diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c
index 5a50d10..7792a44 100644
--- a/drivers/net/vmxnet3/vmxnet3_drv.c
+++ b/drivers/net/vmxnet3/vmxnet3_drv.c
@@ -72,6 +72,7 @@ vmxnet3_enable_all_intrs(struct vmxnet3_adapter *adapter)
 
 	for (i = 0; i < adapter->intr.num_intrs; i++)
 		vmxnet3_enable_intr(adapter, i);
+	adapter->shared->devRead.intrConf.intrCtrl &= ~VMXNET3_IC_DISABLE_ALL;
 }
 
 
@@ -80,6 +81,7 @@ vmxnet3_disable_all_intrs(struct vmxnet3_adapter *adapter)
 {
 	int i;
 
+	adapter->shared->devRead.intrConf.intrCtrl |= VMXNET3_IC_DISABLE_ALL;
 	for (i = 0; i < adapter->intr.num_intrs; i++)
 		vmxnet3_disable_intr(adapter, i);
 }
@@ -1880,6 +1882,7 @@ vmxnet3_setup_driver_shared(struct vmxnet3_adapter *adapter)
 		devRead->intrConf.modLevels[i] = adapter->intr.mod_levels[i];
 
 	devRead->intrConf.eventIntrIdx = adapter->intr.event_intr_idx;
+	devRead->intrConf.intrCtrl |= VMXNET3_IC_DISABLE_ALL;
 
 	/* rx filter settings */
 	devRead->rxFilterConf.rxMode = 0;


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

* [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [3/5] Initialize link state at probe time
  2010-07-07  9:28 ` [PATCH 2.6.35-rc1] net: vmxnet3 fixes [3/5] Initialize link state at probe time Shreyas Bhatewara
  2010-07-07  9:31   ` [PATCH 2.6.35-rc1] net: vmxnet3 fixes [4/5] Do not reset when the device is not opened Shreyas Bhatewara
  2010-07-07  9:34   ` [PATCH 2.6.35-rc1] net: vmxnet3 fixes [5/5] Respect the interrupt type in VM configuration Shreyas Bhatewara
@ 2010-07-14  0:48   ` Shreyas Bhatewara
  2010-07-14 21:10     ` David Miller
  2 siblings, 1 reply; 32+ messages in thread
From: Shreyas Bhatewara @ 2010-07-14  0:48 UTC (permalink / raw)
  To: netdev; +Cc: linux-kernel, pv-drivers


Initialize vmxnet3 link state at probe time

This change initializes the state of link at the time when driver is
loaded. The ethtool output for 'link detected' and 'link speed'
is thus valid even before the interface is brought up.

Signed-off-by: Shreyas Bhatewara <sbhatewara@vmware.com>

---
 drivers/net/vmxnet3/vmxnet3_drv.c |   13 ++++++++-----
 1 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c
index 7792a44..1e31d40 100644
--- a/drivers/net/vmxnet3/vmxnet3_drv.c
+++ b/drivers/net/vmxnet3/vmxnet3_drv.c
@@ -130,7 +130,7 @@ vmxnet3_tq_stop(struct vmxnet3_tx_queue *tq, struct vmxnet3_adapter *adapter)
  * Check the link state. This may start or stop the tx queue.
  */
 static void
-vmxnet3_check_link(struct vmxnet3_adapter *adapter)
+vmxnet3_check_link(struct vmxnet3_adapter *adapter, bool affectTxQueue)
 {
 	u32 ret;
 
@@ -143,14 +143,16 @@ vmxnet3_check_link(struct vmxnet3_adapter *adapter)
 		if (!netif_carrier_ok(adapter->netdev))
 			netif_carrier_on(adapter->netdev);
 
-		vmxnet3_tq_start(&adapter->tx_queue, adapter);
+		if (affectTxQueue)
+			vmxnet3_tq_start(&adapter->tx_queue, adapter);
 	} else {
 		printk(KERN_INFO "%s: NIC Link is Down\n",
 		       adapter->netdev->name);
 		if (netif_carrier_ok(adapter->netdev))
 			netif_carrier_off(adapter->netdev);
 
-		vmxnet3_tq_stop(&adapter->tx_queue, adapter);
+		if (affectTxQueue)
+			vmxnet3_tq_stop(&adapter->tx_queue, adapter);
 	}
 }
 
@@ -165,7 +167,7 @@ vmxnet3_process_events(struct vmxnet3_adapter *adapter)
 
 	/* Check if link state has changed */
 	if (events & VMXNET3_ECR_LINK)
-		vmxnet3_check_link(adapter);
+		vmxnet3_check_link(adapter, true);
 
 	/* Check if there is an error on xmit/recv queues */
 	if (events & (VMXNET3_ECR_TQERR | VMXNET3_ECR_RQERR)) {
@@ -1947,7 +1949,7 @@ vmxnet3_activate_dev(struct vmxnet3_adapter *adapter)
 	 * Check link state when first activating device. It will start the
 	 * tx queue if the link is up.
 	 */
-	vmxnet3_check_link(adapter);
+	vmxnet3_check_link(adapter, true);
 
 	napi_enable(&adapter->napi);
 	vmxnet3_enable_all_intrs(adapter);
@@ -2549,6 +2551,7 @@ vmxnet3_probe_device(struct pci_dev *pdev,
 	}
 
 	set_bit(VMXNET3_STATE_BIT_QUIESCED, &adapter->state);
+	vmxnet3_check_link(adapter, false);
 	atomic_inc(&devices_found);
 	return 0;


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

* [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [4/5] Do not reset when the device is not opened
  2010-07-07  9:31   ` [PATCH 2.6.35-rc1] net: vmxnet3 fixes [4/5] Do not reset when the device is not opened Shreyas Bhatewara
@ 2010-07-14  0:49     ` Shreyas Bhatewara
  2010-07-14 21:07       ` David Miller
  0 siblings, 1 reply; 32+ messages in thread
From: Shreyas Bhatewara @ 2010-07-14  0:49 UTC (permalink / raw)
  To: netdev; +Cc: linux-kernel, pv-drivers, ronghua, matthieu


Do not reset when the device is not opened

If a reset is scheduled, and the device goes thru close and open, it
may happen that reset and open may run in parallel.
The reset code now bails out if the device is not opened.
 
Signed-off-by: Ronghua Zang <ronghua@vmware.com>
Signed-off-by: Matthieu Bucchianeri <matthieu@vmware.com>
Signed-off-by: Shreyas Bhatewara <sbhatewara@vmware.com>

---

 drivers/net/vmxnet3/vmxnet3_drv.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c
index 1e31d40..ffd6a9b 100644
--- a/drivers/net/vmxnet3/vmxnet3_drv.c
+++ b/drivers/net/vmxnet3/vmxnet3_drv.c
@@ -2417,8 +2417,9 @@ vmxnet3_reset_work(struct work_struct *data)
 	if (test_and_set_bit(VMXNET3_STATE_BIT_RESETTING, &adapter->state))
 		return;
 
-	/* if the device is closed, we must leave it alone */
-	if (netif_running(adapter->netdev)) {
+	/* if the device is closed or is being opened, we must leave it alone */
+	if (netif_running(adapter->netdev) &&
+	    (adapter->netdev->flags & IFF_UP)) {
 		printk(KERN_INFO "%s: resetting\n", adapter->netdev->name);
 		vmxnet3_quiesce_dev(adapter);
 		vmxnet3_reset_dev(adapter);

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

* [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [5/5] Respect the interrupt type in VM configuration
  2010-07-07  9:34   ` [PATCH 2.6.35-rc1] net: vmxnet3 fixes [5/5] Respect the interrupt type in VM configuration Shreyas Bhatewara
@ 2010-07-14  0:51     ` Shreyas Bhatewara
  2010-07-14 21:11       ` David Miller
  0 siblings, 1 reply; 32+ messages in thread
From: Shreyas Bhatewara @ 2010-07-14  0:51 UTC (permalink / raw)
  To: netdev; +Cc: linux-kernel, pv-drivers


Do not ignore interrupt type in VM configuration

When interrupt type is not auto, do not ignore the interrupt type set from
VM configuration.
Driver may not always respect the interrupt type in configuration but it 
will certainly try to. Eg: if MSIx is configured and enabling MSIx fails 
in driver, it fall back on MSI and then on INTx.

Signed-off-by: Shreyas Bhatewara <sbhatewara@vmware.com>

---

 drivers/net/vmxnet3/vmxnet3_drv.c     |   13 ++++++++++---
 drivers/net/vmxnet3/vmxnet3_ethtool.c |    2 +-
 drivers/net/vmxnet3/vmxnet3_int.h     |    6 +++---
 3 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c
index ffd6a9b..74fe3f0 100644
--- a/drivers/net/vmxnet3/vmxnet3_drv.c
+++ b/drivers/net/vmxnet3/vmxnet3_drv.c
@@ -2355,9 +2355,13 @@ vmxnet3_alloc_intr_resources(struct vmxnet3_adapter *adapter)
 	adapter->intr.mask_mode = (cfg >> 2) & 0x3;
 
 	if (adapter->intr.type == VMXNET3_IT_AUTO) {
-		int err;
+		adapter->intr.type = VMXNET3_IT_MSIX;
+	}
 
 #ifdef CONFIG_PCI_MSI
+	if (adapter->intr.type == VMXNET3_IT_MSIX) {
+		int err;
+
 		adapter->intr.msix_entries[0].entry = 0;
 		err = pci_enable_msix(adapter->pdev, adapter->intr.msix_entries,
 				      VMXNET3_LINUX_MAX_MSIX_VECT);
@@ -2366,15 +2370,18 @@ vmxnet3_alloc_intr_resources(struct vmxnet3_adapter *adapter)
 			adapter->intr.type = VMXNET3_IT_MSIX;
 			return;
 		}
-#endif
+		adapter->intr.type = VMXNET3_IT_MSI;
+	}
 
+	if (adapter->intr.type == VMXNET3_IT_MSI) {
+		int err;
 		err = pci_enable_msi(adapter->pdev);
 		if (!err) {
 			adapter->intr.num_intrs = 1;
-			adapter->intr.type = VMXNET3_IT_MSI;
 			return;
 		}
 	}
+#endif /* CONFIG_PCI_MSI */
 
 	adapter->intr.type = VMXNET3_IT_INTX;
 
diff --git a/drivers/net/vmxnet3/vmxnet3_ethtool.c b/drivers/net/vmxnet3/vmxnet3_ethtool.c
index de1ba14..52128c7 100644
--- a/drivers/net/vmxnet3/vmxnet3_ethtool.c
+++ b/drivers/net/vmxnet3/vmxnet3_ethtool.c
@@ -291,7 +291,7 @@ vmxnet3_set_flags(struct net_device *netdev, u32 data)
 
 		/* update harware LRO capability accordingly */
 		if (lro_requested)
-			adapter->shared->devRead.misc.uptFeatures &= UPT1_F_LRO;
+			adapter->shared->devRead.misc.uptFeatures |= UPT1_F_LRO;
 		else
 			adapter->shared->devRead.misc.uptFeatures &=
 								~UPT1_F_LRO;
diff --git a/drivers/net/vmxnet3/vmxnet3_int.h b/drivers/net/vmxnet3/vmxnet3_int.h
index bbed330..d17fae6 100644
--- a/drivers/net/vmxnet3/vmxnet3_int.h
+++ b/drivers/net/vmxnet3/vmxnet3_int.h
@@ -68,10 +68,10 @@
 /*
  * Version numbers
  */
-#define VMXNET3_DRIVER_VERSION_STRING   "1.0.5.0-k"
+#define VMXNET3_DRIVER_VERSION_STRING   "1.0.13.0-k"
 
 /* a 32-bit int, each byte encode a verion number in VMXNET3_DRIVER_VERSION */
-#define VMXNET3_DRIVER_VERSION_NUM      0x01000500
+#define VMXNET3_DRIVER_VERSION_NUM      0x01000B00
 
 
 /*
@@ -150,7 +150,7 @@ vmxnet3_cmd_ring_desc_avail(struct vmxnet3_cmd_ring *ring)
 }
 
 static inline bool
-vmxnet3_cmd_ring_desc_empty(struct vmxnet3_cmd_ring *ring)
+vmxnet3_cmd_ring_desc_empty(const struct vmxnet3_cmd_ring *ring)
 {
 	return (ring->next2comp == ring->next2fill);
 }


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

* Re: [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [2/5] Interrupt control bitmap
  2010-07-14  0:48   ` [PATCH 2.6.35-rc1] net-next: " Shreyas Bhatewara
@ 2010-07-14 21:04     ` David Miller
  2010-07-16  1:20       ` Shreyas Bhatewara
  0 siblings, 1 reply; 32+ messages in thread
From: David Miller @ 2010-07-14 21:04 UTC (permalink / raw)
  To: sbhatewara; +Cc: netdev, linux-kernel, pv-drivers, ronghua

From: Shreyas Bhatewara <sbhatewara@vmware.com>
Date: Tue, 13 Jul 2010 17:48:04 -0700 (PDT)

> -	__le32		reserved[3];
> +	__le32		intrCtrl;
> +	__le32		reserved[2];
>  };
>  
 ...
> +	adapter->shared->devRead.intrConf.intrCtrl &= ~VMXNET3_IC_DISABLE_ALL;
 ...
> +	adapter->shared->devRead.intrConf.intrCtrl |= VMXNET3_IC_DISABLE_ALL;
 ...
> +	devRead->intrConf.intrCtrl |= VMXNET3_IC_DISABLE_ALL;

You need to use cpu_to_le32() and similar when accessing this value.

If you run "sparse" with endianness checking enabled you'll see
warnings showing alerting you to this issue...

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

* Re: [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [4/5] Do not reset when the device is not opened
  2010-07-14  0:49     ` [PATCH 2.6.35-rc1] net-next: " Shreyas Bhatewara
@ 2010-07-14 21:07       ` David Miller
  2010-07-16  1:20         ` Shreyas Bhatewara
  0 siblings, 1 reply; 32+ messages in thread
From: David Miller @ 2010-07-14 21:07 UTC (permalink / raw)
  To: sbhatewara; +Cc: netdev, linux-kernel, pv-drivers, ronghua, matthieu

From: Shreyas Bhatewara <sbhatewara@vmware.com>
Date: Tue, 13 Jul 2010 17:49:52 -0700 (PDT)

> 
> Do not reset when the device is not opened
> 
> If a reset is scheduled, and the device goes thru close and open, it
> may happen that reset and open may run in parallel.
> The reset code now bails out if the device is not opened.
>  
> Signed-off-by: Ronghua Zang <ronghua@vmware.com>
> Signed-off-by: Matthieu Bucchianeri <matthieu@vmware.com>
> Signed-off-by: Shreyas Bhatewara <sbhatewara@vmware.com>

Testing IFF_UP is just making your race hole a little smaller but
it isn't fixing the problem.

In fact, there really isn't any legitimate reason for a driver
to test the IFF_UP state bit.  netif_running() is the correct
test, and asynchronous work queue things like resets should
synchronize with open/close using the RTNL lock so that your
test of netif_running() is a stable one.

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

* Re: [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [3/5] Initialize link state at probe time
  2010-07-14  0:48   ` [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [3/5] Initialize link state at probe time Shreyas Bhatewara
@ 2010-07-14 21:10     ` David Miller
  2010-07-16  1:20       ` Shreyas Bhatewara
  0 siblings, 1 reply; 32+ messages in thread
From: David Miller @ 2010-07-14 21:10 UTC (permalink / raw)
  To: sbhatewara; +Cc: netdev, linux-kernel, pv-drivers

From: Shreyas Bhatewara <sbhatewara@vmware.com>
Date: Tue, 13 Jul 2010 17:48:55 -0700 (PDT)

> 
> Initialize vmxnet3 link state at probe time
> 
> This change initializes the state of link at the time when driver is
> loaded. The ethtool output for 'link detected' and 'link speed'
> is thus valid even before the interface is brought up.
> 
> Signed-off-by: Shreyas Bhatewara <sbhatewara@vmware.com>

You should never, ever, call netif_start_queue() on a device which has
not been brought up.

But that is what this patch is doing.

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

* Re: [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [5/5] Respect the interrupt type in VM configuration
  2010-07-14  0:51     ` [PATCH 2.6.35-rc1] net-next: " Shreyas Bhatewara
@ 2010-07-14 21:11       ` David Miller
  2010-07-16  1:21         ` Shreyas Bhatewara
  0 siblings, 1 reply; 32+ messages in thread
From: David Miller @ 2010-07-14 21:11 UTC (permalink / raw)
  To: sbhatewara; +Cc: netdev, linux-kernel, pv-drivers

From: Shreyas Bhatewara <sbhatewara@vmware.com>
Date: Tue, 13 Jul 2010 17:51:39 -0700 (PDT)

> 
> Do not ignore interrupt type in VM configuration
> 
> When interrupt type is not auto, do not ignore the interrupt type set from
> VM configuration.
> Driver may not always respect the interrupt type in configuration but it 
> will certainly try to. Eg: if MSIx is configured and enabling MSIx fails 
> in driver, it fall back on MSI and then on INTx.
> 
> Signed-off-by: Shreyas Bhatewara <sbhatewara@vmware.com>
 ...
> @@ -291,7 +291,7 @@ vmxnet3_set_flags(struct net_device *netdev, u32 data)
>  
>  		/* update harware LRO capability accordingly */
>  		if (lro_requested)
> -			adapter->shared->devRead.misc.uptFeatures &= UPT1_F_LRO;
> +			adapter->shared->devRead.misc.uptFeatures |= UPT1_F_LRO;
>  		else

This change has nothing to do with respecting the VM interrupt setting.

Do not stuff unrelated changes into a commit.

I have to be frank with you, this patch series... it stinks.

It papers over races with incorrect tests, unrelated changes are mixed
together, it fixes RX ring exhaustion incorrectly, it illegals starts a
transmit queue before the device is every brought up, etc.

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

* Re: [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [2/5] Interrupt control bitmap
  2010-07-14 21:04     ` David Miller
@ 2010-07-16  1:20       ` Shreyas Bhatewara
  2010-07-16  5:19         ` David Miller
  0 siblings, 1 reply; 32+ messages in thread
From: Shreyas Bhatewara @ 2010-07-16  1:20 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-kernel, pv-drivers, Ronghua Zhang



On Wed, 14 Jul 2010, David Miller wrote:

> From: Shreyas Bhatewara <sbhatewara@vmware.com>
> Date: Tue, 13 Jul 2010 17:48:04 -0700 (PDT)
> 
> > -	__le32		reserved[3];
> > +	__le32		intrCtrl;
> > +	__le32		reserved[2];
> >  };
> >  
>  ...
> > +	adapter->shared->devRead.intrConf.intrCtrl &= ~VMXNET3_IC_DISABLE_ALL;
>  ...
> > +	adapter->shared->devRead.intrConf.intrCtrl |= VMXNET3_IC_DISABLE_ALL;
>  ...
> > +	devRead->intrConf.intrCtrl |= VMXNET3_IC_DISABLE_ALL;
> 
> You need to use cpu_to_le32() and similar when accessing this value.
> 
> If you run "sparse" with endianness checking enabled you'll see
> warnings showing alerting you to this issue...
> 

Reposting the patch with the endianness fix.

---

A new bit map 'intrCtrl' is introduced in the DriverShared area. The 
driver Ashould update VMXNET3_IC_DISABLE_ALL bit before writing IMR.

Signed-off-by: Ronghua Zang <ronghua@vmware.com>
Signed-off-by: Shreyas Bhatewara <sbhatewara@vmware.com>

---

 drivers/net/vmxnet3/vmxnet3_defs.h |    6 +++++-
 drivers/net/vmxnet3/vmxnet3_drv.c  |    5 +++++
 2 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/drivers/net/vmxnet3/vmxnet3_defs.h b/drivers/net/vmxnet3/vmxnet3_defs.h
index b4889e6..ca7727b 100644
--- a/drivers/net/vmxnet3/vmxnet3_defs.h
+++ b/drivers/net/vmxnet3/vmxnet3_defs.h
@@ -464,6 +464,9 @@ enum vmxnet3_intr_type {
 /* addition 1 for events */
 #define VMXNET3_MAX_INTRS      25
 
+/* value of intrCtrl */
+#define VMXNET3_IC_DISABLE_ALL  0x1   /* bit 0 */
+
 
 struct Vmxnet3_IntrConf {
 	bool		autoMask;
@@ -471,7 +474,8 @@ struct Vmxnet3_IntrConf {
 	u8		eventIntrIdx;
 	u8		modLevels[VMXNET3_MAX_INTRS];	/* moderation level for
 							 * each intr */
-	__le32		reserved[3];
+	__le32		intrCtrl;
+	__le32		reserved[2];
 };
 
 /* one bit per VLAN ID, the size is in the units of u32	*/
diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c
index 5a50d10..df3d2ac 100644
--- a/drivers/net/vmxnet3/vmxnet3_drv.c
+++ b/drivers/net/vmxnet3/vmxnet3_drv.c
@@ -72,6 +72,8 @@ vmxnet3_enable_all_intrs(struct vmxnet3_adapter *adapter)
 
 	for (i = 0; i < adapter->intr.num_intrs; i++)
 		vmxnet3_enable_intr(adapter, i);
+	adapter->shared->devRead.intrConf.intrCtrl &=
+					cpu_to_le32(~VMXNET3_IC_DISABLE_ALL);
 }
 
 
@@ -80,6 +82,8 @@ vmxnet3_disable_all_intrs(struct vmxnet3_adapter *adapter)
 {
 	int i;
 
+	adapter->shared->devRead.intrConf.intrCtrl |=
+					cpu_to_le32(VMXNET3_IC_DISABLE_ALL);
 	for (i = 0; i < adapter->intr.num_intrs; i++)
 		vmxnet3_disable_intr(adapter, i);
 }
@@ -1880,6 +1884,7 @@ vmxnet3_setup_driver_shared(struct vmxnet3_adapter *adapter)
 		devRead->intrConf.modLevels[i] = adapter->intr.mod_levels[i];
 
 	devRead->intrConf.eventIntrIdx = adapter->intr.event_intr_idx;
+	devRead->intrConf.intrCtrl |= cpu_to_le32(VMXNET3_IC_DISABLE_ALL);
 
 	/* rx filter settings */
 	devRead->rxFilterConf.rxMode = 0;


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

* Re: [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [3/5] Initialize link state at probe time
  2010-07-14 21:10     ` David Miller
@ 2010-07-16  1:20       ` Shreyas Bhatewara
  2010-07-16  5:44         ` David Miller
  0 siblings, 1 reply; 32+ messages in thread
From: Shreyas Bhatewara @ 2010-07-16  1:20 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-kernel, pv-drivers


On Wed, 14 Jul 2010, David Miller wrote:

> From: Shreyas Bhatewara <sbhatewara@vmware.com>
> Date: Tue, 13 Jul 2010 17:48:55 -0700 (PDT)
> 
> > 
> > Initialize vmxnet3 link state at probe time
> > 
> > This change initializes the state of link at the time when driver is
> > loaded. The ethtool output for 'link detected' and 'link speed'
> > is thus valid even before the interface is brought up.
> > 
> > Signed-off-by: Shreyas Bhatewara <sbhatewara@vmware.com>
> 
> You should never, ever, call netif_start_queue() on a device which has
> not been brought up.
> 
> But that is what this patch is doing.
> 

I do not understand why you say so. vmxnet3_check_link() is called in 
probe() with affectTxQueue as false. Hence netif_start_queue() will not be 
called before device is brought up.
vmxnet3_check_link() is again called with affectTxQueue as true in 
vmxnet3_activate_dev() after device was activated.

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

* Re: [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [4/5] Do not reset when the device is not opened
  2010-07-14 21:07       ` David Miller
@ 2010-07-16  1:20         ` Shreyas Bhatewara
  2010-07-16  1:32           ` David Miller
  0 siblings, 1 reply; 32+ messages in thread
From: Shreyas Bhatewara @ 2010-07-16  1:20 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, linux-kernel, pv-drivers, Ronghua Zhang, Matthieu Bucchianeri



On Wed, 14 Jul 2010, David Miller wrote:

> From: Shreyas Bhatewara <sbhatewara@vmware.com>
> Date: Tue, 13 Jul 2010 17:49:52 -0700 (PDT)
> 
> > 
> > Do not reset when the device is not opened
> > 
> > If a reset is scheduled, and the device goes thru close and open, it
> > may happen that reset and open may run in parallel.
> > The reset code now bails out if the device is not opened.
> >  
> > Signed-off-by: Ronghua Zang <ronghua@vmware.com>
> > Signed-off-by: Matthieu Bucchianeri <matthieu@vmware.com>
> > Signed-off-by: Shreyas Bhatewara <sbhatewara@vmware.com>
> 
> Testing IFF_UP is just making your race hole a little smaller but
> it isn't fixing the problem.
> 
> In fact, there really isn't any legitimate reason for a driver
> to test the IFF_UP state bit.  netif_running() is the correct
> test, and asynchronous work queue things like resets should
> synchronize with open/close using the RTNL lock so that your
> test of netif_running() is a stable one.
> 

Is this what you suggest :

---

Hold rtnl_lock to get the right link state.

While asynchronously resetting the device, hold rtnl_lock to get the 
right value from netif_running. If a reset is scheduled, and the device
goes thru close and open, it may happen that reset and open may run in 
parallel. Holding rtnl_lock will avoid this.

---

 drivers/net/vmxnet3/vmxnet3_drv.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c
index 1b0ce8c..c4d7e42 100644
--- a/drivers/net/vmxnet3/vmxnet3_drv.c
+++ b/drivers/net/vmxnet3/vmxnet3_drv.c
@@ -2420,6 +2420,7 @@ vmxnet3_reset_work(struct work_struct *data)
 		return;
 
 	/* if the device is closed, we must leave it alone */
+	rtnl_lock();
 	if (netif_running(adapter->netdev)) {
 		printk(KERN_INFO "%s: resetting\n", adapter->netdev->name);
 		vmxnet3_quiesce_dev(adapter);
@@ -2428,6 +2429,7 @@ vmxnet3_reset_work(struct work_struct *data)
 	} else {
 		printk(KERN_INFO "%s: already closed\n", adapter->netdev->name);
 	}
+	rtnl_unlock();
 
 	clear_bit(VMXNET3_STATE_BIT_RESETTING, &adapter->state);
 }

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

* Re: [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [5/5] Respect the interrupt type in VM configuration
  2010-07-14 21:11       ` David Miller
@ 2010-07-16  1:21         ` Shreyas Bhatewara
  2010-07-16  1:28           ` [PATCH 2.6.35-rc1] net-next: fix LRO feature update in vmxnet3 Shreyas Bhatewara
  2010-07-19 20:16           ` [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [5/5] Respect the interrupt type in VM configuration David Miller
  0 siblings, 2 replies; 32+ messages in thread
From: Shreyas Bhatewara @ 2010-07-16  1:21 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-kernel, pv-drivers



On Wed, 14 Jul 2010, David Miller wrote:

> From: Shreyas Bhatewara <sbhatewara@vmware.com>
> Date: Tue, 13 Jul 2010 17:51:39 -0700 (PDT)
> 
> > 
> > Do not ignore interrupt type in VM configuration
> > 
> > When interrupt type is not auto, do not ignore the interrupt type set from
> > VM configuration.
> > Driver may not always respect the interrupt type in configuration but it 
> > will certainly try to. Eg: if MSIx is configured and enabling MSIx fails 
> > in driver, it fall back on MSI and then on INTx.
> > 
> > Signed-off-by: Shreyas Bhatewara <sbhatewara@vmware.com>
>  ...
> > @@ -291,7 +291,7 @@ vmxnet3_set_flags(struct net_device *netdev, u32 data)
> >  
> >  		/* update harware LRO capability accordingly */
> >  		if (lro_requested)
> > -			adapter->shared->devRead.misc.uptFeatures &= UPT1_F_LRO;
> > +			adapter->shared->devRead.misc.uptFeatures |= UPT1_F_LRO;
> >  		else
> 
> This change has nothing to do with respecting the VM interrupt setting.
> 
> Do not stuff unrelated changes into a commit.
> ...

Sorry about that. This is a fix I wanted to add and I did not operate stg
correctly. Reposting the patch w/o the feature update bits.


---

Respect the interrupt type set in VM configuration.

When interrupt type is not auto, do not ignore the interrupt type set from
VM configuration.

Signed-off-by: Shreyas Bhatewara <sbhatewara@vmware.com>

---


 drivers/net/vmxnet3/vmxnet3_drv.c |   13 ++++++++++---
 drivers/net/vmxnet3/vmxnet3_int.h |    4 ++--
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c
index c4d7e42..9282635 100644
--- a/drivers/net/vmxnet3/vmxnet3_drv.c
+++ b/drivers/net/vmxnet3/vmxnet3_drv.c
@@ -2357,9 +2357,13 @@ vmxnet3_alloc_intr_resources(struct vmxnet3_adapter *adapter)
 	adapter->intr.mask_mode = (cfg >> 2) & 0x3;
 
 	if (adapter->intr.type == VMXNET3_IT_AUTO) {
-		int err;
+		adapter->intr.type = VMXNET3_IT_MSIX;
+	}
 
 #ifdef CONFIG_PCI_MSI
+	if (adapter->intr.type == VMXNET3_IT_MSIX) {
+		int err;
+
 		adapter->intr.msix_entries[0].entry = 0;
 		err = pci_enable_msix(adapter->pdev, adapter->intr.msix_entries,
 				      VMXNET3_LINUX_MAX_MSIX_VECT);
@@ -2368,15 +2372,18 @@ vmxnet3_alloc_intr_resources(struct vmxnet3_adapter *adapter)
 			adapter->intr.type = VMXNET3_IT_MSIX;
 			return;
 		}
-#endif
+		adapter->intr.type = VMXNET3_IT_MSI;
+	}
 
+	if (adapter->intr.type == VMXNET3_IT_MSI) {
+		int err;
 		err = pci_enable_msi(adapter->pdev);
 		if (!err) {
 			adapter->intr.num_intrs = 1;
-			adapter->intr.type = VMXNET3_IT_MSI;
 			return;
 		}
 	}
+#endif /* CONFIG_PCI_MSI */
 
 	adapter->intr.type = VMXNET3_IT_INTX;
 
diff --git a/drivers/net/vmxnet3/vmxnet3_int.h b/drivers/net/vmxnet3/vmxnet3_int.h
index 9c2fe9a..d17fae6 100644
--- a/drivers/net/vmxnet3/vmxnet3_int.h
+++ b/drivers/net/vmxnet3/vmxnet3_int.h
@@ -68,10 +68,10 @@
 /*
  * Version numbers
  */
-#define VMXNET3_DRIVER_VERSION_STRING   "1.0.5.0-k"
+#define VMXNET3_DRIVER_VERSION_STRING   "1.0.13.0-k"
 
 /* a 32-bit int, each byte encode a verion number in VMXNET3_DRIVER_VERSION */
-#define VMXNET3_DRIVER_VERSION_NUM      0x01000500
+#define VMXNET3_DRIVER_VERSION_NUM      0x01000B00
 
 
 /*

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

* [PATCH 2.6.35-rc1] net-next: fix LRO feature update in vmxnet3
  2010-07-16  1:21         ` Shreyas Bhatewara
@ 2010-07-16  1:28           ` Shreyas Bhatewara
  2010-07-16  5:17             ` David Miller
  2010-07-19 20:16           ` [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [5/5] Respect the interrupt type in VM configuration David Miller
  1 sibling, 1 reply; 32+ messages in thread
From: Shreyas Bhatewara @ 2010-07-16  1:28 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-kernel, pv-drivers


From: Shreyas Bhatewara <sbhatewara@vmware.com>

Fix LRO feature update.

Signed-off-by: Shreyas Bhatewara <sbhatewara@vmware.com>

---

 drivers/net/vmxnet3/vmxnet3_ethtool.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/vmxnet3/vmxnet3_ethtool.c b/drivers/net/vmxnet3/vmxnet3_ethtool.c
index de1ba14..7e4b5a8 100644
--- a/drivers/net/vmxnet3/vmxnet3_ethtool.c
+++ b/drivers/net/vmxnet3/vmxnet3_ethtool.c
@@ -291,10 +291,11 @@ vmxnet3_set_flags(struct net_device *netdev, u32 data)
 
 		/* update harware LRO capability accordingly */
 		if (lro_requested)
-			adapter->shared->devRead.misc.uptFeatures &= UPT1_F_LRO;
+			adapter->shared->devRead.misc.uptFeatures |=
+						cpu_to_le64(UPT1_F_LRO);
 		else
 			adapter->shared->devRead.misc.uptFeatures &=
-								~UPT1_F_LRO;
+						cpu_to_le64(~UPT1_F_LRO);
 		VMXNET3_WRITE_BAR1_REG(adapter, VMXNET3_REG_CMD,
 				       VMXNET3_CMD_UPDATE_FEATURE);
 	}

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

* Re: [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [4/5] Do not reset when the device is not opened
  2010-07-16  1:20         ` Shreyas Bhatewara
@ 2010-07-16  1:32           ` David Miller
  2010-07-16  8:17             ` Shreyas Bhatewara
  0 siblings, 1 reply; 32+ messages in thread
From: David Miller @ 2010-07-16  1:32 UTC (permalink / raw)
  To: sbhatewara; +Cc: netdev, linux-kernel, pv-drivers, ronghua, matthieu

From: Shreyas Bhatewara <sbhatewara@vmware.com>
Date: Thu, 15 Jul 2010 18:20:52 -0700 (PDT)

> Is this what you suggest :
> 
> ---
> 
> Hold rtnl_lock to get the right link state.

It ought to work, but make sure that it is legal to take the
RTNL semaphore in all contexts in which this code block
might be called.

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

* Re: [PATCH 2.6.35-rc1] net-next: fix LRO feature update in vmxnet3
  2010-07-16  1:28           ` [PATCH 2.6.35-rc1] net-next: fix LRO feature update in vmxnet3 Shreyas Bhatewara
@ 2010-07-16  5:17             ` David Miller
  0 siblings, 0 replies; 32+ messages in thread
From: David Miller @ 2010-07-16  5:17 UTC (permalink / raw)
  To: sbhatewara; +Cc: netdev, linux-kernel, pv-drivers

From: Shreyas Bhatewara <sbhatewara@vmware.com>
Date: Thu, 15 Jul 2010 18:28:31 -0700 (PDT)

> 
> From: Shreyas Bhatewara <sbhatewara@vmware.com>
> 
> Fix LRO feature update.
> 
> Signed-off-by: Shreyas Bhatewara <sbhatewara@vmware.com>

Applied, thanks.

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

* Re: [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [2/5] Interrupt control bitmap
  2010-07-16  1:20       ` Shreyas Bhatewara
@ 2010-07-16  5:19         ` David Miller
  0 siblings, 0 replies; 32+ messages in thread
From: David Miller @ 2010-07-16  5:19 UTC (permalink / raw)
  To: sbhatewara; +Cc: netdev, linux-kernel, pv-drivers, ronghua

From: Shreyas Bhatewara <sbhatewara@vmware.com>
Date: Thu, 15 Jul 2010 18:20:03 -0700 (PDT)

> Reposting the patch with the endianness fix.
> 
> ---
> 
> A new bit map 'intrCtrl' is introduced in the DriverShared area. The 
> driver Ashould update VMXNET3_IC_DISABLE_ALL bit before writing IMR.
> 
> Signed-off-by: Ronghua Zang <ronghua@vmware.com>
> Signed-off-by: Shreyas Bhatewara <sbhatewara@vmware.com>

Applied.

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

* Re: [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [3/5] Initialize link state at probe time
  2010-07-16  1:20       ` Shreyas Bhatewara
@ 2010-07-16  5:44         ` David Miller
  2010-07-16  7:51           ` Shreyas Bhatewara
  0 siblings, 1 reply; 32+ messages in thread
From: David Miller @ 2010-07-16  5:44 UTC (permalink / raw)
  To: sbhatewara; +Cc: netdev, linux-kernel, pv-drivers

From: Shreyas Bhatewara <sbhatewara@vmware.com>
Date: Thu, 15 Jul 2010 18:20:14 -0700 (PDT)

> 
> On Wed, 14 Jul 2010, David Miller wrote:
> 
>> From: Shreyas Bhatewara <sbhatewara@vmware.com>
>> Date: Tue, 13 Jul 2010 17:48:55 -0700 (PDT)
>> 
>> > 
>> > Initialize vmxnet3 link state at probe time
>> > 
>> > This change initializes the state of link at the time when driver is
>> > loaded. The ethtool output for 'link detected' and 'link speed'
>> > is thus valid even before the interface is brought up.
>> > 
>> > Signed-off-by: Shreyas Bhatewara <sbhatewara@vmware.com>
>> 
>> You should never, ever, call netif_start_queue() on a device which has
>> not been brought up.
>> 
>> But that is what this patch is doing.
>> 
> 
> I do not understand why you say so. vmxnet3_check_link() is called in 
> probe() with affectTxQueue as false. Hence netif_start_queue() will not be 
> called before device is brought up.
> vmxnet3_check_link() is again called with affectTxQueue as true in 
> vmxnet3_activate_dev() after device was activated.

Aha, I see how the logic works now.

But still there is a problem with this patch, please remove the
driver version bump and resubmit.

You should only version bump at the last patch in a series.

Thanks.

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

* Re: [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [3/5] Initialize link state at probe time
  2010-07-16  5:44         ` David Miller
@ 2010-07-16  7:51           ` Shreyas Bhatewara
  2010-07-18 21:49             ` David Miller
  0 siblings, 1 reply; 32+ messages in thread
From: Shreyas Bhatewara @ 2010-07-16  7:51 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-kernel, pv-drivers



On Thu, 15 Jul 2010, David Miller wrote:

> From: Shreyas Bhatewara <sbhatewara@vmware.com>
> Date: Thu, 15 Jul 2010 18:20:14 -0700 (PDT)
> 
> > 
> > On Wed, 14 Jul 2010, David Miller wrote:
> > 
> >> From: Shreyas Bhatewara <sbhatewara@vmware.com>
> >> Date: Tue, 13 Jul 2010 17:48:55 -0700 (PDT)
> >> 
> >> > 
> >> > Initialize vmxnet3 link state at probe time
> >> > 
> >> > This change initializes the state of link at the time when driver is
> >> > loaded. The ethtool output for 'link detected' and 'link speed'
> >> > is thus valid even before the interface is brought up.
> >> > 
> >> > Signed-off-by: Shreyas Bhatewara <sbhatewara@vmware.com>
> >> 
> >> You should never, ever, call netif_start_queue() on a device which has
> >> not been brought up.
> >> 
> >> But that is what this patch is doing.
> >> 
> > 
> > I do not understand why you say so. vmxnet3_check_link() is called in 
> > probe() with affectTxQueue as false. Hence netif_start_queue() will not be 
> > called before device is brought up.
> > vmxnet3_check_link() is again called with affectTxQueue as true in 
> > vmxnet3_activate_dev() after device was activated.
> 
> Aha, I see how the logic works now.
> 
> But still there is a problem with this patch, please remove the
> driver version bump and resubmit.
> 
> You should only version bump at the last patch in a series.
> 
> Thanks.
> 

---

Initialize vmxnet3 link state at probe time

This change initializes the state of link at the time when driver is
loaded. The ethtool output for 'link detected' and 'link speed'
is thus valid even before the interface is brought up.

Signed-off-by: Shreyas Bhatewara <sbhatewara@vmware.com>

---
 drivers/net/vmxnet3/vmxnet3_drv.c |   13 ++++++++-----
 1 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c
index 7792a44..1e31d40 100644
--- a/drivers/net/vmxnet3/vmxnet3_drv.c
+++ b/drivers/net/vmxnet3/vmxnet3_drv.c
@@ -130,7 +130,7 @@ vmxnet3_tq_stop(struct vmxnet3_tx_queue *tq, struct vmxnet3_adapter *adapter)
  * Check the link state. This may start or stop the tx queue.
  */
 static void
-vmxnet3_check_link(struct vmxnet3_adapter *adapter)
+vmxnet3_check_link(struct vmxnet3_adapter *adapter, bool affectTxQueue)
 {
 	u32 ret;
 
@@ -143,14 +143,16 @@ vmxnet3_check_link(struct vmxnet3_adapter *adapter)
 		if (!netif_carrier_ok(adapter->netdev))
 			netif_carrier_on(adapter->netdev);
 
-		vmxnet3_tq_start(&adapter->tx_queue, adapter);
+		if (affectTxQueue)
+			vmxnet3_tq_start(&adapter->tx_queue, adapter);
 	} else {
 		printk(KERN_INFO "%s: NIC Link is Down\n",
 		       adapter->netdev->name);
 		if (netif_carrier_ok(adapter->netdev))
 			netif_carrier_off(adapter->netdev);
 
-		vmxnet3_tq_stop(&adapter->tx_queue, adapter);
+		if (affectTxQueue)
+			vmxnet3_tq_stop(&adapter->tx_queue, adapter);
 	}
 }
 
@@ -165,7 +167,7 @@ vmxnet3_process_events(struct vmxnet3_adapter *adapter)
 
 	/* Check if link state has changed */
 	if (events & VMXNET3_ECR_LINK)
-		vmxnet3_check_link(adapter);
+		vmxnet3_check_link(adapter, true);
 
 	/* Check if there is an error on xmit/recv queues */
 	if (events & (VMXNET3_ECR_TQERR | VMXNET3_ECR_RQERR)) {
@@ -1947,7 +1949,7 @@ vmxnet3_activate_dev(struct vmxnet3_adapter *adapter)
 	 * Check link state when first activating device. It will start the
 	 * tx queue if the link is up.
 	 */
-	vmxnet3_check_link(adapter);
+	vmxnet3_check_link(adapter, true);
 
 	napi_enable(&adapter->napi);
 	vmxnet3_enable_all_intrs(adapter);
@@ -2549,6 +2551,7 @@ vmxnet3_probe_device(struct pci_dev *pdev,
 	}
 
 	set_bit(VMXNET3_STATE_BIT_QUIESCED, &adapter->state);
+	vmxnet3_check_link(adapter, false);
 	atomic_inc(&devices_found);
 	return 0;


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

* Re: [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [4/5] Do not reset when the device is not opened
  2010-07-16  1:32           ` David Miller
@ 2010-07-16  8:17             ` Shreyas Bhatewara
  2010-07-17 23:35               ` David Miller
  0 siblings, 1 reply; 32+ messages in thread
From: Shreyas Bhatewara @ 2010-07-16  8:17 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, linux-kernel, pv-drivers, Ronghua Zhang, Matthieu Bucchianeri



On Thu, 15 Jul 2010, David Miller wrote:

> From: Shreyas Bhatewara <sbhatewara@vmware.com>
> Date: Thu, 15 Jul 2010 18:20:52 -0700 (PDT)
> 
> > Is this what you suggest :
> > 
> > ---
> > 
> > Hold rtnl_lock to get the right link state.
> 
> It ought to work, but make sure that it is legal to take the
> RTNL semaphore in all contexts in which this code block
> might be called.
> 

This code block is called only from the workqueue handler, which runs in
process context, so it is legal to take rtnl semaphore.
Tested this code by simulating event interrupts (which schedule this 
code) at considerable frequency while the interface was brought up and
down in a loop. Similar stress testing had revealed the bug originally. 

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

* Re: [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [4/5] Do not reset when the device is not opened
  2010-07-16  8:17             ` Shreyas Bhatewara
@ 2010-07-17 23:35               ` David Miller
  2010-07-19 17:02                 ` Shreyas Bhatewara
  0 siblings, 1 reply; 32+ messages in thread
From: David Miller @ 2010-07-17 23:35 UTC (permalink / raw)
  To: sbhatewara; +Cc: netdev, linux-kernel, pv-drivers, ronghua, matthieu

From: Shreyas Bhatewara <sbhatewara@vmware.com>
Date: Fri, 16 Jul 2010 01:17:29 -0700 (PDT)

> 
> 
> On Thu, 15 Jul 2010, David Miller wrote:
> 
>> From: Shreyas Bhatewara <sbhatewara@vmware.com>
>> Date: Thu, 15 Jul 2010 18:20:52 -0700 (PDT)
>> 
>> > Is this what you suggest :
>> > 
>> > ---
>> > 
>> > Hold rtnl_lock to get the right link state.
>> 
>> It ought to work, but make sure that it is legal to take the
>> RTNL semaphore in all contexts in which this code block
>> might be called.
>> 
> 
> This code block is called only from the workqueue handler, which runs in
> process context, so it is legal to take rtnl semaphore.
> Tested this code by simulating event interrupts (which schedule this 
> code) at considerable frequency while the interface was brought up and
> down in a loop. Similar stress testing had revealed the bug originally. 

Awesome, please submit this formally.  The copy you sent lacked a commit
message and signoff.

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

* Re: [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [3/5] Initialize link state at probe time
  2010-07-16  7:51           ` Shreyas Bhatewara
@ 2010-07-18 21:49             ` David Miller
  0 siblings, 0 replies; 32+ messages in thread
From: David Miller @ 2010-07-18 21:49 UTC (permalink / raw)
  To: sbhatewara; +Cc: netdev, linux-kernel, pv-drivers

From: Shreyas Bhatewara <sbhatewara@vmware.com>
Date: Fri, 16 Jul 2010 00:51:14 -0700 (PDT)

> Initialize vmxnet3 link state at probe time
> 
> This change initializes the state of link at the time when driver is
> loaded. The ethtool output for 'link detected' and 'link speed'
> is thus valid even before the interface is brought up.
> 
> Signed-off-by: Shreyas Bhatewara <sbhatewara@vmware.com>

Applied.

I'm still (patiently) waiting for the formal resubmission of patch #4
so I can also then apply patch #5.  Please post it at your next
possible convenience.

Thanks.

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

* Re: [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [4/5] Do not reset when the device is not opened
  2010-07-17 23:35               ` David Miller
@ 2010-07-19 17:02                 ` Shreyas Bhatewara
  2010-07-19 20:16                   ` David Miller
  0 siblings, 1 reply; 32+ messages in thread
From: Shreyas Bhatewara @ 2010-07-19 17:02 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, linux-kernel, pv-drivers, Ronghua Zhang, Matthieu Bucchianeri



On Sat, 17 Jul 2010, David Miller wrote:

> From: Shreyas Bhatewara <sbhatewara@vmware.com>
> Date: Fri, 16 Jul 2010 01:17:29 -0700 (PDT)
> 
> > 
> > 
> > On Thu, 15 Jul 2010, David Miller wrote:
> > 
> >> From: Shreyas Bhatewara <sbhatewara@vmware.com>
> >> Date: Thu, 15 Jul 2010 18:20:52 -0700 (PDT)
> >> 
> >> > Is this what you suggest :
> >> > 
> >> > ---
> >> > 
> >> > Hold rtnl_lock to get the right link state.
> >> 
> >> It ought to work, but make sure that it is legal to take the
> >> RTNL semaphore in all contexts in which this code block
> >> might be called.
> >> 
> > 
> > This code block is called only from the workqueue handler, which runs in
> > process context, so it is legal to take rtnl semaphore.
> > Tested this code by simulating event interrupts (which schedule this 
> > code) at considerable frequency while the interface was brought up and
> > down in a loop. Similar stress testing had revealed the bug originally. 
> 
> Awesome, please submit this formally.  The copy you sent lacked a commit
> message and signoff.
> 

Reposting the patch formally.

David,
Thanks for your coperation.

->Shreyas

---
From: Shreyas Bhatewara <sbhatewara@vmware.com>

Hold rtnl_lock to get the right link state.

While asynchronously resetting the device, hold rtnl_lock to get the
right value from netif_running. If a reset is scheduled, and the device
goes thru close and open, it may happen that reset and open may run in
parallel. Holding rtnl_lock will avoid this.

Signed-off-by: Shreyas Bhatewara <sbhatewara@vmware.com>

---

 drivers/net/vmxnet3/vmxnet3_drv.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c
index 1b0ce8c..c4d7e42 100644
--- a/drivers/net/vmxnet3/vmxnet3_drv.c
+++ b/drivers/net/vmxnet3/vmxnet3_drv.c
@@ -2420,6 +2420,7 @@ vmxnet3_reset_work(struct work_struct *data)
 		return;
 
 	/* if the device is closed, we must leave it alone */
+	rtnl_lock();
 	if (netif_running(adapter->netdev)) {
 		printk(KERN_INFO "%s: resetting\n", adapter->netdev->name);
 		vmxnet3_quiesce_dev(adapter);
@@ -2428,6 +2429,7 @@ vmxnet3_reset_work(struct work_struct *data)
 	} else {
 		printk(KERN_INFO "%s: already closed\n", adapter->netdev->name);
 	}
+	rtnl_unlock();
 
 	clear_bit(VMXNET3_STATE_BIT_RESETTING, &adapter->state);
 }

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

* Re: [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [4/5] Do not reset when the device is not opened
  2010-07-19 17:02                 ` Shreyas Bhatewara
@ 2010-07-19 20:16                   ` David Miller
  0 siblings, 0 replies; 32+ messages in thread
From: David Miller @ 2010-07-19 20:16 UTC (permalink / raw)
  To: sbhatewara; +Cc: netdev, linux-kernel, pv-drivers, ronghua, matthieu

From: Shreyas Bhatewara <sbhatewara@vmware.com>
Date: Mon, 19 Jul 2010 10:02:13 -0700 (PDT)

> Hold rtnl_lock to get the right link state.
> 
> While asynchronously resetting the device, hold rtnl_lock to get the
> right value from netif_running. If a reset is scheduled, and the device
> goes thru close and open, it may happen that reset and open may run in
> parallel. Holding rtnl_lock will avoid this.
> 
> Signed-off-by: Shreyas Bhatewara <sbhatewara@vmware.com>

Applied.

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

* Re: [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [5/5] Respect the interrupt type in VM configuration
  2010-07-16  1:21         ` Shreyas Bhatewara
  2010-07-16  1:28           ` [PATCH 2.6.35-rc1] net-next: fix LRO feature update in vmxnet3 Shreyas Bhatewara
@ 2010-07-19 20:16           ` David Miller
  1 sibling, 0 replies; 32+ messages in thread
From: David Miller @ 2010-07-19 20:16 UTC (permalink / raw)
  To: sbhatewara; +Cc: netdev, linux-kernel, pv-drivers

From: Shreyas Bhatewara <sbhatewara@vmware.com>
Date: Thu, 15 Jul 2010 18:21:27 -0700 (PDT)

> Respect the interrupt type set in VM configuration.
> 
> When interrupt type is not auto, do not ignore the interrupt type set from
> VM configuration.
> 
> Signed-off-by: Shreyas Bhatewara <sbhatewara@vmware.com>

Applied.

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

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

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-07-07  9:00 [PATCH 2.6.35-rc1] net: vmxnet3 fixes [0/5] Spare skb to avoid starvation Shreyas Bhatewara
2010-07-07  9:24 ` [PATCH 2.6.35-rc1] net: vmxnet3 fixes [2/5] Interrupt control bitmap Shreyas Bhatewara
2010-07-14  0:48   ` [PATCH 2.6.35-rc1] net-next: " Shreyas Bhatewara
2010-07-14 21:04     ` David Miller
2010-07-16  1:20       ` Shreyas Bhatewara
2010-07-16  5:19         ` David Miller
2010-07-07  9:28 ` [PATCH 2.6.35-rc1] net: vmxnet3 fixes [3/5] Initialize link state at probe time Shreyas Bhatewara
2010-07-07  9:31   ` [PATCH 2.6.35-rc1] net: vmxnet3 fixes [4/5] Do not reset when the device is not opened Shreyas Bhatewara
2010-07-14  0:49     ` [PATCH 2.6.35-rc1] net-next: " Shreyas Bhatewara
2010-07-14 21:07       ` David Miller
2010-07-16  1:20         ` Shreyas Bhatewara
2010-07-16  1:32           ` David Miller
2010-07-16  8:17             ` Shreyas Bhatewara
2010-07-17 23:35               ` David Miller
2010-07-19 17:02                 ` Shreyas Bhatewara
2010-07-19 20:16                   ` David Miller
2010-07-07  9:34   ` [PATCH 2.6.35-rc1] net: vmxnet3 fixes [5/5] Respect the interrupt type in VM configuration Shreyas Bhatewara
2010-07-14  0:51     ` [PATCH 2.6.35-rc1] net-next: " Shreyas Bhatewara
2010-07-14 21:11       ` David Miller
2010-07-16  1:21         ` Shreyas Bhatewara
2010-07-16  1:28           ` [PATCH 2.6.35-rc1] net-next: fix LRO feature update in vmxnet3 Shreyas Bhatewara
2010-07-16  5:17             ` David Miller
2010-07-19 20:16           ` [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [5/5] Respect the interrupt type in VM configuration David Miller
2010-07-14  0:48   ` [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [3/5] Initialize link state at probe time Shreyas Bhatewara
2010-07-14 21:10     ` David Miller
2010-07-16  1:20       ` Shreyas Bhatewara
2010-07-16  5:44         ` David Miller
2010-07-16  7:51           ` Shreyas Bhatewara
2010-07-18 21:49             ` David Miller
2010-07-07 21:40 ` [PATCH 2.6.35-rc1] net: vmxnet3 fixes [0/5] Spare skb to avoid starvation David Miller
2010-07-07 22:19   ` Shreyas Bhatewara
2010-07-07 22:25     ` David Miller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.