Re: [PATCH] mlx4: Add support for EEH error recovery

From: Or Gerlitz <redacted>
Date: Sun, 22 Jul 2012 13:26:32 +0300

is there anything in the code you added which maybe implicitly
assumes PPC arch?

He implemented support for a standard PCI API in the kernel, he
happened to test it on a particular platform, and I think that's
the long and short of it.

On 07/22/2012 09:15 PM, David Miller wrote:

From: Or Gerlitz <redacted>
Date: Sun, 22 Jul 2012 13:26:32 +0300

quoted

is there anything in the code you added which maybe implicitly
assumes PPC arch?

He implemented support for a standard PCI API in the kernel, he
happened to test it on a particular platform, and I think that's
the long and short of it.

Exactly. The callbacks implemented are from standard PCI error recovery
(Documentation/PCI/pci-error-recovery.txt) and the changes doesn't
assume any platform in specific. The code was tested only on powerpc
systems since I don't have any mlx4 card on other platforms, however,
these changes shouldn't make the error recover any worse than the
current state.

-- 
Kleber Sacilotto de Souza
IBM Linux Technology Center

On 7/23/2012 4:18 PM, Kleber Sacilotto de Souza wrote:

Exactly. The callbacks implemented are from standard PCI error recovery
(Documentation/PCI/pci-error-recovery.txt) and the changes doesn't
assume any platform in specific. The code was tested only on powerpc
systems [...]

So how did you test that? using the kernel provided error injection 
support and user space tool (which?) or in another way? we've trying 
quickly here to inject errors using /sbin/ear-inject from 
ras-utils-6.1-1.el6.x86_64 on a kernel built with

CONFIG_PCIEAER=y
CONFIG_PCIEAER_INJECT=m

and it failed to inject errors, SB details.

Or.

since I don't have any mlx4 card on other platforms, however,
these changes shouldn't make the error recover any worse than the
current state.

# lspci | grep 08.00.1
08:00.1 Ethernet controller: Intel Corporation 82575EB Gigabit Network 
Connection (rev 02)

# cat /tmp/intel.aer
AER
BUS 8 DEV 0 FN 1
COR_STATUS BAD_TLP
HEADER_LOG 0 1 2 3

# /sbin/aer-inject < /tmp/intel.aer
Error: Failed to write, Invalid argument

# strace -F -f /sbin/aer-inject < /tmp/intel.aer
[...]

open("/dev/aer_inject", O_WRONLY)       = 3
write(3, "\10\0\1\0\0\0\0\0@\0\0\0\0\0\0\0\1\0\0\0\2\0\0\0\3\0\0\0", 
28) = -1 EINVAL (Invalid argument)
write(2, "Error: ", 7Error: )                  = 7
write(2, "Failed to write", 15Failed to write)         = 15
write(2, ", Invalid argument\n", 19, Invalid argument
)    = 19
exit_group(-1)                          = ?

On 07/23/2012 10:45 AM, Or Gerlitz wrote:

On 7/23/2012 4:18 PM, Kleber Sacilotto de Souza wrote:

quoted

Exactly. The callbacks implemented are from standard PCI error recovery
(Documentation/PCI/pci-error-recovery.txt) and the changes doesn't
assume any platform in specific. The code was tested only on powerpc
systems [...]

So how did you test that? using the kernel provided error injection
support and user space tool (which?) or in another way? we've trying
quickly here to inject errors using /sbin/ear-inject from
ras-utils-6.1-1.el6.x86_64 on a kernel built with

CONFIG_PCIEAER=y
CONFIG_PCIEAER_INJECT=m

For powerpc we have an IBM internal user space tool that injects the
error on the bus with the aid of the system firmware. The kernel used
was built with the option:

CONFIG_EEH=y

and without the AER options. I will run some more tests with the AER
options activated.

and it failed to inject errors, SB details.

Or.

quoted

since I don't have any mlx4 card on other platforms, however,
these changes shouldn't make the error recover any worse than the
current state.

quoted

# lspci | grep 08.00.1
08:00.1 Ethernet controller: Intel Corporation 82575EB Gigabit Network
Connection (rev 02)

quoted

# cat /tmp/intel.aer
AER
BUS 8 DEV 0 FN 1
COR_STATUS BAD_TLP
HEADER_LOG 0 1 2 3

quoted

# /sbin/aer-inject < /tmp/intel.aer
Error: Failed to write, Invalid argument

quoted

