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

pkg: add missing xfrm-no-track rules from ipv6 #24557

Merged
merged 2 commits into from
Mar 27, 2023

Conversation

jschwinger233
Copy link
Member

@jschwinger233 jschwinger233 commented Mar 24, 2023

By right there should be a rule to let ipsec skb bypass conntrack:

-A CILIUM_PRE_raw -m mark --mark 0xd00/0xf00 -m comment --comment "cilium-xfrm-notrack:" -j CT --notrack

However ipv6 missed it and this commit adds the rule back.

Fixes: #23481

Add missing xfrm-no-track rules for IPv6 IPSec. This fixes a connectivity issue for IPv6 IPSec with externalTrafficPolicy=local.

By right there should be a rule to let ipsec skb bypass conntrack:

-A CILIUM_PRE_raw -m mark --mark 0xd00/0xf00 -m comment --comment "cilium-xfrm-notrack:" -j CT --notrack

However ipv6 missed it and this commit adds the rule back.

Fixes: cilium#23481

Signed-off-by: Zhichuan Liang <gray.liang@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 Mar 24, 2023
@jschwinger233 jschwinger233 added the release-note/minor This PR changes functionality that users may find relevant to operating Cilium. label Mar 24, 2023
@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 Mar 24, 2023
This reverts commit b1b9fbd and
re-enables test for IPv6 externalTrafficPolicy=Local E/W for IPsec.

Signed-off-by: Zhichuan Liang <gray.liang@isovalent.com>
@jschwinger233 jschwinger233 marked this pull request as ready for review March 24, 2023 13:05
@jschwinger233 jschwinger233 requested review from a team as code owners March 24, 2023 13:05
@julianwiedmann
Copy link
Member

/test

@pchaigno pchaigno added area/encryption Impacts encryption support such as IPSec, WireGuard, or kTLS. feature/ipv6 Relates to IPv6 protocol support labels Mar 24, 2023
Copy link
Member

@pchaigno pchaigno left a comment

Choose a reason for hiding this comment

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

Nice! Thanks for tracking and fixing this!

I see it's labeled as release-note/minor, but it feels more like a release-note/bug, no? The release note should also describe the user impact. It currently describes the change itself.
Finally, I think we should figure out where we need to backport.

pkg/datapath/iptables/iptables.go Show resolved Hide resolved
@jschwinger233 jschwinger233 added release-note/bug This PR fixes an issue in a previous release of Cilium. and removed release-note/minor This PR changes functionality that users may find relevant to operating Cilium. labels Mar 27, 2023
@jschwinger233
Copy link
Member Author

jschwinger233 commented Mar 27, 2023

I think we should figure out where we need to backport.

I noticed there is no PR labelled with backport/1.10 and earlier versions, so I add needs-backport/1.11 for this, if it's not appropriate please let me know.

@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.11.16 Mar 27, 2023
Copy link
Member

@brb brb left a comment

Choose a reason for hiding this comment

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

🚀

@julianwiedmann
Copy link
Member

[word-smith'ed the release-note. Also adding backport labels for 1.12 and 1.13]

@julianwiedmann julianwiedmann added sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. needs-backport/1.12 labels Mar 27, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.13.2 Mar 27, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.12.9 Mar 27, 2023
@pchaigno pchaigno merged commit dd0a8f5 into cilium:master Mar 27, 2023
@jschwinger233
Copy link
Member Author

Backport PR will be taken care of by renovate bot, right? Is there anything I should follow up?

@jschwinger233 jschwinger233 deleted the gray/fix-ipsec-eTP branch March 27, 2023 10:48
@tklauser tklauser mentioned this pull request Mar 28, 2023
5 tasks
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.11 in 1.11.16 Mar 28, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.11 in 1.11.16 Mar 28, 2023
@tklauser tklauser mentioned this pull request Mar 28, 2023
6 tasks
@tklauser tklauser mentioned this pull request Mar 28, 2023
9 tasks
@tklauser tklauser added backport-pending/1.13 The backport for Cilium 1.13.x for this PR is in progress. and removed needs-backport/1.13 labels Mar 28, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.13 in 1.13.2 Mar 28, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.13 in 1.13.2 Mar 28, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.12 in 1.12.9 Mar 28, 2023
@tklauser tklauser added backport-done/1.12 The backport for Cilium 1.12.x for this PR is done. and removed backport-pending/1.12 labels Mar 29, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.12 to Backport done to v1.12 in 1.12.9 Mar 29, 2023
@tklauser tklauser added backport-done/1.11 The backport for Cilium 1.11.x for this PR is done. and removed backport-pending/1.11 labels Mar 29, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.11 to Backport done to v1.11 in 1.11.16 Mar 29, 2023
@julianwiedmann julianwiedmann added backport-done/1.13 The backport for Cilium 1.13.x for this PR is done. and removed backport-pending/1.13 The backport for Cilium 1.13.x for this PR is in progress. labels Apr 14, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.13 to Backport done to v1.13 in 1.13.2 Apr 14, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.13 to Backport done to v1.13 in 1.13.2 Apr 14, 2023
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. backport-done/1.11 The backport for Cilium 1.11.x for this PR is done. backport-done/1.12 The backport for Cilium 1.12.x for this PR is done. backport-done/1.13 The backport for Cilium 1.13.x for this PR is done. feature/ipv6 Relates to IPv6 protocol support release-note/bug This PR fixes an issue in a previous release of Cilium. sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages.
Projects
No open projects
1.11.16
Backport done to v1.11
1.12.9
Backport done to v1.12
1.13.2
Backport done to v1.13
Development

Successfully merging this pull request may close these issues.

ipsec: IPv6 externalTrafficPolicy=Local E/W is broken when accessed from non-backend node
5 participants