From: Breno Leitao leitao@debian.org

Introduces a enum to track netconsole target state which is going to replace the enabled boolean.

Signed-off-by: Breno Leitao leitao@debian.org Signed-off-by: Andre Carvalho asantostc@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 194570443493b1417570d3fe3250281cffe01316..b5e6a9fdc3155196d1fd7e81def584360ecbc3d2 100644 --- a/drivers/net/netconsole.c +++ b/drivers/net/netconsole.c @@ -117,6 +117,11 @@ enum sysdata_feature { SYSDATA_MSGID = BIT(3), };

+enum target_state { + STATE_DISABLED, + STATE_ENABLED, +}; + /** * struct netconsole_target - Represents a configured netconsole target. * @list: Links this target into the target_list.

From: Breno Leitao leitao@debian.org

When the low level interface brings a netconsole target down, record this using a new STATE_DEACTIVATED state. This allows netconsole to distinguish between targets explicitly disabled by users and those deactivated due to interface state changes.

It also enables automatic recovery and re-enabling of targets if the underlying low-level interfaces come back online.

From a code perspective, anything that is not STATE_ENABLED is disabled. Mark the device that is down due to NETDEV_UNREGISTER as STATE_DEACTIVATED, this, should be the same as STATE_DISABLED from a code perspective.

Signed-off-by: Breno Leitao leitao@debian.org Signed-off-by: Andre Carvalho asantostc@gmail.com --- drivers/net/netconsole.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c index 688ed670b37b56ab4a03d43fb3de94ca0e6a8360..59d770bb4baa5f9616b10c0dfb39ed45a4eb7710 100644 --- a/drivers/net/netconsole.c +++ b/drivers/net/netconsole.c @@ -120,6 +120,7 @@ enum sysdata_feature { enum target_state { STATE_DISABLED, STATE_ENABLED, + STATE_DEACTIVATED, };

/** @@ -575,6 +576,14 @@ static ssize_t enabled_store(struct config_item *item, if (ret) goto out_unlock;

+ /* When the user explicitly enables or disables a target that is + * currently deactivated, reset its state to disabled. The DEACTIVATED + * state only tracks interface-driven deactivation and should _not_ + * persist when the user manually changes the target's enabled state. + */ + if (nt->state == STATE_DEACTIVATED) + nt->state = STATE_DISABLED; + ret = -EINVAL; current_enabled = nt->state == STATE_ENABLED; if (enabled == current_enabled) { @@ -1446,7 +1455,7 @@ static int netconsole_netdev_event(struct notifier_block *this, case NETDEV_RELEASE: case NETDEV_JOIN: case NETDEV_UNREGISTER: - nt->state = STATE_DISABLED; + nt->state = STATE_DEACTIVATED; list_move(&nt->list, &target_cleanup_list); stopped = true; }

Attempt to resume a previously deactivated target when the associated interface comes back (NETDEV_UP event is received) by calling __netpoll_setup_hold on the device.

For targets that were initally setup by mac address, their address is also compared with the interface address (while still verifying that the interface name matches).

__netpoll_setup_hold assumes RTNL is held (which is guaranteed to be the case when handling the event) and holds a reference to the device in case of success. This reference will be removed upon target (or netconsole) removal by netpoll_cleanup.

Target transitions to STATE_DISABLED in case of failures resuming it to avoid retrying the same target indefinitely.

Signed-off-by: Andre Carvalho asantostc@gmail.com --- drivers/net/netconsole.c | 38 ++++++++++++++++++++++++++++++++++---- 1 file changed, 34 insertions(+), 4 deletions(-)

diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c index 59d770bb4baa5f9616b10c0dfb39ed45a4eb7710..96485e979e61e0ed6c850ae3b29f46d529923f2d 100644 --- a/drivers/net/netconsole.c +++ b/drivers/net/netconsole.c @@ -135,10 +135,12 @@ enum target_state { * @stats: Packet send stats for the target. Used for debugging. * @state: State of the target. * Visible from userspace (read-write). - * We maintain a strict 1:1 correspondence between this and - * whether the corresponding netpoll is active or inactive. + * From a userspace perspective, the target is either enabled or + * disabled. Internally, although both STATE_DISABLED and + * STATE_DEACTIVATED correspond to inactive netpoll the latter is + * due to interface state changes and may recover automatically. * Also, other parameters of a target may be modified at - * runtime only when it is disabled (state == STATE_DISABLED). + * runtime only when it is disabled (state != STATE_ENABLED). * @extended: Denotes whether console is extended or not. * @release: Denotes whether kernel release version should be prepended * to the message. Depends on extended console. @@ -1430,6 +1432,31 @@ static int prepare_extradata(struct netconsole_target *nt) } #endif /* CONFIG_NETCONSOLE_DYNAMIC */

+/* Attempts to resume logging to a deactivated target. */ +static void maybe_resume_target(struct netconsole_target *nt, + struct net_device *ndev) +{ + int ret; + + if (strncmp(nt->np.dev_name, ndev->name, IFNAMSIZ)) + return; + + /* for targets specified by mac, also verify it matches the addr */ + if (!is_broadcast_ether_addr(nt->np.dev_mac) && + memcmp(nt->np.dev_mac, ndev->dev_addr, ETH_ALEN)) + return; + + ret = __netpoll_setup_hold(&nt->np, ndev); + if (ret) { + /* netpoll fails setup once, do not try again. */ + nt->state = STATE_DISABLED; + } else { + nt->state = STATE_ENABLED; + pr_info("network logging resumed on interface %s\n", + nt->np.dev_name); + } +} + /* Handle network interface device notifications */ static int netconsole_netdev_event(struct notifier_block *this, unsigned long event, void *ptr) @@ -1440,7 +1467,8 @@ static int netconsole_netdev_event(struct notifier_block *this, bool stopped = false;

if (!(event == NETDEV_CHANGENAME || event == NETDEV_UNREGISTER || - event == NETDEV_RELEASE || event == NETDEV_JOIN)) + event == NETDEV_RELEASE || event == NETDEV_JOIN || + event == NETDEV_UP)) goto done;

