[PATCH 0/3] netpoll: Code organization improvements

[PATCH 0/3] netpoll: Code organization improvements

[Breno Leitao]

The netpoll_setup() function has grown complex over time, mixing
different error handling and concerns like carrier waiting, IPv4 address
retrieval, and IPv6 address retrieval all within a single function,
which is huge (127 LoC).

This patch series refactors the netpoll_setup() function to improve code
organization and readability by extracting logical blocks into dedicated
helper functions. netpoll_setup() length is reduced to 72 LoC.

This series breaks down these responsibilities into focused helper
functions.

The changes are purely structural with no functional modifications.

This changes were tested with the netconsole tests and the netpoll
selftest (WIP)[1]

Link: https://lore.kernel.org/all/20250612-netpoll_test-v1-1-4774fd95933f@debian.org/ [1]

Signed-off-by: Breno Leitao <leitao@debian.org>
---
Breno Leitao (3):
      netpoll: Extract carrier wait function
      netpoll: extract IPv4 address retrieval into helper function
      netpoll: Extract IPv6 address retrieval function

 net/core/netpoll.c | 152 ++++++++++++++++++++++++++++++++---------------------
 1 file changed, 91 insertions(+), 61 deletions(-)
---
base-commit: ec315832f6f98f0fa5719b8b5dd2214ca44ef3f1
change-id: 20250617-netpoll_ip_ref-dca7934ea548

Best regards,
-- 
Breno Leitao <leitao@debian.org>

[PATCH 1/3] netpoll: Extract carrier wait function

[Breno Leitao]

Extract the carrier waiting logic into a dedicated helper function
netpoll_wait_carrier() to improve code readability and reduce
duplication in netpoll_setup().

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 net/core/netpoll.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index 07c453864a7df..473d0006cca1f 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -583,6 +583,21 @@ static char *egress_dev(struct netpoll *np, char *buf)
 	return buf;
 }
 
+static void netpoll_wait_carrier(struct netpoll *np, struct net_device *ndev,
+				 unsigned int timeout)
+{
+	unsigned long atmost;
+
+	atmost = jiffies + timeout * HZ;
+	while (!netif_carrier_ok(ndev)) {
+		if (time_after(jiffies, atmost)) {
+			np_notice(np, "timeout waiting for carrier\n");
+			break;
+		}
+		msleep(1);
+	}
+}
+
 int netpoll_setup(struct netpoll *np)
 {
 	struct net *net = current->nsproxy->net_ns;
@@ -613,28 +628,17 @@ int netpoll_setup(struct netpoll *np)
 	}
 
 	if (!netif_running(ndev)) {
-		unsigned long atmost;
-
 		np_info(np, "device %s not up yet, forcing it\n",
 			egress_dev(np, buf));
 
 		err = dev_open(ndev, NULL);
-
 		if (err) {
 			np_err(np, "failed to open %s\n", ndev->name);
 			goto put;
 		}
 
 		rtnl_unlock();
-		atmost = jiffies + carrier_timeout * HZ;
-		while (!netif_carrier_ok(ndev)) {
-			if (time_after(jiffies, atmost)) {
-				np_notice(np, "timeout waiting for carrier\n");
-				break;
-			}
-			msleep(1);
-		}
-
+		netpoll_wait_carrier(np, ndev, carrier_timeout);
 		rtnl_lock();
 	}
 

-- 
2.47.1

[PATCH 2/3] netpoll: extract IPv4 address retrieval into helper function

[Breno Leitao]

Move the IPv4 address retrieval logic from netpoll_setup() into a
separate netpoll_take_ipv4() function to improve code organization
and readability. This change consolidates the IPv4-specific logic
and error handling into a dedicated function while maintaining
the same functionality.

No functional changes.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 net/core/netpoll.c | 48 +++++++++++++++++++++++++++++++-----------------
 1 file changed, 31 insertions(+), 17 deletions(-)

diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index 473d0006cca1f..6ab494559b5c6 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -598,13 +598,41 @@ static void netpoll_wait_carrier(struct netpoll *np, struct net_device *ndev,
 	}
 }
 
