Re: [PATCH net-next v11 7/7] selftests: netconsole: validate target resume

[Andre Carvalho <asantostc@gmail.com>]

Hello Matthieu,

On Wed, Jul 08, 2026 at 05:50:53PM +0200, Matthieu Baerts wrote:
> 
> > +function trigger_reactivation() {
> > +	# Add back low level module
> > +	modprobe netdevsim
> > +	# Recreate namespace and two interfaces
> > +	set_network
> > +	# Restore MACs
> > +	ip netns exec "${NAMESPACE}" ip link set "${DSTIF}" \
> > +		address "${SAVED_DSTMAC}"
> > +	if [ "${BINDMODE}" == "mac" ]; then
> > +		ip link set dev "${SRCIF}" down
> > +		ip link set dev "${SRCIF}" address "${SAVED_SRCMAC}"
> > +		# Rename device in order to trigger target resume, as initial
> > +		# when device was recreated it didn't have correct mac address.
> > +		ip link set dev "${SRCIF}" name "${TARGET}"
> 
> When I execute the test, the "ifname" bind mode works without issues,
> but the "mac" one not. From what I see, the socat process doesn't get
> any UDP packet when expected. I wonder if the problem might not come
> from here: the interface is disabled before changing the MAC address and
> renaming the interface, but not re-enabled at the end. Is it normal?

Yes, the expectation is that the target would be automatically re-enabled by
netconsole. That is why we don't 'up' the interface here explicitly.

The problem is that the test has an implict dependency on interface mac address
changing when we recreate them, which is why we have to do this whole down/update/rename
flow to trigger the reactivation in this case.

> If I add 'up' at the end of this last line here, or if I remove the
> whole if-statement block, the test passes.

For cases where the mac is persistent (e.g with systemd MACAddressPolicy=persistent),
the actual re-enablement should "just work", which I suspect is why it works when
you remove the whole if-statement block.

I think to make the test more robust to such scenarios, we should skip the
if-statement block when we detect that the interface came back with the same mac
as previously. Something like this:

if [ "${BINDMODE}" == "mac" ]; then
	CURR_SRCMAC=$(mac_get "${SRCIF}")
	if [ "${CURR_SRCMAC}" == "${SAVED_SRCMAC}" ]; then
		# Interface came back with same mac, no need to restore
		return
	fi
	ip link set dev "${SRCIF}" down
	ip link set dev "${SRCIF}" address "${SAVED_SRCMAC}"
	# Rename device in order to trigger target resume, as initial
	# when device was recreated it didn't have correct mac address.
	ip link set dev "${SRCIF}" name "${TARGET}"
fi

Could you confirm if this fixes the selftest in your environment?

> The "network logging resumed on interface" seems to suggest that the
> previous patch of this series here, commit 220dbe3c76ed ("netconsole:
> resume previously deactivated target"), managed to resume the previously
> activated target, but not in my case.

Thanks for including the logs, this helps quite a bit and I think confirms that
the interface comes back with the same mac address as previously, and is indeed
resumed, I believe the down/update/rename portion of the test then drops the target
and causes the failure.

> What I don't understand is why is it working on the Netdev CI, and not
> on my side. The main difference is that I might be missing some
> userspace packages -- but I don't see what can be missing here, all
> other netconsole tests pass -- a specific kernel config, or not applied
> patch. Or something different on the host -- in a container on my side
> -- but there shouldn't be any interactions with the host here,
> everything is happening in the VM. Any ideas? :)

Netdev CI executes with MACAddressPolicy=none (which is why I added the if-statement
block [1]) and I think it might explain the behaviour difference.

> 
> > +	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
> 
> Can we remove this? Without that, it is hard to understand what went
> wrong in case of issues.

Can you elaborate? This is typical for netconsole tests and I think required to
ensure the write will show up on netconsole. I'm not opposed to changing it if
there is a reason, but we probably want to make it consistent with other tests too.

> > +	# Send the message
> > +	echo "${MSG}: ${TARGET}" > /dev/kmsg
> > +	# Wait until socat saves the file to disk
> > +	busywait "${BUSYWAIT_TIMEOUT}" test -s "${OUTPUT_FILE}"
> 
> In my case, the script was stopping here, without any message. I can
> send a patch adding "|| true" to go to the next instruction, and display
> "FAIL: File was not generated.".
> 

This seems reasonable to me.

> > +	# 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
> > +
> > +	echo "${BINDMODE} : Test passed" >&2
> > +done
> > +
> > +trap - EXIT
> > +exit "${EXIT_STATUS}"
> > 
> 
> Cheers,
> Matt

[1] https://lore.kernel.org/all/20260117092227.741cba3b@kernel.org/

-- 
Andre Carvalho