-
Notifications
You must be signed in to change notification settings - Fork 3.4k
bpf: add IPv6 mcast addr helpers and cleanup #39579
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
Conversation
e041068
to
d76f3d9
Compare
30042e5
to
c7e2acf
Compare
Added utility |
b26352c
to
90f80b1
Compare
@gentoo-root on the complexity issue, pushed a revised version + , the diff from the original commit:
I think this looks much better now. Asking for a re-review, JIC. |
Add IPv6 multicast helper functions that construct the multicast IPv6 and MAC addresses respectively given a unicast IPv6 address. Cleaned up repeated and/or unnecessary code from tests and pktgen.h. Added simple (and stupid) unit, just in case. Signed-off-by: Marc Suñé <marc.sune@isovalent.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pushed a revised version
Looks good to me, thank you!
Can someone please merge? No more reviews needed, no need for a release note, no need for tests (will only be exercised by #39648. Thx Cc @joestringer |
/test |
Not strictly true. Any refactor can cause build breakage, complexity issues, etc. The rule is: We run the tests. If they're failing, we can investigate. I'll share an example of the triage that can help to identify issues and communicate the status of CI for this specific PR. Failure triage:
I retriggered the tests and marked the PR for auto-merge. |
This is not a refactor. The 3 new functions are not called by any program other than the unit test ipv6_test.c, so they will not even be part of the binary. I believe some smoke tests that run on every PR, so not with /test, build with ipv6 enabled, so I don't see how this can be an issue in this very particular case. I understand and furiously agree in general. |
I stand corrected, I scanned the PR too quickly to notice. |
Hitting some issues with merge queue; latest is a sigstore infrastructure unavailability: https://github.com/cilium/cilium/actions/runs/15332419707/job/43142145733#step:23:32 . I'll retry. |
Add IPv6 multicast helper functions that construct the multicast IPv6 and MAC addresses respectively given a unicast IPv6 address.
Cleaned up repeated and/or unnecessary code from tests and pktgen.h.
Added simple (and stupid) unit, just in case.
Please ensure your pull request adheres to the following guidelines:
Minor cleanup, that simplifies IPv6 L2 announcement code. No release note needed