Skip to content

egressgw: set ifindex for egress interface in IPv6 policy entry #38961

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
Closed
wants to merge 2 commits into from
Closed

egressgw: set ifindex for egress interface in IPv6 policy entry #38961

wants to merge 2 commits into from

Conversation

julianwiedmann
Copy link
Member

@julianwiedmann julianwiedmann commented Apr 15, 2025 •
edited
Loading

While we can still touch the IPv6 policy entry format without worrying about compatibility, take the opportunity to add support for pushing down the ifindex of the egress interface (#36151).

This avoids the usability annoyance of adding custom routes on the gateway node, as described in #30488.

@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 Apr 15, 2025
@julianwiedmann julianwiedmann added release-note/minor This PR changes functionality that users may find relevant to operating Cilium. feature/egress-gateway Impacts the egress IP gateway feature. labels Apr 15, 2025
@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 Apr 15, 2025
@julianwiedmann julianwiedmann added release-note/misc This PR makes changes that have no direct user impact. dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. and removed release-note/minor This PR changes functionality that users may find relevant to operating Cilium. labels Apr 15, 2025
@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 Apr 15, 2025
@julianwiedmann julianwiedmann force-pushed the 1.18-egressgw-ifindex branch from 17c3535 to f60e8df Compare April 15, 2025 12:55
rgo3
rgo3 reviewed Apr 15, 2025
View reviewed changes
@github-actions GitHub Actions
Copy link

This pull request has been automatically marked as stale because it
has not had recent activity. It will be closed if no further activity
occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label May 16, 2025
@julianwiedmann julianwiedmann removed the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label May 16, 2025
@julianwiedmann julianwiedmann force-pushed the 1.18-egressgw-ifindex branch from f60e8df to a670b80 Compare May 20, 2025 08:06
@julianwiedmann
Copy link
Member Author

/test

@julianwiedmann julianwiedmann marked this pull request as ready for review May 20, 2025 09:47
@julianwiedmann julianwiedmann requested review from a team as code owners May 20, 2025 09:47
@julianwiedmann julianwiedmann requested a review from ysksuzuki May 20, 2025 09:47
@julianwiedmann
Copy link
Member Author

(it's too late to do this for IPv4 unfortunately, without adding a new map entry format. I'll add a follow-up issue in case anyone is interested in implementing those bits.)

@julianwiedmann julianwiedmann enabled auto-merge May 20, 2025 09:49
@julianwiedmann julianwiedmann added the feature/ipv6 Relates to IPv6 protocol support label May 20, 2025
@julianwiedmann julianwiedmann force-pushed the 1.18-egressgw-ifindex branch from a670b80 to 9bb9095 Compare May 29, 2025 05:10
@julianwiedmann
Copy link
Member Author

/test

@julianwiedmann julianwiedmann mentioned this pull request May 29, 2025
Merged
@ysksuzuki
Copy link
Member

Looks like the changes ran into the complexity issue. Holding off the review for now

    verifier_test.go:245: Error: program tail_handle_snat_fwd_ipv6: load program: argument list too long: BPF program is too large. Processed 1000001 insn (1166 line(s) omitted)
        Verifier error tail: load program: argument list too long:
        	(1157 line(s) omitted)
        	1902: (b4) w4 = 1                     ; R4_w=1
        	1903: (85) call bpf_map_update_elem#2         ; R0=scalar()
        	1904: (b4) w1 = 6                     ; R1_w=6
        	; if (__snat_create(&cilium_snat_v6_external, &rtuple, &rstate) == 0)
        	1905: (16) if w0 == 0x0 goto pc+512   ; R0=scalar()
        	; __u32 m = (__u32)(val) * n;
        	1906: (04) w6 += 1                    ; R6_w=scalar(umin=32769,umax=98304,var_off=(0x0; 0x1ffff))
        	1907: (54) w6 &= 65535
        	BPF program is too large. Processed 1000001 insn
        	processed 1000001 insns (limit 1000000) max_states_per_insn 45 total_states 49280 peak_states 2010 mark_read 66

@julianwiedmann
egressgw: datapath support for ifindex in IPv6 policy entry
131c8cc 
Enable the EGW datapath on the gateway node to obtain the ifindex of the
egress interface from the EGW policy entry, instead of needing a FIB
lookup. We can make this change for IPv6 as the policy entry format is not
fix yet - IPv4 will require a v2 of the map.

No change in behavior, as the agent still pushes down 0 as the ifindex
value (see the change in updateEgressRules6()). The needed controlplane
changes follow in a subsequent patch.

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
@julianwiedmann
egressgw: controlplane support for egress ifindex in IPv6 policy entry
66e5f88 
For CEGPs that select the egress interface by name (`interface` parameter)
or by default-route, also push the interface ifindex down into the
datapath. This avoids custom routing setup for forwarding EGW traffic to
the correct network interface.

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
@julianwiedmann julianwiedmann force-pushed the 1.18-egressgw-ifindex branch from 9bb9095 to 66e5f88 Compare May 30, 2025 04:25
@julianwiedmann julianwiedmann marked this pull request as draft May 30, 2025 04:27
auto-merge was automatically disabled May 30, 2025 04:27

Pull request was converted to draft

@joestringer joestringer added the dont-merge/wait-until-release Freeze window for current release is blocking non-bugfix PRs label Jun 20, 2025
@joestringer joestringer removed the dont-merge/wait-until-release Freeze window for current release is blocking non-bugfix PRs label Jul 1, 2025
@github-actions GitHub Actions
Copy link

github-actions bot commented Aug 1, 2025

This pull request has been automatically marked as stale because it
has not had recent activity. It will be closed if no further activity
occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label Aug 1, 2025
@github-actions GitHub Actions
Copy link

This pull request has not seen any activity since it was marked stale.
Closing.

@github-actions github-actions bot closed this Aug 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rgo3 rgo3 rgo3 left review comments

@ysksuzuki ysksuzuki Awaiting requested review from ysksuzuki ysksuzuki is a code owner automatically assigned from cilium/egress-gateway

Assignees
No one assigned
Labels
feature/egress-gateway Impacts the egress IP gateway feature. feature/ipv6 Relates to IPv6 protocol support release-note/misc This PR makes changes that have no direct user impact. stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@julianwiedmann @ysksuzuki @rgo3 @joestringer