* [PATCH net-next 0/5] netconsole: support automatic target recovery
@ 2025-09-09 21:12 Andre Carvalho
2025-09-09 21:12 ` [PATCH net-next 1/5] netconsole: add target_state enum Andre Carvalho
` (4 more replies)
0 siblings, 5 replies; 10+ messages in thread
From: Andre Carvalho @ 2025-09-09 21:12 UTC (permalink / raw)
To: Breno Leitao, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Shuah Khan
Cc: netdev, linux-kernel, linux-kselftest, Andre Carvalho
This patch series introduces target resume capability to netconsole
allowing it to recover targets when underlying low-level interface comes
back online.
Signed-off-by: Andre Carvalho <asantostc@gmail.com>
---
Andre Carvalho (3):
netconsole: convert 'enabled' flag to enum for clearer state management
netconsole: resume previously deactivated target
selftests: netconsole: validate target reactivation
Breno Leitao (2):
netconsole: add target_state enum
netconsole: add STATE_DEACTIVATED to track targets disabled by low level
drivers/net/netconsole.c | 110 ++++++++++++++++-----
tools/testing/selftests/drivers/net/Makefile | 1 +
.../selftests/drivers/net/lib/sh/lib_netcons.sh | 23 +++++
.../selftests/drivers/net/netcons_resume.sh | 68 +++++++++++++
4 files changed, 175 insertions(+), 27 deletions(-)
---
base-commit: 3b4296f5893d3a4e19edfc3800cb79381095e55f
change-id: 20250816-netcons-retrigger-a4f547bfc867
Best regards,
--
Andre Carvalho <asantostc@gmail.com>
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH net-next 1/5] netconsole: add target_state enum 2025-09-09 21:12 [PATCH net-next 0/5] netconsole: support automatic target recovery Andre Carvalho @ 2025-09-09 21:12 ` Andre Carvalho 2025-09-09 21:12 ` [PATCH net-next 2/5] netconsole: convert 'enabled' flag to enum for clearer state management Andre Carvalho ` (3 subsequent siblings) 4 siblings, 0 replies; 10+ messages in thread From: Andre Carvalho @ 2025-09-09 21:12 UTC (permalink / raw) To: Breno Leitao, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Shuah Khan Cc: netdev, linux-kernel, linux-kselftest, Andre Carvalho From: Breno Leitao <leitao@debian.org> Introduces a enum to track netconsole target state which is going to replace the enabled boolean. Signed-off-by: Breno Leitao <leitao@debian.org> Signed-off-by: Andre Carvalho <asantostc@gmail.com> --- drivers/net/netconsole.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c index 194570443493b1417570d3fe3250281cffe01316..b5e6a9fdc3155196d1fd7e81def584360ecbc3d2 100644 --- a/drivers/net/netconsole.c +++ b/drivers/net/netconsole.c @@ -117,6 +117,11 @@ enum sysdata_feature { SYSDATA_MSGID = BIT(3), }; +enum target_state { + STATE_DISABLED, + STATE_ENABLED, +}; + /** * struct netconsole_target - Represents a configured netconsole target. * @list: Links this target into the target_list. -- 2.51.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH net-next 2/5] netconsole: convert 'enabled' flag to enum for clearer state management 2025-09-09 21:12 [PATCH net-next 0/5] netconsole: support automatic target recovery Andre Carvalho 2025-09-09 21:12 ` [PATCH net-next 1/5] netconsole: add target_state enum Andre Carvalho @ 2025-09-09 21:12 ` Andre Carvalho 2025-09-09 21:12 ` [PATCH net-next 3/5] netconsole: add STATE_DEACTIVATED to track targets disabled by low level Andre Carvalho ` (2 subsequent siblings) 4 siblings, 0 replies; 10+ messages in thread From: Andre Carvalho @ 2025-09-09 21:12 UTC (permalink / raw) To: Breno Leitao, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Shuah Khan Cc: netdev, linux-kernel, linux-kselftest, Andre Carvalho This patch refactors the netconsole driver's target enabled state from a simple boolean to an explicit enum (`target_state`). This allow the states to be expanded to a new state in the upcoming change. Co-developed-by: Breno Leitao <leitao@debian.org> Signed-off-by: Breno Leitao <leitao@debian.org> Signed-off-by: Andre Carvalho <asantostc@gmail.com> --- drivers/net/netconsole.c | 52 ++++++++++++++++++++++++++---------------------- 1 file changed, 28 insertions(+), 24 deletions(-) diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c index b5e6a9fdc3155196d1fd7e81def584360ecbc3d2..688ed670b37b56ab4a03d43fb3de94ca0e6a8360 100644 --- a/drivers/net/netconsole.c +++ b/drivers/net/netconsole.c @@ -132,12 +132,12 @@ enum target_state { * @sysdata_fields: Sysdata features enabled. * @msgcounter: Message sent counter. * @stats: Packet send stats for the target. Used for debugging. - * @enabled: On / off knob to enable / disable target. + * @state: State of the target. * Visible from userspace (read-write). * We maintain a strict 1:1 correspondence between this and * whether the corresponding netpoll is active or inactive. * Also, other parameters of a target may be modified at - * runtime only when it is disabled (enabled == 0). + * runtime only when it is disabled (state == STATE_DISABLED). * @extended: Denotes whether console is extended or not. * @release: Denotes whether kernel release version should be prepended * to the message. Depends on extended console. @@ -165,7 +165,7 @@ struct netconsole_target { u32 msgcounter; #endif struct netconsole_target_stats stats; - bool enabled; + enum target_state state; bool extended; bool release; struct netpoll np; @@ -257,6 +257,7 @@ static struct netconsole_target *alloc_and_init(void) nt->np.local_port = 6665; nt->np.remote_port = 6666; eth_broadcast_addr(nt->np.remote_mac); + nt->state = STATE_DISABLED; return nt; } @@ -275,7 +276,7 @@ static void netconsole_process_cleanups_core(void) mutex_lock(&target_cleanup_list_lock); list_for_each_entry_safe(nt, tmp, &target_cleanup_list, list) { /* all entries in the cleanup_list needs to be disabled */ - WARN_ON_ONCE(nt->enabled); + WARN_ON_ONCE(nt->state == STATE_ENABLED); do_netpoll_cleanup(&nt->np); /* moved the cleaned target to target_list. Need to hold both * locks @@ -398,7 +399,7 @@ static void trim_newline(char *s, size_t maxlen) static ssize_t enabled_show(struct config_item *item, char *buf) { - return sysfs_emit(buf, "%d\n", to_target(item)->enabled); + return sysfs_emit(buf, "%d\n", to_target(item)->state == STATE_ENABLED); } static ssize_t extended_show(struct config_item *item, char *buf) @@ -565,8 +566,8 @@ static ssize_t enabled_store(struct config_item *item, const char *buf, size_t count) { struct netconsole_target *nt = to_target(item); + bool enabled, current_enabled; unsigned long flags; - bool enabled; ssize_t ret; mutex_lock(&dynamic_netconsole_mutex); @@ -575,9 +576,10 @@ static ssize_t enabled_store(struct config_item *item, goto out_unlock; ret = -EINVAL; - if (enabled == nt->enabled) { + current_enabled = nt->state == STATE_ENABLED; + if (enabled == current_enabled) { pr_info("network logging has already %s\n", - nt->enabled ? "started" : "stopped"); + current_enabled ? "started" : "stopped"); goto out_unlock; } @@ -610,16 +612,16 @@ static ssize_t enabled_store(struct config_item *item, if (ret) goto out_unlock; - nt->enabled = true; + nt->state = STATE_ENABLED; pr_info("network logging started\n"); } else { /* false */ /* We need to disable the netconsole before cleaning it up * otherwise we might end up in write_msg() with - * nt->np.dev == NULL and nt->enabled == true + * nt->np.dev == NULL and nt->state == STATE_ENABLED */ mutex_lock(&target_cleanup_list_lock); spin_lock_irqsave(&target_list_lock, flags); - nt->enabled = false; + nt->state = STATE_DISABLED; /* Remove the target from the list, while holding * target_list_lock */ @@ -648,7 +650,7 @@ static ssize_t release_store(struct config_item *item, const char *buf, ssize_t ret; mutex_lock(&dynamic_netconsole_mutex); - if (nt->enabled) { + if (nt->state == STATE_ENABLED) { pr_err("target (%s) is enabled, disable to update parameters\n", config_item_name(&nt->group.cg_item)); ret = -EINVAL; @@ -675,7 +677,7 @@ static ssize_t extended_store(struct config_item *item, const char *buf, ssize_t ret; mutex_lock(&dynamic_netconsole_mutex); - if (nt->enabled) { + if (nt->state == STATE_ENABLED) { pr_err("target (%s) is enabled, disable to update parameters\n", config_item_name(&nt->group.cg_item)); ret = -EINVAL; @@ -699,7 +701,7 @@ static ssize_t dev_name_store(struct config_item *item, const char *buf, struct netconsole_target *nt = to_target(item); mutex_lock(&dynamic_netconsole_mutex); - if (nt->enabled) { + if (nt->state == STATE_ENABLED) { pr_err("target (%s) is enabled, disable to update parameters\n", config_item_name(&nt->group.cg_item)); mutex_unlock(&dynamic_netconsole_mutex); @@ -720,7 +722,7 @@ static ssize_t local_port_store(struct config_item *item, const char *buf, ssize_t ret = -EINVAL; mutex_lock(&dynamic_netconsole_mutex); - if (nt->enabled) { + if (nt->state == STATE_ENABLED) { pr_err("target (%s) is enabled, disable to update parameters\n", config_item_name(&nt->group.cg_item)); goto out_unlock; @@ -742,7 +744,7 @@ static ssize_t remote_port_store(struct config_item *item, ssize_t ret = -EINVAL; mutex_lock(&dynamic_netconsole_mutex); - if (nt->enabled) { + if (nt->state == STATE_ENABLED) { pr_err("target (%s) is enabled, disable to update parameters\n", config_item_name(&nt->group.cg_item)); goto out_unlock; @@ -765,7 +767,7 @@ static ssize_t local_ip_store(struct config_item *item, const char *buf, int ipv6; mutex_lock(&dynamic_netconsole_mutex); - if (nt->enabled) { + if (nt->state == STATE_ENABLED) { pr_err("target (%s) is enabled, disable to update parameters\n", config_item_name(&nt->group.cg_item)); goto out_unlock; @@ -790,7 +792,7 @@ static ssize_t remote_ip_store(struct config_item *item, const char *buf, int ipv6; mutex_lock(&dynamic_netconsole_mutex); - if (nt->enabled) { + if (nt->state == STATE_ENABLED) { pr_err("target (%s) is enabled, disable to update parameters\n", config_item_name(&nt->group.cg_item)); goto out_unlock; @@ -839,7 +841,7 @@ static ssize_t remote_mac_store(struct config_item *item, const char *buf, ssize_t ret = -EINVAL; mutex_lock(&dynamic_netconsole_mutex); - if (nt->enabled) { + if (nt->state == STATE_ENABLED) { pr_err("target (%s) is enabled, disable to update parameters\n", config_item_name(&nt->group.cg_item)); goto out_unlock; @@ -1315,7 +1317,7 @@ static void drop_netconsole_target(struct config_group *group, * The target may have never been enabled, or was manually disabled * before being removed so netpoll may have already been cleaned up. */ - if (nt->enabled) + if (nt->state == STATE_ENABLED) netpoll_cleanup(&nt->np); config_item_put(&nt->group.cg_item); @@ -1444,7 +1446,7 @@ static int netconsole_netdev_event(struct notifier_block *this, case NETDEV_RELEASE: case NETDEV_JOIN: case NETDEV_UNREGISTER: - nt->enabled = false; + nt->state = STATE_DISABLED; list_move(&nt->list, &target_cleanup_list); stopped = true; } @@ -1711,7 +1713,8 @@ static void write_ext_msg(struct console *con, const char *msg, spin_lock_irqsave(&target_list_lock, flags); list_for_each_entry(nt, &target_list, list) - if (nt->extended && nt->enabled && netif_running(nt->np.dev)) + if (nt->extended && nt->state == STATE_ENABLED && + netif_running(nt->np.dev)) send_ext_msg_udp(nt, msg, len); spin_unlock_irqrestore(&target_list_lock, flags); } @@ -1731,7 +1734,8 @@ static void write_msg(struct console *con, const char *msg, unsigned int len) spin_lock_irqsave(&target_list_lock, flags); list_for_each_entry(nt, &target_list, list) { - if (!nt->extended && nt->enabled && netif_running(nt->np.dev)) { + if (!nt->extended && nt->state == STATE_ENABLED && + netif_running(nt->np.dev)) { /* * We nest this inside the for-each-target loop above * so that we're able to get as much logging out to @@ -1887,7 +1891,7 @@ static struct netconsole_target *alloc_param_target(char *target_config, */ goto fail; } else { - nt->enabled = true; + nt->state = STATE_ENABLED; } populate_configfs_item(nt, cmdline_count); -- 2.51.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH net-next 3/5] netconsole: add STATE_DEACTIVATED to track targets disabled by low level 2025-09-09 21:12 [PATCH net-next 0/5] netconsole: support automatic target recovery Andre Carvalho 2025-09-09 21:12 ` [PATCH net-next 1/5] netconsole: add target_state enum Andre Carvalho 2025-09-09 21:12 ` [PATCH net-next 2/5] netconsole: convert 'enabled' flag to enum for clearer state management Andre Carvalho @ 2025-09-09 21:12 ` Andre Carvalho 2025-09-09 21:12 ` [PATCH net-next 4/5] netconsole: resume previously deactivated target Andre Carvalho 2025-09-09 21:12 ` [PATCH net-next 5/5] selftests: netconsole: validate target reactivation Andre Carvalho 4 siblings, 0 replies; 10+ messages in thread From: Andre Carvalho @ 2025-09-09 21:12 UTC (permalink / raw) To: Breno Leitao, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Shuah Khan Cc: netdev, linux-kernel, linux-kselftest, Andre Carvalho From: Breno Leitao <leitao@debian.org> When the low level interface brings a netconsole target down, record this using a new STATE_DEACTIVATED state. This allows netconsole to distinguish between targets explicitly disabled by users and those deactivated due to interface state changes. It also enables automatic recovery and re-enabling of targets if the underlying low-level interfaces come back online. From a code perspective, anything that is not STATE_ENABLED is disabled. Mark the device that is down due to NETDEV_UNREGISTER as STATE_DEACTIVATED, this, should be the same as STATE_DISABLED from a code perspective. Signed-off-by: Breno Leitao <leitao@debian.org> Signed-off-by: Andre Carvalho <asantostc@gmail.com> --- drivers/net/netconsole.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c index 688ed670b37b56ab4a03d43fb3de94ca0e6a8360..59d770bb4baa5f9616b10c0dfb39ed45a4eb7710 100644 --- a/drivers/net/netconsole.c +++ b/drivers/net/netconsole.c @@ -120,6 +120,7 @@ enum sysdata_feature { enum target_state { STATE_DISABLED, STATE_ENABLED, + STATE_DEACTIVATED, }; /** @@ -575,6 +576,14 @@ static ssize_t enabled_store(struct config_item *item, if (ret) goto out_unlock; + /* When the user explicitly enables or disables a target that is + * currently deactivated, reset its state to disabled. The DEACTIVATED + * state only tracks interface-driven deactivation and should _not_ + * persist when the user manually changes the target's enabled state. + */ + if (nt->state == STATE_DEACTIVATED) + nt->state = STATE_DISABLED; + ret = -EINVAL; current_enabled = nt->state == STATE_ENABLED; if (enabled == current_enabled) { @@ -1446,7 +1455,7 @@ static int netconsole_netdev_event(struct notifier_block *this, case NETDEV_RELEASE: case NETDEV_JOIN: case NETDEV_UNREGISTER: - nt->state = STATE_DISABLED; + nt->state = STATE_DEACTIVATED; list_move(&nt->list, &target_cleanup_list); stopped = true; } -- 2.51.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH net-next 4/5] netconsole: resume previously deactivated target 2025-09-09 21:12 [PATCH net-next 0/5] netconsole: support automatic target recovery Andre Carvalho ` (2 preceding siblings ...) 2025-09-09 21:12 ` [PATCH net-next 3/5] netconsole: add STATE_DEACTIVATED to track targets disabled by low level Andre Carvalho @ 2025-09-09 21:12 ` Andre Carvalho 2025-09-10 11:05 ` kernel test robot 2025-09-10 19:50 ` Breno Leitao 2025-09-09 21:12 ` [PATCH net-next 5/5] selftests: netconsole: validate target reactivation Andre Carvalho 4 siblings, 2 replies; 10+ messages in thread From: Andre Carvalho @ 2025-09-09 21:12 UTC (permalink / raw) To: Breno Leitao, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Shuah Khan Cc: netdev, linux-kernel, linux-kselftest, Andre Carvalho Attempt to resume a previously deactivated target when the associated interface comes back (NETDEV_UP event is received). Target transitions to STATE_DISABLED in case of failures resuming it to avoid retrying the same target indefinitely. Signed-off-by: Andre Carvalho <asantostc@gmail.com> --- drivers/net/netconsole.c | 46 ++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 42 insertions(+), 4 deletions(-) diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c index 59d770bb4baa5f9616b10c0dfb39ed45a4eb7710..397e6543b3d9aeda6da450823adee09cb3e9ae70 100644 --- a/drivers/net/netconsole.c +++ b/drivers/net/netconsole.c @@ -135,10 +135,12 @@ enum target_state { * @stats: Packet send stats for the target. Used for debugging. * @state: State of the target. * Visible from userspace (read-write). - * We maintain a strict 1:1 correspondence between this and - * whether the corresponding netpoll is active or inactive. + * From a userspace perspective, the target is either enabled or + * disabled. Internally, although both STATE_DISABLED and + * STATE_DEACTIVATED correspond to inactive netpoll the latter is + * due to interface state changes and may recover automatically. * Also, other parameters of a target may be modified at - * runtime only when it is disabled (state == STATE_DISABLED). + * runtime only when it is disabled (state != STATE_ENABLED). * @extended: Denotes whether console is extended or not. * @release: Denotes whether kernel release version should be prepended * to the message. Depends on extended console. @@ -172,6 +174,7 @@ struct netconsole_target { struct netpoll np; /* protected by target_list_lock */ char buf[MAX_PRINT_CHUNK]; + struct work_struct resume_wq; }; #ifdef CONFIG_NETCONSOLE_DYNAMIC @@ -237,6 +240,33 @@ static void populate_configfs_item(struct netconsole_target *nt, } #endif /* CONFIG_NETCONSOLE_DYNAMIC */ +/* Attempts to resume logging to a deactivated target. */ +static void resume_target(struct netconsole_target *nt) +{ + int ret; + + if (nt->state != STATE_DEACTIVATED) + return; + + ret = netpoll_setup(&nt->np); + if (ret) { + /* netpoll fails to register once, do not try again. */ + nt->state = STATE_DISABLED; + } else { + pr_info("network logging resumed on interface %s\n", + nt->np.dev_name); + nt->state = STATE_ENABLED; + } +} + +static void resume_work_handler(struct work_struct *work) +{ + struct netconsole_target *nt = + container_of(work, struct netconsole_target, resume_wq); + + resume_target(nt); +} + /* Allocate and initialize with defaults. * Note that these targets get their config_item fields zeroed-out. */ @@ -260,6 +290,8 @@ static struct netconsole_target *alloc_and_init(void) eth_broadcast_addr(nt->np.remote_mac); nt->state = STATE_DISABLED; + INIT_WORK(&nt->resume_wq, resume_work_handler); + return nt; } @@ -1440,7 +1472,8 @@ static int netconsole_netdev_event(struct notifier_block *this, bool stopped = false; if (!(event == NETDEV_CHANGENAME || event == NETDEV_UNREGISTER || - event == NETDEV_RELEASE || event == NETDEV_JOIN)) + event == NETDEV_RELEASE || event == NETDEV_JOIN || + event == NETDEV_UP)) goto done; mutex_lock(&target_cleanup_list_lock); @@ -1460,6 +1493,10 @@ static int netconsole_netdev_event(struct notifier_block *this, stopped = true; } } + if (nt->state == STATE_DEACTIVATED && event == NETDEV_UP) { + if (!strncmp(nt->np.dev_name, dev->name, IFNAMSIZ)) + schedule_work(&nt->resume_wq); + } netconsole_target_put(nt); } spin_unlock_irqrestore(&target_list_lock, flags); @@ -1915,6 +1952,7 @@ static struct netconsole_target *alloc_param_target(char *target_config, static void free_param_target(struct netconsole_target *nt) { netpoll_cleanup(&nt->np); + cancel_work_sync(&nt->resume_wq); kfree(nt); } -- 2.51.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next 4/5] netconsole: resume previously deactivated target 2025-09-09 21:12 ` [PATCH net-next 4/5] netconsole: resume previously deactivated target Andre Carvalho @ 2025-09-10 11:05 ` kernel test robot 2025-09-10 19:50 ` Breno Leitao 1 sibling, 0 replies; 10+ messages in thread From: kernel test robot @ 2025-09-10 11:05 UTC (permalink / raw) To: Andre Carvalho, Breno Leitao, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Shuah Khan Cc: oe-kbuild-all, netdev, linux-kernel, linux-kselftest, Andre Carvalho Hi Andre, kernel test robot noticed the following build warnings: [auto build test WARNING on 3b4296f5893d3a4e19edfc3800cb79381095e55f] url: https://github.com/intel-lab-lkp/linux/commits/Andre-Carvalho/netconsole-add-target_state-enum/20250910-051601 base: 3b4296f5893d3a4e19edfc3800cb79381095e55f patch link: https://lore.kernel.org/r/20250909-netcons-retrigger-v1-4-3aea904926cf%40gmail.com patch subject: [PATCH net-next 4/5] netconsole: resume previously deactivated target config: i386-randconfig-014-20250910 (https://download.01.org/0day-ci/archive/20250910/202509101818.wKicbxgJ-lkp@intel.com/config) compiler: gcc-14 (Debian 14.2.0-19) 14.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250910/202509101818.wKicbxgJ-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202509101818.wKicbxgJ-lkp@intel.com/ All warnings (new ones prefixed by >>): >> Warning: drivers/net/netconsole.c:177 struct member 'resume_wq' not described in 'netconsole_target' -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next 4/5] netconsole: resume previously deactivated target 2025-09-09 21:12 ` [PATCH net-next 4/5] netconsole: resume previously deactivated target Andre Carvalho 2025-09-10 11:05 ` kernel test robot @ 2025-09-10 19:50 ` Breno Leitao 2025-09-11 8:05 ` Andre Carvalho 1 sibling, 1 reply; 10+ messages in thread From: Breno Leitao @ 2025-09-10 19:50 UTC (permalink / raw) To: Andre Carvalho Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Shuah Khan, netdev, linux-kernel, linux-kselftest On Tue, Sep 09, 2025 at 10:12:15PM +0100, Andre Carvalho wrote: > @@ -1460,6 +1493,10 @@ static int netconsole_netdev_event(struct notifier_block *this, > stopped = true; > } > } > + if (nt->state == STATE_DEACTIVATED && event == NETDEV_UP) { > + if (!strncmp(nt->np.dev_name, dev->name, IFNAMSIZ)) Don't you need to check for dev_mac here as well? > + schedule_work(&nt->resume_wq); I would prefer to have the enablement done inline here, instead of scheduling a task. It will be safer, given no one else is traversing the list and accessing the element, given you have the target_list_lock in netconsole_netdev_event, but, you don't have it in the resume_wq. Given that we don't have a lock per target, target_list_lock is the current lock protecting any simultaneous modification to the targets. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next 4/5] netconsole: resume previously deactivated target 2025-09-10 19:50 ` Breno Leitao @ 2025-09-11 8:05 ` Andre Carvalho 0 siblings, 0 replies; 10+ messages in thread From: Andre Carvalho @ 2025-09-11 8:05 UTC (permalink / raw) To: Breno Leitao Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Shuah Khan, netdev, linux-kernel, linux-kselftest On Wed, Sep 10, 2025 at 12:50:07PM -0700, Breno Leitao wrote: > > + if (nt->state == STATE_DEACTIVATED && event == NETDEV_UP) { > > + if (!strncmp(nt->np.dev_name, dev->name, IFNAMSIZ)) > > Don't you need to check for dev_mac here as well? I believe so. Will fix that and try to cover this case on the selftest too. > > + schedule_work(&nt->resume_wq); > > I would prefer to have the enablement done inline here, instead of > scheduling a task. That makes sense. I believe I'll need an alternative to netpoll_setup that can be called with rtnl already held. I'll attempt to do this for v2. Thanks for the review! ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH net-next 5/5] selftests: netconsole: validate target reactivation 2025-09-09 21:12 [PATCH net-next 0/5] netconsole: support automatic target recovery Andre Carvalho ` (3 preceding siblings ...) 2025-09-09 21:12 ` [PATCH net-next 4/5] netconsole: resume previously deactivated target Andre Carvalho @ 2025-09-09 21:12 ` Andre Carvalho 2025-09-10 19:59 ` Breno Leitao 4 siblings, 1 reply; 10+ messages in thread From: Andre Carvalho @ 2025-09-09 21:12 UTC (permalink / raw) To: Breno Leitao, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Shuah Khan Cc: netdev, linux-kernel, linux-kselftest, Andre Carvalho Introduce a new netconsole selftest to validate that netconsole is able to resume a deactivated target when the low level interface comes back. The test setups the network using netdevsim, creates a netconsole target and then remove/add netdevsim in order to bring the same interfaces back. Afterwards, the test validates that the target works as expected. Signed-off-by: Andre Carvalho <asantostc@gmail.com> --- tools/testing/selftests/drivers/net/Makefile | 1 + .../selftests/drivers/net/lib/sh/lib_netcons.sh | 23 ++++++++ .../selftests/drivers/net/netcons_resume.sh | 68 ++++++++++++++++++++++ 3 files changed, 92 insertions(+) diff --git a/tools/testing/selftests/drivers/net/Makefile b/tools/testing/selftests/drivers/net/Makefile index 984ece05f7f92e836592107ba4c692da6d8ce1b3..f47c4d57f7b4ce82b0b59bee4c87a9660819675e 100644 --- a/tools/testing/selftests/drivers/net/Makefile +++ b/tools/testing/selftests/drivers/net/Makefile @@ -17,6 +17,7 @@ TEST_PROGS := \ netcons_fragmented_msg.sh \ netcons_overflow.sh \ netcons_sysdata.sh \ + netcons_resume.sh \ netpoll_basic.py \ ping.py \ queues.py \ diff --git a/tools/testing/selftests/drivers/net/lib/sh/lib_netcons.sh b/tools/testing/selftests/drivers/net/lib/sh/lib_netcons.sh index 8e1085e896472d5c87ec8b236240878a5b2d00d2..ba7c865b1be3b60f53ea548aba269059ca74aee6 100644 --- a/tools/testing/selftests/drivers/net/lib/sh/lib_netcons.sh +++ b/tools/testing/selftests/drivers/net/lib/sh/lib_netcons.sh @@ -350,6 +350,29 @@ function check_netconsole_module() { fi } +function wait_target_state() { + local TARGET=${1} + local STATE=${2} + local FILENAME="${NETCONS_CONFIGFS}"/"${TARGET}"/"enabled" + + if [ "${STATE}" == "enabled" ] + then + ENABLED=1 + else + ENABLED=0 + fi + + if [ ! -f "$FILENAME" ]; then + echo "FAIL: Target does not exist." >&2 + exit "${ksft_fail}" + fi + + slowwait 2 sh -c 'test -n "$(grep '"'${ENABLED}'"' '"'${FILENAME}'"')"' || { + echo "FAIL: ${TARGET} is not ${STATE}." >&2 + exit "${ksft_fail}" + } +} + # A wrapper to translate protocol version to udp version function wait_for_port() { local NAMESPACE=${1} diff --git a/tools/testing/selftests/drivers/net/netcons_resume.sh b/tools/testing/selftests/drivers/net/netcons_resume.sh new file mode 100755 index 0000000000000000000000000000000000000000..7e8ea74821fffdac8be0c3db2f1aa7953b4d5bd5 --- /dev/null +++ b/tools/testing/selftests/drivers/net/netcons_resume.sh @@ -0,0 +1,68 @@ +#!/usr/bin/env bash +# SPDX-License-Identifier: GPL-2.0 + +# This test validates that netconsole is able to resume a target that was +# deactivated when its interface was removed. +# +# The test configures a netconsole dynamic target and then removes netdevsim +# module to cause the interface to disappear. The test veries that the target +# moved to disabled state before adding netdevsim and the interface back. +# +# Finally, the test verifies that the target is re-enabled automatically and +# the message is received on the destination interface. +# +# Author: Andre Carvalho <asantostc@gmail.com> + +set -euo pipefail + +SCRIPTDIR=$(dirname "$(readlink -e "${BASH_SOURCE[0]}")") + +source "${SCRIPTDIR}"/lib/sh/lib_netcons.sh + +modprobe netdevsim 2> /dev/null || true +modprobe netconsole 2> /dev/null || true + +# The content of kmsg will be save to the following file +OUTPUT_FILE="/tmp/${TARGET}" + +# Check for basic system dependency and exit if not found +check_for_dependencies + +# Set current loglevel to KERN_INFO(6), and default to KERN_NOTICE(5) +echo "6 5" > /proc/sys/kernel/printk +# Remove the namespace, interfaces and netconsole target on exit +trap cleanup EXIT + +# Create one namespace and two interfaces +set_network +# Create a dynamic target for netconsole +create_dynamic_target + +# Remove low level module +rmmod netdevsim +# Target should be disabled +wait_target_state "${TARGET}" "disabled" + +# Add back low level module +modprobe netdevsim +# Recreate namespace and two interfaces +set_network +# Target should be enabled again +wait_target_state "${TARGET}" "enabled" + +# Listed for netconsole port inside the namespace and destination +# interface +listen_port_and_save_to "${OUTPUT_FILE}" & +# Wait for socat to start and listen to the port. +wait_local_port_listen "${NAMESPACE}" "${PORT}" udp +# Send the message +echo "${MSG}: ${TARGET}" > /dev/kmsg +# Wait until socat saves the file to disk +busywait "${BUSYWAIT_TIMEOUT}" test -s "${OUTPUT_FILE}" +# Make sure the message was received in the dst part +# and exit +validate_msg "${OUTPUT_FILE}" +# kill socat in case it is still running +pkill_socat + +exit "${ksft_pass}" -- 2.51.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next 5/5] selftests: netconsole: validate target reactivation 2025-09-09 21:12 ` [PATCH net-next 5/5] selftests: netconsole: validate target reactivation Andre Carvalho @ 2025-09-10 19:59 ` Breno Leitao 0 siblings, 0 replies; 10+ messages in thread From: Breno Leitao @ 2025-09-10 19:59 UTC (permalink / raw) To: Andre Carvalho Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Shuah Khan, netdev, linux-kernel, linux-kselftest On Tue, Sep 09, 2025 at 10:12:16PM +0100, Andre Carvalho wrote: > diff --git a/tools/testing/selftests/drivers/net/Makefile b/tools/testing/selftests/drivers/net/Makefile > index 984ece05f7f92e836592107ba4c692da6d8ce1b3..f47c4d57f7b4ce82b0b59bee4c87a9660819675e 100644 > --- a/tools/testing/selftests/drivers/net/Makefile > +++ b/tools/testing/selftests/drivers/net/Makefile > @@ -17,6 +17,7 @@ TEST_PROGS := \ > netcons_fragmented_msg.sh \ > netcons_overflow.sh \ > netcons_sysdata.sh \ > + netcons_resume.sh \ we try to keep these tests alphabetically ordered. > netpoll_basic.py \ > ping.py \ > queues.py \ > diff --git a/tools/testing/selftests/drivers/net/lib/sh/lib_netcons.sh b/tools/testing/selftests/drivers/net/lib/sh/lib_netcons.sh > index 8e1085e896472d5c87ec8b236240878a5b2d00d2..ba7c865b1be3b60f53ea548aba269059ca74aee6 100644 > --- a/tools/testing/selftests/drivers/net/lib/sh/lib_netcons.sh > +++ b/tools/testing/selftests/drivers/net/lib/sh/lib_netcons.sh > @@ -350,6 +350,29 @@ function check_netconsole_module() { > fi > } > > +function wait_target_state() { > + local TARGET=${1} > + local STATE=${2} > + local FILENAME="${NETCONS_CONFIGFS}"/"${TARGET}"/"enabled" > + > + if [ "${STATE}" == "enabled" ] > + then > + ENABLED=1 > + else > + ENABLED=0 > + fi > + > + if [ ! -f "$FILENAME" ]; then > + echo "FAIL: Target does not exist." >&2 > + exit "${ksft_fail}" > + fi > + > + slowwait 2 sh -c 'test -n "$(grep '"'${ENABLED}'"' '"'${FILENAME}'"')"' || { shellcheck is not very happy with this line: https://netdev.bots.linux.dev/static/nipa/1000727/14224835/shellcheck/stderr > diff --git a/tools/testing/selftests/drivers/net/netcons_resume.sh b/tools/testing/selftests/drivers/net/netcons_resume.sh > new file mode 100755 > index 0000000000000000000000000000000000000000..7e8ea74821fffdac8be0c3db2f1aa7953b4d5bd5 > --- /dev/null > +++ b/tools/testing/selftests/drivers/net/netcons_resume.sh > @@ -0,0 +1,68 @@ > +#!/usr/bin/env bash > +# SPDX-License-Identifier: GPL-2.0 > + > +# This test validates that netconsole is able to resume a target that was > +# deactivated when its interface was removed. > +# > +# The test configures a netconsole dynamic target and then removes netdevsim > +# module to cause the interface to disappear. The test veries that the target nit: s/veries/verifies/ ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2025-09-11 8:05 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2025-09-09 21:12 [PATCH net-next 0/5] netconsole: support automatic target recovery Andre Carvalho 2025-09-09 21:12 ` [PATCH net-next 1/5] netconsole: add target_state enum Andre Carvalho 2025-09-09 21:12 ` [PATCH net-next 2/5] netconsole: convert 'enabled' flag to enum for clearer state management Andre Carvalho 2025-09-09 21:12 ` [PATCH net-next 3/5] netconsole: add STATE_DEACTIVATED to track targets disabled by low level Andre Carvalho 2025-09-09 21:12 ` [PATCH net-next 4/5] netconsole: resume previously deactivated target Andre Carvalho 2025-09-10 11:05 ` kernel test robot 2025-09-10 19:50 ` Breno Leitao 2025-09-11 8:05 ` Andre Carvalho 2025-09-09 21:12 ` [PATCH net-next 5/5] selftests: netconsole: validate target reactivation Andre Carvalho 2025-09-10 19:59 ` Breno Leitao
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®