mutex_lock(&target_cleanup_list_lock); @@ -1460,6 +1488,8 @@ static int netconsole_netdev_event(struct notifier_block *this, stopped = true; } } + if (nt->state == STATE_DEACTIVATED && event == NETDEV_UP) + maybe_resume_target(nt, dev); netconsole_target_put(nt); } spin_unlock_irqrestore(&target_list_lock, flags);

Introduce a new netconsole selftest to validate that netconsole is able to resume a deactivated target when the low level interface comes back.

The test setups the network using netdevsim, creates a netconsole target and then remove/add netdevsim in order to bring the same interfaces back. Afterwards, the test validates that the target works as expected.

Targets are created via cmdline parameters to the module to ensure that resuming works for targets bound by interface name and mac address.

Signed-off-by: Andre Carvalho asantostc@gmail.com --- tools/testing/selftests/drivers/net/Makefile | 1 + .../selftests/drivers/net/lib/sh/lib_netcons.sh | 30 ++++++- .../selftests/drivers/net/netcons_resume.sh | 92 ++++++++++++++++++++++ 3 files changed, 120 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/Makefile b/tools/testing/selftests/drivers/net/Makefile index 984ece05f7f92e836592107ba4c692da6d8ce1b3..a40b50c66d530b3fcbeaf93ca46f79380b3a1949 100644 --- a/tools/testing/selftests/drivers/net/Makefile +++ b/tools/testing/selftests/drivers/net/Makefile @@ -16,6 +16,7 @@ TEST_PROGS := \ netcons_cmdline.sh \ netcons_fragmented_msg.sh \ netcons_overflow.sh \ + netcons_resume.sh \ netcons_sysdata.sh \ netpoll_basic.py \ ping.py \ 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 8e1085e896472d5c87ec8b236240878a5b2d00d2..88b4bdfa84cf4ab67ff0e04c3ed88e5ae9df49d2 100644 --- a/tools/testing/selftests/drivers/net/lib/sh/lib_netcons.sh +++ b/tools/testing/selftests/drivers/net/lib/sh/lib_netcons.sh @@ -186,12 +186,13 @@ function do_cleanup() { }

