* [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 related [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 related [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 related [flat|nested] 10+ messages in thread