[PATCH net-next v3 0/5] netconsole: reuse netpoll_parse_ip_addr in configfs helpers

[PATCH net-next v3 0/5] netconsole: reuse netpoll_parse_ip_addr in configfs helpers

[Breno Leitao]

This patchset refactors the IP address parsing logic in the netconsole
driver to eliminate code duplication and improve maintainability. The
changes centralize IPv4 and IPv6 address parsing into a single function
(netpoll_parse_ip_addr). For that, it needs to teach
netpoll_parse_ip_addr() to handle strings with newlines, which is the
type of string coming from configfs.

Background

The netconsole driver currently has duplicate IP address parsing logic
in both local_ip_store() and remote_ip_store() functions. This
duplication increases the risk of inconsistencies and makes the code
harder to maintain.

Benefits

* Reduced code duplication: ~40 lines of duplicate parsing logic eliminated
 * Improved robustness: Centralized parsing reduces the chance of inconsistencies
 * Easier to maintain: Code follow more the netdev way

PS: The patches are very well contained in other to help review.

---
Changes in v3:
- Avoid #ifdef and use if (IS_ENABLED()) instead (Simon)
- Assing an int to a boolean using !! (Simon)
- Link to v2: https://lore.kernel.org/r/20250721-netconsole_ref-v2-0-b42f1833565a@debian.org

Changes in v2:
- Moved the netpoll_parse_ip_addr() to outside the dynamic block (Jakub)
- Link to v1: https://lore.kernel.org/r/20250718-netconsole_ref-v1-0-86ef253b7a7a@debian.org

---
Breno Leitao (5):
      netpoll: Remove unused fields from inet_addr union
      netconsole: move netpoll_parse_ip_addr() earlier for reuse
      netconsole: add support for strings with new line in netpoll_parse_ip_addr
      netconsole: use netpoll_parse_ip_addr in local_ip_store
      netconsole: use netpoll_parse_ip_addr in local_ip_store

 drivers/net/netconsole.c | 84 +++++++++++++++++-------------------------------
 include/linux/netpoll.h  |  3 --
 2 files changed, 30 insertions(+), 57 deletions(-)
---
base-commit: d61f6cb6f6ef3c70d2ccc0d9c85c508cb8017da9
change-id: 20250718-netconsole_ref-c1f7254cfb51

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

[PATCH net-next v3 1/5] netpoll: Remove unused fields from inet_addr union

[Breno Leitao]

Clean up the inet_addr union by removing unused fields that are
redundant with existing members:

This simplifies the union structure while maintaining all necessary
functionality for both IPv4 and IPv6 address handling.

Signed-off-by: Breno Leitao <leitao@debian.org>
Reviewed-by: Simon Horman <horms@kernel.org>
---
 include/linux/netpoll.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/include/linux/netpoll.h b/include/linux/netpoll.h
index 735e65c3cc114..b5ea9882eda8b 100644
--- a/include/linux/netpoll.h
+++ b/include/linux/netpoll.h
@@ -15,10 +15,7 @@
 #include <linux/refcount.h>
 
 union inet_addr {
-	__u32		all[4];
 	__be32		ip;
-	__be32		ip6[4];
-	struct in_addr	in;
 	struct in6_addr	in6;
 };
 

-- 
2.47.1

[PATCH net-next v3 2/5] netconsole: move netpoll_parse_ip_addr() earlier for reuse

[Breno Leitao]

Move netpoll_parse_ip_addr() earlier in the file to be reused in
other functions, such as local_ip_store(). This avoids duplicate
address parsing logic and centralizes validation for both IPv4
and IPv6 string input.

No functional changes intended.

Signed-off-by: Breno Leitao <leitao@debian.org>
Reviewed-by: Simon Horman <horms@kernel.org>
---
 drivers/net/netconsole.c | 40 ++++++++++++++++++++--------------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index e3722de08ea9f..8d1b93264e0fd 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -300,6 +300,26 @@ static void netconsole_print_banner(struct netpoll *np)
 	np_info(np, "remote ethernet address %pM\n", np->remote_mac);
 }
 
