Hi Andre,
Thank you for your reply!
On 08/07/2026 22:24, Andre Carvalho wrote:
> 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.
OK, that's what I thought when I looked at the previous commit of this
series, but it was still feeling "strange" :)
> 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.
OK!
>> 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 see, I just noticed the discussion on lore about that on the previous
patch and on the v10. I now understand why MACAddressPolicy should be
set to 'none', and I can do that on my side.
> 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?
Yes, it fixes the selftest (without MACAddressPolicy=none), thank you!
And maybe this could be used to avoid the 'return':
if [ "${BINDMODE}" == "mac" ] &&
[ "$(mac_get "${SRCIF}")" != "${SAVED_SRCMAC}" ]; then
A detail, up to you :)
I think it would be interesting to have such check. If the MAC policy
can cause issues here, it might be good to also add a warning, or to
continue to fail, but with an explicit error message to look at this.
>>> + 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.
I understand, and I asked because I was not sure if this was needed for
some tests. If it doesn't break some tests, I think it would be good not
to change the loglevel, because on my side, all I had was:
# Running with bind mode: mac
not ok 1 selftests: drivers/net: netcons_resume.sh # exit=1
Even with the suggested modification to have "FAIL: File was not
generated.", that's not helpful enough to understand where the problem
came from. Having the extra info in the console was really helpful, but
it was not obvious I could do that.
There could be a message saying "comment this to have more debug", but
that means a CI will not get such useful debug info in case of error. So
I think this behaviour should be changed. Or at least printing debug
info only in case of errors.
>>> + # 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.
Good! I will send this patch.
Just to avoid a deadlock, do you plan to send one to handle the MAC
address that has not been modified (and eventually one to get more debug
messages in case of errors)?
Cheers,
Matt
