* [PATCH] e1000 in linux-3.18.0: a potential bug
@ 2014-12-20 7:50 Jia-Ju Bai
2014-12-20 13:27 ` Sergei Shtylyov
0 siblings, 1 reply; 4+ messages in thread
From: Jia-Ju Bai @ 2014-12-20 7:50 UTC (permalink / raw)
To: todd.fujinaka, netdev; +Cc: linux.nics, e1000-devel
I have actually tested e1000 driver on the real hardware(Intel 82540EM PCI
Gigabit Ethernet Controller), and find a potential bug:
The target file is drivers/net/ethernet/intel/e1000/e1000_main.c, which is
used to build e1000.ko.
(1) In the normal process, netif_napi_add is called in e1000_probe, but
netif_napi_del is not called in e1000_remove. However, many other ethernet
card drivers call them in pairs, even in the error handling paths, such as
r8169 and igb.
Meanwhile, I also write the patch to fix the bug. I have run the patch on
the hardware, it can work normally and fix the above bug.
diff --git a/drivers/net/ethernet/intel/e1000/e1000_main.c
b/drivers/net/ethernet/intel/e1000/e1000_main.c
index 24f3986..f6def7b 100644
--- a/drivers/net/ethernet/intel/e1000/e1000_main.c
+++ b/drivers/net/ethernet/intel/e1000/e1000_main.c
@@ -1004,7 +1004,7 @@ static int e1000_probe(struct pci_dev *pdev, const
struct pci_device_id *ent)
/* make ready for any if (hw->...) below */
err = e1000_init_hw_struct(adapter, hw);
if (err)
- goto err_sw_init;
+ goto err_dma;
/* there is a workaround being applied below that limits
* 64-bit DMA addresses to 64-bit hardware. There are some
@@ -1239,8 +1239,9 @@ err_eeprom:
iounmap(hw->flash_address);
kfree(adapter->tx_ring);
kfree(adapter->rx_ring);
-err_dma:
err_sw_init:
+ netif_napi_del(&adapter->napi);
+err_dma:
err_mdio_ioremap:
iounmap(hw->ce4100_gbe_mdio_base_virt);
iounmap(hw->hw_addr);
@@ -1271,6 +1272,7 @@ static void e1000_remove(struct pci_dev *pdev)
e1000_down_and_stop(adapter);
e1000_release_manageability(adapter);
+ netif_napi_del(&adapter->napi);
unregister_netdev(netdev);
e1000_phy_hw_reset(hw);
Thanks!
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] e1000 in linux-3.18.0: a potential bug
2014-12-20 7:50 Jia-Ju Bai
@ 2014-12-20 13:27 ` Sergei Shtylyov
0 siblings, 0 replies; 4+ messages in thread
From: Sergei Shtylyov @ 2014-12-20 13:27 UTC (permalink / raw)
To: Jia-Ju Bai, todd.fujinaka, netdev; +Cc: linux.nics, e1000-devel
Hello.
On 12/20/2014 10:50 AM, Jia-Ju Bai wrote:
> I have actually tested e1000 driver on the real hardware(Intel 82540EM PCI
> Gigabit Ethernet Controller), and find a potential bug:
> The target file is drivers/net/ethernet/intel/e1000/e1000_main.c, which is
> used to build e1000.ko.
> (1) In the normal process, netif_napi_add is called in e1000_probe, but
> netif_napi_del is not called in e1000_remove. However, many other ethernet
> card drivers call them in pairs, even in the error handling paths, such as
> r8169 and igb.
> Meanwhile, I also write the patch to fix the bug. I have run the patch on
> the hardware, it can work normally and fix the above bug.
You didn't sign off on your patch, so it can't be applied.
WBR, Sergei
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] e1000 in linux-3.18.0: a potential bug
@ 2014-12-20 13:57 Jia-Ju Bai
2014-12-20 14:15 ` Sergei Shtylyov
0 siblings, 1 reply; 4+ messages in thread
From: Jia-Ju Bai @ 2014-12-20 13:57 UTC (permalink / raw)
To: sergei.shtylyov, netdev
Maybe the email can not be displayed fully...
I send my patch again:
diff --git a/drivers/net/ethernet/intel/e1000/e1000_main.c
b/drivers/net/ethernet/intel/e1000/e1000_main.c
index 24f3986..f6def7b 100644
--- a/drivers/net/ethernet/intel/e1000/e1000_main.c
+++ b/drivers/net/ethernet/intel/e1000/e1000_main.c
@@ -1004,7 +1004,7 @@ static int e1000_probe(struct pci_dev *pdev, const
struct pci_device_id *ent)
/* make ready for any if (hw->...) below */
err = e1000_init_hw_struct(adapter, hw);
if (err)
- goto err_sw_init;
+ goto err_dma;
/* there is a workaround being applied below that limits
* 64-bit DMA addresses to 64-bit hardware. There are some
@@ -1239,8 +1239,9 @@ err_eeprom:
iounmap(hw->flash_address);
kfree(adapter->tx_ring);
kfree(adapter->rx_ring);
-err_dma:
err_sw_init:
+ netif_napi_del(&adapter->napi);
+err_dma:
err_mdio_ioremap:
iounmap(hw->ce4100_gbe_mdio_base_virt);
iounmap(hw->hw_addr);
@@ -1271,6 +1272,7 @@ static void e1000_remove(struct pci_dev *pdev)
e1000_down_and_stop(adapter);
e1000_release_manageability(adapter);
+ netif_napi_del(&adapter->napi);
unregister_netdev(netdev);
e1000_phy_hw_reset(hw);
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] e1000 in linux-3.18.0: a potential bug
2014-12-20 13:57 [PATCH] e1000 in linux-3.18.0: a potential bug Jia-Ju Bai
@ 2014-12-20 14:15 ` Sergei Shtylyov
0 siblings, 0 replies; 4+ messages in thread
From: Sergei Shtylyov @ 2014-12-20 14:15 UTC (permalink / raw)
To: Jia-Ju Bai, netdev
On 12/20/2014 4:57 PM, Jia-Ju Bai wrote:
> Maybe the email can not be displayed fully...
It was displayed alright.
> I send my patch again:
There was no need, especially without the patch description.
WBR, Sergei
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-12-20 14:15 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-12-20 13:57 [PATCH] e1000 in linux-3.18.0: a potential bug Jia-Ju Bai
2014-12-20 14:15 ` Sergei Shtylyov
-- strict thread matches above, loose matches on Subject: below --
2014-12-20 7:50 Jia-Ju Bai
2014-12-20 13:27 ` Sergei Shtylyov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).