# strace -F -f /sbin/aer-inject < /tmp/intel.aer
[...]

quoted

open("/dev/aer_inject", O_WRONLY)       = 3
write(3, "\10\0\1\0\0\0\0\0@\0\0\0\0\0\0\0\1\0\0\0\2\0\0\0\3\0\0\0",
28) = -1 EINVAL (Invalid argument)
write(2, "Error: ", 7Error: )                  = 7
write(2, "Failed to write", 15Failed to write)         = 15
write(2, ", Invalid argument\n", 19, Invalid argument
)    = 19
exit_group(-1)                          = ?

-- 
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Kleber Sacilotto de Souza
IBM Linux Technology Center

On 07/23/2012 03:12 PM, Kleber Sacilotto de Souza wrote:

On 07/23/2012 10:45 AM, Or Gerlitz wrote:

quoted

On 7/23/2012 4:18 PM, Kleber Sacilotto de Souza wrote:

quoted

Exactly. The callbacks implemented are from standard PCI error recovery
(Documentation/PCI/pci-error-recovery.txt) and the changes doesn't
assume any platform in specific. The code was tested only on powerpc
systems [...]

So how did you test that? using the kernel provided error injection
support and user space tool (which?) or in another way? we've trying
quickly here to inject errors using /sbin/ear-inject from
ras-utils-6.1-1.el6.x86_64 on a kernel built with

CONFIG_PCIEAER=y
CONFIG_PCIEAER_INJECT=m

For powerpc we have an IBM internal user space tool that injects the
error on the bus with the aid of the system firmware. The kernel used
was built with the option:

CONFIG_EEH=y

and without the AER options. I will run some more tests with the AER
options activated.

I tested the powerpc error injection with

CONFIG_EEH=y
CONFIG_PCIEAER=y
CONFIG_PCIEAER_INJECT=m

and with the aer_inject module loaded and it didn't affect the EEH
recovery, the adapter recovered as expected.

quoted

and it failed to inject errors, SB details.

Or.

quoted

since I don't have any mlx4 card on other platforms, however,
these changes shouldn't make the error recover any worse than the
current state.

quoted

# lspci | grep 08.00.1
08:00.1 Ethernet controller: Intel Corporation 82575EB Gigabit Network
Connection (rev 02)

quoted

# cat /tmp/intel.aer
AER
BUS 8 DEV 0 FN 1
COR_STATUS BAD_TLP
HEADER_LOG 0 1 2 3

quoted

# /sbin/aer-inject < /tmp/intel.aer
Error: Failed to write, Invalid argument

quoted

# strace -F -f /sbin/aer-inject < /tmp/intel.aer
[...]

quoted

open("/dev/aer_inject", O_WRONLY)       = 3
write(3, "\10\0\1\0\0\0\0\0@\0\0\0\0\0\0\0\1\0\0\0\2\0\0\0\3\0\0\0",
28) = -1 EINVAL (Invalid argument)
write(2, "Error: ", 7Error: )                  = 7
write(2, "Failed to write", 15Failed to write)         = 15
write(2, ", Invalid argument\n", 19, Invalid argument
)    = 19
exit_group(-1)                          = ?

-- 
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Kleber Sacilotto de Souza
IBM Linux Technology Center

Kleber Sacilotto de Souza [off-list ref] wrote:

quoted

For powerpc we have an IBM internal user space tool that injects the
error on the bus with the aid of the system firmware. The kernel used
was built with the option:
CONFIG_EEH=y
and without the AER options. I will run some more tests with the AER
options activated.

I tested the powerpc error injection with

CONFIG_EEH=y
CONFIG_PCIEAER=y
CONFIG_PCIEAER_INJECT=m

and with the aer_inject module loaded and it didn't affect the EEH
recovery, the adapter recovered as expected.

I wasn't sure to follow what did you mean by "it didn't affect the EEH
recovery", how did you use the aer_inject module, is that through
user-space tool which is available for us?

Or.

From: Or Gerlitz <redacted>
Date: Tue, 24 Jul 2012 00:26:51 +0300

I wasn't sure to follow what did you mean by "it didn't affect the EEH
recovery", how did you use the aer_inject module, is that through
user-space tool which is available for us?

Can we please move forward, if he implemented the feature properly
and he tested it successfully, unless you can find a logic or
stylistic flaw in his patch please ACK it.

You can't hold his changes back while you work out how _YOU_ can
test it to your liking.

On Tue, Jul 24, 2012 at 12:34 AM, David Miller [off-list ref] wrote:

