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

bpf: nodeport: remove lb4_populate_ports() #25063

Merged
merged 1 commit into from Apr 24, 2023

Conversation

julianwiedmann
Copy link
Member

lb4_extract_tuple() already extracts the full tuple, there's no need to extract the ports again. On top it also deals with L4 ports for fragmented packets. We just need to adjust __snat_v6_has_v4_complete() so that it load the ports from the right locations.

We can also trust that the L4 proto type has been checked by lb4_extract_tuple(), and thus only need to special-case ICMP.

lb4_extract_tuple() already extracts the full tuple, there's no need to
extract the ports again. On top it also deals with L4 ports for fragmented
packets. We just need to adjust __snat_v6_has_v4_complete() so that it
load the ports from the right locations.

We can also trust that the L4 proto type has been checked by
lb4_extract_tuple(), and thus only need to special-case ICMP.

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 Apr 22, 2023
@julianwiedmann julianwiedmann requested a review from a team as a code owner April 22, 2023 17:22
@julianwiedmann
Copy link
Member Author

julianwiedmann commented Apr 22, 2023

/test

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

Click to show.

Test Name

K8sAgentPolicyTest Multi-node policy test validates ingress CIDR-dependent L4 With host policy Connectivity is restored after importing ingress policy

Failure Output

FAIL: Timed out after 1.000s.

Jenkins URL: https://jenkins.cilium.io/job/Cilium-PR-K8s-1.27-kernel-net-next/88/

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

@julianwiedmann
Copy link
Member Author

julianwiedmann commented Apr 24, 2023

/mlh new-flake Cilium-PR-K8s-1.27-kernel-net-next

👍 created #25074

@julianwiedmann
Copy link
Member Author

/test-1.27-net-next

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Apr 24, 2023
@julianwiedmann julianwiedmann merged commit 29dafd8 into cilium:main Apr 24, 2023
57 checks passed
@julianwiedmann julianwiedmann deleted the 1.14-nodeport-l4-ports branch April 24, 2023 10:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

None yet

2 participants