* [PATCH net-2.6 1/2] sfc: Fix crash in legacy onterrupt handler during ring reallocation
2010-12-07 21:40 pull request: sfc-2.6 2010-12-07 Ben Hutchings
@ 2010-12-07 21:42 ` Ben Hutchings
2010-12-07 21:42 ` [PATCH net-2.6 2/2] sfc: Fix NAPI list corruption " Ben Hutchings
2010-12-07 21:45 ` pull request: sfc-2.6 2010-12-07 Ben Hutchings
2 siblings, 0 replies; 5+ messages in thread
From: Ben Hutchings @ 2010-12-07 21:42 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers
If we are using a legacy interrupt, our IRQ may be shared and our
interrupt handler may be called even though interrupts are disabled on
the NIC. When we change ring sizes, we reallocate the event queue and
the interrupt handler may use an invalid pointer when called for
another device's interrupt.
Maintain a legacy_irq_enabled flag and test that at the top of the
interrupt handler. Note that this problem results from the need to
work around broken INT_ISR0 reads, and does not affect the legacy
interrupt handler for Falcon A1.
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
drivers/net/sfc/efx.c | 12 ++++++++++--
drivers/net/sfc/net_driver.h | 2 ++
drivers/net/sfc/nic.c | 6 ++++++
3 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/drivers/net/sfc/efx.c b/drivers/net/sfc/efx.c
index 05df20e..d06cb74 100644
--- a/drivers/net/sfc/efx.c
+++ b/drivers/net/sfc/efx.c
@@ -335,8 +335,10 @@ void efx_process_channel_now(struct efx_channel *channel)
/* Disable interrupts and wait for ISRs to complete */
efx_nic_disable_interrupts(efx);
- if (efx->legacy_irq)
+ if (efx->legacy_irq) {
synchronize_irq(efx->legacy_irq);
+ efx->legacy_irq_enabled = false;
+ }
if (channel->irq)
synchronize_irq(channel->irq);
@@ -351,6 +353,8 @@ void efx_process_channel_now(struct efx_channel *channel)
efx_channel_processed(channel);
napi_enable(&channel->napi_str);
+ if (efx->legacy_irq)
+ efx->legacy_irq_enabled = true;
efx_nic_enable_interrupts(efx);
}
@@ -1400,6 +1404,8 @@ static void efx_start_all(struct efx_nic *efx)
efx_start_channel(channel);
}
+ if (efx->legacy_irq)
+ efx->legacy_irq_enabled = true;
efx_nic_enable_interrupts(efx);
/* Switch to event based MCDI completions after enabling interrupts.
@@ -1460,8 +1466,10 @@ static void efx_stop_all(struct efx_nic *efx)
/* Disable interrupts and wait for ISR to complete */
efx_nic_disable_interrupts(efx);
- if (efx->legacy_irq)
+ if (efx->legacy_irq) {
synchronize_irq(efx->legacy_irq);
+ efx->legacy_irq_enabled = false;
+ }
efx_for_each_channel(channel, efx) {
if (channel->irq)
synchronize_irq(channel->irq);
diff --git a/drivers/net/sfc/net_driver.h b/drivers/net/sfc/net_driver.h
index 0a7e26d..b137c88 100644
--- a/drivers/net/sfc/net_driver.h
+++ b/drivers/net/sfc/net_driver.h
@@ -621,6 +621,7 @@ struct efx_filter_state;
* @pci_dev: The PCI device
* @type: Controller type attributes
* @legacy_irq: IRQ number
+ * @legacy_irq_enabled: Are IRQs enabled on NIC (INT_EN_KER register)?
* @workqueue: Workqueue for port reconfigures and the HW monitor.
* Work items do not hold and must not acquire RTNL.
* @workqueue_name: Name of workqueue
@@ -709,6 +710,7 @@ struct efx_nic {
struct pci_dev *pci_dev;
const struct efx_nic_type *type;
int legacy_irq;
+ bool legacy_irq_enabled;
struct workqueue_struct *workqueue;
char workqueue_name[16];
struct work_struct reset_work;
diff --git a/drivers/net/sfc/nic.c b/drivers/net/sfc/nic.c
index 41c36b9..67cb0c9 100644
--- a/drivers/net/sfc/nic.c
+++ b/drivers/net/sfc/nic.c
@@ -1418,6 +1418,12 @@ static irqreturn_t efx_legacy_interrupt(int irq, void *dev_id)
u32 queues;
int syserr;
+ /* Could this be ours? If interrupts are disabled then the
+ * channel state may not be valid.
+ */
+ if (!efx->legacy_irq_enabled)
+ return result;
+
/* Read the ISR which also ACKs the interrupts */
efx_readd(efx, ®, FR_BZ_INT_ISR0);
queues = EFX_EXTRACT_DWORD(reg, 0, 31);
--
1.7.3.2
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH net-2.6 2/2] sfc: Fix NAPI list corruption during ring reallocation
2010-12-07 21:40 pull request: sfc-2.6 2010-12-07 Ben Hutchings
2010-12-07 21:42 ` [PATCH net-2.6 1/2] sfc: Fix crash in legacy onterrupt handler during ring reallocation Ben Hutchings
@ 2010-12-07 21:42 ` Ben Hutchings
2010-12-07 21:45 ` pull request: sfc-2.6 2010-12-07 Ben Hutchings
2 siblings, 0 replies; 5+ messages in thread
From: Ben Hutchings @ 2010-12-07 21:42 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers
Call netif_napi_{add,del}() on the NAPI contexts in the new and
old channels, respectively.
Since efx_init_napi() cannot fail, make its return type void.
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
drivers/net/sfc/efx.c | 31 +++++++++++++++++++------------
1 files changed, 19 insertions(+), 12 deletions(-)
diff --git a/drivers/net/sfc/efx.c b/drivers/net/sfc/efx.c
index d06cb74..fb83cdd 100644
--- a/drivers/net/sfc/efx.c
+++ b/drivers/net/sfc/efx.c
@@ -197,7 +197,9 @@ MODULE_PARM_DESC(debug, "Bitmapped debugging message enable value");
static void efx_remove_channels(struct efx_nic *efx);
static void efx_remove_port(struct efx_nic *efx);
+static void efx_init_napi(struct efx_nic *efx);
static void efx_fini_napi(struct efx_nic *efx);
+static void efx_fini_napi_channel(struct efx_channel *channel);
static void efx_fini_struct(struct efx_nic *efx);
static void efx_start_all(struct efx_nic *efx);
static void efx_stop_all(struct efx_nic *efx);
@@ -430,6 +432,7 @@ efx_alloc_channel(struct efx_nic *efx, int i, struct efx_channel *old_channel)
*channel = *old_channel;
+ channel->napi_dev = NULL;
memset(&channel->eventq, 0, sizeof(channel->eventq));
rx_queue = &channel->rx_queue;
@@ -740,9 +743,13 @@ efx_realloc_channels(struct efx_nic *efx, u32 rxq_entries, u32 txq_entries)
if (rc)
goto rollback;
+ efx_init_napi(efx);
+
/* Destroy old channels */
- for (i = 0; i < efx->n_channels; i++)
+ for (i = 0; i < efx->n_channels; i++) {
+ efx_fini_napi_channel(other_channel[i]);
efx_remove_channel(other_channel[i]);
+ }
out:
/* Free unused channel structures */
for (i = 0; i < efx->n_channels; i++)
@@ -1601,7 +1608,7 @@ static int efx_ioctl(struct net_device *net_dev, struct ifreq *ifr, int cmd)
*
**************************************************************************/
-static int efx_init_napi(struct efx_nic *efx)
+static void efx_init_napi(struct efx_nic *efx)
{
struct efx_channel *channel;
@@ -1610,18 +1617,21 @@ static int efx_init_napi(struct efx_nic *efx)
netif_napi_add(channel->napi_dev, &channel->napi_str,
efx_poll, napi_weight);
}
- return 0;
+}
+
+static void efx_fini_napi_channel(struct efx_channel *channel)
+{
+ if (channel->napi_dev)
+ netif_napi_del(&channel->napi_str);
+ channel->napi_dev = NULL;
}
static void efx_fini_napi(struct efx_nic *efx)
{
struct efx_channel *channel;
- efx_for_each_channel(channel, efx) {
- if (channel->napi_dev)
- netif_napi_del(&channel->napi_str);
- channel->napi_dev = NULL;
- }
+ efx_for_each_channel(channel, efx)
+ efx_fini_napi_channel(channel);
}
/**************************************************************************
@@ -2343,9 +2353,7 @@ static int efx_pci_probe_main(struct efx_nic *efx)
if (rc)
goto fail1;
- rc = efx_init_napi(efx);
- if (rc)
- goto fail2;
+ efx_init_napi(efx);
rc = efx->type->init(efx);
if (rc) {
@@ -2376,7 +2384,6 @@ static int efx_pci_probe_main(struct efx_nic *efx)
efx->type->fini(efx);
fail3:
efx_fini_napi(efx);
- fail2:
efx_remove_all(efx);
fail1:
return rc;
--
1.7.3.2
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: pull request: sfc-2.6 2010-12-07
2010-12-07 21:40 pull request: sfc-2.6 2010-12-07 Ben Hutchings
2010-12-07 21:42 ` [PATCH net-2.6 1/2] sfc: Fix crash in legacy onterrupt handler during ring reallocation Ben Hutchings
2010-12-07 21:42 ` [PATCH net-2.6 2/2] sfc: Fix NAPI list corruption " Ben Hutchings
@ 2010-12-07 21:45 ` Ben Hutchings
2010-12-08 18:41 ` David Miller
2 siblings, 1 reply; 5+ messages in thread
From: Ben Hutchings @ 2010-12-07 21:45 UTC (permalink / raw)
To: David Miller; +Cc: netdev, sf-linux-drivers
On Tue, 2010-12-07 at 21:40 +0000, Ben Hutchings wrote:
> The following changes since commit 46bcf14f44d8f31ecfdc8b6708ec15a3b33316d9:
>
> filter: fix sk_filter rcu handling (2010-12-06 09:29:43 -0800)
>
> are available in the git repository at:
> git://git.kernel.org/linux/kernel/git/bwh/sfc-2.6.git sfc-2.6.37
That should of course be:
git://git.kernel.org/pub/scm/linux/kernel/git/bwh/sfc-2.6.git sfc-2.6.37
Ben.
> (or will be shortly).
>
> These fix bugs introduced in 2.6.37-rc1. Please pull.
>
> Ben.
>
> Ben Hutchings (2):
> sfc: Fix crash in legacy onterrupt handler during ring reallocation
> sfc: Fix NAPI list corruption during ring reallocation
>
> drivers/net/sfc/efx.c | 43 ++++++++++++++++++++++++++++-------------
> drivers/net/sfc/net_driver.h | 2 +
> drivers/net/sfc/nic.c | 6 +++++
> 3 files changed, 37 insertions(+), 14 deletions(-)
>
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: pull request: sfc-2.6 2010-12-07
2010-12-07 21:45 ` pull request: sfc-2.6 2010-12-07 Ben Hutchings
@ 2010-12-08 18:41 ` David Miller
0 siblings, 0 replies; 5+ messages in thread
From: David Miller @ 2010-12-08 18:41 UTC (permalink / raw)
To: bhutchings; +Cc: netdev, linux-net-drivers
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Tue, 07 Dec 2010 21:45:35 +0000
> On Tue, 2010-12-07 at 21:40 +0000, Ben Hutchings wrote:
>> The following changes since commit 46bcf14f44d8f31ecfdc8b6708ec15a3b33316d9:
>>
>> filter: fix sk_filter rcu handling (2010-12-06 09:29:43 -0800)
>>
>> are available in the git repository at:
>> git://git.kernel.org/linux/kernel/git/bwh/sfc-2.6.git sfc-2.6.37
>
> That should of course be:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/bwh/sfc-2.6.git sfc-2.6.37
>
> Ben.
Pulled, thanks a lot Ben.
^ permalink raw reply [flat|nested] 5+ messages in thread