Linux Kernel Selftest development
 help / color / mirror / Atom feed
* [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 related	[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 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

* [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 related	[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 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

* 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

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