2010-07-07 09:07:34

by Shreyas Bhatewara

Subject: [PATCH 2.6.35-rc1] net: vmxnet3 fixes [0/5] Spare skb to avoid starvation

From: Shreyas Bhatewara <[email protected]>

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 <[email protected]>
Signed-off-by: Shreyas Bhatewara <[email protected]>

---

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

2010-07-07 09:24:18

by Shreyas Bhatewara

Subject: [PATCH 2.6.35-rc1] net: vmxnet3 fixes [2/5] Interrupt control bitmap

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 <[email protected]>
Signed-off-by: Shreyas Bhatewara <[email protected]>

---

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


/*

2010-07-07 09:28:39

by Shreyas Bhatewara

Subject: [PATCH 2.6.35-rc1] net: vmxnet3 fixes [3/5] Initialize link state at probe time

Author: Shreyas Bhatewara <[email protected]>

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 <[email protected]>

---

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


/*

2010-07-07 09:31:41

by Shreyas Bhatewara

Subject: [PATCH 2.6.35-rc1] net: vmxnet3 fixes [4/5] Do not reset when the device is not opened

commit bc8b3f0b3978d3c0a201926b2e2bd5c732e0352e
Author: Shreyas Bhatewara <[email protected]>
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 <[email protected]>
Signed-off-by: Matthieu Bucchianeri <[email protected]>
Signed-off-by: Shreyas Bhatewara <[email protected]>

---

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


/*

2010-07-07 09:34:38

by Shreyas Bhatewara

Subject: [PATCH 2.6.35-rc1] net: vmxnet3 fixes [5/5] Respect the interrupt type in VM configuration

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 <[email protected]>

---

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


/*

2010-07-07 21:44:39

by David Miller

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

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

Thanks.

2010-07-07 22:19:12

by Shreyas Bhatewara

Subject: RE: [PATCH 2.6.35-rc1] net: vmxnet3 fixes [0/5] Spare skb to avoid starvation

> -----Original Message-----
> From: David Miller [mailto:[email protected]]
> Sent: Wednesday, July 07, 2010 2:41 PM
> To: Shreyas Bhatewara
> Cc: [email protected]; [email protected]; pv-
> [email protected]; 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.

2010-07-07 22:25:33

by David Miller

Subject: Re: [PATCH 2.6.35-rc1] net: vmxnet3 fixes [0/5] Spare skb to avoid starvation

From: Shreyas Bhatewara <[email protected]>
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.

2010-07-14 00:48:07

by Shreyas Bhatewara

Subject: [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [2/5] Interrupt control bitmap

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 <[email protected]>
Signed-off-by: Shreyas Bhatewara <[email protected]>

---

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;

2010-07-14 00:48:57

by Shreyas Bhatewara

Subject: [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [3/5] Initialize link state at probe time

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 <[email protected]>

---
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;

2010-07-14 00:49:54

by Shreyas Bhatewara

Subject: [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [4/5] Do not reset when the device is not opened

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 <[email protected]>
Signed-off-by: Matthieu Bucchianeri <[email protected]>
Signed-off-by: Shreyas Bhatewara <[email protected]>

---

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

2010-07-14 00:51:41

by Shreyas Bhatewara

Subject: [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [5/5] Respect the interrupt type in VM configuration

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 <[email protected]>

---

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);
}

2010-07-14 21:04:19

by David Miller

Subject: Re: [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [2/5] Interrupt control bitmap

From: Shreyas Bhatewara <[email protected]>
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...

2010-07-14 21:07:05

by David Miller

Subject: Re: [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [4/5] Do not reset when the device is not opened

From: Shreyas Bhatewara <[email protected]>
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 <[email protected]>
> Signed-off-by: Matthieu Bucchianeri <[email protected]>
> Signed-off-by: Shreyas Bhatewara <[email protected]>

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.

2010-07-14 21:10:40

by David Miller

Subject: Re: [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [3/5] Initialize link state at probe time

From: Shreyas Bhatewara <[email protected]>
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 <[email protected]>

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.

2010-07-14 21:11:07

by David Miller

Subject: Re: [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [5/5] Respect the interrupt type in VM configuration

From: Shreyas Bhatewara <[email protected]>
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 <[email protected]>
...
> @@ -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.

2010-07-16 01:20:06

by Shreyas Bhatewara

Subject: Re: [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [2/5] Interrupt control bitmap

On Wed, 14 Jul 2010, David Miller wrote:

> From: Shreyas Bhatewara <[email protected]>
> 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 <[email protected]>
Signed-off-by: Shreyas Bhatewara <[email protected]>

---

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;

2010-07-16 01:20:17

by Shreyas Bhatewara

Subject: Re: [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [3/5] Initialize link state at probe time

On Wed, 14 Jul 2010, David Miller wrote:

> From: Shreyas Bhatewara <[email protected]>
> 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 <[email protected]>
>
> 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.

2010-07-16 01:20:54

by Shreyas Bhatewara

Subject: Re: [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [4/5] Do not reset when the device is not opened

On Wed, 14 Jul 2010, David Miller wrote:

> From: Shreyas Bhatewara <[email protected]>
> 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 <[email protected]>
> > Signed-off-by: Matthieu Bucchianeri <[email protected]>
> > Signed-off-by: Shreyas Bhatewara <[email protected]>
>
> 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);
}

2010-07-16 01:21:30

by Shreyas Bhatewara

Subject: Re: [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [5/5] Respect the interrupt type in VM configuration

On Wed, 14 Jul 2010, David Miller wrote:

> From: Shreyas Bhatewara <[email protected]>
> 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 <[email protected]>
> ...
> > @@ -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 <[email protected]>

---


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


/*

2010-07-16 01:28:34

by Shreyas Bhatewara

Subject: [PATCH 2.6.35-rc1] net-next: fix LRO feature update in vmxnet3

From: Shreyas Bhatewara <[email protected]>

Fix LRO feature update.

Signed-off-by: Shreyas Bhatewara <[email protected]>

---

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);
}

2010-07-16 01:31:56

by David Miller

Subject: Re: [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [4/5] Do not reset when the device is not opened

From: Shreyas Bhatewara <[email protected]>
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.

2010-07-16 05:17:25

by David Miller

Subject: Re: [PATCH 2.6.35-rc1] net-next: fix LRO feature update in vmxnet3

From: Shreyas Bhatewara <[email protected]>
Date: Thu, 15 Jul 2010 18:28:31 -0700 (PDT)

>
> From: Shreyas Bhatewara <[email protected]>
>
> Fix LRO feature update.
>
> Signed-off-by: Shreyas Bhatewara <[email protected]>

Applied, thanks.

2010-07-16 05:18:48

by David Miller

Subject: Re: [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [2/5] Interrupt control bitmap

From: Shreyas Bhatewara <[email protected]>
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 <[email protected]>
> Signed-off-by: Shreyas Bhatewara <[email protected]>

Applied.

2010-07-16 05:44:21

by David Miller

Subject: Re: [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [3/5] Initialize link state at probe time

From: Shreyas Bhatewara <[email protected]>
Date: Thu, 15 Jul 2010 18:20:14 -0700 (PDT)

>
> On Wed, 14 Jul 2010, David Miller wrote:
>
>> From: Shreyas Bhatewara <[email protected]>
>> 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 <[email protected]>
>>
>> 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.

2010-07-16 07:51:19

by Shreyas Bhatewara

Subject: Re: [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [3/5] Initialize link state at probe time

On Thu, 15 Jul 2010, David Miller wrote:

> From: Shreyas Bhatewara <[email protected]>
> Date: Thu, 15 Jul 2010 18:20:14 -0700 (PDT)
>
> >
> > On Wed, 14 Jul 2010, David Miller wrote:
> >
> >> From: Shreyas Bhatewara <[email protected]>
> >> 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 <[email protected]>
> >>
> >> 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 <[email protected]>

---
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;

2010-07-16 08:17:36

by Shreyas Bhatewara

Subject: Re: [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [4/5] Do not reset when the device is not opened

On Thu, 15 Jul 2010, David Miller wrote:

> From: Shreyas Bhatewara <[email protected]>
> 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.

2010-07-17 23:35:24

by David Miller

Subject: Re: [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [4/5] Do not reset when the device is not opened

From: Shreyas Bhatewara <[email protected]>
Date: Fri, 16 Jul 2010 01:17:29 -0700 (PDT)

>
>
> On Thu, 15 Jul 2010, David Miller wrote:
>
>> From: Shreyas Bhatewara <[email protected]>
>> 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.

2010-07-18 21:49:14

by David Miller

Subject: Re: [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [3/5] Initialize link state at probe time

From: Shreyas Bhatewara <[email protected]>
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 <[email protected]>

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.

2010-07-19 17:02:16

by Shreyas Bhatewara

Subject: Re: [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [4/5] Do not reset when the device is not opened

On Sat, 17 Jul 2010, David Miller wrote:

> From: Shreyas Bhatewara <[email protected]>
> Date: Fri, 16 Jul 2010 01:17:29 -0700 (PDT)
>
> >
> >
> > On Thu, 15 Jul 2010, David Miller wrote:
> >
> >> From: Shreyas Bhatewara <[email protected]>
> >> 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 <[email protected]>

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 <[email protected]>

---

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);
}

2010-07-19 20:15:55

by David Miller

Subject: Re: [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [4/5] Do not reset when the device is not opened

From: Shreyas Bhatewara <[email protected]>
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 <[email protected]>

Applied.

2010-07-19 20:16:09

by David Miller

Subject: Re: [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [5/5] Respect the interrupt type in VM configuration

From: Shreyas Bhatewara <[email protected]>
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 <[email protected]>

Applied.