Added support for L2 BridgeMDB entries #32894
Conversation
github-actions
bot
added
network
documentation
tests
please-review
|
Important An -rc1 tag has been created and a release is being prepared, so please note that PRs introducing new features and APIs will be held back until the new version has been released. |
|
looks superficially ok, but please have another look at the coding style: https://systemd.io/CODING_STYLE/#formatting and make sure that the patch matches this and the rest of the code. |
poettering
added
reviewed/needs-rework 🔨
github-actions
bot
added
please-review
| "%s: MulticastGroupAddress= is the multicast all nodes address. " | ||
| "Ignoring [BridgeMDB] section from line %u.", | ||
| mdb->section->filename, mdb->section->line); | ||
| } |
There was a problem hiding this comment.
coding style: no {} for single line if blocks
There was a problem hiding this comment.
This one was already here, wasn't added by me. I think it's to make sure no compiler interprets this as "else if", because of the nested if statements.
src/network/networkd-bridge-mdb.c
Outdated
| "Ignoring [BridgeMDB] section from line %u.", | ||
| mdb->section->filename, mdb->section->line); | ||
|
|
||
| if (mdb->family == AF_INET) { |
There was a problem hiding this comment.
mght actually be a candidate for switch statement too
There was a problem hiding this comment.
All of the L3 checks were already present and I didn't change them. Just changed their indentation to put them in the switch statement.
Changing this into a switch statement sounds like a good idea though, it gets rid of the nested if statements, which force us to use brackets to prevent "else" ambiguity. Should I do it as part of this PR?
There was a problem hiding this comment.
It might make sense to check for AF_INET6 on the "else" instead of just catching everything, and putting an "assert_not_reached" on the else case. I'm hesitant to change code behaviour that is not directly related to the L2 BridgeMDB entries, which is the topic of this PR though.
BridgeMDB entries for Ipv4 and Ipv6 have been supported in networkd for some time.
This PR adds support for adding L2 entries in the multicast group database, which was implemented in Linux 5.11