[PATCH net-next v4 0/7] netconsole: validate a target's IP address configuration

[PATCH net-next v4 0/7] netconsole: validate a target's IP address configuration

[Gustavo Luiz Duarte]

This series adds two validations to the target configuration when the
user tries to enable it: first whether remote_ip was set, and second
whether local_ip and remote_ip address families match. Refuse to enable
the target if any of those validations fail.

These validations are already done for the target passed on the
command-line, so this aligns dynamic targets with the command-line
behavior.

The first two patches replace the per-target 'ipv6' flag with a
per-address 'family' field, which makes it easier to detect these error
conditions. Patches 3 and 4 implement the actual validations.

Patches 5-7 are follow-ups from previous reviews: move inet_addr from
netpoll.h into netconsole.c, show an unset address as an empty string
rather than "0.0.0.0", document local_ip auto-selection.

Signed-off-by: Gustavo Luiz Duarte <gustavold@gmail.com>
---
Changes in v4:
- No changes, just reposting post merge window close.
- Link to v3: https://patch.msgid.link/20260814-netcons_ipv6-v3-0-bc0915e8c75f@gmail.com

Changes in v3:
- Document local_ip auto-selection
- Link to v2: https://patch.msgid.link/20260810-netcons_ipv6-v2-0-3d4fc987a90f@gmail.com

Changes in v2:
- Show empty string in configfs for an unset IP address
- Moved inet_addr definition from netpoll.h to netconsole.c
- Moved address checks out of rtnl_lock()
- Link to v1: https://patch.msgid.link/20260805-netcons_ipv6-v1-0-170a35b92da1@gmail.com

To: Breno Leitao <leitao@debian.org>
To: Andrew Lunn <andrew+netdev@lunn.ch>
To: "David S. Miller" <davem@davemloft.net>
To: Eric Dumazet <edumazet@google.com>
To: Jakub Kicinski <kuba@kernel.org>
To: Paolo Abeni <pabeni@redhat.com>
To: Simon Horman <horms@kernel.org>
To: Jonathan Corbet <corbet@lwn.net>
To: Shuah Khan <skhan@linuxfoundation.org>
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-doc@vger.kernel.org

---
Gustavo Luiz Duarte (7):
      netconsole: add an address family to struct inet_addr
      netconsole: use the address family instead of the ipv6 flag
      netconsole: reject enabling a target with no remote IP address
      netconsole: reject a target mixing IPv4 and IPv6 addresses
      netconsole: show empty string for an unset IP address
      netconsole: move struct inet_addr into netconsole.c
      docs: netconsole: document local_ip auto-selection

 Documentation/networking/netconsole.rst |   5 ++
 drivers/net/netconsole.c                | 132 ++++++++++++++++----------------
 include/linux/netpoll.h                 |   5 --
 3 files changed, 70 insertions(+), 72 deletions(-)
---
base-commit: 7042c8c193e5d634198b7c766bb3a01c8e3ee0e2
change-id: 20260730-netcons_ipv6-565d55f55729

Best regards,
--  
Gustavo Luiz Duarte <gustavold@gmail.com>

[PATCH net-next v4 1/7] netconsole: add an address family to struct inet_addr

[Gustavo Luiz Duarte]

netconsole_target stores a single 'bool ipv6' to denote the target's
address family. This makes it hard to detect conditions like "no address
set" or ipv4/ipv6 mixup between local_ip and remote_ip.

Add a 'family' field to inet_addr so each address stores its own address
family: AF_UNSPEC while unset, else AF_INET or AF_INET6.

Nothing reads the new field yet. The next patch switches the users over
and removes the bool. No functional change.

Reviewed-by: Breno Leitao <leitao@debian.org>
Signed-off-by: Gustavo Luiz Duarte <gustavold@gmail.com>
---
 drivers/net/netconsole.c | 24 +++++++++++++++---------
 include/linux/netpoll.h  | 10 +++++++---
 2 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index b358e5c36735..432b66cf111a 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -225,7 +225,7 @@ struct netconsole_target {
 	bool			extended;
 	bool			release;
 	struct netpoll		np;
-	union inet_addr		local_ip, remote_ip;
+	struct inet_addr	local_ip, remote_ip;
 	bool			ipv6;
 	u16			local_port, remote_port;
 	u8			remote_mac[ETH_ALEN];
@@ -428,6 +428,7 @@ static int netcons_take_ipv6(struct netconsole_target *nt,
 				continue;
 			/* Got the IP, let's return */
 			nt->local_ip.in6 = ifp->addr;
+			nt->local_ip.family = AF_INET6;
 			err = 0;
 			break;
 		}
@@ -469,6 +470,7 @@ static int netcons_take_ipv4(struct netconsole_target *nt,
 	}
 
 	nt->local_ip.ip = ifa->ifa_local;
+	nt->local_ip.family = AF_INET;
 	np_info(np, "local IP %pI4\n", &nt->local_ip.ip);
 
 	return 0;
@@ -741,10 +743,10 @@ static void netconsole_print_banner(struct netconsole_target *nt)
 	np_info(np, "remote ethernet address %pM\n", nt->remote_mac);
 }
 
-/* Parse the string and populate the `inet_addr` union. Return 0 if IPv4 is
+/* Parse the string and populate the `inet_addr` struct. Return 0 if IPv4 is
  * populated, 1 if IPv6 is populated, and -1 upon failure.
  */
-static int netpoll_parse_ip_addr(const char *str, union inet_addr *addr)
+static int netpoll_parse_ip_addr(const char *str, struct inet_addr *addr)
 {
 	const char *end = NULL;
 	int len;
@@ -756,14 +758,18 @@ static int netpoll_parse_ip_addr(const char *str, union inet_addr *addr)
 	if (str[len - 1] == '\n')
 		len -= 1;
 
-	if (in4_pton(str, len, (void *)addr, -1, &end) > 0 &&
-	    (!end || *end == 0 || *end == '\n'))
+	if (in4_pton(str, len, (void *)&addr->ip, -1, &end) > 0 &&
+	    (!end || *end == 0 || *end == '\n')) {
+		addr->family = AF_INET;
 		return 0;
+	}
 
 	if (IS_ENABLED(CONFIG_IPV6) &&
-	    in6_pton(str, len, (void *)addr, -1, &end) > 0 &&
-	    (!end || *end == 0 || *end == '\n'))
+	    in6_pton(str, len, (void *)&addr->in6, -1, &end) > 0 &&
+	    (!end || *end == 0 || *end == '\n')) {
+		addr->family = AF_INET6;
 		return 1;
+	}
 
 	return -1;
 }
@@ -871,7 +877,7 @@ static ssize_t local_ip_show(struct config_item *item, char *buf)
 	if (nt->ipv6)
 		return sysfs_emit(buf, "%pI6c\n", &nt->local_ip.in6);
 	else
-		return sysfs_emit(buf, "%pI4\n", &nt->local_ip);
+		return sysfs_emit(buf, "%pI4\n", &nt->local_ip.ip);
 }
 
 static ssize_t remote_ip_show(struct config_item *item, char *buf)
@@ -881,7 +887,7 @@ static ssize_t remote_ip_show(struct config_item *item, char *buf)
 	if (nt->ipv6)
 		return sysfs_emit(buf, "%pI6c\n", &nt->remote_ip.in6);
 	else
