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

Revert and fix ip rules #25350

Merged
merged 17 commits into from May 23, 2023
Merged

Revert and fix ip rules #25350

merged 17 commits into from May 23, 2023

Conversation

NikAleksandrov
Copy link

@NikAleksandrov NikAleksandrov commented May 10, 2023 •
edited

This set is a second attempt at reverting the revert PR #24756, which was reverting #24288 and #24577

The upgrade problem mentioned in PR #24756 was due to wrongfully assuming that DeleteRule()
was handling both IPv4 and IPv6. Turns out there was a specific DeleteRuleIPv6() function
which seems unnecessary, so there is 1 additional patch prior to the last revert which changes DeleteRule() and adds
a new family argument, then removes DeleteRuleIPv6(). The problem manifested only when both IPv4 and IPv6 were
enabled due to the IPv6 rule deletion which was actually removing the IPv4 rule.

Note I have removed the "Revert revert" in the patch titles to make checkpatch happy.

The upgrade was tested as:

$ helm install cilium install/cilium/cilium --version 1.13.2 --namespace kube-system --set ipv6.enabled=true

<make sure ip -d rule show has "proto unspec">

$ helm upgrade cilium install/kubernetes/cilium  --set image.override=192.168.122.1:5000/cilium/cilium-dev:latest --set ipv6.enabled=true --namespace kube-system
# upgrade to a local registry with patched cilium image

<make sure ip -d rule show now has "proto kernel" and there is connectivity>

@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 May 10, 2023
@NikAleksandrov NikAleksandrov self-assigned this May 10, 2023
@NikAleksandrov NikAleksandrov force-pushed the revert-and-fix-ip-rules branch 5 times, most recently from f2739c2 to 348bba8 Compare May 10, 2023 10:09
pengbinbin1 and others added 17 commits May 10, 2023 13:09
@pengbinbin1 @NikAleksandrov
fix the comments about Delete
b1517dc 
The endpoint's IP is not released int the function Delete, it is release
in the function EndpointDeleted. This comments will confused the code
reader.

Signed-off-by: pengbinbin1 <pengbiny@163.com>
@NikAleksandrov
vendor: update vishvananda/netlink/
1da125c 
Update vishvananda/netlink/ so we can make use of fib rule
RTA_PROTOCOL attribute.

Signed-off-by: Nikolay Aleksandrov <nikolay@isovalent.com>
@NikAleksandrov
datapath/linux/route: fix CI expectations for rule string format
13dd701 
This reverts commit 53fef54.

Signed-off-by: Nikolay Aleksandrov <nikolay@isovalent.com>
@NikAleksandrov
linux_defaults: add default rt protocol to use for fib rules and routes
279da4f 
This reverts commit 368ec8e.

Signed-off-by: Nikolay Aleksandrov <nikolay@isovalent.com>
@NikAleksandrov
linux_defaults: add local lookup priority to use for fib rules
4024d85 
This reverts commit ed5114d.

Signed-off-by: Nikolay Aleksandrov <nikolay@isovalent.com>
@NikAleksandrov
init.sh: install ip rules and routes with proto kernel
1618f63 
This reverts commit dbce5f1.

Signed-off-by: Nikolay Aleksandrov <nikolay@isovalent.com>
@NikAleksandrov
init.sh: take local lookup rule priority as an argument
24dc4e4 
This reverts commit 9e62a84.

Signed-off-by: Nikolay Aleksandrov <nikolay@isovalent.com>
@NikAleksandrov
datapath/linux/route: add support for rule protocol
21a44da 
This reverts commit 5fb791d.

Signed-off-by: Nikolay Aleksandrov <nikolay@isovalent.com>
@NikAleksandrov
datapath/linux/route: use proto kernel when installing routes
96380d3 
This reverts commit 0f3e989.

Signed-off-by: Nikolay Aleksandrov <nikolay@isovalent.com>
@NikAleksandrov
egressgw: use proto kernel for fib routes and rules
6ceb86d 
This reverts commit 3271cb2.

Signed-off-by: Nikolay Aleksandrov <nikolay@isovalent.com>
@NikAleksandrov
datapath/loader: use proto kernel for ENI fib rules and routes
b47af02 
This reverts commit afdc51f.

Signed-off-by: Nikolay Aleksandrov <nikolay@isovalent.com>
@NikAleksandrov
datapath/linux/routing: use proto kernel for fib routes and rules
76cd5db 
This reverts commit 9b5e74b.

Signed-off-by: Nikolay Aleksandrov <nikolay@isovalent.com>
@NikAleksandrov
datapath/linux/node: use proto kernel for fib rules and routes
5febf4a 
This reverts commit 2b6d5c4.

