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

ndb: make non-unique FDB index more unique #1158

Merged
merged 1 commit into from
Jan 10, 2024

Conversation

pkulev
Copy link
Contributor

@pkulev pkulev commented Dec 25, 2023

I found out that brmsg_schema has unique index on (ifindex, lladdr, vlan), so that gives an answer to me what happend in the linked issue.

There can be multicast FDBs with single ifindex and no NDA_VLAN at all (that means NDA_VLAN = 0), but NDA_DST will be different.

We can clearly see that in the example from the issue:

~# bridge fdb show | grep 00:00:00:00:00:00
00:00:00:00:00:00 dev vxln7 dst 10.1.0.107 self extern_learn permanent
00:00:00:00:00:00 dev vxln7 dst 10.1.0.11 self extern_learn permanent

This change of index breaks backward compatibility for filters like this:

fdb = ndb.fdb.dump()
fdb.select_records(dst=lambda dst: dst is not None)
do_something_with(list(fdb))

Now it's more like identity check:

fdb.select_records(dst=lambda dst: dst)

Closes: #1091

There can be multicast FDBs with single ifindex and no NDA_VLAN
at all (that means NDA_VLAN = 0), but NDA_DST will be different.
@pkulev
Copy link
Contributor Author

pkulev commented Jan 10, 2024

@svinota could you please review this PR and tell me what do you think about it?

@svinota
Copy link
Owner

svinota commented Jan 10, 2024

Thanks, looking how to merge (a small change required, but I fix it)

@svinota
Copy link
Owner

svinota commented Jan 10, 2024

Fixed, running CI locally. I will merge the the PR manually and update you here.

svinota added a commit that referenced this pull request Jan 10, 2024
svinota added a commit that referenced this pull request Jan 10, 2024
@svinota svinota merged commit 799a5fb into svinota:master Jan 10, 2024
12 of 14 checks passed
@pkulev pkulev deleted the bugfix/fdb-missing-records branch January 10, 2024 13:40
@svinota
Copy link
Owner

svinota commented Jan 10, 2024

The PR uses NDA_DST which is empty in most cases, so I added a small fix that ignores empty NDA_DST when running IPRoute.fdb() and IPRoute.neigh().

@svinota
Copy link
Owner

svinota commented Jan 10, 2024

Pls check if this will work for you.

@pkulev
Copy link
Contributor Author

pkulev commented Jan 10, 2024

Thanks! I'll test changes tomorrow.

@pkulev
Copy link
Contributor Author

pkulev commented Jan 11, 2024

@svinota, yep it worked well, thank you! Could you please make a new release version?

@svinota
Copy link
Owner

svinota commented Jan 11, 2024

Sure thing, to be tagged tonight

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.

2 participants