* [Intel-wired-lan] [PATCH net v2] igb: re-assign hw address pointer on reset after PCI error
@ 2016-11-10 18:46 ` Guilherme G. Piccoli
0 siblings, 0 replies; 8+ messages in thread
From: Guilherme G. Piccoli @ 2016-11-10 18:46 UTC (permalink / raw)
To: intel-wired-lan
Whenever the igb driver detects the result of a read operation returns
a value composed only by F's (like 0xFFFFFFFF), it will detach the
net_device, clear the hw_addr pointer and warn to the user that adapter's
link is lost - those steps happen on igb_rd32().
In case a PCI error happens on Power architecture, there's a recovery
mechanism called EEH, that will reset the PCI slot and call driver's
handlers to reset the adapter and network functionality as well.
We observed that once hw_addr is NULL after the error is detected on
igb_rd32(), it's never assigned back, so in the process of resetting
the network functionality we got a NULL pointer dereference in both
igb_configure_tx_ring() and igb_configure_rx_ring(). In order to avoid
such bug, this patch re-assigns the hw_addr value in the slot_reset
handler.
Reported-by: Anthony H. Thai <ahthai@us.ibm.com>
Reported-by: Harsha Thyagaraja <hathyaga@in.ibm.com>
Signed-off-by: Guilherme G. Piccoli <gpiccoli@linux.vnet.ibm.com>
---
drivers/net/ethernet/intel/igb/igb_main.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index edc9a6a..136ee9e 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -7878,6 +7878,11 @@ static pci_ers_result_t igb_io_slot_reset(struct pci_dev *pdev)
pci_enable_wake(pdev, PCI_D3hot, 0);
pci_enable_wake(pdev, PCI_D3cold, 0);
+ /* In case of PCI error, adapter lose its HW address
+ * so we should re-assign it here.
+ */
+ hw->hw_addr = adapter->io_addr;
+
igb_reset(adapter);
wr32(E1000_WUS, ~0);
result = PCI_ERS_RESULT_RECOVERED;
--
2.1.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH net v2] igb: re-assign hw address pointer on reset after PCI error
@ 2016-11-10 18:46 ` Guilherme G. Piccoli
0 siblings, 0 replies; 8+ messages in thread
From: Guilherme G. Piccoli @ 2016-11-10 18:46 UTC (permalink / raw)
To: intel-wired-lan
Cc: jeffrey.t.kirsher, gpiccoli, netdev, alexander.duyck,
todd.fujinaka
Whenever the igb driver detects the result of a read operation returns
a value composed only by F's (like 0xFFFFFFFF), it will detach the
net_device, clear the hw_addr pointer and warn to the user that adapter's
link is lost - those steps happen on igb_rd32().
In case a PCI error happens on Power architecture, there's a recovery
mechanism called EEH, that will reset the PCI slot and call driver's
handlers to reset the adapter and network functionality as well.
We observed that once hw_addr is NULL after the error is detected on
igb_rd32(), it's never assigned back, so in the process of resetting
the network functionality we got a NULL pointer dereference in both
igb_configure_tx_ring() and igb_configure_rx_ring(). In order to avoid
such bug, this patch re-assigns the hw_addr value in the slot_reset
handler.
Reported-by: Anthony H. Thai <ahthai@us.ibm.com>
Reported-by: Harsha Thyagaraja <hathyaga@in.ibm.com>
Signed-off-by: Guilherme G. Piccoli <gpiccoli@linux.vnet.ibm.com>
---
drivers/net/ethernet/intel/igb/igb_main.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index edc9a6a..136ee9e 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -7878,6 +7878,11 @@ static pci_ers_result_t igb_io_slot_reset(struct pci_dev *pdev)
pci_enable_wake(pdev, PCI_D3hot, 0);
pci_enable_wake(pdev, PCI_D3cold, 0);
+ /* In case of PCI error, adapter lose its HW address
+ * so we should re-assign it here.
+ */
+ hw->hw_addr = adapter->io_addr;
+
igb_reset(adapter);
wr32(E1000_WUS, ~0);
result = PCI_ERS_RESULT_RECOVERED;
--
2.1.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Intel-wired-lan] [PATCH net v2] igb: re-assign hw address pointer on reset after PCI error
2016-11-10 18:46 ` Guilherme G. Piccoli
@ 2016-12-02 12:55 ` Guilherme G. Piccoli
-1 siblings, 0 replies; 8+ messages in thread
From: Guilherme G. Piccoli @ 2016-12-02 12:55 UTC (permalink / raw)
To: intel-wired-lan
On 11/10/2016 04:46 PM, Guilherme G. Piccoli wrote:
> Whenever the igb driver detects the result of a read operation returns
> a value composed only by F's (like 0xFFFFFFFF), it will detach the
> net_device, clear the hw_addr pointer and warn to the user that adapter's
> link is lost - those steps happen on igb_rd32().
>
> In case a PCI error happens on Power architecture, there's a recovery
> mechanism called EEH, that will reset the PCI slot and call driver's
> handlers to reset the adapter and network functionality as well.
>
> We observed that once hw_addr is NULL after the error is detected on
> igb_rd32(), it's never assigned back, so in the process of resetting
> the network functionality we got a NULL pointer dereference in both
> igb_configure_tx_ring() and igb_configure_rx_ring(). In order to avoid
> such bug, this patch re-assigns the hw_addr value in the slot_reset
> handler.
>
> Reported-by: Anthony H. Thai <ahthai@us.ibm.com>
> Reported-by: Harsha Thyagaraja <hathyaga@in.ibm.com>
> Signed-off-by: Guilherme G. Piccoli <gpiccoli@linux.vnet.ibm.com>
> ---
> drivers/net/ethernet/intel/igb/igb_main.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
> index edc9a6a..136ee9e 100644
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -7878,6 +7878,11 @@ static pci_ers_result_t igb_io_slot_reset(struct pci_dev *pdev)
> pci_enable_wake(pdev, PCI_D3hot, 0);
> pci_enable_wake(pdev, PCI_D3cold, 0);
>
> + /* In case of PCI error, adapter lose its HW address
> + * so we should re-assign it here.
> + */
> + hw->hw_addr = adapter->io_addr;
> +
> igb_reset(adapter);
> wr32(E1000_WUS, ~0);
> result = PCI_ERS_RESULT_RECOVERED;
>
Ping?
Sorry to annoy, any news on this?
Thanks in advance!
Cheers,
Guilherme
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net v2] igb: re-assign hw address pointer on reset after PCI error
@ 2016-12-02 12:55 ` Guilherme G. Piccoli
0 siblings, 0 replies; 8+ messages in thread
From: Guilherme G. Piccoli @ 2016-12-02 12:55 UTC (permalink / raw)
To: intel-wired-lan, jeffrey.t.kirsher, alexander.duyck; +Cc: netdev, todd.fujinaka
On 11/10/2016 04:46 PM, Guilherme G. Piccoli wrote:
> Whenever the igb driver detects the result of a read operation returns
> a value composed only by F's (like 0xFFFFFFFF), it will detach the
> net_device, clear the hw_addr pointer and warn to the user that adapter's
> link is lost - those steps happen on igb_rd32().
>
> In case a PCI error happens on Power architecture, there's a recovery
> mechanism called EEH, that will reset the PCI slot and call driver's
> handlers to reset the adapter and network functionality as well.
>
> We observed that once hw_addr is NULL after the error is detected on
> igb_rd32(), it's never assigned back, so in the process of resetting
> the network functionality we got a NULL pointer dereference in both
> igb_configure_tx_ring() and igb_configure_rx_ring(). In order to avoid
> such bug, this patch re-assigns the hw_addr value in the slot_reset
> handler.
>
> Reported-by: Anthony H. Thai <ahthai@us.ibm.com>
> Reported-by: Harsha Thyagaraja <hathyaga@in.ibm.com>
> Signed-off-by: Guilherme G. Piccoli <gpiccoli@linux.vnet.ibm.com>
> ---
> drivers/net/ethernet/intel/igb/igb_main.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
> index edc9a6a..136ee9e 100644
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -7878,6 +7878,11 @@ static pci_ers_result_t igb_io_slot_reset(struct pci_dev *pdev)
> pci_enable_wake(pdev, PCI_D3hot, 0);
> pci_enable_wake(pdev, PCI_D3cold, 0);
>
> + /* In case of PCI error, adapter lose its HW address
> + * so we should re-assign it here.
> + */
> + hw->hw_addr = adapter->io_addr;
> +
> igb_reset(adapter);
> wr32(E1000_WUS, ~0);
> result = PCI_ERS_RESULT_RECOVERED;
>
Ping?
Sorry to annoy, any news on this?
Thanks in advance!
Cheers,
Guilherme
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Intel-wired-lan] [PATCH net v2] igb: re-assign hw address pointer on reset after PCI error
2016-12-02 12:55 ` Guilherme G. Piccoli
@ 2016-12-02 16:41 ` Keller, Jacob E
-1 siblings, 0 replies; 8+ messages in thread
From: Keller, Jacob E @ 2016-12-02 16:41 UTC (permalink / raw)
To: intel-wired-lan
On Fri, 2016-12-02 at 10:55 -0200, Guilherme G. Piccoli wrote:
> On 11/10/2016 04:46 PM, Guilherme G. Piccoli wrote:
> > Whenever the igb driver detects the result of a read operation
> > returns
> > a value composed only by F's (like 0xFFFFFFFF), it will detach the
> > net_device, clear the hw_addr pointer and warn to the user that
> > adapter's
> > link is lost - those steps happen on igb_rd32().
> >
> > In case a PCI error happens on Power architecture, there's a
> > recovery
> > mechanism called EEH, that will reset the PCI slot and call
> > driver's
> > handlers to reset the adapter and network functionality as well.
> >
> > We observed that once hw_addr is NULL after the error is detected
> > on
> > igb_rd32(), it's never assigned back, so in the process of
> > resetting
> > the network functionality we got a NULL pointer dereference in both
> > igb_configure_tx_ring() and igb_configure_rx_ring(). In order to
> > avoid
> > such bug, this patch re-assigns the hw_addr value in the slot_reset
> > handler.
> >
> > Reported-by: Anthony H. Thai <ahthai@us.ibm.com>
> > Reported-by: Harsha Thyagaraja <hathyaga@in.ibm.com>
> > Signed-off-by: Guilherme G. Piccoli <gpiccoli@linux.vnet.ibm.com>
> > ---
> > ?drivers/net/ethernet/intel/igb/igb_main.c | 5 +++++
> > ?1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/intel/igb/igb_main.c
> > b/drivers/net/ethernet/intel/igb/igb_main.c
> > index edc9a6a..136ee9e 100644
> > --- a/drivers/net/ethernet/intel/igb/igb_main.c
> > +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> > @@ -7878,6 +7878,11 @@ static pci_ers_result_t
> > igb_io_slot_reset(struct pci_dev *pdev)
> > ? pci_enable_wake(pdev, PCI_D3hot, 0);
> > ? pci_enable_wake(pdev, PCI_D3cold, 0);
> >
> > + /* In case of PCI error, adapter lose its HW
> > address
> > + ?* so we should re-assign it here.
> > + ?*/
> > + hw->hw_addr = adapter->io_addr;
> > +
> > ? igb_reset(adapter);
> > ? wr32(E1000_WUS, ~0);
> > ? result = PCI_ERS_RESULT_RECOVERED;
> >
>
> Ping?
>
> Sorry to annoy, any news on this?
> Thanks in advance!
>
> Cheers,
>
>
> Guilherme
>
This seems reasonable. It's similar to what fm10k driver does under
this circumstance.
Thanks,
Jake
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net v2] igb: re-assign hw address pointer on reset after PCI error
@ 2016-12-02 16:41 ` Keller, Jacob E
0 siblings, 0 replies; 8+ messages in thread
From: Keller, Jacob E @ 2016-12-02 16:41 UTC (permalink / raw)
To: gpiccoli@linux.vnet.ibm.com, intel-wired-lan@lists.osuosl.org,
Kirsher, Jeffrey T, alexander.duyck@gmail.com
Cc: netdev@vger.kernel.org, Fujinaka, Todd
On Fri, 2016-12-02 at 10:55 -0200, Guilherme G. Piccoli wrote:
> On 11/10/2016 04:46 PM, Guilherme G. Piccoli wrote:
> > Whenever the igb driver detects the result of a read operation
> > returns
> > a value composed only by F's (like 0xFFFFFFFF), it will detach the
> > net_device, clear the hw_addr pointer and warn to the user that
> > adapter's
> > link is lost - those steps happen on igb_rd32().
> >
> > In case a PCI error happens on Power architecture, there's a
> > recovery
> > mechanism called EEH, that will reset the PCI slot and call
> > driver's
> > handlers to reset the adapter and network functionality as well.
> >
> > We observed that once hw_addr is NULL after the error is detected
> > on
> > igb_rd32(), it's never assigned back, so in the process of
> > resetting
> > the network functionality we got a NULL pointer dereference in both
> > igb_configure_tx_ring() and igb_configure_rx_ring(). In order to
> > avoid
> > such bug, this patch re-assigns the hw_addr value in the slot_reset
> > handler.
> >
> > Reported-by: Anthony H. Thai <ahthai@us.ibm.com>
> > Reported-by: Harsha Thyagaraja <hathyaga@in.ibm.com>
> > Signed-off-by: Guilherme G. Piccoli <gpiccoli@linux.vnet.ibm.com>
> > ---
> > drivers/net/ethernet/intel/igb/igb_main.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/intel/igb/igb_main.c
> > b/drivers/net/ethernet/intel/igb/igb_main.c
> > index edc9a6a..136ee9e 100644
> > --- a/drivers/net/ethernet/intel/igb/igb_main.c
> > +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> > @@ -7878,6 +7878,11 @@ static pci_ers_result_t
> > igb_io_slot_reset(struct pci_dev *pdev)
> > pci_enable_wake(pdev, PCI_D3hot, 0);
> > pci_enable_wake(pdev, PCI_D3cold, 0);
> >
> > + /* In case of PCI error, adapter lose its HW
> > address
> > + * so we should re-assign it here.
> > + */
> > + hw->hw_addr = adapter->io_addr;
> > +
> > igb_reset(adapter);
> > wr32(E1000_WUS, ~0);
> > result = PCI_ERS_RESULT_RECOVERED;
> >
>
> Ping?
>
> Sorry to annoy, any news on this?
> Thanks in advance!
>
> Cheers,
>
>
> Guilherme
>
This seems reasonable. It's similar to what fm10k driver does under
this circumstance.
Thanks,
Jake
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Intel-wired-lan] [PATCH net v2] igb: re-assign hw address pointer on reset after PCI error
2016-11-10 18:46 ` Guilherme G. Piccoli
@ 2016-12-13 20:51 ` Brown, Aaron F
-1 siblings, 0 replies; 8+ messages in thread
From: Brown, Aaron F @ 2016-12-13 20:51 UTC (permalink / raw)
To: intel-wired-lan
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Guilherme G. Piccoli
> Sent: Thursday, November 10, 2016 10:47 AM
> To: intel-wired-lan at lists.osuosl.org
> Cc: netdev at vger.kernel.org; gpiccoli at linux.vnet.ibm.com
> Subject: [Intel-wired-lan] [PATCH net v2] igb: re-assign hw address pointer on
> reset after PCI error
>
> Whenever the igb driver detects the result of a read operation returns
> a value composed only by F's (like 0xFFFFFFFF), it will detach the
> net_device, clear the hw_addr pointer and warn to the user that adapter's
> link is lost - those steps happen on igb_rd32().
>
> In case a PCI error happens on Power architecture, there's a recovery
> mechanism called EEH, that will reset the PCI slot and call driver's
> handlers to reset the adapter and network functionality as well.
>
> We observed that once hw_addr is NULL after the error is detected on
> igb_rd32(), it's never assigned back, so in the process of resetting
> the network functionality we got a NULL pointer dereference in both
> igb_configure_tx_ring() and igb_configure_rx_ring(). In order to avoid
> such bug, this patch re-assigns the hw_addr value in the slot_reset
> handler.
>
> Reported-by: Anthony H. Thai <ahthai@us.ibm.com>
> Reported-by: Harsha Thyagaraja <hathyaga@in.ibm.com>
> Signed-off-by: Guilherme G. Piccoli <gpiccoli@linux.vnet.ibm.com>
> ---
> drivers/net/ethernet/intel/igb/igb_main.c | 5 +++++
> 1 file changed, 5 insertions(+)
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [Intel-wired-lan] [PATCH net v2] igb: re-assign hw address pointer on reset after PCI error
@ 2016-12-13 20:51 ` Brown, Aaron F
0 siblings, 0 replies; 8+ messages in thread
From: Brown, Aaron F @ 2016-12-13 20:51 UTC (permalink / raw)
To: Guilherme G. Piccoli, intel-wired-lan@lists.osuosl.org
Cc: netdev@vger.kernel.org
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@lists.osuosl.org] On
> Behalf Of Guilherme G. Piccoli
> Sent: Thursday, November 10, 2016 10:47 AM
> To: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org; gpiccoli@linux.vnet.ibm.com
> Subject: [Intel-wired-lan] [PATCH net v2] igb: re-assign hw address pointer on
> reset after PCI error
>
> Whenever the igb driver detects the result of a read operation returns
> a value composed only by F's (like 0xFFFFFFFF), it will detach the
> net_device, clear the hw_addr pointer and warn to the user that adapter's
> link is lost - those steps happen on igb_rd32().
>
> In case a PCI error happens on Power architecture, there's a recovery
> mechanism called EEH, that will reset the PCI slot and call driver's
> handlers to reset the adapter and network functionality as well.
>
> We observed that once hw_addr is NULL after the error is detected on
> igb_rd32(), it's never assigned back, so in the process of resetting
> the network functionality we got a NULL pointer dereference in both
> igb_configure_tx_ring() and igb_configure_rx_ring(). In order to avoid
> such bug, this patch re-assigns the hw_addr value in the slot_reset
> handler.
>
> Reported-by: Anthony H. Thai <ahthai@us.ibm.com>
> Reported-by: Harsha Thyagaraja <hathyaga@in.ibm.com>
> Signed-off-by: Guilherme G. Piccoli <gpiccoli@linux.vnet.ibm.com>
> ---
> drivers/net/ethernet/intel/igb/igb_main.c | 5 +++++
> 1 file changed, 5 insertions(+)
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-12-13 20:51 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-11-10 18:46 [Intel-wired-lan] [PATCH net v2] igb: re-assign hw address pointer on reset after PCI error Guilherme G. Piccoli
2016-11-10 18:46 ` Guilherme G. Piccoli
2016-12-02 12:55 ` [Intel-wired-lan] " Guilherme G. Piccoli
2016-12-02 12:55 ` Guilherme G. Piccoli
2016-12-02 16:41 ` [Intel-wired-lan] " Keller, Jacob E
2016-12-02 16:41 ` Keller, Jacob E
2016-12-13 20:51 ` [Intel-wired-lan] " Brown, Aaron F
2016-12-13 20:51 ` Brown, Aaron F
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.