[PATCH net-next 4/5] netconsole: resume previously deactivated target

Andre Carvalho posted 5 patches 3 weeks, 2 days ago
There is a newer version of this series
[PATCH net-next 4/5] netconsole: resume previously deactivated target
Posted by Andre Carvalho 3 weeks, 2 days ago
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
Re: [PATCH net-next 4/5] netconsole: resume previously deactivated target
Posted by Breno Leitao 3 weeks, 1 day ago
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.
Re: [PATCH net-next 4/5] netconsole: resume previously deactivated target
Posted by Andre Carvalho 3 weeks ago
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!
Re: [PATCH net-next 4/5] netconsole: resume previously deactivated target
Posted by kernel test robot 3 weeks, 1 day ago
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