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

EgressGW cleanups #21719

Merged
merged 3 commits into from
Oct 28, 2022
Merged

EgressGW cleanups #21719

merged 3 commits into from
Oct 28, 2022

Conversation

julianwiedmann
Copy link
Member

@julianwiedmann julianwiedmann commented Oct 13, 2022

This pulls various pieces of EgressGW code into egress_policies.h. We also clean up the unused dependency on TUNNEL_MAP.

Fixes: #19785

Egress Gateway: move code into its own header file, and remove the dependency on TUNNEL_MAP.

@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. feature/egress-gateway Impacts the egress IP gateway feature. labels Oct 13, 2022
@julianwiedmann
Copy link
Member Author

/test

@julianwiedmann
Copy link
Member Author

/test-1.25-net-next

@julianwiedmann
Copy link
Member Author

/test

@julianwiedmann
Copy link
Member Author

CI failures in Check connectivity with transparent encryption and direct routing are unrelated, and a known issue.

@julianwiedmann julianwiedmann marked this pull request as ready for review October 14, 2022 06:53
@julianwiedmann julianwiedmann requested a review from a team as a code owner October 14, 2022 06:53
bpf/bpf_lxc.c Show resolved Hide resolved
bpf/lib/nat.h Outdated Show resolved Hide resolved
bpf/lib/egress_policies.h Outdated Show resolved Hide resolved
Move most of the Egress Gateway-specific code into its header file, so that
readers don't have to concern themselves with all the details.

Fixes: cilium#19785
Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
encap_and_redirect_with_nodeid() doesn't do any tunnel lookup. Consequently
it also doesn't return DROP_NO_TUNNEL_ENDPOINT.

None of the callers is trying to handle such an error either, so this was
presumably just a copy&paste typo.

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
In a direct-routing config, we don't actually require the TUNNEL_MAP
for EgressGW.

In the from-container path, handle_ipv4_from_lxc() gets the tunnel_endpoint
from the EgressGW policy (and can trust that it's != 0). So extract an
optimized __encap_and_redirect_lxc() that doesn't depend on TUNNEL_MAP.

In the reply path, rev_nodeport_lb4() queries the IPCache to
obtain the source node of an EgressGW connection. If that fails, there's
no further fallback to the TUNNEL_MAP.

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

julianwiedmann commented Oct 21, 2022

/test

Job 'Cilium-PR-K8s-1.23-kernel-5.4' failed:

Click to show.

Test Name

K8sDatapathConfig Host firewall With VXLAN

Failure Output

FAIL: Failed to reach 10.0.0.120:80 from testclient-host-prq6b

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

@julianwiedmann
Copy link
Member Author

/test-1.23-5.4

@julianwiedmann
Copy link
Member Author

Travis fail is #21730.

@julianwiedmann
Copy link
Member Author

@jibi - as discussed offline. Slightly changed the helper layout, so that they focus on wrapping the call to lookup_ip4_egress_gw_policy() and the relevant result processing.

Copy link
Member

@jibi jibi 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, thanks! Just a couple more nits that can be addressed when we touch again this logic

bpf/bpf_lxc.c Show resolved Hide resolved
bpf/bpf_lxc.c Show resolved Hide resolved
@julianwiedmann julianwiedmann added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Oct 26, 2022
@nbusseneau nbusseneau merged commit 34e824d into cilium:master Oct 28, 2022
@julianwiedmann julianwiedmann deleted the egressgw-cleanups branch October 28, 2022 13:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/egress-gateway Impacts the egress IP gateway feature. 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.

egressgw: decouple egress gateway logic from rev_nodeport_lb4()
4 participants