Signed-off-by: Nikolay Aleksandrov <nikolay@isovalent.com>
@NikAleksandrov
datapath: remove RouteProtocolIPSec and use proto kernel
0b3ad81 
This reverts commit 05593ee.

Signed-off-by: Nikolay Aleksandrov <nikolay@isovalent.com>
@NikAleksandrov
bugtool: dump fib rule protocol
4fea039 
This reverts commit a9cad19.

Signed-off-by: Nikolay Aleksandrov <nikolay@isovalent.com>
@NikAleksandrov
datapath/linux/route: DeleteRule takes family as an argument
dfeb732 
Remove the IPv6-specific DeleteRuleIPv6() and add a family argument to
DeleteRule so we can use the same function for both families.

Signed-off-by: Nikolay Aleksandrov <nikolay@isovalent.com>
@NikAleksandrov
datapath/linux: make sure we have a local rule with proto kernel
5ae2df5 
This reverts commit 9d60341.

Signed-off-by: Nikolay Aleksandrov <nikolay@isovalent.com>
@NikAleksandrov NikAleksandrov marked this pull request as ready for review May 10, 2023 10:09
@NikAleksandrov NikAleksandrov requested a review from a team as a code owner May 10, 2023 10:09
@michi-covalent michi-covalent removed this from Needs backport from main in 1.13.7 Sep 9, 2023
@jrajahalme jrajahalme added this to Needs backport from main in 1.13.9 Oct 17, 2023
@jrajahalme jrajahalme removed this from Needs backport from main in 1.13.8 Oct 17, 2023
@jrajahalme jrajahalme removed this from Needs backport from main in 1.13.3 Oct 17, 2023
@jrajahalme
Copy link
Member

Removing release blocker 1.14 as this is already in v1.14: a63aabc

@jrajahalme jrajahalme removed the release-blocker/1.14 This issue will prevent the release of the next version of Cilium. label Oct 17, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from main in 1.13.3 Oct 17, 2023
@nathanjsweet nathanjsweet removed this from Needs backport from main in 1.13.9 Nov 12, 2023
@nathanjsweet nathanjsweet added this to Needs backport from main in 1.13.10 Nov 12, 2023
@nathanjsweet nathanjsweet removed this from Needs backport from main in 1.13.3 Nov 12, 2023
@nebril nebril added this to Needs backport from main in 1.13.11 Dec 11, 2023
@nebril nebril removed this from Needs backport from main in 1.13.10 Dec 11, 2023
@gentoo-root gentoo-root added this to Needs backport from main in 1.13.12 Jan 17, 2024
@gentoo-root gentoo-root removed this from Needs backport from main in 1.13.11 Jan 17, 2024
@julianwiedmann julianwiedmann added affects/v1.13 This issue affects v1.13 branch and removed needs-backport/1.13 This PR / issue needs backporting to the v1.13 branch labels Feb 10, 2024
@julianwiedmann
Copy link
Member

I dropped the PR from the 1.13 backport queue for now, let's not set wrong expectations.

@michi-covalent michi-covalent added this to Needs backport from main in 1.13.13 Feb 13, 2024
@michi-covalent michi-covalent removed this from Needs backport from main in 1.13.12 Feb 13, 2024
@thorn3r thorn3r removed this from Needs backport from main in 1.13.13 Mar 13, 2024
@thorn3r thorn3r added this to Needs backport from main in 1.13.13 Mar 13, 2024
@thorn3r thorn3r added this to Needs backport from main in 1.13.14 Mar 13, 2024
@thorn3r thorn3r removed this from Needs backport from main in 1.13.13 Mar 13, 2024
@thorn3r thorn3r added this to Needs backport from main in 1.13.15 Mar 26, 2024
@thorn3r thorn3r removed this from Needs backport from main in 1.13.14 Mar 26, 2024
@asauber asauber added this to Needs backport from main in 1.13.16 Apr 11, 2024
@asauber asauber removed this from Needs backport from main in 1.13.15 Apr 11, 2024
@nebril nebril added this to Needs backport from main in 1.13.17 May 10, 2024
@nebril nebril removed this from Needs backport from main in 1.13.16 May 10, 2024
@nebril nebril removed this from Needs backport from main in 1.13.17 May 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brb brb brb approved these changes

@borkmann borkmann borkmann approved these changes

@asauber asauber asauber approved these changes

@aanm aanm aanm approved these changes

@YutaroHayakawa YutaroHayakawa YutaroHayakawa approved these changes

@pchaigno pchaigno Awaiting requested review from pchaigno

Assignees

@NikAleksandrov NikAleksandrov

Labels
affects/v1.13 This issue affects v1.13 branch backport/author The backport will be carried out by the author of the PR. 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