* Re: [Intel-wired-lan] [PATCH v2 iwl-net] igc: don't fail igc_probe() on LED setup error
2025-09-10 13:47 [PATCH v2 iwl-net] igc: don't fail igc_probe() on LED setup error Kohei Enju
@ 2025-09-10 13:50 ` Paul Menzel
2025-09-11 6:36 ` Loktionov, Aleksandr
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Paul Menzel @ 2025-09-10 13:50 UTC (permalink / raw)
To: Kohei Enju
Cc: intel-wired-lan, netdev, Tony Nguyen, Przemek Kitszel,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Kurt Kanzenbach, Aleksandr Loktionov,
Vitaly Lifshits, kohei.enju
Dear Kohei,
Thank you for your patch.
Am 10.09.25 um 15:47 schrieb Kohei Enju:
> When igc_led_setup() fails, igc_probe() fails and triggers kernel panic
> in free_netdev() since unregister_netdev() is not called. [1]
> This behavior can be tested using fault-injection framework, especially
> the failslab feature. [2]
>
> Since LED support is not mandatory, treat LED setup failures as
> non-fatal and continue probe with a warning message, consequently
> avoiding the kernel panic.
>
> [1]
> kernel BUG at net/core/dev.c:12047!
> Oops: invalid opcode: 0000 [#1] SMP NOPTI
> CPU: 0 UID: 0 PID: 937 Comm: repro-igc-led-e Not tainted 6.17.0-rc4-enjuk-tnguy-00865-gc4940196ab02 #64 PREEMPT(voluntary)
> Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
> RIP: 0010:free_netdev+0x278/0x2b0
> [...]
> Call Trace:
> <TASK>
> igc_probe+0x370/0x910
> local_pci_probe+0x3a/0x80
> pci_device_probe+0xd1/0x200
> [...]
>
> [2]
> #!/bin/bash -ex
>
> FAILSLAB_PATH=/sys/kernel/debug/failslab/
> DEVICE=0000:00:05.0
> START_ADDR=$(grep " igc_led_setup" /proc/kallsyms \
> | awk '{printf("0x%s", $1)}')
> END_ADDR=$(printf "0x%x" $((START_ADDR + 0x100)))
>
> echo $START_ADDR > $FAILSLAB_PATH/require-start
> echo $END_ADDR > $FAILSLAB_PATH/require-end
> echo 1 > $FAILSLAB_PATH/times
> echo 100 > $FAILSLAB_PATH/probability
> echo N > $FAILSLAB_PATH/ignore-gfp-wait
>
> echo $DEVICE > /sys/bus/pci/drivers/igc/bind
Sweet!
> Fixes: ea578703b03d ("igc: Add support for LEDs on i225/i226")
> Signed-off-by: Kohei Enju <enjuk@amazon.com>
> ---
> Changes:
> v1->v2:
> - don't fail probe when led setup fails
> - rephrase subject and commit message
> v1: https://lore.kernel.org/intel-wired-lan/20250906055239.29396-1-enjuk@amazon.com/
> ---
> drivers/net/ethernet/intel/igc/igc.h | 1 +
> drivers/net/ethernet/intel/igc/igc_main.c | 12 +++++++++---
> 2 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/igc/igc.h b/drivers/net/ethernet/intel/igc/igc.h
> index 266bfcf2a28f..a427f05814c1 100644
> --- a/drivers/net/ethernet/intel/igc/igc.h
> +++ b/drivers/net/ethernet/intel/igc/igc.h
> @@ -345,6 +345,7 @@ struct igc_adapter {
> /* LEDs */
> struct mutex led_mutex;
> struct igc_led_classdev *leds;
> + bool leds_available;
> };
>
> void igc_up(struct igc_adapter *adapter);
> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
> index e79b14d50b24..728d7ca5338b 100644
> --- a/drivers/net/ethernet/intel/igc/igc_main.c
> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
> @@ -7335,8 +7335,14 @@ static int igc_probe(struct pci_dev *pdev,
>
> if (IS_ENABLED(CONFIG_IGC_LEDS)) {
> err = igc_led_setup(adapter);
> - if (err)
> - goto err_register;
> + if (err) {
> + netdev_warn_once(netdev,
> + "LED init failed (%d); continuing without LED support\n",
> + err);
> + adapter->leds_available = false;
> + } else {
> + adapter->leds_available = true;
> + }
> }
>
> return 0;
> @@ -7392,7 +7398,7 @@ static void igc_remove(struct pci_dev *pdev)
> cancel_work_sync(&adapter->watchdog_task);
> hrtimer_cancel(&adapter->hrtimer);
>
> - if (IS_ENABLED(CONFIG_IGC_LEDS))
> + if (IS_ENABLED(CONFIG_IGC_LEDS) && adapter->leds_available)
> igc_led_free(adapter);
>
> /* Release control of h/w to f/w. If f/w is AMT enabled, this
Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
Kind regards,
Paul
^ permalink raw reply [flat|nested] 6+ messages in thread* RE: [PATCH v2 iwl-net] igc: don't fail igc_probe() on LED setup error
2025-09-10 13:47 [PATCH v2 iwl-net] igc: don't fail igc_probe() on LED setup error Kohei Enju
2025-09-10 13:50 ` [Intel-wired-lan] " Paul Menzel
@ 2025-09-11 6:36 ` Loktionov, Aleksandr
2025-09-11 7:04 ` Lifshits, Vitaly
2025-09-11 7:27 ` Kurt Kanzenbach
2025-09-14 16:48 ` [Intel-wired-lan] " Mor Bar-Gabay
3 siblings, 1 reply; 6+ messages in thread
From: Loktionov, Aleksandr @ 2025-09-11 6:36 UTC (permalink / raw)
To: Kohei Enju, intel-wired-lan@lists.osuosl.org,
netdev@vger.kernel.org
Cc: Nguyen, Anthony L, Kitszel, Przemyslaw, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Kurt Kanzenbach, Lifshits, Vitaly, kohei.enju@gmail.com
> -----Original Message-----
> From: Kohei Enju <enjuk@amazon.com>
> Sent: Wednesday, September 10, 2025 3:47 PM
> To: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org
> Cc: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel,
> Przemyslaw <przemyslaw.kitszel@intel.com>; Andrew Lunn
> <andrew+netdev@lunn.ch>; David S. Miller <davem@davemloft.net>; Eric
> Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo
> Abeni <pabeni@redhat.com>; Kurt Kanzenbach <kurt@linutronix.de>;
> Loktionov, Aleksandr <aleksandr.loktionov@intel.com>; Lifshits, Vitaly
> <vitaly.lifshits@intel.com>; kohei.enju@gmail.com; Kohei Enju
> <enjuk@amazon.com>
> Subject: [PATCH v2 iwl-net] igc: don't fail igc_probe() on LED setup
> error
>
...
>
> FAILSLAB_PATH=/sys/kernel/debug/failslab/
> DEVICE=0000:00:05.0
> START_ADDR=$(grep " igc_led_setup" /proc/kallsyms \
> | awk '{printf("0x%s", $1)}')
> END_ADDR=$(printf "0x%x" $((START_ADDR + 0x100)))
>
> echo $START_ADDR > $FAILSLAB_PATH/require-start
> echo $END_ADDR > $FAILSLAB_PATH/require-end
> echo 1 > $FAILSLAB_PATH/times
> echo 100 > $FAILSLAB_PATH/probability
> echo N > $FAILSLAB_PATH/ignore-gfp-wait
>
> echo $DEVICE > /sys/bus/pci/drivers/igc/bind
>
Using fault-injection test using failslab - excellent!
> Fixes: ea578703b03d ("igc: Add support for LEDs on i225/i226")
> Signed-off-by: Kohei Enju <enjuk@amazon.com>
> ---
> Changes:
> v1->v2:
> - don't fail probe when led setup fails
> - rephrase subject and commit message
> v1: https://lore.kernel.org/intel-wired-lan/20250906055239.29396-1-
> enjuk@amazon.com/
> ---
...
> /* Release control of h/w to f/w. If f/w is AMT enabled, this
> --
> 2.48.1
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2 iwl-net] igc: don't fail igc_probe() on LED setup error
2025-09-11 6:36 ` Loktionov, Aleksandr
@ 2025-09-11 7:04 ` Lifshits, Vitaly
0 siblings, 0 replies; 6+ messages in thread
From: Lifshits, Vitaly @ 2025-09-11 7:04 UTC (permalink / raw)
To: Loktionov, Aleksandr, Kohei Enju,
intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org
Cc: Nguyen, Anthony L, Kitszel, Przemyslaw, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Kurt Kanzenbach, kohei.enju@gmail.com
On 9/11/2025 9:36 AM, Loktionov, Aleksandr wrote:
>
>
>> -----Original Message-----
>> From: Kohei Enju <enjuk@amazon.com>
>> Sent: Wednesday, September 10, 2025 3:47 PM
>> To: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org
>> Cc: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel,
>> Przemyslaw <przemyslaw.kitszel@intel.com>; Andrew Lunn
>> <andrew+netdev@lunn.ch>; David S. Miller <davem@davemloft.net>; Eric
>> Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo
>> Abeni <pabeni@redhat.com>; Kurt Kanzenbach <kurt@linutronix.de>;
>> Loktionov, Aleksandr <aleksandr.loktionov@intel.com>; Lifshits, Vitaly
>> <vitaly.lifshits@intel.com>; kohei.enju@gmail.com; Kohei Enju
>> <enjuk@amazon.com>
>> Subject: [PATCH v2 iwl-net] igc: don't fail igc_probe() on LED setup
>> error
>>
>
> ...
>
>>
>> FAILSLAB_PATH=/sys/kernel/debug/failslab/
>> DEVICE=0000:00:05.0
>> START_ADDR=$(grep " igc_led_setup" /proc/kallsyms \
>> | awk '{printf("0x%s", $1)}')
>> END_ADDR=$(printf "0x%x" $((START_ADDR + 0x100)))
>>
>> echo $START_ADDR > $FAILSLAB_PATH/require-start
>> echo $END_ADDR > $FAILSLAB_PATH/require-end
>> echo 1 > $FAILSLAB_PATH/times
>> echo 100 > $FAILSLAB_PATH/probability
>> echo N > $FAILSLAB_PATH/ignore-gfp-wait
>>
>> echo $DEVICE > /sys/bus/pci/drivers/igc/bind
>>
> Using fault-injection test using failslab - excellent!
>
>> Fixes: ea578703b03d ("igc: Add support for LEDs on i225/i226")
>> Signed-off-by: Kohei Enju <enjuk@amazon.com>
>> ---
>> Changes:
>> v1->v2:
>> - don't fail probe when led setup fails
>> - rephrase subject and commit message
>> v1: https://lore.kernel.org/intel-wired-lan/20250906055239.29396-1-
>> enjuk@amazon.com/
>> ---
>
> ...
>
>
>> /* Release control of h/w to f/w. If f/w is AMT enabled, this
>> --
>> 2.48.1
>
>
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Reviewed-by: Vitaly Lifshits <vitaly.lifshits@intel.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 iwl-net] igc: don't fail igc_probe() on LED setup error
2025-09-10 13:47 [PATCH v2 iwl-net] igc: don't fail igc_probe() on LED setup error Kohei Enju
2025-09-10 13:50 ` [Intel-wired-lan] " Paul Menzel
2025-09-11 6:36 ` Loktionov, Aleksandr
@ 2025-09-11 7:27 ` Kurt Kanzenbach
2025-09-14 16:48 ` [Intel-wired-lan] " Mor Bar-Gabay
3 siblings, 0 replies; 6+ messages in thread
From: Kurt Kanzenbach @ 2025-09-11 7:27 UTC (permalink / raw)
To: Kohei Enju, intel-wired-lan, netdev
Cc: Tony Nguyen, Przemek Kitszel, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Aleksandr Loktionov,
Vitaly Lifshits, kohei.enju, Kohei Enju
[-- Attachment #1: Type: text/plain, Size: 1602 bytes --]
On Wed Sep 10 2025, Kohei Enju wrote:
> When igc_led_setup() fails, igc_probe() fails and triggers kernel panic
> in free_netdev() since unregister_netdev() is not called. [1]
> This behavior can be tested using fault-injection framework, especially
> the failslab feature. [2]
>
> Since LED support is not mandatory, treat LED setup failures as
> non-fatal and continue probe with a warning message, consequently
> avoiding the kernel panic.
>
> [1]
> kernel BUG at net/core/dev.c:12047!
> Oops: invalid opcode: 0000 [#1] SMP NOPTI
> CPU: 0 UID: 0 PID: 937 Comm: repro-igc-led-e Not tainted 6.17.0-rc4-enjuk-tnguy-00865-gc4940196ab02 #64 PREEMPT(voluntary)
> Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
> RIP: 0010:free_netdev+0x278/0x2b0
> [...]
> Call Trace:
> <TASK>
> igc_probe+0x370/0x910
> local_pci_probe+0x3a/0x80
> pci_device_probe+0xd1/0x200
> [...]
>
> [2]
> #!/bin/bash -ex
>
> FAILSLAB_PATH=/sys/kernel/debug/failslab/
> DEVICE=0000:00:05.0
> START_ADDR=$(grep " igc_led_setup" /proc/kallsyms \
> | awk '{printf("0x%s", $1)}')
> END_ADDR=$(printf "0x%x" $((START_ADDR + 0x100)))
>
> echo $START_ADDR > $FAILSLAB_PATH/require-start
> echo $END_ADDR > $FAILSLAB_PATH/require-end
> echo 1 > $FAILSLAB_PATH/times
> echo 100 > $FAILSLAB_PATH/probability
> echo N > $FAILSLAB_PATH/ignore-gfp-wait
>
> echo $DEVICE > /sys/bus/pci/drivers/igc/bind
>
> Fixes: ea578703b03d ("igc: Add support for LEDs on i225/i226")
> Signed-off-by: Kohei Enju <enjuk@amazon.com>
Reviewed-by: Kurt Kanzenbach <kurt@linutronix.de>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Intel-wired-lan] [PATCH v2 iwl-net] igc: don't fail igc_probe() on LED setup error
2025-09-10 13:47 [PATCH v2 iwl-net] igc: don't fail igc_probe() on LED setup error Kohei Enju
` (2 preceding siblings ...)
2025-09-11 7:27 ` Kurt Kanzenbach
@ 2025-09-14 16:48 ` Mor Bar-Gabay
3 siblings, 0 replies; 6+ messages in thread
From: Mor Bar-Gabay @ 2025-09-14 16:48 UTC (permalink / raw)
To: Kohei Enju, intel-wired-lan, netdev
Cc: Tony Nguyen, Przemek Kitszel, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Kurt Kanzenbach,
Aleksandr Loktionov, Vitaly Lifshits, kohei.enju
On 10/09/2025 16:47, Kohei Enju wrote:
> When igc_led_setup() fails, igc_probe() fails and triggers kernel panic
> in free_netdev() since unregister_netdev() is not called. [1]
> This behavior can be tested using fault-injection framework, especially
> the failslab feature. [2]
>
> Since LED support is not mandatory, treat LED setup failures as
> non-fatal and continue probe with a warning message, consequently
> avoiding the kernel panic.
>
> [1]
> kernel BUG at net/core/dev.c:12047!
> Oops: invalid opcode: 0000 [#1] SMP NOPTI
> CPU: 0 UID: 0 PID: 937 Comm: repro-igc-led-e Not tainted 6.17.0-rc4-enjuk-tnguy-00865-gc4940196ab02 #64 PREEMPT(voluntary)
> Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
> RIP: 0010:free_netdev+0x278/0x2b0
> [...]
> Call Trace:
> <TASK>
> igc_probe+0x370/0x910
> local_pci_probe+0x3a/0x80
> pci_device_probe+0xd1/0x200
> [...]
>
> [2]
> #!/bin/bash -ex
>
> FAILSLAB_PATH=/sys/kernel/debug/failslab/
> DEVICE=0000:00:05.0
> START_ADDR=$(grep " igc_led_setup" /proc/kallsyms \
> | awk '{printf("0x%s", $1)}')
> END_ADDR=$(printf "0x%x" $((START_ADDR + 0x100)))
>
> echo $START_ADDR > $FAILSLAB_PATH/require-start
> echo $END_ADDR > $FAILSLAB_PATH/require-end
> echo 1 > $FAILSLAB_PATH/times
> echo 100 > $FAILSLAB_PATH/probability
> echo N > $FAILSLAB_PATH/ignore-gfp-wait
>
> echo $DEVICE > /sys/bus/pci/drivers/igc/bind
>
> Fixes: ea578703b03d ("igc: Add support for LEDs on i225/i226")
> Signed-off-by: Kohei Enju <enjuk@amazon.com>
> Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> Reviewed-by: Vitaly Lifshits <vitaly.lifshits@intel.com>
> Reviewed-by: Kurt Kanzenbach <kurt@linutronix.de>
> ---
> Changes:
> v1->v2:
> - don't fail probe when led setup fails
> - rephrase subject and commit message
> v1: https://lore.kernel.org/intel-wired-lan/20250906055239.29396-1-enjuk@amazon.com/
> ---
> drivers/net/ethernet/intel/igc/igc.h | 1 +
> drivers/net/ethernet/intel/igc/igc_main.c | 12 +++++++++---
> 2 files changed, 10 insertions(+), 3 deletions(-)
>
Tested-by: Mor Bar-Gabay <morx.bar.gabay@intel.com>
^ permalink raw reply [flat|nested] 6+ messages in thread