Can we please move forward, if he implemented the feature properly
and he tested it successfully, unless you can find a logic or
stylistic flaw in his patch please ACK it.

You can't hold his changes back while you work out how _YOU_ can
test it to your liking.

Hi Dave,

We're trying to act in  R/R (Responsive and Responsible) manner -
namely Shlomo did code review of the patches and we want to further
evaluate them by testing, I think its fully legitimate to test a patch
before ACK-ing.  Doing these types of tests isn't around my personal
typical daily menu and I'm asking for some directives from the author
on how to issue that testing, I don't see what wrong here. We're
planning anyway to go deeper around this area and enhance the PCI
hotplug /error handling related code in the driver, so there's an
initial learing curve here, makes sense? we can move the Q&A for the
testing to be off-list if you prefer it to go that way.

Or.

From: Or Gerlitz <redacted>
Date: Tue, 24 Jul 2012 00:42:08 +0300

On Tue, Jul 24, 2012 at 12:34 AM, David Miller [off-list ref] wrote:

quoted

Can we please move forward, if he implemented the feature properly
and he tested it successfully, unless you can find a logic or
stylistic flaw in his patch please ACK it.

You can't hold his changes back while you work out how _YOU_ can
test it to your liking.

Hi Dave,

We're trying to act in  R/R (Responsive and Responsible) manner -
namely Shlomo did code review of the patches and we want to further
evaluate them by testing, I think its fully legitimate to test a patch
before ACK-ing.  Doing these types of tests isn't around my personal
typical daily menu and I'm asking for some directives from the author
on how to issue that testing, I don't see what wrong here. We're
planning anyway to go deeper around this area and enhance the PCI
hotplug /error handling related code in the driver, so there's an
initial learing curve here, makes sense? we can move the Q&A for the
testing to be off-list if you prefer it to go that way.

But ACK his patch, because you have not found any problems with it.

This is taking days, and you're stalling further progress.

I never let patches rot in patchwork more than a few days, as this
patch has already.

Either ACK or provide a legitimate reason to reject it now.

On Tue, Jul 24, 2012 at 12:44 AM, David Miller [off-list ref] wrote:

But ACK his patch, because you have not found any problems with it.

Again, we wanted to test the patch before providing ACK, and it takes us a
bit to catch up on testing of this area, happens.

This is taking days, and you're stalling further progress.
I never let patches rot in patchwork more than a few days, as this patch has already.
Either ACK or provide a legitimate reason to reject it now.

understood. As its a bit off working hours here now, lets see what
tomorrow yield testing wise - but by tomorrow we either ACK or provide
reason to reject/change it.

Or.

From: Or Gerlitz <redacted>
Date: Tue, 24 Jul 2012 01:02:10 +0300

As its a bit off working hours here now, lets see what tomorrow
yield testing wise - but by tomorrow we either ACK or provide reason
to reject/change it.

Thank you.

On 07/23/2012 06:26 PM, Or Gerlitz wrote:

Kleber Sacilotto de Souza [off-list ref] wrote:

quoted

quoted

For powerpc we have an IBM internal user space tool that injects the
error on the bus with the aid of the system firmware. The kernel used
was built with the option:
CONFIG_EEH=y
and without the AER options. I will run some more tests with the AER
options activated.

quoted

I tested the powerpc error injection with

CONFIG_EEH=y
CONFIG_PCIEAER=y
CONFIG_PCIEAER_INJECT=m

and with the aer_inject module loaded and it didn't affect the EEH
recovery, the adapter recovered as expected.

I wasn't sure to follow what did you mean by "it didn't affect the EEH
recovery", how did you use the aer_inject module, is that through
user-space tool which is available for us?

I wanted to say that I was testing before only with the EEH option
activated, then I activated the AER options on my powerpc system just to
make sure these options when activate wouldn't affect the EEH recovery.
I haven't injected and AER error since I don't have a system with
hardware support for it.


Thanks,
-- 
Kleber Sacilotto de Souza
IBM Linux Technology Center

On 7/24/2012 4:12 PM, Kleber Sacilotto de Souza wrote:

On 07/23/2012 06:26 PM, Or Gerlitz wrote:

quoted

Kleber Sacilotto de Souza [off-list ref] wrote:

quoted

quoted

For powerpc we have an IBM internal user space tool that injects the
error on the bus with the aid of the system firmware. The kernel used
was built with the option:
CONFIG_EEH=y
and without the AER options. I will run some more tests with the AER
options activated.

