Skip to content

Support XDP Nodeport & Tunnel config#21364

Merged
ti-mo merged 5 commits intocilium:masterfrom
julianwiedmann:xdp-tunnel
Oct 5, 2022
Merged

Support XDP Nodeport & Tunnel config#21364
ti-mo merged 5 commits intocilium:masterfrom
julianwiedmann:xdp-tunnel

Conversation

@julianwiedmann
Copy link
Copy Markdown
Member

@julianwiedmann julianwiedmann commented Sep 20, 2022 •
edited
Loading

[follow-on to the recent EgressGW + XDP work]

In order to support XDP Nodeport acceleration in a tunnel config, the XDP Nodeport code needs to encap packets before forwarding them to a backend on a different node. As we currently can't bpf_redirect() to cilium_vxlan / cilium_geneve from within XDP, the datapath supports punting such packets from XDP to TC-Ingress (and then handling the redirect from there).

This patchset converts the existing TUNNEL_MODE sections in lib/nodeport.h to support such punting from XDP to TC-Ingress. Performance won't be as good as pure-XDP.

XDP NodePort Acceleration can also be used for clusters in tunnel mode.

@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 Sep 20, 2022
@julianwiedmann julianwiedmann force-pushed the xdp-tunnel branch 2 times, most recently from b982d32 to 8485461 Compare September 20, 2022 08:09
@julianwiedmann
Copy link
Copy Markdown
Member Author

julianwiedmann commented Sep 20, 2022

/test

@julianwiedmann
Copy link
Copy Markdown
Member Author

julianwiedmann commented Sep 20, 2022

/test

@julianwiedmann
Copy link
Copy Markdown
Member Author

julianwiedmann commented Sep 20, 2022

/test-1.25-net-next

@julianwiedmann julianwiedmann added area/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. release-note/minor This PR changes functionality that users may find relevant to operating Cilium. labels Sep 20, 2022
@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 Sep 20, 2022
@julianwiedmann julianwiedmann changed the title WIP: XDP tunnel Support XDP Nodeport & Tunnel config Sep 20, 2022
@julianwiedmann julianwiedmann marked this pull request as ready for review September 20, 2022 16:28
@julianwiedmann julianwiedmann requested review from a team as code owners September 20, 2022 16:28
qmonnet
qmonnet approved these changes Sep 29, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@qmonnet qmonnet left a comment

Choose a reason for hiding this comment

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

Looks good to me, thank you!

aspsk
aspsk approved these changes Oct 3, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@aspsk aspsk left a comment

Choose a reason for hiding this comment

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

LGTM

@julianwiedmann julianwiedmann force-pushed the xdp-tunnel branch 2 times, most recently from 54cae6f to 7f2aad7 Compare October 3, 2022 11:40
@julianwiedmann
Copy link
Copy Markdown
Member Author

julianwiedmann commented Oct 3, 2022

Needed a rebase to resolve a small conflict in daemon/cmd/kube_proxy_replacement.go, and picked up the pending review comments.

@julianwiedmann
Copy link
Copy Markdown
Member Author

julianwiedmann commented Oct 3, 2022

/test

@julianwiedmann
Copy link
Copy Markdown
Member Author

julianwiedmann commented Oct 3, 2022 •
edited by maintainer-s-little-helper bot
Loading

/test

Job 'Cilium-PR-K8s-1.25-kernel-net-next' failed:

Click to show.

Test Name

K8sAgentPolicyTest Multi-node policy test with L7 policy using connectivity-check to check datapath

Failure Output

FAIL: cannot install connectivity-check

If it is a flake and a GitHub issue doesn't already exist to track it, comment /mlh new-flake Cilium-PR-K8s-1.25-kernel-net-next so I can create one.

@julianwiedmann
bpf: nodeport: prepare for SNAT_DONE handling in XDP context
0599eba 
Generalize the SNAT_DONE helpers, so that they can also be used from XDP
context. No change in behaviour for now, the actual XDP implementation
comes in a subsequent patch.

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
@julianwiedmann
bpf: nodeport: track & transfer SNAT_DONE flag from XDP context
180f282 
The comment in XDP's ctx_snat_done_set() is no longer accurate. While a
packet doesn't pass through any Egress hook in XDP, it might be punted up
to TC-Ingress and later pass through such a hook as SKB. So keep track of
whether XDP has done the SNAT processing, and transfer the flag to a punted
skb.

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
@julianwiedmann
bpf: nodeport: support XDP tunnel punting for egress traffic
76eba7a 
When running in a cluster config with tunneling, the nodeport code forwards
traffic to a backend on another node by first calling __encap_with_nodeid()
and then redirecting the packet to the tunnel interface.

To enable this for XDP Acceleration, we need to support that
__encap_with_nodeid() can also return CTX_ACT_OK. In this case the packet
gets punted up to TC-Ingress of the physical interface, where the usual
tunnel processing happens.

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
@julianwiedmann
datapath, test: allow to combine XDP Nodeport Acceleration with Tunne…
8d72fb7 
…ling

The Nodeport code now supports egressing into a tunnel interface when in
XDP context (although at lower performance, due to the punt to TC-Ingress).

As the TUNNEL_MODE macro is only set up by init.sh, shuffle building the
XDP program around.

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
@julianwiedmann
bpf: nodeport: clean up naming for tail_nodeport_nat_ipv6_egress()
c39254a 
Keep it consistent with the other tail_nodeport_nat_*() functions.

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
@julianwiedmann
Copy link
Copy Markdown
Member Author

julianwiedmann commented Oct 4, 2022

/test

@julianwiedmann
Copy link
Copy Markdown
Member Author

julianwiedmann commented Oct 4, 2022

/test-1.25-net-next

@julianwiedmann julianwiedmann added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Oct 4, 2022
@ti-mo ti-mo merged commit d522921 into cilium:master Oct 5, 2022
@julianwiedmann julianwiedmann deleted the xdp-tunnel branch December 13, 2022 13:36
@julianwiedmann julianwiedmann mentioned this pull request Dec 19, 2022
Closed
@ysksuzuki ysksuzuki mentioned this pull request Jan 6, 2023
Closed
@julianwiedmann julianwiedmann mentioned this pull request May 17, 2023
Merged
@smagnani96 smagnani96 mentioned this pull request May 6, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@qmonnet qmonnet qmonnet approved these changes

@aspsk aspsk aspsk approved these changes

@nbusseneau nbusseneau nbusseneau approved these changes

@dylandreimerink dylandreimerink Awaiting requested review from dylandreimerink dylandreimerink was automatically assigned from cilium/loader

+1 more reviewer

@jibi jibi jibi approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/minor This PR changes functionality that users may find relevant to operating Cilium.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@julianwiedmann @jibi @aspsk @nbusseneau @qmonnet @ti-mo