+static int netpoll_parse_ip_addr(const char *str, union inet_addr *addr)
+{
+	const char *end;
+
+	if (!strchr(str, ':') &&
+	    in4_pton(str, -1, (void *)addr, -1, &end) > 0) {
+		if (!*end)
+			return 0;
+	}
+	if (in6_pton(str, -1, addr->in6.s6_addr, -1, &end) > 0) {
+#if IS_ENABLED(CONFIG_IPV6)
+		if (!*end)
+			return 1;
+#else
+		return -1;
+#endif
+	}
+	return -1;
+}
+
 #ifdef	CONFIG_NETCONSOLE_DYNAMIC
 
 /*
@@ -1742,26 +1762,6 @@ static void write_msg(struct console *con, const char *msg, unsigned int len)
 	spin_unlock_irqrestore(&target_list_lock, flags);
 }
 
-static int netpoll_parse_ip_addr(const char *str, union inet_addr *addr)
-{
-	const char *end;
-
-	if (!strchr(str, ':') &&
-	    in4_pton(str, -1, (void *)addr, -1, &end) > 0) {
-		if (!*end)
-			return 0;
-	}
-	if (in6_pton(str, -1, addr->in6.s6_addr, -1, &end) > 0) {
-#if IS_ENABLED(CONFIG_IPV6)
-		if (!*end)
-			return 1;
-#else
-		return -1;
-#endif
-	}
-	return -1;
-}
-
 static int netconsole_parser_cmdline(struct netpoll *np, char *opt)
 {
 	bool ipversion_set = false;

-- 
2.47.1

[PATCH net-next v3 3/5] netconsole: add support for strings with new line in netpoll_parse_ip_addr

[Breno Leitao]

The current IP address parsing logic fails when the input string
contains a trailing newline character. This can occur when IP
addresses are provided through configfs, which contains newlines in
a const buffer.

Teach netpoll_parse_ip_addr() how to ignore newlines at the end of the
IPs. Also, simplify the code by:

 * No need to check for separators. Try to parse ipv4, if it fails try
   ipv6 similarly to ceph_pton()
 * If ipv6 is not supported, don't call in6_pton() at all.

Signed-off-by: Breno Leitao <leitao@debian.org>
Reviewed-by: Simon Horman <horms@kernel.org>
---
 drivers/net/netconsole.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index 8d1b93264e0fd..3188cc180a934 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -303,20 +303,20 @@ static void netconsole_print_banner(struct netpoll *np)
 static int netpoll_parse_ip_addr(const char *str, union inet_addr *addr)
 {
 	const char *end;
+	int len;
 
-	if (!strchr(str, ':') &&
-	    in4_pton(str, -1, (void *)addr, -1, &end) > 0) {
-		if (!*end)
-			return 0;
-	}
-	if (in6_pton(str, -1, addr->in6.s6_addr, -1, &end) > 0) {
-#if IS_ENABLED(CONFIG_IPV6)
-		if (!*end)
-			return 1;
-#else
+	len = strlen(str);
+	if (!len)
 		return -1;
-#endif
-	}
+
+	if (str[len - 1] == '\n')
+		len -= 1;
+
+	if (in4_pton(str, len, (void *)addr, -1, &end) > 0)
+		return 0;
+	if (IS_ENABLED(CONFIG_IPV6) &&
+	    in6_pton(str, len, addr->in6.s6_addr, -1, &end) > 0)
+		return 1;
 	return -1;
 }
 

-- 
2.47.1

[PATCH net-next v3 4/5] netconsole: use netpoll_parse_ip_addr in local_ip_store

[Breno Leitao]

Replace manual IP address parsing with a call to netpoll_parse_ip_addr
in local_ip_store(), simplifying the code and reducing the chance of
errors.

Also, remove the pr_err() if the user enters an invalid value in
configfs entries. pr_err() is not the best way to alert user that the
configuration is invalid.

Signed-off-by: Breno Leitao <leitao@debian.org>
Reviewed-by: Simon Horman <horms@kernel.org>
---
 drivers/net/netconsole.c | 22 +++++-----------------
 1 file changed, 5 insertions(+), 17 deletions(-)

diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index 3188cc180a934..358db590a5046 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -750,6 +750,7 @@ static ssize_t local_ip_store(struct config_item *item, const char *buf,
 {
 	struct netconsole_target *nt = to_target(item);
 	ssize_t ret = -EINVAL;
+	int ipv6;
 
 	mutex_lock(&dynamic_netconsole_mutex);
 	if (nt->enabled) {
@@ -758,23 +759,10 @@ static ssize_t local_ip_store(struct config_item *item, const char *buf,
 		goto out_unlock;
 	}
 
-	if (strnchr(buf, count, ':')) {
-		const char *end;
-
-		if (in6_pton(buf, count, nt->np.local_ip.in6.s6_addr, -1, &end) > 0) {
-			if (*end && *end != '\n') {
-				pr_err("invalid IPv6 address at: <%c>\n", *end);
-				goto out_unlock;
-			}
-			nt->np.ipv6 = true;
-		} else
-			goto out_unlock;
-	} else {
-		if (!nt->np.ipv6)
-			nt->np.local_ip.ip = in_aton(buf);
-		else
-			goto out_unlock;
-	}
+	ipv6 = netpoll_parse_ip_addr(buf, &nt->np.local_ip);
+	if (ipv6 == -1)
+		goto out_unlock;
+	nt->np.ipv6 = !!ipv6;
 
 	ret = strnlen(buf, count);
 out_unlock:

-- 
2.47.1

[PATCH net-next v3 5/5] netconsole: use netpoll_parse_ip_addr in local_ip_store

[Breno Leitao]

Replace manual IP address parsing with a call to netpoll_parse_ip_addr
in remote_ip_store(), simplifying the code and reducing the chance of
errors.

The error message got removed, since it is not a good practice to
pr_err() if used pass a wrong value in configfs.

Signed-off-by: Breno Leitao <leitao@debian.org>
Reviewed-by: Simon Horman <horms@kernel.org>
---
 drivers/net/netconsole.c | 22 +++++-----------------
 1 file changed, 5 insertions(+), 17 deletions(-)

diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index 358db590a5046..ef7d385a28151 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -775,6 +775,7 @@ static ssize_t remote_ip_store(struct config_item *item, const char *buf,
 {
 	struct netconsole_target *nt = to_target(item);
 	ssize_t ret = -EINVAL;
+	int ipv6;
 
 	mutex_lock(&dynamic_netconsole_mutex);
 	if (nt->enabled) {
@@ -783,23 +784,10 @@ static ssize_t remote_ip_store(struct config_item *item, const char *buf,
 		goto out_unlock;
 	}
 
-	if (strnchr(buf, count, ':')) {
-		const char *end;
-
-		if (in6_pton(buf, count, nt->np.remote_ip.in6.s6_addr, -1, &end) > 0) {
-			if (*end && *end != '\n') {
-				pr_err("invalid IPv6 address at: <%c>\n", *end);
-				goto out_unlock;
-			}
-			nt->np.ipv6 = true;
-		} else
-			goto out_unlock;
-	} else {
-		if (!nt->np.ipv6)
-			nt->np.remote_ip.ip = in_aton(buf);
-		else
-			goto out_unlock;
-	}
+	ipv6 = netpoll_parse_ip_addr(buf, &nt->np.remote_ip);
+	if (ipv6 == -1)
+		goto out_unlock;
+	nt->np.ipv6 = !!ipv6;
 
 	ret = strnlen(buf, count);
 out_unlock:

-- 
2.47.1

Re: [PATCH net-next v3 3/5] netconsole: add support for strings with new line in netpoll_parse_ip_addr

[Jakub Kicinski]

On Wed, 23 Jul 2025 10:20:31 -0700 Breno Leitao wrote:
>  	const char *end;
> +	int len;
>  
> -	if (!strchr(str, ':') &&
> -	    in4_pton(str, -1, (void *)addr, -1, &end) > 0) {
> -		if (!*end)
> -			return 0;
> -	}
> -	if (in6_pton(str, -1, addr->in6.s6_addr, -1, &end) > 0) {
> -#if IS_ENABLED(CONFIG_IPV6)
> -		if (!*end)
> -			return 1;
> -#else
> +	len = strlen(str);
> +	if (!len)
>  		return -1;
> -#endif
> -	}
> +
> +	if (str[len - 1] == '\n')
> +		len -= 1;
> +
> +	if (in4_pton(str, len, (void *)addr, -1, &end) > 0)
> +		return 0;
> +	if (IS_ENABLED(CONFIG_IPV6) &&
> +	    in6_pton(str, len, addr->in6.s6_addr, -1, &end) > 0)
> +		return 1;
>  	return -1;

Looks like we're removing the validation for reaching the end of 
the buffer? This should at least be explained in the commit message.
AFAICT for IPv4 for example 1.2.3.4:xyz would have been rejected,
now it's accepted. Personally I find the

	if (*end && *end != '\n')

removed by subsequent patch cleaner than modifying the input string.

I'll apply the first patch of the series..

Re: [PATCH net-next v3 0/5] netconsole: reuse netpoll_parse_ip_addr in configfs helpers

[patchwork-bot+netdevbpf]

Hello:

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

On Wed, 23 Jul 2025 10:20:28 -0700 you wrote:
> This patchset refactors the IP address parsing logic in the netconsole
> driver to eliminate code duplication and improve maintainability. The
> changes centralize IPv4 and IPv6 address parsing into a single function
> (netpoll_parse_ip_addr). For that, it needs to teach
> netpoll_parse_ip_addr() to handle strings with newlines, which is the
> type of string coming from configfs.
> 
> [...]

Here is the summary with links:
  - [net-next,v3,1/5] netpoll: Remove unused fields from inet_addr union
    https://git.kernel.org/netdev/net-next/c/33360f2508e0
  - [net-next,v3,2/5] netconsole: move netpoll_parse_ip_addr() earlier for reuse
    (no matching commit)
  - [net-next,v3,3/5] netconsole: add support for strings with new line in netpoll_parse_ip_addr
    (no matching commit)
  - [net-next,v3,4/5] netconsole: use netpoll_parse_ip_addr in local_ip_store
    (no matching commit)
  - [net-next,v3,5/5] netconsole: use netpoll_parse_ip_addr in local_ip_store
    (no matching commit)

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