function cleanup() { + local TARGETPATH=${1:-${NETCONS_PATH}} # delete netconsole dynamic reconfiguration - echo 0 > "${NETCONS_PATH}"/enabled + echo 0 > "${TARGETPATH}"/enabled # Remove all the keys that got created during the selftest - find "${NETCONS_PATH}/userdata/" -mindepth 1 -type d -delete + find "${TARGETPATH}/userdata/" -mindepth 1 -type d -delete # Remove the configfs entry - rmdir "${NETCONS_PATH}" + rmdir "${TARGETPATH}"

do_cleanup } @@ -350,6 +351,29 @@ function check_netconsole_module() { fi }

+function wait_target_state() { + local TARGET=${1} + local STATE=${2} + local FILE="${NETCONS_CONFIGFS}"/"${TARGET}"/"enabled" + + if [ "${STATE}" == "enabled" ] + then + ENABLED=1 + else + ENABLED=0 + fi + + if [ ! -f "$FILE" ]; then + echo "FAIL: Target does not exist." >&2 + exit "${ksft_fail}" + fi + + slowwait 2 sh -c "test -n "$(grep "${ENABLED}" "${FILE}")"" || { + echo "FAIL: ${TARGET} is not ${STATE}." >&2 + exit "${ksft_fail}" + } +} + # A wrapper to translate protocol version to udp version function wait_for_port() { local NAMESPACE=${1} diff --git a/tools/testing/selftests/drivers/net/netcons_resume.sh b/tools/testing/selftests/drivers/net/netcons_resume.sh new file mode 100755 index 0000000000000000000000000000000000000000..404df7abef1bcdbd29a128c304ac9b39f19fc82d --- /dev/null +++ b/tools/testing/selftests/drivers/net/netcons_resume.sh @@ -0,0 +1,92 @@ +#!/usr/bin/env bash +# SPDX-License-Identifier: GPL-2.0 + +# This test validates that netconsole is able to resume a target that was +# deactivated when its interface was removed when the interface is brought +# back up. +# +# The test configures a netconsole target and then removes netdevsim module to +# cause the interface to disappear. Targets are configured via cmdline to ensure +# targets bound by interface name and mac address can be resumed. +# The test verifies that the target moved to disabled state before adding +# netdevsim and the interface back. +# +# Finally, the test verifies that the target is re-enabled automatically and +# the message is received on the destination interface. +# +# Author: Andre Carvalho asantostc@gmail.com + +set -euo pipefail + +SCRIPTDIR=$(dirname "$(readlink -e "${BASH_SOURCE[0]}")") + +source "${SCRIPTDIR}"/lib/sh/lib_netcons.sh + +modprobe netdevsim 2> /dev/null || true +rmmod netconsole 2> /dev/null || true + +check_netconsole_module + +# Run the test twice, with different cmdline parameters +for BINDMODE in "ifname" "mac" +do + echo "Running with bind mode: ${BINDMODE}" >&2 + # Set current loglevel to KERN_INFO(6), and default to KERN_NOTICE(5) + echo "6 5" > /proc/sys/kernel/printk + + # Create one namespace and two interfaces + set_network + trap do_cleanup EXIT + + # Create the command line for netconsole, with the configuration from + # the function above + CMDLINE=$(create_cmdline_str "${BINDMODE}") + + # The content of kmsg will be save to the following file + OUTPUT_FILE="/tmp/${TARGET}-${BINDMODE}" + + # Load the module, with the cmdline set + modprobe netconsole "${CMDLINE}" + # Expose cmdline target in configfs + mkdir ${NETCONS_CONFIGFS}"/cmdline0" + trap 'cleanup "${NETCONS_CONFIGFS}"/cmdline0' EXIT + + # Target should be enabled + wait_target_state "cmdline0" "enabled" + + # Remove low level module + rmmod netdevsim + # Target should be disabled + wait_target_state "cmdline0" "disabled" + + # Add back low level module + modprobe netdevsim + # Recreate namespace and two interfaces + set_network + # Target should be enabled again + wait_target_state "cmdline0" "enabled" + + # Listen 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}" + + # kill socat in case it is still running + pkill_socat + # Cleanup & unload the module + cleanup "${NETCONS_CONFIGFS}/cmdline0" + rmmod netconsole + trap - EXIT + + echo "${BINDMODE} : Test passed" >&2 +done + +exit "${ksft_pass}"