Merge branch 'netpoll-untangle-netconsole-and-netpoll'

Breno Leitao says:

====================
netpoll: Untangle netconsole and netpoll

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.com/ [1]

v2: https://lore.kernel.org/20250611-rework-v2-0-ab1d92b458ca@debian.org
v1: https://lore.kernel.org/20250610-rework-v1-0-7cfde283f246@debian.org
====================

Link: https://patch.msgid.link/20250613-rework-v3-0-0752bf2e6912@debian.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
This commit is contained in:
Jakub Kicinski 2025-06-16 15:18:35 -07:00
commit 7aa3f99156
6 changed files with 240 additions and 155 deletions

View File

@ -278,6 +278,23 @@ static void netconsole_process_cleanups_core(void)
mutex_unlock(&target_cleanup_list_lock);
}
static void netconsole_print_banner(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
/*
@ -534,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)
@ -1659,6 +1676,120 @@ 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;
char *cur = opt;
char *delim;
int ipv6;
if (*cur != '@') {
delim = strchr(cur, '@');
if (!delim)
goto parse_failed;
*delim = 0;
if (kstrtou16(cur, 10, &np->local_port))
goto parse_failed;
cur = delim;
}
cur++;
if (*cur != '/') {
ipversion_set = true;
delim = strchr(cur, '/');
if (!delim)
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 */
delim = strchr(cur, ',');
if (!delim)
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 */
delim = strchr(cur, '@');
if (!delim)
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 */
delim = strchr(cur, '/');
if (!delim)
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;
}
netconsole_print_banner(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)
@ -1688,7 +1819,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;

View File

@ -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;
@ -65,11 +72,8 @@ 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_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);

View File

@ -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)
@ -499,43 +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 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)
{
@ -546,95 +502,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 =
@ -863,7 +730,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 +752,6 @@ void __netpoll_cleanup(struct netpoll *np)
skb_pool_flush(np);
}
EXPORT_SYMBOL_GPL(__netpoll_cleanup);
void __netpoll_free(struct netpoll *np)
{

View File

@ -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 \

View File

@ -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
@ -128,16 +139,9 @@ function disable_release_append() {
echo 1 > "${NETCONS_PATH}"/enabled
}
function cleanup() {
function do_cleanup() {
local NSIM_DEV_SYS_DEL="/sys/bus/netdevsim/del_device"
# delete netconsole dynamic reconfiguration
echo 0 > "${NETCONS_PATH}"/enabled
# Remove all the keys that got created during the selftest
find "${NETCONS_PATH}/userdata/" -mindepth 1 -type d -delete
# Remove the configfs entry
rmdir "${NETCONS_PATH}"
# Delete netdevsim devices
echo "$NSIM_DEV_2_ID" > "$NSIM_DEV_SYS_DEL"
echo "$NSIM_DEV_1_ID" > "$NSIM_DEV_SYS_DEL"
@ -149,6 +153,17 @@ function cleanup() {
echo "${DEFAULT_PRINTK_VALUES}" > /proc/sys/kernel/printk
}
function cleanup() {
# delete netconsole dynamic reconfiguration
echo 0 > "${NETCONS_PATH}"/enabled
# Remove all the keys that got created during the selftest
find "${NETCONS_PATH}/userdata/" -mindepth 1 -type d -delete
# Remove the configfs entry
rmdir "${NETCONS_PATH}"
do_cleanup
}
function set_user_data() {
if [[ ! -d "${NETCONS_PATH}""/userdata" ]]
then
@ -169,13 +184,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 +199,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 +285,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
}

View File

@ -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 do_cleanup 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}"