-		return sysfs_emit(buf, "%pI4\n", &nt->remote_ip);
+		return sysfs_emit(buf, "%pI4\n", &nt->remote_ip.ip);
 }
 
 static ssize_t local_mac_show(struct config_item *item, char *buf)
diff --git a/include/linux/netpoll.h b/include/linux/netpoll.h
index 1c6b1eec5efd..de97f001a0f9 100644
--- a/include/linux/netpoll.h
+++ b/include/linux/netpoll.h
@@ -16,9 +16,13 @@
 #include <linux/ip.h>
 #include <linux/udp.h>
 
-union inet_addr {
-	__be32		ip;
-	struct in6_addr	in6;
+struct inet_addr {
+	/* Address family: AF_UNSPEC when unset, else AF_INET or AF_INET6 */
+	u8			family;
+	union {
+		__be32		ip;
+		struct in6_addr	in6;
+	};
 };
 
 struct netpoll {

-- 
2.55.0

[PATCH net-next v4 2/7] netconsole: use the address family instead of the ipv6 flag

[Gustavo Luiz Duarte]

Now that we have the address family in inet_addr, use that and remove
nt->ipv6.

We no longer need netcons_local_ip_unset() to check that all bytes are
zeroes, as that is now denoted by (family == AF_UNSPEC).

Reviewed-by: Breno Leitao <leitao@debian.org>
Signed-off-by: Gustavo Luiz Duarte <gustavold@gmail.com>
---
 drivers/net/netconsole.c | 82 ++++++++++++++----------------------------------
 1 file changed, 23 insertions(+), 59 deletions(-)

diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index 432b66cf111a..351754d53cf9 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -195,7 +195,6 @@ struct netcons_userdata {
  *		local_mac	(read-only)
  * @local_ip:	Source IP address of the target (read-write).
  * @remote_ip:	Destination IP address of the target (read-write).
- * @ipv6:	Whether the target addresses are IPv6 (read-write).
  * @local_port:	Source UDP port of the target (read-write).
  * @remote_port: Destination UDP port of the target (read-write).
  * @remote_mac:	Destination ethernet address of the target (read-write).
@@ -226,7 +225,6 @@ struct netconsole_target {
 	bool			release;
 	struct netpoll		np;
 	struct inet_addr	local_ip, remote_ip;
-	bool			ipv6;
 	u16			local_port, remote_port;
 	u8			remote_mac[ETH_ALEN];
 	/* protected by target_list_lock; +1 gives scnprintf() room for its
@@ -476,23 +474,6 @@ static int netcons_take_ipv4(struct netconsole_target *nt,
 	return 0;
 }
 
-/*
- * Test whether the caller left nt->local_ip unset, so that
- * netcons_netpoll_setup() should auto-populate it from the egress device.
- *
- * nt->local_ip is a union of __be32 (IPv4) and struct in6_addr (IPv6),
- * so an IPv6 address whose first 4 bytes are zero (e.g. ::1, ::2,
- * IPv4-mapped ::ffff:a.b.c.d) must not be tested via the IPv4 arm —
- * doing so would misclassify a caller-supplied address as unset and
- * silently overwrite it with whatever address the device exposes.
- */
-static bool netcons_local_ip_unset(const struct netconsole_target *nt)
-{
-	if (nt->ipv6)
-		return ipv6_addr_any(&nt->local_ip.in6);
-	return !nt->local_ip.ip;
-}
-
 static int netcons_netpoll_setup(struct netconsole_target *nt)
 {
 	struct net *net = current->nsproxy->net_ns;
@@ -538,16 +519,13 @@ static int netcons_netpoll_setup(struct netconsole_target *nt)
 		rtnl_lock();
 	}
 
-	if (netcons_local_ip_unset(nt)) {
-		if (!nt->ipv6) {
-			err = netcons_take_ipv4(nt, ndev);
-			if (err)
-				goto put;
-		} else {
+	if (nt->local_ip.family == AF_UNSPEC) {
+		if (nt->remote_ip.family == AF_INET6)
 			err = netcons_take_ipv6(nt, ndev);
-			if (err)
-				goto put;
-		}
+		else
+			err = netcons_take_ipv4(nt, ndev);
+		if (err)
+			goto put;
 		ip_overwritten = true;
 	}
 
@@ -729,22 +707,22 @@ static void netconsole_print_banner(struct netconsole_target *nt)
 	struct netpoll *np = &nt->np;
 
 	np_info(np, "local port %d\n", nt->local_port);
-	if (nt->ipv6)
+	if (nt->local_ip.family == AF_INET6)
 		np_info(np, "local IPv6 address %pI6c\n", &nt->local_ip.in6);
 	else
 		np_info(np, "local IPv4 address %pI4\n", &nt->local_ip.ip);
 	np_info(np, "interface name '%s'\n", np->dev_name);
 	np_info(np, "local ethernet address '%pM'\n", np->dev_mac);
 	np_info(np, "remote port %d\n", nt->remote_port);
-	if (nt->ipv6)
+	if (nt->remote_ip.family == AF_INET6)
 		np_info(np, "remote IPv6 address %pI6c\n", &nt->remote_ip.in6);
 	else
 		np_info(np, "remote IPv4 address %pI4\n", &nt->remote_ip.ip);
 	np_info(np, "remote ethernet address %pM\n", nt->remote_mac);
 }
 
-/* Parse the string and populate the `inet_addr` struct. Return 0 if IPv4 is
- * populated, 1 if IPv6 is populated, and -1 upon failure.
+/* Parse the string and populate the `inet_addr` struct. Return 0 on success
+ * and -1 upon failure.
  */
 static int netpoll_parse_ip_addr(const char *str, struct inet_addr *addr)
 {
@@ -768,7 +746,7 @@ static int netpoll_parse_ip_addr(const char *str, struct inet_addr *addr)
 	    in6_pton(str, len, (void *)&addr->in6, -1, &end) > 0 &&
 	    (!end || *end == 0 || *end == '\n')) {
 		addr->family = AF_INET6;
-		return 1;
+		return 0;
 	}
 
 	return -1;
@@ -874,7 +852,7 @@ static ssize_t local_ip_show(struct config_item *item, char *buf)
 {
 	struct netconsole_target *nt = to_target(item);
 
-	if (nt->ipv6)
+	if (nt->local_ip.family == AF_INET6)
 		return sysfs_emit(buf, "%pI6c\n", &nt->local_ip.in6);
 	else
 		return sysfs_emit(buf, "%pI4\n", &nt->local_ip.ip);
@@ -884,7 +862,7 @@ static ssize_t remote_ip_show(struct config_item *item, char *buf)
 {
 	struct netconsole_target *nt = to_target(item);
 
-	if (nt->ipv6)
+	if (nt->remote_ip.family == AF_INET6)
 		return sysfs_emit(buf, "%pI6c\n", &nt->remote_ip.in6);
 	else
 		return sysfs_emit(buf, "%pI4\n", &nt->remote_ip.ip);
@@ -1232,7 +1210,6 @@ 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;
 
 	dynamic_netconsole_mutex_lock();
 	if (nt->state == STATE_ENABLED) {
@@ -1241,10 +1218,8 @@ static ssize_t local_ip_store(struct config_item *item, const char *buf,
 		goto out_unlock;
 	}
 
-	ipv6 = netpoll_parse_ip_addr(buf, &nt->local_ip);
-	if (ipv6 == -1)
+	if (netpoll_parse_ip_addr(buf, &nt->local_ip) < 0)
 		goto out_unlock;
-	nt->ipv6 = !!ipv6;
 
 	ret = count;
 out_unlock:
@@ -1257,7 +1232,6 @@ 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;
 
 	dynamic_netconsole_mutex_lock();
 	if (nt->state == STATE_ENABLED) {
@@ -1266,10 +1240,8 @@ static ssize_t remote_ip_store(struct config_item *item, const char *buf,
 		goto out_unlock;
 	}
 
-	ipv6 = netpoll_parse_ip_addr(buf, &nt->remote_ip);
-	if (ipv6 == -1)
+	if (netpoll_parse_ip_addr(buf, &nt->remote_ip) < 0)
 		goto out_unlock;
-	nt->ipv6 = !!ipv6;
 
 	ret = count;
 out_unlock:
@@ -2078,7 +2050,7 @@ static void netpoll_udp_checksum(struct netconsole_target *nt,
 
 	/* check needs to be set, since it will be consumed in csum_partial */
 	udph->check = 0;
-	if (nt->ipv6)
+	if (nt->remote_ip.family == AF_INET6)
 		udph->check = csum_ipv6_magic(&nt->local_ip.in6,
 					      &nt->remote_ip.in6,
 					      udp_len, IPPROTO_UDP,
@@ -2119,7 +2091,7 @@ static void push_eth(struct netconsole_target *nt, struct sk_buff *skb)
 	skb_reset_mac_header(skb);
 	ether_addr_copy(eth->h_source, np->dev->dev_addr);
 	ether_addr_copy(eth->h_dest, nt->remote_mac);
-	if (nt->ipv6)
+	if (nt->remote_ip.family == AF_INET6)
 		eth->h_proto = htons(ETH_P_IPV6);
 	else
 		eth->h_proto = htons(ETH_P_IP);
@@ -2188,7 +2160,7 @@ static int netpoll_send_udp(struct netconsole_target *nt, const char *msg,
 		WARN_ON_ONCE(!irqs_disabled());
 
 	udp_len = len + sizeof(struct udphdr);
-	if (nt->ipv6)
+	if (nt->remote_ip.family == AF_INET6)
 		ip_len = udp_len + sizeof(struct ipv6hdr);
 	else
 		ip_len = udp_len + sizeof(struct iphdr);
@@ -2204,7 +2176,7 @@ static int netpoll_send_udp(struct netconsole_target *nt, const char *msg,
 	skb_put(skb, len);
 
 	push_udp(nt, skb, len);
-	if (nt->ipv6)
+	if (nt->remote_ip.family == AF_INET6)
 		push_ipv6(nt, skb, len);
 	else
 		push_ipv4(nt, skb, len);
@@ -2533,10 +2505,8 @@ __releases(&target_list_lock)
 static int netconsole_parser_cmdline(struct netconsole_target *nt, char *opt)
 {
 	struct netpoll *np = &nt->np;
-	bool ipversion_set = false;
 	char *cur = opt;
 	char *delim;
-	int ipv6;
 
 	if (*cur != '@') {
 		delim = strchr(cur, '@');
@@ -2550,16 +2520,12 @@ static int netconsole_parser_cmdline(struct netconsole_target *nt, char *opt)
 	cur++;
 
 	if (*cur != '/') {
-		ipversion_set = true;
 		delim = strchr(cur, '/');
 		if (!delim)
 			goto parse_failed;
 		*delim = 0;
-		ipv6 = netpoll_parse_ip_addr(cur, &nt->local_ip);
-		if (ipv6 < 0)
+		if (netpoll_parse_ip_addr(cur, &nt->local_ip) < 0)
 			goto parse_failed;
-		else
-			nt->ipv6 = (bool)ipv6;
 		cur = delim;
 	}
 	cur++;
@@ -2601,13 +2567,11 @@ static int netconsole_parser_cmdline(struct netconsole_target *nt, char *opt)
 	if (!delim)
 		goto parse_failed;
 	*delim = 0;
-	ipv6 = netpoll_parse_ip_addr(cur, &nt->remote_ip);
-	if (ipv6 < 0)
+	if (netpoll_parse_ip_addr(cur, &nt->remote_ip) < 0)
 		goto parse_failed;
-	else if (ipversion_set && nt->ipv6 != (bool)ipv6)
+	if (nt->local_ip.family != AF_UNSPEC &&
+	    nt->local_ip.family != nt->remote_ip.family)
 		goto parse_failed;
-	else
-		nt->ipv6 = (bool)ipv6;
 	cur = delim + 1;
 
 	if (*cur != 0) {

-- 
2.55.0

[PATCH net-next v4 3/7] netconsole: reject enabling a target with no remote IP address

[Gustavo Luiz Duarte]

The command-line path already requires a remote address, but if a user
creates a dynamic target and enables it without setting a remote
address, we currently try sending netconsole traffic to "0.0.0.0".

Refuse to enable a target if the remote address is unset.

Reviewed-by: Breno Leitao <leitao@debian.org>
Signed-off-by: Gustavo Luiz Duarte <gustavold@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 351754d53cf9..4af4f3039d4c 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -483,6 +483,11 @@ static int netcons_netpoll_setup(struct netconsole_target *nt)
 	bool ip_overwritten = false;
 	int err;
 
+	if (nt->remote_ip.family == AF_UNSPEC) {
+		np_err(np, "remote IP address not configured, aborting\n");
+		return -EDESTADDRREQ;
+	}
+
 	rtnl_lock();
 	if (np->dev_name[0])
 		ndev = __dev_get_by_name(net, np->dev_name);

-- 
2.55.0

[PATCH net-next v4 4/7] netconsole: reject a target mixing IPv4 and IPv6 addresses

[Gustavo Luiz Duarte]

The local_ip and remote_ip configfs attributes are written independently
and nothing stops a user from mixing ipv4 and ipv6. This leads to an
ipv4 address being zero-extended into an ipv6 header or an ipv6 address
being truncated to its first 4 bytes for an ipv4 header.

The command-line parser already refuses such a mismatch. This adds a
similar check when a dynamic target is being enabled.

Reviewed-by: Breno Leitao <leitao@debian.org>
Signed-off-by: Gustavo Luiz Duarte <gustavold@gmail.com>
---
 drivers/net/netconsole.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index 4af4f3039d4c..abcc8515ddd2 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -488,6 +488,12 @@ static int netcons_netpoll_setup(struct netconsole_target *nt)
 		return -EDESTADDRREQ;
 	}
 
+	if (nt->local_ip.family != AF_UNSPEC &&
+	    nt->local_ip.family != nt->remote_ip.family) {
+		np_err(np, "local and remote IP address families differ, aborting\n");
+		return -EINVAL;
+	}
+
 	rtnl_lock();
 	if (np->dev_name[0])
 		ndev = __dev_get_by_name(net, np->dev_name);

-- 
2.55.0

[PATCH net-next v4 5/7] netconsole: show empty string for an unset IP address

[Gustavo Luiz Duarte]

An unset address (local_ip/remote_ip), now denoted by AF_UNSPEC,
currently shows "0.0.0.0" in configfs. Print an empty string instead,
which is more clear.

In netconsole_print_banner() we can be even more explicit about it, as
we don't have the risk of userspace trying to parse it.

Reviewed-by: Breno Leitao <leitao@debian.org>
Signed-off-by: Gustavo Luiz Duarte <gustavold@gmail.com>
---
 drivers/net/netconsole.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index abcc8515ddd2..1f18d0a7d2dd 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -718,14 +718,18 @@ static void netconsole_print_banner(struct netconsole_target *nt)
 	struct netpoll *np = &nt->np;
 
 	np_info(np, "local port %d\n", nt->local_port);
-	if (nt->local_ip.family == AF_INET6)
+	if (nt->local_ip.family == AF_UNSPEC)
+		np_info(np, "local IP unset\n");
+	else if (nt->local_ip.family == AF_INET6)
 		np_info(np, "local IPv6 address %pI6c\n", &nt->local_ip.in6);
 	else
 		np_info(np, "local IPv4 address %pI4\n", &nt->local_ip.ip);
 	np_info(np, "interface name '%s'\n", np->dev_name);
 	np_info(np, "local ethernet address '%pM'\n", np->dev_mac);
 	np_info(np, "remote port %d\n", nt->remote_port);
-	if (nt->remote_ip.family == AF_INET6)
+	if (nt->remote_ip.family == AF_UNSPEC)
+		np_info(np, "remote IP unset\n");
+	else if (nt->remote_ip.family == AF_INET6)
 		np_info(np, "remote IPv6 address %pI6c\n", &nt->remote_ip.in6);
 	else
 		np_info(np, "remote IPv4 address %pI4\n", &nt->remote_ip.ip);
@@ -863,6 +867,8 @@ static ssize_t local_ip_show(struct config_item *item, char *buf)
 {
 	struct netconsole_target *nt = to_target(item);
 
+	if (nt->local_ip.family == AF_UNSPEC)
+		return sysfs_emit(buf, "\n");
 	if (nt->local_ip.family == AF_INET6)
 		return sysfs_emit(buf, "%pI6c\n", &nt->local_ip.in6);
 	else
@@ -873,6 +879,8 @@ static ssize_t remote_ip_show(struct config_item *item, char *buf)
 {
 	struct netconsole_target *nt = to_target(item);
 
+	if (nt->remote_ip.family == AF_UNSPEC)
+		return sysfs_emit(buf, "\n");
 	if (nt->remote_ip.family == AF_INET6)
 		return sysfs_emit(buf, "%pI6c\n", &nt->remote_ip.in6);
 	else

-- 
2.55.0

[PATCH net-next v4 6/7] netconsole: move struct inet_addr into netconsole.c

[Gustavo Luiz Duarte]

The struct inet_addr lives in netpoll.h, but since commit a1116396476f
("netconsole: move local_ip/remote_ip/ipv6 to netconsole_target") the
only user is netconsole. Move the definition into netconsole.c

Suggested-by: Breno Leitao <leitao@debian.org>
Reviewed-by: Breno Leitao <leitao@debian.org>
Signed-off-by: Gustavo Luiz Duarte <gustavold@gmail.com>
---
 drivers/net/netconsole.c | 9 +++++++++
 include/linux/netpoll.h  | 9 ---------
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index 1f18d0a7d2dd..267254f046de 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -151,6 +151,15 @@ enum target_state {
 	STATE_DEACTIVATED,
 };
 
+struct inet_addr {
+	/* Address family: AF_UNSPEC when unset, else AF_INET or AF_INET6 */
+	u8			family;
+	union {
+		__be32		ip;
+		struct in6_addr	in6;
+	};
+};
+
 /**
  * struct netcons_userdata - Formatted userdata payload of a target.
  * @rcu:	Used to free the payload after a grace period.
diff --git a/include/linux/netpoll.h b/include/linux/netpoll.h
index de97f001a0f9..ec0821a5b02d 100644
--- a/include/linux/netpoll.h
+++ b/include/linux/netpoll.h
@@ -16,15 +16,6 @@
 #include <linux/ip.h>
 #include <linux/udp.h>
 
-struct inet_addr {
-	/* Address family: AF_UNSPEC when unset, else AF_INET or AF_INET6 */
-	u8			family;
-	union {
-		__be32		ip;
-		struct in6_addr	in6;
-	};
-};
-
 struct netpoll {
 	struct net_device *dev;
 	netdevice_tracker dev_tracker;

-- 
2.55.0

[PATCH net-next v4 7/7] docs: netconsole: document local_ip auto-selection

[Gustavo Luiz Duarte]

An unset local_ip is auto-populated from the egress interface at enable
time, but any value the user writes, including 0.0.0.0, is used as-is.
This deviates from the behavior prior to "netconsole: use the address
family instead of the ipv6 flag", which interpreted 0.0.0.0 as unset.

Document this so the distinction between "unset" and "set to 0.0.0.0" is
clear.

Reviewed-by: Breno Leitao <leitao@debian.org>
Signed-off-by: Gustavo Luiz Duarte <gustavold@gmail.com>
---
 Documentation/networking/netconsole.rst | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/networking/netconsole.rst b/Documentation/networking/netconsole.rst
index 4ab5d7b05cf1..694bca4803d5 100644
--- a/Documentation/networking/netconsole.rst
+++ b/Documentation/networking/netconsole.rst
@@ -141,6 +141,11 @@ The interface exposes these parameters of a netconsole target to userspace:
 	transmit_errors	Number of packet send errors		(read-only)
 	=============== =================================       ============
 
+local_ip is optional: if it is left unset, netconsole derives a source
+address from the egress interface when the target is enabled. Writing any
+value to it, including 0.0.0.0, uses that value verbatim and disables the
+automatic selection.
+
 The "enabled" attribute is also used to control whether the parameters of
 a target can be updated or not -- you can modify the parameters of only
 disabled targets (i.e. if "enabled" is 0).

-- 
2.55.0

Re: [PATCH net-next v4 1/7] netconsole: add an address family to struct inet_addr

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 0 (1) · Low: 1

- [Low] Stale comment: the block comment above netcons_local_ip_unset()
  in drivers/net/netconsole.c still states "nt->local_ip is a union of…

Pre-existing issues:
- [Medium] netpoll_parse_ip_addr() (drivers/net/netconsole.c:751-773)
  commits address bytes into the caller's live struct inet_addr before…

--

netconsole: add an address family to struct inet_addr

Converts union inet_addr into a struct that carries a per-address family
field (AF_UNSPEC when unset, else AF_INET/AF_INET6) and sets that field in
the netconsole paths that populate local_ip/remote_ip. The %pI4 users are
updated to point at the union member instead of the containing object. No
reader of the new field is added yet.

> diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
> index b358e5c367351..432b66cf111a5 100644
> --- a/drivers/net/netconsole.c
> +++ b/drivers/net/netconsole.c

[ ... ]

> @@ -741,10 +743,10 @@ static void netconsole_print_banner(struct netconsole_target *nt)
>  	np_info(np, "remote ethernet address %pM\n", nt->remote_mac);
>  }
>  
> -/* Parse the string and populate the `inet_addr` union. Return 0 if IPv4 is
> +/* Parse the string and populate the `inet_addr` struct. Return 0 if IPv4 is
>   * populated, 1 if IPv6 is populated, and -1 upon failure.
>   */

[Severity: Low]
This comment was updated from "union" to "struct", but the block comment
above netcons_local_ip_unset() in the same file still describes the old
layout:

drivers/net/netconsole.c:
 * nt->local_ip is a union of __be32 (IPv4) and struct in6_addr (IPv6),
 * so an IPv6 address whose first 4 bytes are zero (e.g. ::1, ::2,
 * IPv4-mapped ::ffff:a.b.c.d) must not be tested via the IPv4 arm —

After this patch nt->local_ip is a struct whose address payload no longer
starts at offset 0, and it carries its own family discriminator. Should
that comment be refreshed too?

The reasoning about "first 4 bytes" is exactly the assumption this patch
had to fix at the three %pI4 sites (local_ip_show(), remote_ip_show() and
netcons_take_ipv4()), so leaving it in place is a little confusing.

For completeness: the netcons_local_ip_unset() helper and this comment are
removed by the following patch in the series ("netconsole: use the address
family instead of the ipv6 flag"), so the stale text does not survive the
series.

> -static int netpoll_parse_ip_addr(const char *str, union inet_addr *addr)
> +static int netpoll_parse_ip_addr(const char *str, struct inet_addr *addr)
>  {
>  	const char *end = NULL;
>  	int len;
> @@ -756,14 +758,18 @@ static int netpoll_parse_ip_addr(const char *str, union inet_addr *addr)
>  	if (str[len - 1] == '\n')
>  		len -= 1;
>  
> -	if (in4_pton(str, len, (void *)addr, -1, &end) > 0 &&
> -	    (!end || *end == 0 || *end == '\n'))
> +	if (in4_pton(str, len, (void *)&addr->ip, -1, &end) > 0 &&
> +	    (!end || *end == 0 || *end == '\n')) {
> +		addr->family = AF_INET;
>  		return 0;
> +	}

[Severity: Medium]
This isn't a bug introduced by this patch, but can a rejected write leave
the target's address half-updated?

in4_pton() copies the parsed bytes into the destination before returning,
so the trailing-character check here runs after the store has already
happened:

net/core/utils.c:in4_pton() {
	...
	ret = 1;
	memcpy(dst, dbuf, sizeof(dbuf));
out:
	if (end)
		*end = s;
	return ret;
}

For input like "1.2.3.4x", in4_pton() returns > 0 and has written four
bytes into addr->ip, then (*end == 'x') fails the check, in6_pton() also
fails, and -1 is returned. The callers parse straight into the live target
and bail out without restoring the old value:

drivers/net/netconsole.c:remote_ip_store() {
	...
	ipv6 = netpoll_parse_ip_addr(buf, &nt->remote_ip);
	if (ipv6 == -1)
		goto out_unlock;
	nt->ipv6 = !!ipv6;
	...
}

local_ip_store() does the same for nt->local_ip. So a write that returns
-EINVAL to userspace can still change the configured address that
push_ipv4()/push_ipv6() and netcons_take_ipv6() later use.

Before this patch nt->ipv6 was the field left describing the previous
contents; with this patch addr->family is now also inconsistent with the
stored bytes on that path, even though the new field documents:

include/linux/netpoll.h:
	/* Address family: AF_UNSPEC when unset, else AF_INET or AF_INET6 */
	u8			family;

Would it be reasonable for netpoll_parse_ip_addr() to parse into a local
struct inet_addr and have the callers commit both the bytes and the family
only on success?

The behaviour is unchanged at the end of the series: netpoll_parse_ip_addr()
still writes into the caller's struct before validating, and
local_ip_store()/remote_ip_store() still return the error without restoring
the previous value.

>  
>  	if (IS_ENABLED(CONFIG_IPV6) &&
> -	    in6_pton(str, len, (void *)addr, -1, &end) > 0 &&
> -	    (!end || *end == 0 || *end == '\n'))
> +	    in6_pton(str, len, (void *)&addr->in6, -1, &end) > 0 &&
> +	    (!end || *end == 0 || *end == '\n')) {
> +		addr->family = AF_INET6;
>  		return 1;
> +	}
>  
>  	return -1;
>  }

[ ... ]

> diff --git a/include/linux/netpoll.h b/include/linux/netpoll.h
> index 1c6b1eec5efd6..de97f001a0f9c 100644
> --- a/include/linux/netpoll.h
> +++ b/include/linux/netpoll.h
> @@ -16,9 +16,13 @@
>  #include <linux/ip.h>
>  #include <linux/udp.h>
>  
> -union inet_addr {
> -	__be32		ip;
> -	struct in6_addr	in6;
> +struct inet_addr {
> +	/* Address family: AF_UNSPEC when unset, else AF_INET or AF_INET6 */
> +	u8			family;
> +	union {
> +		__be32		ip;
> +		struct in6_addr	in6;
> +	};
>  };

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260903-netcons_ipv6-v4-0-bdd183c844d3%40gmail.com

Re: [PATCH net-next v4 2/7] netconsole: use the address family instead of the ipv6 flag

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 2 · Low: 1

- [Medium] netconsole: writing "0.0.0.0" (or "::") to local_ip no longer
  requests automatic source-address selection; the zero address is now…
- [Medium] At this commit a configfs target can be enabled with local_ip
  and remote_ip of different families, and the transmit path then reads…
- [Low] The readers converted by this patch treat the newly three-valued
  `family` field as two-valued: local_ip_show()…

--

netconsole: use the address family instead of the ipv6 flag

This drops the per-target nt->ipv6 boolean and derives IPv4 vs IPv6
behaviour from the family field stored in struct inet_addr instead.  The
netcons_local_ip_unset() helper is removed and the "local address not
configured" test becomes local_ip.family == AF_UNSPEC.  A few questions
about behaviour changes that come with this conversion follow.

> diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
> index 432b66cf111a5..351754d53cf97 100644
> --- a/drivers/net/netconsole.c
> +++ b/drivers/net/netconsole.c

[ ... ]

> @@ -476,23 +474,6 @@ static int netcons_take_ipv4(struct netconsole_target *nt,
>  	return 0;
>  }
>  
> -/*
> - * Test whether the caller left nt->local_ip unset, so that
> - * netcons_netpoll_setup() should auto-populate it from the egress device.
> - *
> - * nt->local_ip is a union of __be32 (IPv4) and struct in6_addr (IPv6),
> - * so an IPv6 address whose first 4 bytes are zero (e.g. ::1, ::2,
> - * IPv4-mapped ::ffff:a.b.c.d) must not be tested via the IPv4 arm —
> - * doing so would misclassify a caller-supplied address as unset and
> - * silently overwrite it with whatever address the device exposes.
> - */
> -static bool netcons_local_ip_unset(const struct netconsole_target *nt)
> -{
> -	if (nt->ipv6)
> -		return ipv6_addr_any(&nt->local_ip.in6);
> -	return !nt->local_ip.ip;
> -}
> -
>  static int netcons_netpoll_setup(struct netconsole_target *nt)
>  {
>  	struct net *net = current->nsproxy->net_ns;
> @@ -538,16 +519,13 @@ static int netcons_netpoll_setup(struct netconsole_target *nt)
>  		rtnl_lock();
>  	}
>  
> -	if (netcons_local_ip_unset(nt)) {
> -		if (!nt->ipv6) {
> -			err = netcons_take_ipv4(nt, ndev);
> -			if (err)
> -				goto put;
> -		} else {
> +	if (nt->local_ip.family == AF_UNSPEC) {
> +		if (nt->remote_ip.family == AF_INET6)
>  			err = netcons_take_ipv6(nt, ndev);
> -			if (err)
> -				goto put;
> -		}
> +		else
> +			err = netcons_take_ipv4(nt, ndev);
> +		if (err)
> +			goto put;
>  		ip_overwritten = true;
>  	}

[Severity: Medium]

Is the replacement of netcons_local_ip_unset() with the AF_UNSPEC test
intended to also change the user-visible meaning of an all-zero local
address?

Before this patch the auto-populate test was purely value based
(!nt->local_ip.ip for IPv4, ipv6_addr_any() for IPv6), which matches the
long-standing netpoll behaviour where a zero local address requests
selection from the egress device.  After this patch the only way to reach
netcons_take_ipv4()/netcons_take_ipv6() is family == AF_UNSPEC, but
netpoll_parse_ip_addr() stamps a concrete family on every successful
parse, including "0.0.0.0" and "::":

	if (in4_pton(str, len, (void *)&addr->ip, -1, &end) > 0 &&
	    (!end || *end == 0 || *end == '\n')) {
		addr->family = AF_INET;
		return 0;
	}

So writing "0.0.0.0" to local_ip, or booting with
netconsole=@0.0.0.0/eth0,6666@a.b.c.d/, now enables the target without
error and every frame is built with a zero source address in push_ipv4()
and in the UDP pseudo-header, which the receiver or reverse path
filtering will drop, silently losing console output.

Related to that: the only remaining writer of AF_UNSPEC is the failure
path in netcons_netpoll_setup():

	if (ip_overwritten)
		memset(&nt->local_ip, 0, sizeof(nt->local_ip));

Once local_ip has been written once from configfs, is there any way for
userspace to get back to the auto-selection state?

The commit message says:

    We no longer need netcons_local_ip_unset() to check that all bytes are
    zeroes, as that is now denoted by (family == AF_UNSPEC).

Could this be reworded to state that the zeros-means-unset input is no
longer accepted, rather than presenting the two tests as equivalent?  A
later patch in the series ("docs: netconsole: document local_ip
auto-selection") documents the new behaviour, but the behaviour change
itself is not mentioned here.

> @@ -729,22 +707,22 @@ static void netconsole_print_banner(struct netconsole_target *nt)
>  	struct netpoll *np = &nt->np;
>  
>  	np_info(np, "local port %d\n", nt->local_port);
> -	if (nt->ipv6)
> +	if (nt->local_ip.family == AF_INET6)
>  		np_info(np, "local IPv6 address %pI6c\n", &nt->local_ip.in6);
>  	else
>  		np_info(np, "local IPv4 address %pI4\n", &nt->local_ip.ip);
>  	np_info(np, "interface name '%s'\n", np->dev_name);
>  	np_info(np, "local ethernet address '%pM'\n", np->dev_mac);
>  	np_info(np, "remote port %d\n", nt->remote_port);
> -	if (nt->ipv6)
> +	if (nt->remote_ip.family == AF_INET6)
>  		np_info(np, "remote IPv6 address %pI6c\n", &nt->remote_ip.in6);
>  	else
>  		np_info(np, "remote IPv4 address %pI4\n", &nt->remote_ip.ip);
>  	np_info(np, "remote ethernet address %pM\n", nt->remote_mac);
>  }
>  
> -/* Parse the string and populate the `inet_addr` struct. Return 0 if IPv4 is
> - * populated, 1 if IPv6 is populated, and -1 upon failure.
> +/* Parse the string and populate the `inet_addr` struct. Return 0 on success
> + * and -1 upon failure.
>   */
>  static int netpoll_parse_ip_addr(const char *str, struct inet_addr *addr)
>  {

[ ... ]

> @@ -874,7 +852,7 @@ static ssize_t local_ip_show(struct config_item *item, char *buf)
>  {
>  	struct netconsole_target *nt = to_target(item);
>  
> -	if (nt->ipv6)
> +	if (nt->local_ip.family == AF_INET6)
>  		return sysfs_emit(buf, "%pI6c\n", &nt->local_ip.in6);
>  	else
>  		return sysfs_emit(buf, "%pI4\n", &nt->local_ip.ip);
> @@ -884,7 +862,7 @@ static ssize_t remote_ip_show(struct config_item *item, char *buf)
>  {
>  	struct netconsole_target *nt = to_target(item);
>  
> -	if (nt->ipv6)
> +	if (nt->remote_ip.family == AF_INET6)
>  		return sysfs_emit(buf, "%pI6c\n", &nt->remote_ip.in6);
>  	else
>  		return sysfs_emit(buf, "%pI4\n", &nt->remote_ip.ip);

[Severity: Low]

The family field now has three states, per the comment on struct
inet_addr:

	/* Address family: AF_UNSPEC when unset, else AF_INET or AF_INET6 */
	u8			family;

but local_ip_show(), remote_ip_show() and netconsole_print_banner() all
use a two-way test, so AF_UNSPEC falls into the IPv4 else-branch.  Should
an unset address print as 0.0.0.0?

For a target with an IPv6 remote_ip and no local_ip written, cat local_ip
returns "0.0.0.0" while cat remote_ip returns an IPv6 literal, and before
this patch the same target had nt->ipv6 set so local_ip read back as
"::".  The 0.0.0.0 result is also indistinguishable from a deliberately
configured zero address, so a save-and-restore of the configuration turns
an auto-select target into a literal zero source address.

This is handled later in the series by "netconsole: show empty string for
an unset IP address", which adds an AF_UNSPEC case to both show handlers
and "local IP unset"/"remote IP unset" to the banner, so it only affects
the intermediate tree state.

[ ... ]

> @@ -1257,7 +1232,6 @@ 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;
>  
>  	dynamic_netconsole_mutex_lock();
>  	if (nt->state == STATE_ENABLED) {
> @@ -1266,10 +1240,8 @@ static ssize_t remote_ip_store(struct config_item *item, const char *buf,
>  		goto out_unlock;
>  	}
>  
> -	ipv6 = netpoll_parse_ip_addr(buf, &nt->remote_ip);
> -	if (ipv6 == -1)
> +	if (netpoll_parse_ip_addr(buf, &nt->remote_ip) < 0)
>  		goto out_unlock;
> -	nt->ipv6 = !!ipv6;
>  
>  	ret = count;
>  out_unlock:

[Severity: Medium]

With the shared nt->ipv6 flag gone, local_ip_store() and remote_ip_store()
each set only their own address's family, and nothing couples them.  What
keeps a configfs target from being enabled with local_ip as IPv4 and
remote_ip as IPv6?

netcons_netpoll_setup() at this commit only has the AF_UNSPEC
auto-populate branch, no family agreement check, so such a target reaches
STATE_ENABLED.  The transmit path then keys everything off remote_ip
while reading the local_ip union through the IPv6 arm:

drivers/net/netconsole.c:push_ipv6() {
	...
	ip6h->saddr = nt->local_ip.in6;
	ip6h->daddr = nt->remote_ip.in6;
	...
}

Only 4 bytes were written into that union by local_ip_store(), so the
emitted IPv6 source address is the IPv4 value followed by 12 bytes of
whatever the union previously held (zeros, or bytes of an earlier IPv6
address).  netpoll_udp_checksum() below feeds the same mismatched arm to
csum_ipv6_magic().

> @@ -2078,7 +2050,7 @@ static void netpoll_udp_checksum(struct netconsole_target *nt,
>  
>  	/* check needs to be set, since it will be consumed in csum_partial */
>  	udph->check = 0;
> -	if (nt->ipv6)
> +	if (nt->remote_ip.family == AF_INET6)
>  		udph->check = csum_ipv6_magic(&nt->local_ip.in6,
>  					      &nt->remote_ip.in6,
>  					      udp_len, IPPROTO_UDP,

[ ... ]

> @@ -2601,13 +2567,11 @@ static int netconsole_parser_cmdline(struct netconsole_target *nt, char *opt)
>  	if (!delim)
>  		goto parse_failed;
>  	*delim = 0;
> -	ipv6 = netpoll_parse_ip_addr(cur, &nt->remote_ip);
> -	if (ipv6 < 0)
> +	if (netpoll_parse_ip_addr(cur, &nt->remote_ip) < 0)
>  		goto parse_failed;
> -	else if (ipversion_set && nt->ipv6 != (bool)ipv6)
> +	if (nt->local_ip.family != AF_UNSPEC &&
> +	    nt->local_ip.family != nt->remote_ip.family)
>  		goto parse_failed;
> -	else
> -		nt->ipv6 = (bool)ipv6;
>  	cur = delim + 1;

The family agreement invariant that the transmit code depends on is
enforced here for cmdline targets, but not for configfs targets at this
commit.  Would it be worth folding the equivalent check into
netcons_netpoll_setup() in the same patch that removes nt->ipv6?  A
following patch in the series ("netconsole: reject a target mixing IPv4
and IPv6 addresses") does add it, so the window is limited to this
intermediate state.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260903-netcons_ipv6-v4-0-bdd183c844d3%40gmail.com

Re: [PATCH net-next v4 3/7] netconsole: reject enabling a target with no remote IP address

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 0 · Low: 1 (1)

- [Low] Documentation gap: after this patch `echo 1 > enabled` on a
  freshly created dynamic configfs target hard-fails with -EDESTADDRREQ…

Pre-existing issues:
- [Low] Concern (validated as NOT a defect after series verification,
  retained only per the validation-policy retention rule): claim that…

--

netconsole: reject enabling a target with no remote IP address

This adds an early check in netcons_netpoll_setup() so that enabling a
target whose remote address was never set fails with -EDESTADDRREQ instead
of sending netconsole traffic to 0.0.0.0. Only the configfs (dynamic
target) path is affected, since the command-line path already requires a
remote address.

No Link: tags present in the commit header.

> diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
> index 351754d53cf97..4af4f3039d4cd 100644
> --- a/drivers/net/netconsole.c
> +++ b/drivers/net/netconsole.c
> @@ -483,6 +483,11 @@ static int netcons_netpoll_setup(struct netconsole_target *nt)
>  	bool ip_overwritten = false;
>  	int err;
>  
> +	if (nt->remote_ip.family == AF_UNSPEC) {
> +		np_err(np, "remote IP address not configured, aborting\n");
> +		return -EDESTADDRREQ;
> +	}
> +

[Severity: Low]
Should Documentation/networking/netconsole.rst be updated along with this?

The "Dynamic reconfiguration" section still says:

  Note that newly created targets have default parameter values (as mentioned
  above) and are disabled by default -- they must first be enabled by writing
  "1" to the "enabled" attribute (usually after setting parameters accordingly)
  as described below.

and the attribute table still lists remote_ip as an ordinary read-write
parameter:

	remote_ip	Remote agent's IP address		(read-write)

After this change the following sequence fails with -EDESTADDRREQ:

  mkdir /sys/kernel/config/netconsole/t1
  echo 1 > t1/enabled

Reading the rest of the series, the last patch ("docs: netconsole: document
local_ip auto-selection") only adds a paragraph stating that local_ip is
optional; nothing marks remote_ip as required before enabling, and the new
errno is not mentioned anywhere. Would a sentence in that section saying
remote_ip must be set before enabling be worth adding?

[Severity: Low]
This isn't a bug introduced by this patch, but a note on how the new gate
interacts with an explicitly written 0.0.0.0.

The check keys on nt->remote_ip.family == AF_UNSPEC, while
netpoll_parse_ip_addr() sets AF_INET for an explicit value:

  if (in4_pton(str, len, (void *)&addr->ip, -1, &end) > 0 &&
      (!end || *end == 0 || *end == '\n')) {
	  addr->family = AF_INET;
	  return 0;
  }

So this still succeeds and sends to 0.0.0.0:

  echo 0.0.0.0 > t1/remote_ip
  echo 1 > t1/enabled

That looks intentional: the later patch "docs: netconsole: document local_ip
auto-selection" documents "Writing any value to it, including 0.0.0.0, uses
that value verbatim and disables the automatic selection", and it matches
the command-line path which also accepts an explicit tgt-ip of 0.0.0.0. The
related read-back ambiguity, where remote_ip_show() rendered AF_UNSPEC as
"0.0.0.0", is also addressed later in the series by "netconsole: show empty
string for an unset IP address". Mentioning it only so the distinction
between "unset" and "explicitly 0.0.0.0" is on record for this hunk.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260903-netcons_ipv6-v4-0-bdd183c844d3%40gmail.com

Re: [PATCH net-next v4 4/7] netconsole: reject a target mixing IPv4 and IPv6 addresses

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 1 · Low: 0

- [Medium] The new family-equality check in netcons_netpoll_setup()
  (drivers/net/netconsole.c:491-495) cannot distinguish a user-…

--

netconsole: reject a target mixing IPv4 and IPv6 addresses

This adds a check in netcons_netpoll_setup() that refuses to enable a
dynamic target whose local_ip and remote_ip belong to different address
families, matching what the command-line parser already does.

> diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
> index 4af4f3039d4cd..abcc8515ddd2b 100644
> --- a/drivers/net/netconsole.c
> +++ b/drivers/net/netconsole.c
> @@ -488,6 +488,12 @@ static int netcons_netpoll_setup(struct netconsole_target *nt)
>  		return -EDESTADDRREQ;
>  	}
>  
> +	if (nt->local_ip.family != AF_UNSPEC &&
> +	    nt->local_ip.family != nt->remote_ip.family) {
> +		np_err(np, "local and remote IP address families differ, aborting\n");
> +		return -EINVAL;
> +	}
> +

[Severity: Medium]
Can this check tell apart a local_ip the user configured from one that
netconsole itself stamped into the target?

Later in the same function, when no local address was configured,
netcons_netpoll_setup() fills one in from the device and records the
family:

	if (nt->local_ip.family == AF_UNSPEC) {
		if (nt->remote_ip.family == AF_INET6)
			err = netcons_take_ipv6(nt, ndev);
		else
			err = netcons_take_ipv4(nt, ndev);
		if (err)
			goto put;
		ip_overwritten = true;
	}

netcons_take_ipv4() sets nt->local_ip.family = AF_INET and
netcons_take_ipv6() sets nt->local_ip.family = AF_INET6.

The derived value is only rolled back on the failure path:

put:
	DEBUG_NET_WARN_ON_ONCE(np->dev);
	if (ip_overwritten)
		memset(&nt->local_ip, 0, sizeof(nt->local_ip));

so after a successful enable the auto-taken family stays in the target.
The disable branch of enabled_store() only touches nt->state and moves
the target to target_cleanup_list, and local_ip_store() cannot write
AF_UNSPEC back because netpoll_parse_ip_addr() rejects an empty write
(if (!len) return -1;) and always sets AF_INET or AF_INET6 on success.

Does that make the documented reconfiguration flow in
Documentation/networking/netconsole.rst fail after a family switch?

 echo 0 > enabled			# disable the target (if required)
 echo 10.0.0.4 > remote_ip		# update some parameter
 echo 1 > enabled			# enable target again

Concretely:

  1. create a target with remote_ip=10.0.0.4 and no local_ip, enable it
     -> netcons_take_ipv4() sets local_ip.family = AF_INET
  2. echo 0 > enabled
  3. echo fd00::1 > remote_ip     (accepted, remote_ip_store() only
     refuses writes while nt->state == STATE_ENABLED)
  4. echo 1 > enabled             -> the new check returns -EINVAL and
     prints "local and remote IP address families differ", although no
     local address was ever configured by the user

The only way back to auto-selection appears to be writing an explicit
local address of the new family, or removing and recreating the target
directory.

Is the deactivated-target case affected too?  remote_ip_store() also
accepts a family change while nt->state == STATE_DEACTIVATED, and when
the interface comes back resume_target() treats the new -EINVAL as a
one-shot setup failure:

	if (netcons_netpoll_setup(nt)) {
		/* netpoll fails setup once, do not try again. */
		netconsole_skb_pool_flush(nt);
		nt->state = STATE_DISABLED;
		return;
	}

which stops logging on that target with no retry.

Would it make sense to also clear the derived local_ip when the target
is disabled, or to remember that it was auto-taken, so the equality test
only applies to a local address the user actually wrote?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260903-netcons_ipv6-v4-0-bdd183c844d3%40gmail.com

Re: [PATCH net-next v4 5/7] netconsole: show empty string for an unset IP address

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 1 · Low: 1

- [Medium] ABI show/store asymmetry for the configfs attributes local_ip
  and remote_ip: after this patch local_ip_show()/remote_ip_show() emit…
- [Low] The patch changes the on-read format of two documented configfs
  attributes (local_ip, remote_ip) from an always-parseable IPv4/IPv6…

--

netconsole: show empty string for an unset IP address

An unset local_ip/remote_ip is now denoted by AF_UNSPEC, and this patch
makes the configfs show handlers print an empty string instead of
"0.0.0.0" for that state. netconsole_print_banner() prints an explicit
"unset" message for the same case.

> diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
> index abcc8515ddd2b..1f18d0a7d2ddb 100644
> --- a/drivers/net/netconsole.c
> +++ b/drivers/net/netconsole.c

[ ... ]

> @@ -863,6 +867,8 @@ static ssize_t local_ip_show(struct config_item *item, char *buf)
>  {
>  	struct netconsole_target *nt = to_target(item);
>  
> +	if (nt->local_ip.family == AF_UNSPEC)
> +		return sysfs_emit(buf, "\n");

[Severity: Medium]
Can the value that local_ip_show() now produces be written back into the
same attribute?

A freshly created target has family == AF_UNSPEC, so local_ip_show()
emits just "\n". Feeding that back through local_ip_store() reaches
netpoll_parse_ip_addr():

drivers/net/netconsole.c:netpoll_parse_ip_addr() {
	len = strlen(str);
	if (!len)
		return -1;

	if (str[len - 1] == '\n')
		len -= 1;

	if (in4_pton(str, len, (void *)&addr->ip, -1, &end) > 0 &&
	...
}

An input of "" fails the !len test, and "\n" is trimmed to len 0 so both
in4_pton() and in6_pton() fail. local_ip_store() then returns -EINVAL:

drivers/net/netconsole.c:local_ip_store() {
	...
	if (netpoll_parse_ip_addr(buf, &nt->local_ip) < 0)
		goto out_unlock;	/* ret == -EINVAL */
	...
}

Before this change the show side printed "0.0.0.0", which the store side
accepted, so "cat local_ip > local_ip" and generic save/restore of all
target attributes round-tripped.

Should netpoll_parse_ip_addr() (or the store handlers) gain an
empty-string branch that sets family = AF_UNSPEC?  As it stands there
seems to be no way to move an address back to the unset state through
the configfs ABI, and this remains true at the end of the series
(62342076a29c).

[Severity: Low]
Should Documentation/networking/netconsole.rst be updated along with
this?  The attribute table there still describes both attributes as
plain read-write IP addresses:

	local_ip	Source IP address to use		(read-write)
	remote_ip	Remote agent's IP address		(read-write)

and the example section still shows "cat cmdline0/remote_ip" producing an
address.  The later patch in the series, "docs: netconsole: document
local_ip auto-selection", describes the auto-derivation of an unset
local_ip, but it does not mention that reading an unset local_ip or
remote_ip now yields an empty value, nor that such a value cannot be
written back.

>  	if (nt->local_ip.family == AF_INET6)
>  		return sysfs_emit(buf, "%pI6c\n", &nt->local_ip.in6);
>  	else
> @@ -873,6 +879,8 @@ static ssize_t remote_ip_show(struct config_item *item, char *buf)
>  {
>  	struct netconsole_target *nt = to_target(item);
>  
> +	if (nt->remote_ip.family == AF_UNSPEC)
> +		return sysfs_emit(buf, "\n");

[Severity: Medium]
Same question for remote_ip: remote_ip_store() -> netpoll_parse_ip_addr()
rejects the "\n" that remote_ip_show() emits, so
"cat remote_ip > remote_ip" returns -EINVAL on a target whose remote
address was never set.

>  	if (nt->remote_ip.family == AF_INET6)
>  		return sysfs_emit(buf, "%pI6c\n", &nt->remote_ip.in6);
>  	else

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260903-netcons_ipv6-v4-0-bdd183c844d3%40gmail.com

Re: [PATCH net-next v4 4/7] netconsole: reject a target mixing IPv4 and IPv6 addresses

[Jakub Kicinski]

On Tue, 08 Sep 2026 19:29:36 +0000 netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider:
> 
> Critical: 0 · High: 0 · Medium: 1 · Low: 0
> 
> - [Medium] The new family-equality check in netcons_netpoll_setup()
>   (drivers/net/netconsole.c:491-495) cannot distinguish a user-…

This one looks semi-legit, if you think switching address families
is worth supporting please follow up.

Re: [PATCH net-next v4 0/7] netconsole: validate a target's IP address configuration

[patchwork-bot+netdevbpf]

Hello:

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

On Thu, 03 Sep 2026 17:25:59 +0100 you wrote:
> This series adds two validations to the target configuration when the
> user tries to enable it: first whether remote_ip was set, and second
> whether local_ip and remote_ip address families match. Refuse to enable
> the target if any of those validations fail.
> 
> These validations are already done for the target passed on the
> command-line, so this aligns dynamic targets with the command-line
> behavior.
> 
> [...]

Here is the summary with links:
  - [net-next,v4,1/7] netconsole: add an address family to struct inet_addr
    https://git.kernel.org/netdev/net-next/c/f6e96d72f64a
  - [net-next,v4,2/7] netconsole: use the address family instead of the ipv6 flag
    https://git.kernel.org/netdev/net-next/c/46b3f9e7e761
  - [net-next,v4,3/7] netconsole: reject enabling a target with no remote IP address
    https://git.kernel.org/netdev/net-next/c/2e949954908b
  - [net-next,v4,4/7] netconsole: reject a target mixing IPv4 and IPv6 addresses
    https://git.kernel.org/netdev/net-next/c/32535a1ea962
  - [net-next,v4,5/7] netconsole: show empty string for an unset IP address
    https://git.kernel.org/netdev/net-next/c/56ddc8d48e75
  - [net-next,v4,6/7] netconsole: move struct inet_addr into netconsole.c
    https://git.kernel.org/netdev/net-next/c/7be9bfb7689e
  - [net-next,v4,7/7] docs: netconsole: document local_ip auto-selection
    https://git.kernel.org/netdev/net-next/c/a4b9392ef046

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