Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

1.x: nl_bridge: properly handle L2 neighbors moving between ports #458

Draft
wants to merge 3 commits into
base: 1.x
Choose a base branch
from

Conversation

KanjiMonster
Copy link
Contributor

This is a backport of #415 and #428 (follow-up fix)

nl_cache_search() uses nl_object_identical()[1] to lookup the target
object. This limits the check to unique attributes, which does not
include the ifindex [2].

Therefore nl_cache_search() may return an object for a different port,
in case it moved, which we then may wrongly remove from the cache.

So add a check for the ifindex of the hit before attempting to delete
any neighbors or cache entries.

We could have also alternatively used nl_cache_find() which uses all
attributes, but this function iterates over the whole cache intead of
using the hashtable if possible, so would be much less performant.

[1] https://github.com/thom311/libnl/blob/libnl3_7_0/lib/cache.c#L1113-L1114
[2] https://github.com/thom311/libnl/blob/libnl3_7_0/lib/route/neigh.c#L323

Fixes: 291591d ("l2 aging added")
Signed-off-by: Jonas Gorski <[email protected]>
(cherry picked from commit a566d4c)
Signed-off-by: Jonas Gorski <[email protected]>
Calling nl_cache_add() for an object cache that supports hashing will
refuse to add an object if it already contains an "identical" object,
where identical is considered having the same master, vid and mac for
AF_BRIDGE neighbors.

Therefore we need to remove the old entry first before we can add an
updated one when we handle neighbors moving between ports.

Without it, we may fail to update the flow if the port moves back again
to its original port, as the outdated entry makes us think we already
have a flow for it.

Fixes: de9b16e ("nl_bridge: take over fdb entries instead of creating them")
Signed-off-by: Jonas Gorski <[email protected]>
(cherry picked from commit 82f59be)
Signed-off-by: Jonas Gorski <[email protected]>
When fixing moving neighbors between ports we removed the ifindex from
the filter neigh to find a neighbor at any port with the mac address.

But since we use the same neigh to construct the fdb delete message,
this caused the message to miss the ifindex, and the kernel was
rejecting the deletion. This resulting in us not updating out local
cache of known layer 2 neighbors, and failing to relearn them.

Fix this by just using the found entry from our local cache, n_lookup.

Fixes: a566d4c ("nl_bridge::fdb_timeout(): check ifindex of hit before deletion")
Signed-off-by: Jonas Gorski <[email protected]>
(cherry picked from commit 09bf1a6)
Signed-off-by: Jonas Gorski <[email protected]>
@KanjiMonster KanjiMonster changed the title v1.x: nl_bridge: properly handle L2 neighbors moving between ports 1.x: nl_bridge: properly handle L2 neighbors moving between ports Feb 11, 2025
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.

1 participant