+/*
+ * Take the IPv4 from ndev and populate local_ip structure in netpoll
+ */
+static int netpoll_take_ipv4(struct netpoll *np, struct net_device *ndev)
+{
+	char buf[MAC_ADDR_STR_LEN + 1];
+	const struct in_ifaddr *ifa;
+	struct in_device *in_dev;
+
+	in_dev = __in_dev_get_rtnl(ndev);
+	if (!in_dev) {
+		np_err(np, "no IP address for %s, aborting\n",
+		       egress_dev(np, buf));
+		return -EDESTADDRREQ;
+	}
+
+	ifa = rtnl_dereference(in_dev->ifa_list);
+	if (!ifa) {
+		np_err(np, "no IP address for %s, aborting\n",
+		       egress_dev(np, buf));
+		return -EDESTADDRREQ;
+	}
+
+	np->local_ip.ip = ifa->ifa_local;
+	np_info(np, "local IP %pI4\n", &np->local_ip.ip);
+
+	return 0;
+}
+
 int netpoll_setup(struct netpoll *np)
 {
 	struct net *net = current->nsproxy->net_ns;
 	char buf[MAC_ADDR_STR_LEN + 1];
 	struct net_device *ndev = NULL;
 	bool ip_overwritten = false;
-	struct in_device *in_dev;
 	int err;
 
 	rtnl_lock();
@@ -644,24 +672,10 @@ int netpoll_setup(struct netpoll *np)
 
 	if (!np->local_ip.ip) {
 		if (!np->ipv6) {
-			const struct in_ifaddr *ifa;
-
-			in_dev = __in_dev_get_rtnl(ndev);
-			if (!in_dev)
-				goto put_noaddr;
-
-			ifa = rtnl_dereference(in_dev->ifa_list);
-			if (!ifa) {
-put_noaddr:
-				np_err(np, "no IP address for %s, aborting\n",
-				       egress_dev(np, buf));
-				err = -EDESTADDRREQ;
+			err = netpoll_take_ipv4(np, ndev);
+			if (err)
 				goto put;
-			}
-
-			np->local_ip.ip = ifa->ifa_local;
 			ip_overwritten = true;
-			np_info(np, "local IP %pI4\n", &np->local_ip.ip);
 		} else {
 #if IS_ENABLED(CONFIG_IPV6)
 			struct inet6_dev *idev;

-- 
2.47.1

[PATCH 3/3] netpoll: Extract IPv6 address retrieval function

[Breno Leitao]

Extract the IPv6 address retrieval logic from netpoll_setup() into
a dedicated helper function netpoll_take_ipv6() to improve code
organization and readability.

The function handles obtaining the local IPv6 address from the
network device, including proper address type matching between
local and remote addresses (link-local vs global), and includes
appropriate error handling when IPv6 is not supported or no
suitable address is available.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 net/core/netpoll.c | 76 +++++++++++++++++++++++++++++++-----------------------
 1 file changed, 44 insertions(+), 32 deletions(-)

diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index 6ab494559b5c6..7021ea45a0539 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -598,6 +598,47 @@ static void netpoll_wait_carrier(struct netpoll *np, struct net_device *ndev,
 	}
 }
 
+/*
+ * Take the IPv6 from ndev and populate local_ip structure in netpoll
+ */
+static int netpoll_take_ipv6(struct netpoll *np, struct net_device *ndev)
+{
+	char buf[MAC_ADDR_STR_LEN + 1];
+	int err = -EDESTADDRREQ;
+	struct inet6_dev *idev;
+
+	if (!IS_ENABLED(CONFIG_IPV6)) {
+		np_err(np, "IPv6 is not supported %s, aborting\n",
+		       egress_dev(np, buf));
+		return -EINVAL;
+	}
+
+	idev = __in6_dev_get(ndev);
+	if (idev) {
+		struct inet6_ifaddr *ifp;
+
+		read_lock_bh(&idev->lock);
+		list_for_each_entry(ifp, &idev->addr_list, if_list) {
+			if (!!(ipv6_addr_type(&ifp->addr) & IPV6_ADDR_LINKLOCAL) !=
+				!!(ipv6_addr_type(&np->remote_ip.in6) & IPV6_ADDR_LINKLOCAL))
+				continue;
+			/* Got the IP, let's return */
+			np->local_ip.in6 = ifp->addr;
+			err = 0;
+			break;
+		}
+		read_unlock_bh(&idev->lock);
+	}
+	if (err) {
+		np_err(np, "no IPv6 address for %s, aborting\n",
+		       egress_dev(np, buf));
+		return err;
+	}
+
+	np_info(np, "local IPv6 %pI6c\n", &np->local_ip.in6);
+	return 0;
+}
+
 /*
  * Take the IPv4 from ndev and populate local_ip structure in netpoll
  */
@@ -675,41 +716,12 @@ int netpoll_setup(struct netpoll *np)
 			err = netpoll_take_ipv4(np, ndev);
 			if (err)
 				goto put;
-			ip_overwritten = true;
 		} else {
-#if IS_ENABLED(CONFIG_IPV6)
-			struct inet6_dev *idev;
-
-			err = -EDESTADDRREQ;
-			idev = __in6_dev_get(ndev);
-			if (idev) {
-				struct inet6_ifaddr *ifp;
-
-				read_lock_bh(&idev->lock);
-				list_for_each_entry(ifp, &idev->addr_list, if_list) {
-					if (!!(ipv6_addr_type(&ifp->addr) & IPV6_ADDR_LINKLOCAL) !=
-					    !!(ipv6_addr_type(&np->remote_ip.in6) & IPV6_ADDR_LINKLOCAL))
-						continue;
-					np->local_ip.in6 = ifp->addr;
-					ip_overwritten = true;
-					err = 0;
-					break;
-				}
-				read_unlock_bh(&idev->lock);
-			}
-			if (err) {
-				np_err(np, "no IPv6 address for %s, aborting\n",
-				       egress_dev(np, buf));
+			err = netpoll_take_ipv6(np, ndev);
+			if (err)
 				goto put;
-			} else
-				np_info(np, "local IPv6 %pI6c\n", &np->local_ip.in6);
-#else
-			np_err(np, "IPv6 is not supported %s, aborting\n",
-			       egress_dev(np, buf));
-			err = -EINVAL;
-			goto put;
-#endif
 		}
+		ip_overwritten = true;
 	}
 
 	err = __netpoll_setup(np, ndev);

