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.

Sign up for GitHub

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

Jump to bottom

bpf: minor improvements to XDP punt with XFER_PKT_NO_SVC #23106

Merged
merged 2 commits into from Jan 19, 2023
Merged

bpf: minor improvements to XDP punt with XFER_PKT_NO_SVC #23106

merged 2 commits into from Jan 19, 2023

Conversation

julianwiedmann
Copy link
Member

@julianwiedmann julianwiedmann commented Jan 16, 2023 •
edited

Just two small improvements to the XFER_PKT_NO_SVC processing:

  1. move its processing into the from-netdev code block that deals with punted packets,
  2. add a missing XFER_PKT_NO_SVC for non-SVC traffic

@julianwiedmann
bpf: nodeport: simplify XFER_PKT_NO_SVC transfer
488f3c5 
When XDP punts up a packet with XFER_PKT_NO_SVC set, let the from-netdev TC
program transfer this into the skb representation right away. This way we
don't have to special-case it later.

Clean up the definition of vlan_id in from-netdev while at it.

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
@julianwiedmann
bpf: nodeport: also set XFER_PKT_NO_SVC for packets with non-SVC prot…
75c67d5 
…ocol

When bailing out from the nodeport code because the packet doesn't have a
relevant L4 protocol (ie. ICMP/SCTP/TCP/UDP), set the XFER_PKT_NO_SVC flag.
If the check happened in XDP, doing so allows the subsequent TC program to
skip all nodeport processing.

This is mostly for consistency, there's not a lot of performance gains to
be had for such packets.

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
@julianwiedmann julianwiedmann added sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. release-note/misc This PR makes changes that have no direct user impact. labels Jan 16, 2023
@julianwiedmann julianwiedmann requested a review from a team as a code owner January 16, 2023 10:57
@julianwiedmann
Copy link
Member Author

/test

sahid
sahid approved these changes Jan 16, 2023
View reviewed changes
Copy link
Member

@sahid sahid left a comment

Choose a reason for hiding this comment

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

Sounds good thank you!

bpf/bpf_host.c
@@ -1040,16 +1037,23 @@ handle_srv6(struct __ctx_buff *ctx)
__section("from-netdev")
int cil_from_netdev(struct __ctx_buff *ctx)
{
__u32 __maybe_unused vlan_id;
Copy link
Member

Choose a reason for hiding this comment

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

nit: I would have done this change in a different commit right?

@julianwiedmann
Copy link
Member Author

Reviews are in, CI is green except for the bpf-next fails in ci-datapath that #23171 tracks.

📋 Test Report
❌ 5/31 tests failed (12/237 actions), 0 tests skipped, 0 scenarios skipped:
Test [no-policies]:
❌ no-policies/pod-to-world/http-to-one.one.one.one-0: cilium-test/client-755fb678bd-kncrl (10.244.3.107) -> one.one.one.one-http (one.one.one.one:80)
connectivity test failed: 5 tests failed
❌ no-policies/pod-to-world/https-to-one.one.one.one-0: cilium-test/client-755fb678bd-kncrl (10.244.3.107) -> one.one.one.one-https (one.one.one.one:443)
❌ no-policies/pod-to-world/https-to-one.one.one.one-index-0: cilium-test/client-755fb678bd-kncrl (10.244.3.107) -> one.one.one.one-https-index (one.one.one.one:443)
❌ no-policies/pod-to-world/http-to-one.one.one.one-1: cilium-test/client2-5b97d7bc66-6wmtt (10.244.3.80) -> one.one.one.one-http (one.one.one.one:80)
❌ no-policies/pod-to-world/https-to-one.one.one.one-1: cilium-test/client2-5b97d7bc66-6wmtt (10.244.3.80) -> one.one.one.one-https (one.one.one.one:443)
❌ no-policies/pod-to-world/https-to-one.one.one.one-index-1: cilium-test/client2-5b97d7bc66-6wmtt (10.244.3.80) -> one.one.one.one-https-index (one.one.one.one:443)
Test [to-entities-world]:
❌ to-entities-world/pod-to-world/http-to-one.one.one.one-0: cilium-test/client-755fb678bd-kncrl (10.244.3.107) -> one.one.one.one-http (one.one.one.one:80)
❌ to-entities-world/pod-to-world/http-to-one.one.one.one-1: cilium-test/client2-5b97d7bc66-6wmtt (10.244.3.80) -> one.one.one.one-http (one.one.one.one:80)
Test [client-egress-l7]:
❌ client-egress-l7/pod-to-world/http-to-one.one.one.one-1: cilium-test/client2-5b97d7bc66-6wmtt (10.244.3.80) -> one.one.one.one-http (one.one.one.one:80)
Test [client-egress-l7-named-port]:
❌ client-egress-l7-named-port/pod-to-world/http-to-one.one.one.one-0: cilium-test/client2-5b97d7bc66-6wmtt (10.244.3.80) -> one.one.one.one-http (one.one.one.one:80)
Test [to-fqdns]:
❌ to-fqdns/pod-to-world/http-to-one.one.one.one-0: cilium-test/client-755fb678bd-kncrl (10.244.3.107) -> one.one.one.one-http (one.one.one.one:80)
❌ to-fqdns/pod-to-world/http-to-one.one.one.one-1: cilium-test/client2-5b97d7bc66-6wmtt (10.244.3.80) -> one.one.one.one-http (one.one.one.one:80)
Error: Process completed with exit code 1.

@julianwiedmann julianwiedmann added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jan 19, 2023
@ldelossa ldelossa merged commit 080798f into cilium:master Jan 19, 2023
@julianwiedmann julianwiedmann deleted the bpf-nodeport-xfer-no-svc branch January 19, 2023 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sahid sahid sahid approved these changes

@aspsk aspsk aspsk approved these changes

@borkmann borkmann Awaiting requested review from borkmann

Assignees
No one assigned
Labels
ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact. sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@julianwiedmann @sahid @aspsk @ldelossa