Skip to content

Fix mctpd deleted interface handling#76

Merged
jk-ozlabs merged 15 commits intomainfrom
pr/mctpd-del-interface
May 30, 2025
Merged

Fix mctpd deleted interface handling#76
jk-ozlabs merged 15 commits intomainfrom
pr/mctpd-del-interface

Conversation

@mkj
Copy link
Copy Markdown
Member

@mkj mkj commented May 30, 2025

Previously the duplicate linkmaps would become inconsistent when interfaces were added/removed.
mctpd also attempted to remove a route for a deleted interface, even though the kernel would have already removed that route.

This PR also has a few cleanups encountered while adding a test.

In the process of adding tests, I realised zero-length lladdr (for eg mctpusb) still adds neighbour entries. Those are not necessary, mctpd now omits adding those.

This should fix #55 and fix #73.

mkj added 15 commits May 30, 2025 09:56
Previously the linkmap in ctx->nl_query could be inconsistent with
ctx->nl when interfaces are added/removed (github issues #55 and #73).

All netlink requests are currently performed synchronously, so the
single sd can be used both for update requests from mctpd, and for
fill_linkmap() after a monitor update.

Signed-off-by: Matt Johnston <matt@codeconstruct.com.au>
This accidentally converted the lladdr to a tuple (bytes,) rather than
plain bytes. Does not seem to have any effect.

Signed-off-by: Matt Johnston <matt@codeconstruct.com.au>
Previously the 'c' format would not permit an empty lladdr.
This fixes the encode case, though it's not obvious if this will be
correct in all cases for the decode case.

  File "site-packages/pyroute2/netlink/__init__.py", line 1719, in ft_encode
    struct.pack_into(efmt, self.data, offset, value)
struct.error: char format requires a bytes object of length 1

Signed-off-by: Matt Johnston <matt@codeconstruct.com.au>
Signed-off-by: Matt Johnston <matt@codeconstruct.com.au>
Retrieves the length of an interface's lladdr.

Signed-off-by: Matt Johnston <matt@codeconstruct.com.au>
The neighbour entry is not necessary for eg USB transport
The test harness is updated to treat a missing neighbour entry
as an empty lladdr for lookup.

Signed-off-by: Matt Johnston <matt@codeconstruct.com.au>
Signed-off-by: Matt Johnston <matt@codeconstruct.com.au>
This was already used in meson, but make it clearer in the main
instructions.

Signed-off-by: Matt Johnston <matt@codeconstruct.com.au>
Signed-off-by: Matt Johnston <matt@codeconstruct.com.au>
Also remove an extraneous resp_len log message, and remove unnecessary
newlines from warnx() messages.

Signed-off-by: Matt Johnston <matt@codeconstruct.com.au>
Linux removes routes and neighbour entries on interface removal, so
don't warn when an ifindex has been removed prior to cleaning up a peer
on a deleted interface.

Previously a warning would be reported:

mctpd: emitting endpoint remove: /au/com/codeconstruct/mctp1/networks/1/endpoints/88
Deleting interface #10
Deleting route to peer eid 30 net 1 phys physaddr if 10 hw len 0 0x state 0
mctpd: BUG peer_route_update: Unknown ifindex 10
mctpd: Failed removing route for peer eid 30 net 1 phys physaddr if 10 hw len 0 0x state 0: No such device

Signed-off-by: Matt Johnston <matt@codeconstruct.com.au>
The Linux kernel deletes routes, neighbours, and addresses when an
interface is deleted. Match that behaviour in the test harness.

Signed-off-by: Matt Johnston <matt@codeconstruct.com.au>
This also exercises the zero-length lladdr codepaths in the test
harness.

Signed-off-by: Matt Johnston <matt@codeconstruct.com.au>
Signed-off-by: Matt Johnston <matt@codeconstruct.com.au>
Existing BUG warnings now call bug_warn(). In normal operation the
program code is unchanged (print a warning to logs), in test-mctpd it
will abort.

Signed-off-by: Matt Johnston <matt@codeconstruct.com.au>
@jk-ozlabs jk-ozlabs self-assigned this May 30, 2025
@jk-ozlabs jk-ozlabs merged commit db00b46 into main May 30, 2025
2 checks passed
@mkj mkj deleted the pr/mctpd-del-interface branch May 30, 2025 03:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

mctp interface removal/reattach (hotplug devices), doesnt update nl_query structure Mctpd does not track network interface changes correctly

2 participants