-- 
2.47.1

Re: [PATCH 0/3] netpoll: Code organization improvements

[Breno Leitao]

On Wed, Jun 18, 2025 at 02:32:44AM -0700, Breno Leitao wrote:
> The netpoll_setup() function has grown complex over time, mixing
> different error handling and concerns like carrier waiting, IPv4 address
> retrieval, and IPv6 address retrieval all within a single function,
> which is huge (127 LoC).
> 
> This patch series refactors the netpoll_setup() function to improve code
> organization and readability by extracting logical blocks into dedicated
> helper functions. netpoll_setup() length is reduced to 72 LoC.
> 
> This series breaks down these responsibilities into focused helper
> functions.
> 
> The changes are purely structural with no functional modifications.
> 
> This changes were tested with the netconsole tests and the netpoll
> selftest (WIP)[1]
> 
> Link: https://lore.kernel.org/all/20250612-netpoll_test-v1-1-4774fd95933f@debian.org/ [1]
> 
> Signed-off-by: Breno Leitao <leitao@debian.org>

I forgot to tag this in the header, but this is against 'net-next'.

I will send a v2 tomorrow with the proper "net-next" tag.

Re: [PATCH 0/3] netpoll: Code organization improvements

[Jakub Kicinski]

On Wed, 18 Jun 2025 08:25:49 -0700 Breno Leitao wrote:
> I will send a v2 tomorrow with the proper "net-next" tag.

No need, it's not worth resending for.

Re: [PATCH 1/3] netpoll: Extract carrier wait function

[Simon Horman]

On Wed, Jun 18, 2025 at 02:32:45AM -0700, Breno Leitao wrote:
> Extract the carrier waiting logic into a dedicated helper function
> netpoll_wait_carrier() to improve code readability and reduce
> duplication in netpoll_setup().
> 
> Signed-off-by: Breno Leitao <leitao@debian.org>

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH 2/3] netpoll: extract IPv4 address retrieval into helper function

[Simon Horman]

On Wed, Jun 18, 2025 at 02:32:46AM -0700, Breno Leitao wrote:
> Move the IPv4 address retrieval logic from netpoll_setup() into a
> separate netpoll_take_ipv4() function to improve code organization
> and readability. This change consolidates the IPv4-specific logic
> and error handling into a dedicated function while maintaining
> the same functionality.
> 
> No functional changes.
> 
> Signed-off-by: Breno Leitao <leitao@debian.org>

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH 3/3] netpoll: Extract IPv6 address retrieval function

[Simon Horman]

On Wed, Jun 18, 2025 at 02:32:47AM -0700, Breno Leitao wrote:
> Extract the IPv6 address retrieval logic from netpoll_setup() into
> a dedicated helper function netpoll_take_ipv6() to improve code
> organization and readability.
> 
> The function handles obtaining the local IPv6 address from the
> network device, including proper address type matching between
> local and remote addresses (link-local vs global), and includes
> appropriate error handling when IPv6 is not supported or no
> suitable address is available.
> 
> Signed-off-by: Breno Leitao <leitao@debian.org>

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH 0/3] netpoll: Code organization improvements

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 18 Jun 2025 02:32:44 -0700 you wrote:
> The netpoll_setup() function has grown complex over time, mixing
> different error handling and concerns like carrier waiting, IPv4 address
> retrieval, and IPv6 address retrieval all within a single function,
> which is huge (127 LoC).
> 
> This patch series refactors the netpoll_setup() function to improve code
> organization and readability by extracting logical blocks into dedicated
> helper functions. netpoll_setup() length is reduced to 72 LoC.
> 
> [...]

Here is the summary with links:
  - [1/3] netpoll: Extract carrier wait function
    https://git.kernel.org/netdev/net-next/c/76d30b51e818
  - [2/3] netpoll: extract IPv4 address retrieval into helper function
    https://git.kernel.org/netdev/net-next/c/3699f992e8c2
  - [3/3] netpoll: Extract IPv6 address retrieval function
    https://git.kernel.org/netdev/net-next/c/6ad7969a361c

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html