I tested the powerpc error injection with

CONFIG_EEH=y
CONFIG_PCIEAER=y
CONFIG_PCIEAER_INJECT=m

and with the aer_inject module loaded and it didn't affect the EEH
recovery, the adapter recovered as expected.

I wasn't sure to follow what did you mean by "it didn't affect the EEH
recovery", how did you use the aer_inject module, is that through
user-space tool which is available for us?

I wanted to say that I was testing before only with the EEH option
activated, then I activated the AER options on my powerpc system just to
make sure these options when activate wouldn't affect the EEH recovery.
I haven't injected and AER error since I don't have a system with
hardware support for it.


Thanks,

Hi

Using a special extender card I've powered down the card.
None of the callbacks were called (I added printks to be sure).
Shouldn't one on the callbacks be called?

Shlomo Pongratz.

On 07/24/2012 02:09 PM, Shlomo Pongartz wrote:

On 7/24/2012 4:12 PM, Kleber Sacilotto de Souza wrote:

quoted

On 07/23/2012 06:26 PM, Or Gerlitz wrote:

quoted

Kleber Sacilotto de Souza [off-list ref] wrote:

quoted

quoted

For powerpc we have an IBM internal user space tool that injects the
error on the bus with the aid of the system firmware. The kernel used
was built with the option:
CONFIG_EEH=y
and without the AER options. I will run some more tests with the AER
options activated.

I tested the powerpc error injection with

CONFIG_EEH=y
CONFIG_PCIEAER=y
CONFIG_PCIEAER_INJECT=m

and with the aer_inject module loaded and it didn't affect the EEH
recovery, the adapter recovered as expected.

I wasn't sure to follow what did you mean by "it didn't affect the EEH
recovery", how did you use the aer_inject module, is that through
user-space tool which is available for us?

I wanted to say that I was testing before only with the EEH option
activated, then I activated the AER options on my powerpc system just to
make sure these options when activate wouldn't affect the EEH recovery.
I haven't injected and AER error since I don't have a system with
hardware support for it.


Thanks,

Hi

Using a special extender card I've powered down the card.
None of the callbacks were called (I added printks to be sure).
Shouldn't one on the callbacks be called?

Shlomo Pongratz.

What does this extender card do exactly? If it does hot plugging, it
will call the remove and probe callbacks.


-- 
Kleber Sacilotto de Souza
IBM Linux Technology Center

On Tue, Jul 24, 2012 at 02:35:51PM -0300, Kleber Sacilotto de Souza wrote:

On 07/24/2012 02:09 PM, Shlomo Pongartz wrote:

quoted

On 7/24/2012 4:12 PM, Kleber Sacilotto de Souza wrote:

quoted

On 07/23/2012 06:26 PM, Or Gerlitz wrote:

quoted

Kleber Sacilotto de Souza [off-list ref] wrote:

quoted

quoted

For powerpc we have an IBM internal user space tool that injects the
error on the bus with the aid of the system firmware. The kernel used
was built with the option:
CONFIG_EEH=y
and without the AER options. I will run some more tests with the AER
options activated.

I tested the powerpc error injection with

CONFIG_EEH=y
CONFIG_PCIEAER=y
CONFIG_PCIEAER_INJECT=m

and with the aer_inject module loaded and it didn't affect the EEH
recovery, the adapter recovered as expected.

I wasn't sure to follow what did you mean by "it didn't affect the EEH
recovery", how did you use the aer_inject module, is that through
user-space tool which is available for us?

I wanted to say that I was testing before only with the EEH option
activated, then I activated the AER options on my powerpc system just to
make sure these options when activate wouldn't affect the EEH recovery.
I haven't injected and AER error since I don't have a system with
hardware support for it.


Thanks,

Hi

Using a special extender card I've powered down the card.
None of the callbacks were called (I added printks to be sure).
Shouldn't one on the callbacks be called?

Shlomo Pongratz.

What does this extender card do exactly? If it does hot plugging, it
will call the remove and probe callbacks.


-- 
Kleber Sacilotto de Souza
IBM Linux Technology Center

I assume it just powers down the card, ie, it will not respond anymore
to any bus messages, which may cause an error report.

Shlomo, is it the system you are using AER-capable? From what I see from
code and documentation, you must have root ports which support AER. Is
that the case?

Regards.
Cascardo.

On Tue, Jul 24, 2012 at 02:35:51PM -0300, Kleber Sacilotto de Souza wrote:

On 07/24/2012 02:09 PM, Shlomo Pongartz wrote:

quoted

On 7/24/2012 4:12 PM, Kleber Sacilotto de Souza wrote:

quoted

On 07/23/2012 06:26 PM, Or Gerlitz wrote:

quoted

Kleber Sacilotto de Souza [off-list ref] wrote:

quoted

quoted

For powerpc we have an IBM internal user space tool that injects the
error on the bus with the aid of the system firmware. The kernel used
was built with the option:
CONFIG_EEH=y
and without the AER options. I will run some more tests with the AER
options activated.

I tested the powerpc error injection with

CONFIG_EEH=y
CONFIG_PCIEAER=y
CONFIG_PCIEAER_INJECT=m

and with the aer_inject module loaded and it didn't affect the EEH
recovery, the adapter recovered as expected.

I wasn't sure to follow what did you mean by "it didn't affect the EEH
recovery", how did you use the aer_inject module, is that through
user-space tool which is available for us?

I wanted to say that I was testing before only with the EEH option
activated, then I activated the AER options on my powerpc system just to
make sure these options when activate wouldn't affect the EEH recovery.
I haven't injected and AER error since I don't have a system with
hardware support for it.


Thanks,

Hi

Using a special extender card I've powered down the card.
None of the callbacks were called (I added printks to be sure).
Shouldn't one on the callbacks be called?

Shlomo Pongratz.

What does this extender card do exactly? If it does hot plugging, it
will call the remove and probe callbacks.


--
Kleber Sacilotto de Souza
IBM Linux Technology Center

I assume it just powers down the card, ie, it will not respond anymore
to any bus messages, which may cause an error report.

Shlomo, is it the system you are using AER-capable? From what I see from
code and documentation, you must have root ports which support AER. Is
that the case?

Regards.
Cascardo.


The extender card is a passive card that only enables me to power on/off the PCI card stacked on it.
I'll check the mother board and BIOS ASAP.

Regards,
Shlomo

________________________________________
From: Thadeu Lima de Souza Cascardo [cascardo@linux.vnet.ibm.com]
Sent: Tuesday, July 24, 2012 9:08 PM
To: Kleber Sacilotto de Souza
Cc: Shlomo Pongratz; Or Gerlitz; Or Gerlitz; David Miller; netdev@vger.kernel.org; jackm@dev.mellanox.co.il; Yevgeny Petrilin; brking@linux.vnet.ibm.com
Subject: Re: [PATCH] mlx4: Add support for EEH error recovery

On Tue, Jul 24, 2012 at 02:35:51PM -0300, Kleber Sacilotto de Souza wrote:

On 07/24/2012 02:09 PM, Shlomo Pongartz wrote:

quoted

On 7/24/2012 4:12 PM, Kleber Sacilotto de Souza wrote:

quoted

On 07/23/2012 06:26 PM, Or Gerlitz wrote:

quoted

Kleber Sacilotto de Souza [off-list ref] wrote:

quoted

quoted

For powerpc we have an IBM internal user space tool that injects the
error on the bus with the aid of the system firmware. The kernel used
was built with the option:
CONFIG_EEH=y
and without the AER options. I will run some more tests with the AER
options activated.

I tested the powerpc error injection with

CONFIG_EEH=y
CONFIG_PCIEAER=y
CONFIG_PCIEAER_INJECT=m

and with the aer_inject module loaded and it didn't affect the EEH
recovery, the adapter recovered as expected.

I wasn't sure to follow what did you mean by "it didn't affect the EEH
recovery", how did you use the aer_inject module, is that through
user-space tool which is available for us?

I wanted to say that I was testing before only with the EEH option
activated, then I activated the AER options on my powerpc system just to
make sure these options when activate wouldn't affect the EEH recovery.
I haven't injected and AER error since I don't have a system with
hardware support for it.


Thanks,

Hi

Using a special extender card I've powered down the card.
None of the callbacks were called (I added printks to be sure).
Shouldn't one on the callbacks be called?

Shlomo Pongratz.

What does this extender card do exactly? If it does hot plugging, it
will call the remove and probe callbacks.


--
Kleber Sacilotto de Souza
IBM Linux Technology Center

I assume it just powers down the card, ie, it will not respond anymore
to any bus messages, which may cause an error report.

Shlomo, is it the system you are using AER-capable? From what I see from
code and documentation, you must have root ports which support AER. Is
that the case?

Regards.
Cascardo.