Initially netpoll and netconsole were created together, and some functions are in the wrong file. Seperate netconsole-only functions in netconsole, avoiding exports.
1. Expose netpoll logging macros in the public header to enable consistent log formatting across netpoll consumers.
2. Relocate netconsole-specific functions from netpoll to the netconsole module where they are actually used, reducing unnecessary coupling.
3. Remove unnecessary function exports
4. Rename netpoll parsing functions in netconsole to better reflect their specific usage.
5. Create a test to check that cmdline works fine. This was in my todo list since [1], this was a good time to add it here to make sure this patchset doesn't regress.
PS: The code was split in a way that it is easy to review. When copying the functions from netpoll to netconsole, I do not change than other than adding `static`. This will make checkpatch unhappy, but, further patches will address the issues. It is done this way to make it easy for reviewers.
Link: https://lore.kernel.org/netdev/Z36TlACdNMwFD7wv@dev-ushankar.dev.purestorage... [1] Signed-off-by: Breno Leitao leitao@debian.org --- Changes in v2: - No change in the code. Just rebased the patches onto netnext/main - Link to v1: https://lore.kernel.org/r/20250610-rework-v1-0-7cfde283f246@debian.org
--- Breno Leitao (7): netpoll: remove __netpoll_cleanup from exported API netpoll: expose netpoll logging macros in public header netpoll: relocate netconsole-specific functions to netconsole module netpoll: move netpoll_print_options to netconsole netconsole: rename functions to better reflect their purpose netconsole: improve code style in parser function selftest: netconsole: add test for cmdline configuration
drivers/net/netconsole.c | 137 ++++++++++++++++++++- include/linux/netpoll.h | 10 +- net/core/netpoll.c | 136 +------------------- tools/testing/selftests/drivers/net/Makefile | 1 + .../selftests/drivers/net/lib/sh/lib_netcons.sh | 39 +++++- .../selftests/drivers/net/netcons_cmdline.sh | 52 ++++++++ 6 files changed, 228 insertions(+), 147 deletions(-) --- base-commit: 0097c4195b1d0ca57d15979626c769c74747b5a0 change-id: 20250603-rework-c175cad8d22e
Best regards,
Since commit 97714695ef90 ("net: netconsole: Defer netpoll cleanup to avoid lock release during list traversal"), netconsole no longer uses __netpoll_cleanup(). With no remaining users, remove this function from the exported netpoll API.
The function remains available internally within netpoll for use by netpoll_cleanup().
Signed-off-by: Breno Leitao leitao@debian.org --- include/linux/netpoll.h | 1 - net/core/netpoll.c | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/include/linux/netpoll.h b/include/linux/netpoll.h index 0477208ed9ffa..a637e51152544 100644 --- a/include/linux/netpoll.h +++ b/include/linux/netpoll.h @@ -69,7 +69,6 @@ void netpoll_print_options(struct netpoll *np); int netpoll_parse_options(struct netpoll *np, char *opt); int __netpoll_setup(struct netpoll *np, struct net_device *ndev); int netpoll_setup(struct netpoll *np); -void __netpoll_cleanup(struct netpoll *np); void __netpoll_free(struct netpoll *np); void netpoll_cleanup(struct netpoll *np); void do_netpoll_cleanup(struct netpoll *np); diff --git a/net/core/netpoll.c b/net/core/netpoll.c index 4ddb7490df4b8..a69c2773841a5 100644 --- a/net/core/netpoll.c +++ b/net/core/netpoll.c @@ -863,7 +863,7 @@ static void rcu_cleanup_netpoll_info(struct rcu_head *rcu_head) kfree(npinfo); }
-void __netpoll_cleanup(struct netpoll *np) +static void __netpoll_cleanup(struct netpoll *np) { struct netpoll_info *npinfo;
@@ -885,7 +885,6 @@ void __netpoll_cleanup(struct netpoll *np)
skb_pool_flush(np); } -EXPORT_SYMBOL_GPL(__netpoll_cleanup);
void __netpoll_free(struct netpoll *np) {
Move np_info(), np_err(), and np_notice() macros from internal implementation to the public netpoll header file to make them available for use by netpoll consumers.
These logging macros provide consistent formatting for netpoll-related messages by automatically prefixing log output with the netpoll instance name.
The goal is to use the exact same format that is being displayed today, instead of creating something netconsole-specific.
Signed-off-by: Breno Leitao leitao@debian.org --- include/linux/netpoll.h | 7 +++++++ net/core/netpoll.c | 7 ------- 2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/include/linux/netpoll.h b/include/linux/netpoll.h index a637e51152544..72086b8a3decd 100644 --- a/include/linux/netpoll.h +++ b/include/linux/netpoll.h @@ -42,6 +42,13 @@ struct netpoll { struct work_struct refill_wq; };
+#define np_info(np, fmt, ...) \ + pr_info("%s: " fmt, np->name, ##__VA_ARGS__) +#define np_err(np, fmt, ...) \ + pr_err("%s: " fmt, np->name, ##__VA_ARGS__) +#define np_notice(np, fmt, ...) \ + pr_notice("%s: " fmt, np->name, ##__VA_ARGS__) + struct netpoll_info { refcount_t refcnt;
diff --git a/net/core/netpoll.c b/net/core/netpoll.c index a69c2773841a5..9e86026225a36 100644 --- a/net/core/netpoll.c +++ b/net/core/netpoll.c @@ -58,13 +58,6 @@ static void zap_completion_queue(void); static unsigned int carrier_timeout = 4; module_param(carrier_timeout, uint, 0644);
-#define np_info(np, fmt, ...) \ - pr_info("%s: " fmt, np->name, ##__VA_ARGS__) -#define np_err(np, fmt, ...) \ - pr_err("%s: " fmt, np->name, ##__VA_ARGS__) -#define np_notice(np, fmt, ...) \ - pr_notice("%s: " fmt, np->name, ##__VA_ARGS__) - static netdev_tx_t netpoll_start_xmit(struct sk_buff *skb, struct net_device *dev, struct netdev_queue *txq)
Move netpoll_parse_ip_addr() and netpoll_parse_options() from the generic netpoll module to the netconsole module where they are actually used.
These functions were originally placed in netpoll but are only consumed by netconsole. This refactoring improves code organization by:
- Removing unnecessary exported symbols from netpoll - Making netpoll_parse_options() static (no longer needs global visibility) - Reducing coupling between netpoll and netconsole modules
The functions remain functionally identical - this is purely a code reorganization to better reflect their actual usage patterns. Here are the changes:
1) Move both functions from netpoll to netconsole 2) Add static to netpoll_parse_options() 3) Removed the EXPORT_SYMBOL()
PS: This diff does not change the function format, so, it is easy to review, but, checkpatch will not be happy. A follow-up patch will address the current issues reported by checkpatch.
Signed-off-by: Breno Leitao leitao@debian.org --- drivers/net/netconsole.c | 108 ++++++++++++++++++++++++++++++++++++++++++++++ include/linux/netpoll.h | 1 - net/core/netpoll.c | 109 ----------------------------------------------- 3 files changed, 108 insertions(+), 110 deletions(-)
diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c index 21077aff061c5..6741e5484f053 100644 --- a/drivers/net/netconsole.c +++ b/drivers/net/netconsole.c @@ -1660,6 +1660,114 @@ 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 netpoll_parse_options(struct netpoll *np, char *opt) +{ + char *cur=opt, *delim; + int ipv6; + bool ipversion_set = false; + + if (*cur != '@') { + if ((delim = strchr(cur, '@')) == NULL) + goto parse_failed; + *delim = 0; + if (kstrtou16(cur, 10, &np->local_port)) + goto parse_failed; + cur = delim; + } + cur++; + + if (*cur != '/') { + ipversion_set = true; + if ((delim = strchr(cur, '/')) == NULL) + goto parse_failed; + *delim = 0; + ipv6 = netpoll_parse_ip_addr(cur, &np->local_ip); + if (ipv6 < 0) + goto parse_failed; + else + np->ipv6 = (bool)ipv6; + cur = delim; + } + cur++; + + if (*cur != ',') { + /* parse out dev_name or dev_mac */ + if ((delim = strchr(cur, ',')) == NULL) + goto parse_failed; + *delim = 0; + + np->dev_name[0] = '\0'; + eth_broadcast_addr(np->dev_mac); + if (!strchr(cur, ':')) + strscpy(np->dev_name, cur, sizeof(np->dev_name)); + else if (!mac_pton(cur, np->dev_mac)) + goto parse_failed; + + cur = delim; + } + cur++; + + if (*cur != '@') { + /* dst port */ + if ((delim = strchr(cur, '@')) == NULL) + goto parse_failed; + *delim = 0; + if (*cur == ' ' || *cur == '\t') + np_info(np, "warning: whitespace is not allowed\n"); + if (kstrtou16(cur, 10, &np->remote_port)) + goto parse_failed; + cur = delim; + } + cur++; + + /* dst ip */ + if ((delim = strchr(cur, '/')) == NULL) + goto parse_failed; + *delim = 0; + ipv6 = netpoll_parse_ip_addr(cur, &np->remote_ip); + if (ipv6 < 0) + goto parse_failed; + else if (ipversion_set && np->ipv6 != (bool)ipv6) + goto parse_failed; + else + np->ipv6 = (bool)ipv6; + cur = delim + 1; + + if (*cur != 0) { + /* MAC address */ + if (!mac_pton(cur, np->remote_mac)) + goto parse_failed; + } + + netpoll_print_options(np); + + return 0; + + parse_failed: + np_info(np, "couldn't parse config at '%s'!\n", cur); + return -1; +} + /* Allocate new target (from boot/module param) and setup netpoll for it */ static struct netconsole_target *alloc_param_target(char *target_config, int cmdline_count) diff --git a/include/linux/netpoll.h b/include/linux/netpoll.h index 72086b8a3decd..1b8000954e52a 100644 --- a/include/linux/netpoll.h +++ b/include/linux/netpoll.h @@ -73,7 +73,6 @@ static inline void netpoll_poll_enable(struct net_device *dev) { return; }
int netpoll_send_udp(struct netpoll *np, const char *msg, int len); void netpoll_print_options(struct netpoll *np); -int netpoll_parse_options(struct netpoll *np, char *opt); int __netpoll_setup(struct netpoll *np, struct net_device *ndev); int netpoll_setup(struct netpoll *np); void __netpoll_free(struct netpoll *np); diff --git a/net/core/netpoll.c b/net/core/netpoll.c index 9e86026225a36..d2965c916130d 100644 --- a/net/core/netpoll.c +++ b/net/core/netpoll.c @@ -510,26 +510,6 @@ void netpoll_print_options(struct netpoll *np) } EXPORT_SYMBOL(netpoll_print_options);
-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 void skb_pool_flush(struct netpoll *np) { struct sk_buff_head *skb_pool; @@ -539,95 +519,6 @@ static void skb_pool_flush(struct netpoll *np) skb_queue_purge_reason(skb_pool, SKB_CONSUMED); }
-int netpoll_parse_options(struct netpoll *np, char *opt) -{ - char *cur=opt, *delim; - int ipv6; - bool ipversion_set = false; - - if (*cur != '@') { - if ((delim = strchr(cur, '@')) == NULL) - goto parse_failed; - *delim = 0; - if (kstrtou16(cur, 10, &np->local_port)) - goto parse_failed; - cur = delim; - } - cur++; - - if (*cur != '/') { - ipversion_set = true; - if ((delim = strchr(cur, '/')) == NULL) - goto parse_failed; - *delim = 0; - ipv6 = netpoll_parse_ip_addr(cur, &np->local_ip); - if (ipv6 < 0) - goto parse_failed; - else - np->ipv6 = (bool)ipv6; - cur = delim; - } - cur++; - - if (*cur != ',') { - /* parse out dev_name or dev_mac */ - if ((delim = strchr(cur, ',')) == NULL) - goto parse_failed; - *delim = 0; - - np->dev_name[0] = '\0'; - eth_broadcast_addr(np->dev_mac); - if (!strchr(cur, ':')) - strscpy(np->dev_name, cur, sizeof(np->dev_name)); - else if (!mac_pton(cur, np->dev_mac)) - goto parse_failed; - - cur = delim; - } - cur++; - - if (*cur != '@') { - /* dst port */ - if ((delim = strchr(cur, '@')) == NULL) - goto parse_failed; - *delim = 0; - if (*cur == ' ' || *cur == '\t') - np_info(np, "warning: whitespace is not allowed\n"); - if (kstrtou16(cur, 10, &np->remote_port)) - goto parse_failed; - cur = delim; - } - cur++; - - /* dst ip */ - if ((delim = strchr(cur, '/')) == NULL) - goto parse_failed; - *delim = 0; - ipv6 = netpoll_parse_ip_addr(cur, &np->remote_ip); - if (ipv6 < 0) - goto parse_failed; - else if (ipversion_set && np->ipv6 != (bool)ipv6) - goto parse_failed; - else - np->ipv6 = (bool)ipv6; - cur = delim + 1; - - if (*cur != 0) { - /* MAC address */ - if (!mac_pton(cur, np->remote_mac)) - goto parse_failed; - } - - netpoll_print_options(np); - - return 0; - - parse_failed: - np_info(np, "couldn't parse config at '%s'!\n", cur); - return -1; -} -EXPORT_SYMBOL(netpoll_parse_options); - static void refill_skbs_work_handler(struct work_struct *work) { struct netpoll *np =
Move netpoll_print_options() from net/core/netpoll.c to drivers/net/netconsole.c and make it static. This function is only used by netconsole, so there's no need to export it or keep it in the public netpoll API.
This reduces the netpoll API surface and improves code locality by keeping netconsole-specific functionality within the netconsole driver.
Signed-off-by: Breno Leitao leitao@debian.org --- drivers/net/netconsole.c | 17 +++++++++++++++++ include/linux/netpoll.h | 1 - net/core/netpoll.c | 17 ----------------- 3 files changed, 17 insertions(+), 18 deletions(-)
diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c index 6741e5484f053..1b8f9150eb350 100644 --- a/drivers/net/netconsole.c +++ b/drivers/net/netconsole.c @@ -278,6 +278,23 @@ static void netconsole_process_cleanups_core(void) mutex_unlock(&target_cleanup_list_lock); }
+static void netpoll_print_options(struct netpoll *np) +{ + np_info(np, "local port %d\n", np->local_port); + if (np->ipv6) + np_info(np, "local IPv6 address %pI6c\n", &np->local_ip.in6); + else + np_info(np, "local IPv4 address %pI4\n", &np->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", np->remote_port); + if (np->ipv6) + np_info(np, "remote IPv6 address %pI6c\n", &np->remote_ip.in6); + else + np_info(np, "remote IPv4 address %pI4\n", &np->remote_ip.ip); + np_info(np, "remote ethernet address %pM\n", np->remote_mac); +} + #ifdef CONFIG_NETCONSOLE_DYNAMIC
/* diff --git a/include/linux/netpoll.h b/include/linux/netpoll.h index 1b8000954e52a..735e65c3cc114 100644 --- a/include/linux/netpoll.h +++ b/include/linux/netpoll.h @@ -72,7 +72,6 @@ static inline void netpoll_poll_enable(struct net_device *dev) { return; } #endif
int netpoll_send_udp(struct netpoll *np, const char *msg, int len); -void netpoll_print_options(struct netpoll *np); int __netpoll_setup(struct netpoll *np, struct net_device *ndev); int netpoll_setup(struct netpoll *np); void __netpoll_free(struct netpoll *np); diff --git a/net/core/netpoll.c b/net/core/netpoll.c index d2965c916130d..07c453864a7df 100644 --- a/net/core/netpoll.c +++ b/net/core/netpoll.c @@ -492,23 +492,6 @@ int netpoll_send_udp(struct netpoll *np, const char *msg, int len) } EXPORT_SYMBOL(netpoll_send_udp);
-void netpoll_print_options(struct netpoll *np) -{ - np_info(np, "local port %d\n", np->local_port); - if (np->ipv6) - np_info(np, "local IPv6 address %pI6c\n", &np->local_ip.in6); - else - np_info(np, "local IPv4 address %pI4\n", &np->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", np->remote_port); - if (np->ipv6) - np_info(np, "remote IPv6 address %pI6c\n", &np->remote_ip.in6); - else - np_info(np, "remote IPv4 address %pI4\n", &np->remote_ip.ip); - np_info(np, "remote ethernet address %pM\n", np->remote_mac); -} -EXPORT_SYMBOL(netpoll_print_options);
static void skb_pool_flush(struct netpoll *np) {
Rename netpoll_parse_options() to netconsole_parser_cmdline() and netpoll_print_options() to netconsole_print_banner() to better describe what these functions actually do within the netconsole context.
Also fix minor code style issues including variable declaration ordering and spacing.
These functions are specific to netconsole functionality rather than general netpoll operations, so the new names better reflect their actual purpose.
Signed-off-by: Breno Leitao leitao@debian.org --- drivers/net/netconsole.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c index 1b8f9150eb350..7f1fed7871967 100644 --- a/drivers/net/netconsole.c +++ b/drivers/net/netconsole.c @@ -278,7 +278,7 @@ static void netconsole_process_cleanups_core(void) mutex_unlock(&target_cleanup_list_lock); }
-static void netpoll_print_options(struct netpoll *np) +static void netconsole_print_banner(struct netpoll *np) { np_info(np, "local port %d\n", np->local_port); if (np->ipv6) @@ -551,10 +551,10 @@ static ssize_t enabled_store(struct config_item *item, }
/* - * Skip netpoll_parse_options() -- all the attributes are + * Skip netconsole_parser_cmdline() -- all the attributes are * already configured via configfs. Just print them out. */ - netpoll_print_options(&nt->np); + netconsole_print_banner(&nt->np);
ret = netpoll_setup(&nt->np); if (ret) @@ -1697,11 +1697,12 @@ static int netpoll_parse_ip_addr(const char *str, union inet_addr *addr) return -1; }
-static int netpoll_parse_options(struct netpoll *np, char *opt) +static int netconsole_parser_cmdline(struct netpoll *np, char *opt) { - char *cur=opt, *delim; - int ipv6; bool ipversion_set = false; + char *cur = opt; + char *delim; + int ipv6;
if (*cur != '@') { if ((delim = strchr(cur, '@')) == NULL) @@ -1776,7 +1777,7 @@ static int netpoll_parse_options(struct netpoll *np, char *opt) goto parse_failed; }
- netpoll_print_options(np); + netconsole_print_banner(np);
return 0;
@@ -1814,7 +1815,7 @@ static struct netconsole_target *alloc_param_target(char *target_config, }
/* Parse parameters and setup netpoll */ - err = netpoll_parse_options(&nt->np, target_config); + err = netconsole_parser_cmdline(&nt->np, target_config); if (err) goto fail;
Split assignment from conditional checks and use preferred null pointer check style (!delim instead of == NULL) in netconsole_parser_cmdline(). This improves code readability and follows kernel coding style conventions.
Signed-off-by: Breno Leitao leitao@debian.org --- drivers/net/netconsole.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c index 7f1fed7871967..56d14294ddcaf 100644 --- a/drivers/net/netconsole.c +++ b/drivers/net/netconsole.c @@ -1705,7 +1705,8 @@ static int netconsole_parser_cmdline(struct netpoll *np, char *opt) int ipv6;
if (*cur != '@') { - if ((delim = strchr(cur, '@')) == NULL) + delim = strchr(cur, '@'); + if (!delim) goto parse_failed; *delim = 0; if (kstrtou16(cur, 10, &np->local_port)) @@ -1716,7 +1717,8 @@ static int netconsole_parser_cmdline(struct netpoll *np, char *opt)
if (*cur != '/') { ipversion_set = true; - if ((delim = strchr(cur, '/')) == NULL) + delim = strchr(cur, '/'); + if (!delim) goto parse_failed; *delim = 0; ipv6 = netpoll_parse_ip_addr(cur, &np->local_ip); @@ -1730,7 +1732,8 @@ static int netconsole_parser_cmdline(struct netpoll *np, char *opt)
if (*cur != ',') { /* parse out dev_name or dev_mac */ - if ((delim = strchr(cur, ',')) == NULL) + delim = strchr(cur, ','); + if (!delim) goto parse_failed; *delim = 0;
@@ -1747,7 +1750,8 @@ static int netconsole_parser_cmdline(struct netpoll *np, char *opt)
if (*cur != '@') { /* dst port */ - if ((delim = strchr(cur, '@')) == NULL) + delim = strchr(cur, '@'); + if (!delim) goto parse_failed; *delim = 0; if (*cur == ' ' || *cur == '\t') @@ -1759,7 +1763,8 @@ static int netconsole_parser_cmdline(struct netpoll *np, char *opt) cur++;
/* dst ip */ - if ((delim = strchr(cur, '/')) == NULL) + delim = strchr(cur, '/'); + if (!delim) goto parse_failed; *delim = 0; ipv6 = netpoll_parse_ip_addr(cur, &np->remote_ip);
Add a new selftest to verify netconsole module loading with command line arguments. This test exercises the init_netconsole() path and validates proper parsing of the netconsole= parameter format.
The test: - Loads netconsole module with cmdline configuration instead of dynamic reconfiguration - Validates message transmission through the configured target - Adds helper functions for cmdline string generation and module validation
This complements existing netconsole selftests by covering the module initialization code path that processes boot-time parameters. This test is useful to test issues like the one described in [1].
Link: https://lore.kernel.org/netdev/Z36TlACdNMwFD7wv@dev-ushankar.dev.purestorage... [1] Signed-off-by: Breno Leitao leitao@debian.org --- tools/testing/selftests/drivers/net/Makefile | 1 + .../selftests/drivers/net/lib/sh/lib_netcons.sh | 39 +++++++++++++--- .../selftests/drivers/net/netcons_cmdline.sh | 52 ++++++++++++++++++++++ 3 files changed, 86 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/drivers/net/Makefile b/tools/testing/selftests/drivers/net/Makefile index be780bcb73a3b..bd309b2d39095 100644 --- a/tools/testing/selftests/drivers/net/Makefile +++ b/tools/testing/selftests/drivers/net/Makefile @@ -12,6 +12,7 @@ TEST_GEN_FILES := \ TEST_PROGS := \ napi_id.py \ netcons_basic.sh \ + netcons_cmdline.sh \ netcons_fragmented_msg.sh \ netcons_overflow.sh \ netcons_sysdata.sh \ diff --git a/tools/testing/selftests/drivers/net/lib/sh/lib_netcons.sh b/tools/testing/selftests/drivers/net/lib/sh/lib_netcons.sh index 71a5a8b1712c0..42fd458be56c9 100644 --- a/tools/testing/selftests/drivers/net/lib/sh/lib_netcons.sh +++ b/tools/testing/selftests/drivers/net/lib/sh/lib_netcons.sh @@ -121,6 +121,17 @@ function create_dynamic_target() { echo 1 > "${NETCONS_PATH}"/enabled }
+# Generate the command line argument for netconsole following: +# netconsole=[+][src-port]@[src-ip]/[<dev>],[tgt-port]@<tgt-ip>/[tgt-macaddr] +function create_cmdline_str() { + DSTMAC=$(ip netns exec "${NAMESPACE}" \ + ip link show "${DSTIF}" | awk '/ether/ {print $2}') + SRCPORT="1514" + TGTPORT="6666" + + echo "netconsole="+${SRCPORT}@${SRCIP}/${SRCIF},${TGTPORT}@${DSTIP}/${DSTMAC}"" +} + # Do not append the release to the header of the message function disable_release_append() { echo 0 > "${NETCONS_PATH}"/enabled @@ -169,13 +180,9 @@ function listen_port_and_save_to() { socat UDP-LISTEN:"${PORT}",fork "${OUTPUT}" }
-function validate_result() { +# Only validate that the message arrived properly +function validate_msg() { local TMPFILENAME="$1" - local FORMAT=${2:-"extended"} - - # TMPFILENAME will contain something like: - # 6.11.1-0_fbk0_rc13_509_g30d75cea12f7,13,1822,115075213798,-;netconsole selftest: netcons_gtJHM - # key=value
# Check if the file exists if [ ! -f "$TMPFILENAME" ]; then @@ -188,6 +195,17 @@ function validate_result() { cat "${TMPFILENAME}" >&2 exit "${ksft_fail}" fi +} + +# Validate the message and userdata +function validate_result() { + local TMPFILENAME="$1" + + # TMPFILENAME will contain something like: + # 6.11.1-0_fbk0_rc13_509_g30d75cea12f7,13,1822,115075213798,-;netconsole selftest: netcons_gtJHM + # key=value + + validate_msg "${TMPFILENAME}"
# userdata is not supported on basic format target, # thus, do not validate it. @@ -263,3 +281,12 @@ function pkill_socat() { pkill -f "${PROCESS_NAME}" set -e } + +# Check if netconsole was compiled as a module, otherwise exit +function check_netconsole_module() { + if modinfo netconsole | grep filename: | grep -q builtin + then + echo "SKIP: netconsole should be compiled as a module" >&2 + exit "${ksft_skip}" + fi +} diff --git a/tools/testing/selftests/drivers/net/netcons_cmdline.sh b/tools/testing/selftests/drivers/net/netcons_cmdline.sh new file mode 100755 index 0000000000000..6d743b6e4922b --- /dev/null +++ b/tools/testing/selftests/drivers/net/netcons_cmdline.sh @@ -0,0 +1,52 @@ +#!/usr/bin/env bash +# SPDX-License-Identifier: GPL-2.0 + +# This is a selftest to test cmdline arguments on netconsole. +# It exercises loading of netconsole from cmdline instead of the dynamic +# reconfiguration. This includes parsing the long netconsole= line and all the +# flow through init_netconsole(). +# +# Author: Breno Leitao leitao@debian.org + +set -euo pipefail + +SCRIPTDIR=$(dirname "$(readlink -e "${BASH_SOURCE[0]}")") + +source "${SCRIPTDIR}"/lib/sh/lib_netcons.sh + +check_netconsole_module + +modprobe netdevsim 2> /dev/null || true +rmmod netconsole 2> /dev/null || true + +# The content of kmsg will be save to the following file +OUTPUT_FILE="/tmp/${TARGET}" + +# Check for basic system dependency and exit if not found +# check_for_dependencies +# Set current loglevel to KERN_INFO(6), and default to KERN_NOTICE(5) +echo "6 5" > /proc/sys/kernel/printk +# Remove the namespace and network interfaces +trap cleanup_all_ns EXIT +# Create one namespace and two interfaces +set_network +# Create the command line for netconsole, with the configuration from the +# function above +CMDLINE="$(create_cmdline_str)" + +# Load the module, with the cmdline set +modprobe netconsole "${CMDLINE}" + +# Listed for netconsole port inside the namespace and destination interface +listen_port_and_save_to "${OUTPUT_FILE}" & +# Wait for socat to start and listen to the port. +wait_local_port_listen "${NAMESPACE}" "${PORT}" udp +# Send the message +echo "${MSG}: ${TARGET}" > /dev/kmsg +# Wait until socat saves the file to disk +busywait "${BUSYWAIT_TIMEOUT}" test -s "${OUTPUT_FILE}" +# Make sure the message was received in the dst part +# and exit +validate_msg "${OUTPUT_FILE}" + +exit "${ksft_pass}"
On Wed, 11 Jun 2025 07:18:56 -0700 Breno Leitao wrote:
Add a new selftest to verify netconsole module loading with command line arguments. This test exercises the init_netconsole() path and validates proper parsing of the netconsole= parameter format.
The test:
- Loads netconsole module with cmdline configuration instead of dynamic reconfiguration
- Validates message transmission through the configured target
- Adds helper functions for cmdline string generation and module validation
This complements existing netconsole selftests by covering the module initialization code path that processes boot-time parameters. This test is useful to test issues like the one described in [1].
I think this leaks the IP address, because if netcons_overflow.sh runs afterwards it skips with:
# inet 192.0.2.1/24 scope global eni370np1 # SKIP: IPs already in use. Skipping it
if netcons_overflow.sh runs first everything is fine.
On Thu, Jun 12, 2025 at 03:05:42PM -0700, Jakub Kicinski wrote:
On Wed, 11 Jun 2025 07:18:56 -0700 Breno Leitao wrote:
Add a new selftest to verify netconsole module loading with command line arguments. This test exercises the init_netconsole() path and validates proper parsing of the netconsole= parameter format.
The test:
- Loads netconsole module with cmdline configuration instead of dynamic reconfiguration
- Validates message transmission through the configured target
- Adds helper functions for cmdline string generation and module validation
This complements existing netconsole selftests by covering the module initialization code path that processes boot-time parameters. This test is useful to test issues like the one described in [1].
I think this leaks the IP address, because if netcons_overflow.sh runs afterwards it skips with:
# inet 192.0.2.1/24 scope global eni370np1 # SKIP: IPs already in use. Skipping it
if netcons_overflow.sh runs first everything is fine.
Thanks for the report. I acknowledge there is a bug in the cleanup part of the code. Basically `trap cleanup_all_ns EXIT` is not enought because it was not removing the local netdevsim interface, which must be removed as well. I will update and resend. Good catch, again.
--breno
linux-kselftest-mirror@lists.linaro.org
-
Breno Leitao -
Jakub Kicinski