linux-kernel - Re: [PATCH 5/7] netpoll: Move SKBs pool to netconsole side

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <willemdebruijn.kernel.a0f67bb6112a@gmail.com>
Date: Tue, 02 Sep 2025 18:56:13 -0400
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: Breno Leitao <leitao@...ian.org>, 
 Andrew Lunn <andrew+netdev@...n.ch>, 
 "David S. Miller" <davem@...emloft.net>, 
 Eric Dumazet <edumazet@...gle.com>, 
 Jakub Kicinski <kuba@...nel.org>, 
 Paolo Abeni <pabeni@...hat.com>, 
 Simon Horman <horms@...nel.org>, 
 Sebastian Andrzej Siewior <bigeasy@...utronix.de>, 
 Clark Williams <clrkwllms@...nel.org>, 
 Steven Rostedt <rostedt@...dmis.org>
Cc: netdev@...r.kernel.org, 
 linux-kernel@...r.kernel.org, 
 linux-rt-devel@...ts.linux.dev, 
 kernel-team@...a.com, 
 efault@....de, 
 calvin@...nvd.org, 
 Breno Leitao <leitao@...ian.org>
Subject: Re: [PATCH 5/7] netpoll: Move SKBs pool to netconsole side

Breno Leitao wrote:
> Since netconsole is the sole user of the SKBs pool within netpoll, move
> the pool management into the netconsole driver.
> 
> This change prevents other netpoll users from allocating and holding
> onto skb pool memory unnecessarily, thereby reducing memory usage when
> the pool is not required (which is all the cases except netconsole).
> 
> The skb poll struct is still attached to the netpoll, but, eventually
> this should move to the netconsole target, since it has nothing to do
> with netpoll.
> 
> Signed-off-by: Breno Leitao <leitao@...ian.org>
> ---
>  drivers/net/netconsole.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++--
>  net/core/netpoll.c       | 44 ------------------------------------
>  2 files changed, 56 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
> index 90e359b87469a..3fe55db07cfe5 100644
> --- a/drivers/net/netconsole.c
> +++ b/drivers/net/netconsole.c
> @@ -57,6 +57,19 @@ MODULE_LICENSE("GPL");
>  #define MAX_EXTRADATA_ITEMS		16
>  #define MAX_PRINT_CHUNK			1000
>  
> +/*
> + * We maintain a small pool of fully-sized skbs, to make sure the
> + * message gets out even in extreme OOM situations.
> + */
> +
> +#define MAX_SKBS 32
> +#define MAX_UDP_CHUNK 1460
> +#define MAX_SKB_SIZE							\
> +	(sizeof(struct ethhdr) +					\
> +	 sizeof(struct iphdr) +						\
> +	 sizeof(struct udphdr) +					\
> +	 MAX_UDP_CHUNK)
> +
>  static char config[MAX_PARAM_LENGTH];
>  module_param_string(netconsole, config, MAX_PARAM_LENGTH, 0);
>  MODULE_PARM_DESC(netconsole, " netconsole=[src-port]@[src-ip]/[dev],[tgt-port]@<tgt-ip>/[tgt-macaddr]");
> @@ -172,6 +185,33 @@ struct netconsole_target {
>  	char			buf[MAX_PRINT_CHUNK];
>  };
>  
> +static void refill_skbs(struct netpoll *np)
> +{
> +	struct sk_buff_head *skb_pool;
> +	struct sk_buff *skb;
> +	unsigned long flags;
> +
> +	skb_pool = &np->skb_pool;
> +
> +	spin_lock_irqsave(&skb_pool->lock, flags);
> +	while (skb_pool->qlen < MAX_SKBS) {
> +		skb = alloc_skb(MAX_SKB_SIZE, GFP_ATOMIC);
> +		if (!skb)
> +			break;
> +
> +		__skb_queue_tail(skb_pool, skb);
> +	}
> +	spin_unlock_irqrestore(&skb_pool->lock, flags);
> +}
> +
> +static void refill_skbs_work_handler(struct work_struct *work)
> +{
> +	struct netpoll *np =
> +		container_of(work, struct netpoll, refill_wq);
> +
> +	refill_skbs(np);
> +}
> +
>  #ifdef	CONFIG_NETCONSOLE_DYNAMIC
>  
>  static struct configfs_subsystem netconsole_subsys;
> @@ -341,6 +381,20 @@ static int netpoll_parse_ip_addr(const char *str, union inet_addr *addr)
>  	return -1;
>  }
>  
> +static int setup_netpoll(struct netpoll *np)

Having both netpoll_setup and setup_netpoll is a bit confusing.
Maybe netconsole_setup_netpoll?

> +{
> +	int err;
> +
> +	err = netpoll_setup(np);
> +	if (err)
> +		return err;
> +
> +	refill_skbs(np);
> +	INIT_WORK(&np->refill_wq, refill_skbs_work_handler);
> +
> +	return 0;
> +}
> +
>  #ifdef	CONFIG_NETCONSOLE_DYNAMIC
>  
>  /*
> @@ -615,7 +669,7 @@ static ssize_t enabled_store(struct config_item *item,
>  		 */
>  		netconsole_print_banner(&nt->np);
>  
> -		ret = netpoll_setup(&nt->np);
> +		ret = setup_netpoll(&nt->np);
>  		if (ret)
>  			goto out_unlock;
>  
> @@ -2036,7 +2090,7 @@ static struct netconsole_target *alloc_param_target(char *target_config,
>  	if (err)
>  		goto fail;
>  
> -	err = netpoll_setup(&nt->np);
> +	err = setup_netpoll(&nt->np);
>  	if (err) {
>  		pr_err("Not enabling netconsole for %s%d. Netpoll setup failed\n",
>  		       NETCONSOLE_PARAM_TARGET_PREFIX, cmdline_count);