Skip to content

Conversation

julianwiedmann
Copy link
Member

@julianwiedmann julianwiedmann commented Apr 25, 2025

Also plug the ICMPv6 NA filtering from #38798 into the native-routing paths, so that we avoid false-positives like this one in CI.

Break the long filter strings down into smaller pieces that are more
composable. No change in behavior.

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
Make sure that we only process ICMPv6 packets in this filter.

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
Cilium's Wireguard logic doesn't encrypt NA messages. We currently ignore
such ICMPv6 packets when in overlay routing mode. Do the same in native
routing mode.

Note that this filter actually seems redundant for the overlay case in
encryption_v2 - the `baseTunnelFilter` already filters for UDP packets, so
we shouldn't even see any ICMPv6 packets.

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Apr 25, 2025
@github-actions github-actions bot added cilium-cli This PR contains changes related with cilium-cli cilium-cli-exclusive This PR only impacts cilium-cli binary labels Apr 25, 2025
@julianwiedmann
Copy link
Member Author

/test

@julianwiedmann julianwiedmann changed the title Pr/jwi/main/cli encrypt cli: encryption: improve ICMPv6 NA detection Apr 25, 2025
@julianwiedmann julianwiedmann added area/encryption Impacts encryption support such as IPSec, WireGuard, or kTLS. release-note/ci This PR makes changes to the CI. labels Apr 25, 2025
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Apr 25, 2025
@julianwiedmann julianwiedmann marked this pull request as ready for review April 25, 2025 07:45
@julianwiedmann julianwiedmann requested a review from a team as a code owner April 25, 2025 07:45
Copy link
Contributor

@smagnani96 smagnani96 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks Julian!

@julianwiedmann julianwiedmann added the dont-merge/waiting-for-review Requires further review before merging. label Apr 25, 2025
@julianwiedmann
Copy link
Member Author

@tommyp1ckles I think this is fairly straight-forward, but maybe you have any thoughts. Otherwise please just drop the label :)

@julianwiedmann julianwiedmann removed the dont-merge/waiting-for-review Requires further review before merging. label Apr 29, 2025
@julianwiedmann julianwiedmann added this pull request to the merge queue Apr 29, 2025
Merged via the queue into main with commit 692ac6d Apr 29, 2025
463 of 472 checks passed
@julianwiedmann julianwiedmann deleted the pr/jwi/main/cli-encrypt branch April 29, 2025 05:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/encryption Impacts encryption support such as IPSec, WireGuard, or kTLS. cilium-cli This PR contains changes related with cilium-cli cilium-cli-exclusive This PR only impacts cilium-cli binary release-note/ci This PR makes changes to the CI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants