Re: [PATCH net-next v4 5/7] netconsole: show empty string for an unset IP address
flat view
From: netdev-bot+sashiko@kernel.org
Date: 2026-09-08 19:29:38
Also in:
lkml, netdev
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.
quoted hunk ↗ jump to hunk
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
[ ... ]
quoted hunk ↗ jump to hunk
@@ -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.
quoted hunk ↗ jump to hunk
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