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

Re-introduce 2005 route table to fix L7 proxy issues #29530

Merged
merged 4 commits into from Dec 1, 2023

Conversation

jschwinger233
Copy link
Member

@jschwinger233 jschwinger233 commented Dec 1, 2023

See commit messages for details.

Fixes an L7 proxy issue by re-introducing 2005 route table.

@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 Dec 1, 2023
@jschwinger233 jschwinger233 force-pushed the gray/fix-l7-proxy branch 2 times, most recently from a366380 to 2abe5a5 Compare December 1, 2023 09:11
jschwinger233 and others added 4 commits December 1, 2023 17:18
This is an alternative approach to fix cilium#21954, so that we
can re-introduce the 2005 from-proxy routing rule in following patches
to fix L7 proxy issues.

This commit simply allows packets to WORLD as long as they are from
ingress proxy. This was one of the solution suggested by Martynas, as
recorded in commit message cilium/cilium@c534bb7:

    One fix was to extend the troublesome check
    https://github.com/cilium/cilium/blob/v1.12.3/bpf/bpf_host.c#L626 by
    allowing proxy replies to `WORLD_ID`.

To tell if an skb is originated from ingress proxy, the commit extends
the semantic of existing flags `TC_INDEX_F_SKIP_{INGRESS,EGRESS}_PROXY`,
renames flags to clarify the changed meaning.

Fixes: cilium#21954 (Reply from pod to outside is dropped when L7 ingress policy is used)

Signed-off-by: Zhichuan Liang <gray.liang@isovalent.com>
This commit re-introduced the 2005 routes that were removed by
cilium@9dd6cfc (datapath: remove 2005 route table for ipv6 only)
and cilium@c1a0dba (datapath: remove 2005 route table for ipv4 only).

Signed-off-by: Robin Gögge <r.goegge@gmail.com>
Signed-off-by: Zhichuan Liang <gray.liang@isovalent.com>
Previously cilium#25440 removed
bpf_host's logic for host-to-remote-pod packets.

However, we recently realized such host-to-remote-pod traffic can also
be pod-to-pod traffic passing through L7 proxy. This commit made
bpf_host capable of handling these host-to-remote-pod packets as long as
they are originated from L7 proxy.

Fixes: cilium#25440

Suggested-by: Paul Chaignon <paul.chaignon@gmail.com>
Signed-off-by: Zhichuan Liang <gray.liang@isovalent.com>
Previously we set skb->mark in from_host@cilium_host, expect the mark
to remain unchanged after kernel transmits skb from cilium_host to
cilium_net. The skb->mark is for instance used to transport
IPsec-related information.

However, as of 2023-10-19, kernel 5.10 still misses the backport patch[1]
to fix a bug in skb_scrub_packet() which clears skb->mark for veth_xmit even if the
veth pair is under the same netns:
https://elixir.bootlin.com/linux/v5.10.198/source/include/linux/netdevice.h#L3975

To avoid hitting this issue, this patch sets metadata in skb->cb to
survive skb_scrub_packet(), then to_host@cilium_net can retrieve this
info and set proper mark.

Only from_host bpf is setting cb, while from_lxc bpf is still using mark.

[1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ff70202b2d1a ("dev_forward_skb: do not scrub skb mark within the same name space")

Signed-off-by: Zhichuan Liang <gray.liang@isovalent.com>
@jschwinger233
Copy link
Member Author

/test

@julianwiedmann julianwiedmann added kind/bug This is a bug in the Cilium logic. sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. area/proxy Impacts proxy components, including DNS, Kafka, Envoy and/or XDS servers. release-note/bug This PR fixes an issue in a previous release of Cilium. labels Dec 1, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot removed dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Dec 1, 2023
@julianwiedmann julianwiedmann added needs-backport/1.12 backport/author The backport will be carried out by the author of the PR. needs-backport/1.13 This PR / issue needs backporting to the v1.13 branch needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch labels Dec 1, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from main in 1.14.5 Dec 1, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from main in 1.13.10 Dec 1, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from main in 1.12.17 Dec 1, 2023
@julianwiedmann julianwiedmann added kind/regression This functionality worked fine before, but was broken in a newer release of Cilium. area/encryption Impacts encryption support such as IPSec, WireGuard, or kTLS. feature/ipsec Relates to Cilium's IPsec feature labels Dec 1, 2023
@julianwiedmann julianwiedmann marked this pull request as ready for review December 1, 2023 11:19
@julianwiedmann julianwiedmann requested review from a team as code owners December 1, 2023 11:19
sayboras added a commit that referenced this pull request Mar 17, 2024
This commit is to re-enable proxy rule installation in tunnel mode, as
route 2005 was added back, and we need this rule to handle the
hairpinning trafic in Ingress L7 proxy if the backend is on the same
node.

Relates: 0ebe516
Relates: #29530, #29864

Signed-off-by: Tam Mach <tam.mach@cilium.io>
sayboras added a commit that referenced this pull request Mar 18, 2024
This commit is to re-enable proxy rule installation in tunnel mode, as
route 2005 was added back, and we need this rule to handle the
hairpinning trafic in Ingress L7 proxy if the backend is on the same
node.

Relates: 0ebe516
Relates: #29530, #29864

Signed-off-by: Tam Mach <tam.mach@cilium.io>
sayboras added a commit that referenced this pull request Mar 27, 2024
This commit is to re-enable proxy rule installation in tunnel mode, as
route 2005 was added back, and we need this rule to handle the
hairpinning trafic in Ingress L7 proxy if the backend is on the same
node.

Relates: 0ebe516
Relates: #29530, #29864

Signed-off-by: Tam Mach <tam.mach@cilium.io>
sayboras added a commit that referenced this pull request Apr 6, 2024
This commit is to re-enable proxy rule installation in tunnel mode, as
route 2005 was added back, and we need this rule to handle the
hairpinning trafic in Ingress L7 proxy if the backend is on the same
node.

Relates: 0ebe516
Relates: #29530, #29864

Signed-off-by: Tam Mach <tam.mach@cilium.io>
sayboras added a commit that referenced this pull request Apr 7, 2024
This commit is to re-enable proxy rule installation in tunnel mode, as
route 2005 was added back, and we need this rule to handle the
hairpinning trafic in Ingress L7 proxy if the backend is on the same
node.

Relates: 0ebe516
Relates: #29530, #29864

Signed-off-by: Tam Mach <tam.mach@cilium.io>
sayboras added a commit that referenced this pull request Apr 9, 2024
This commit is to re-enable proxy rule installation in tunnel mode, as
route 2005 was added back, and we need this rule to handle the
hairpinning trafic in Ingress L7 proxy if the backend is on the same
node.

Relates: 0ebe516
Relates: #29530, #29864

Signed-off-by: Tam Mach <tam.mach@cilium.io>
sayboras added a commit that referenced this pull request Apr 10, 2024
This commit is to re-enable proxy rule installation in tunnel mode, as
route 2005 was added back, and we need this rule to handle the
hairpinning trafic in Ingress L7 proxy if the backend is on the same
node.

Relates: 0ebe516
Relates: #29530, #29864

Signed-off-by: Tam Mach <tam.mach@cilium.io>
sayboras added a commit that referenced this pull request Apr 11, 2024
This commit is to re-enable proxy rule installation in tunnel mode, as
route 2005 was added back, and we need this rule to handle the
hairpinning trafic in Ingress L7 proxy if the backend is on the same
node.

Relates: 0ebe516
Relates: #29530, #29864

Signed-off-by: Tam Mach <tam.mach@cilium.io>
sayboras added a commit that referenced this pull request Apr 11, 2024
This commit is to re-enable proxy rule installation in tunnel mode, as
route 2005 was added back, and we need this rule to handle the
hairpinning trafic in Ingress L7 proxy if the backend is on the same
node.

Relates: 0ebe516
Relates: #29530, #29864

Signed-off-by: Tam Mach <tam.mach@cilium.io>
sayboras added a commit that referenced this pull request Apr 16, 2024
This commit is to re-enable proxy rule installation in tunnel mode, as
route 2005 was added back, and we need this rule to handle the
hairpinning trafic in Ingress L7 proxy if the backend is on the same
node.

Relates: 0ebe516
Relates: #29530, #29864

Signed-off-by: Tam Mach <tam.mach@cilium.io>
sayboras added a commit that referenced this pull request Apr 23, 2024
This commit is to re-enable proxy rule installation in tunnel mode, as
route 2005 was added back, and we need this rule to handle the
hairpinning trafic in Ingress L7 proxy if the backend is on the same
node.

Relates: 0ebe516
Relates: #29530, #29864

Signed-off-by: Tam Mach <tam.mach@cilium.io>
sayboras added a commit that referenced this pull request Apr 29, 2024
This commit is to re-enable proxy rule installation in tunnel mode, as
route 2005 was added back, and we need this rule to handle the
hairpinning trafic in Ingress L7 proxy if the backend is on the same
node.

Relates: 0ebe516
Relates: #29530, #29864

Signed-off-by: Tam Mach <tam.mach@cilium.io>
sayboras added a commit that referenced this pull request May 5, 2024
This commit is to re-enable proxy rule installation in tunnel mode, as
route 2005 was added back, and we need this rule to handle the
hairpinning trafic in Ingress L7 proxy if the backend is on the same
node.

Relates: 0ebe516
Relates: #29530, #29864

Signed-off-by: Tam Mach <tam.mach@cilium.io>
sayboras added a commit that referenced this pull request May 6, 2024
This commit is to re-enable proxy rule installation in tunnel mode if
cilium envoy config is enabled. The reason is to handle the reply packet
of hair-pinning traffic in Ingress L7 proxy (i.e. backend is in the same
node).

Relates: 0ebe516
Relates: #29530, #29864

Signed-off-by: Tam Mach <tam.mach@cilium.io>
sayboras added a commit that referenced this pull request May 6, 2024
This commit is to re-enable proxy rule installation in tunnel mode if
cilium envoy config or ipsec is enabled. The reason is to handle the
reply packet of hair-pinning traffic in Ingress L7 proxy (i.e. backend
is in the same node).

Relates: 0ebe516
Relates: #29530, #29864

Signed-off-by: Tam Mach <tam.mach@cilium.io>
sayboras added a commit that referenced this pull request May 6, 2024
This commit is to re-enable proxy rule installation in tunnel mode if
cilium envoy config or ipsec is enabled. The reason is to handle the
reply packet of hair-pinning traffic in Ingress L7 proxy (i.e. backend
is in the same node).

Relates: 0ebe516
Relates: #29530, #29864

Signed-off-by: Tam Mach <tam.mach@cilium.io>
sayboras added a commit that referenced this pull request May 6, 2024
This commit is to re-enable proxy rule installation in tunnel mode if
cilium envoy config or ipsec is enabled. The reason is to handle the
reply packet of hair-pinning traffic in Ingress L7 proxy (i.e. backend
is in the same node).

Relates: 0ebe516
Relates: #29530, #29864

Signed-off-by: Tam Mach <tam.mach@cilium.io>
sayboras added a commit that referenced this pull request May 6, 2024
This commit is to re-enable proxy rule installation in tunnel mode if
cilium envoy config or ipsec is enabled. The reason is to handle the
reply packet of hair-pinning traffic in Ingress L7 proxy (i.e. backend
is in the same node).

Relates: 0ebe516
Relates: #29530, #29864

Signed-off-by: Tam Mach <tam.mach@cilium.io>
sayboras added a commit that referenced this pull request May 8, 2024
This commit is to re-enable proxy rule installation in tunnel mode if
cilium envoy config or ipsec is enabled. The reason is to handle the
reply packet of hair-pinning traffic in Ingress L7 proxy (i.e. backend
is in the same node).

Relates: 0ebe516
Relates: #29530, #29864

Signed-off-by: Tam Mach <tam.mach@cilium.io>
sayboras added a commit that referenced this pull request May 8, 2024
This commit is to re-enable proxy rule installation in native mode if
cilium envoy config or ipsec is enabled. The reason is to handle the
reply packet of hair-pinning traffic in Ingress L7 proxy (i.e. backend
is in the same node).

Relates: 0ebe516
Relates: #29530, #29864

Signed-off-by: Tam Mach <tam.mach@cilium.io>
github-merge-queue bot pushed a commit that referenced this pull request May 10, 2024
This commit is to re-enable proxy rule installation in native mode if
cilium envoy config or ipsec is enabled. The reason is to handle the
reply packet of hair-pinning traffic in Ingress L7 proxy (i.e. backend
is in the same node).

Relates: 0ebe516
Relates: #29530, #29864

Signed-off-by: Tam Mach <tam.mach@cilium.io>
sayboras added a commit that referenced this pull request May 11, 2024
[ upstream commit 32b2784 ]

This commit is to re-enable proxy rule installation in native mode if
cilium envoy config or ipsec is enabled. The reason is to handle the
reply packet of hair-pinning traffic in Ingress L7 proxy (i.e. backend
is in the same node).

Relates: 0ebe516
Relates: #29530, #29864

Signed-off-by: Tam Mach <tam.mach@cilium.io>
sayboras added a commit that referenced this pull request May 11, 2024
[ upstream commit 32b2784 ]

This commit is to re-enable proxy rule installation in native mode if
cilium envoy config or ipsec is enabled. The reason is to handle the
reply packet of hair-pinning traffic in Ingress L7 proxy (i.e. backend
is in the same node).

Relates: 0ebe516
Relates: #29530, #29864

Signed-off-by: Tam Mach <tam.mach@cilium.io>
sayboras added a commit that referenced this pull request May 11, 2024
[ upstream commit 32b2784 ]

This commit is to re-enable proxy rule installation in native mode if
cilium envoy config or ipsec is enabled. The reason is to handle the
reply packet of hair-pinning traffic in Ingress L7 proxy (i.e. backend
is in the same node).

Relates: 0ebe516
Relates: #29530, #29864

Signed-off-by: Tam Mach <tam.mach@cilium.io>
sayboras added a commit that referenced this pull request May 11, 2024
[ upstream commit 32b2784 ]

This commit is to re-enable proxy rule installation in native mode if
cilium envoy config or ipsec is enabled. The reason is to handle the
reply packet of hair-pinning traffic in Ingress L7 proxy (i.e. backend
is in the same node).

Relates: 0ebe516
Relates: #29530, #29864

Signed-off-by: Tam Mach <tam.mach@cilium.io>
ldelossa pushed a commit that referenced this pull request May 12, 2024
[ upstream commit 32b2784 ]

This commit is to re-enable proxy rule installation in native mode if
cilium envoy config or ipsec is enabled. The reason is to handle the
reply packet of hair-pinning traffic in Ingress L7 proxy (i.e. backend
is in the same node).

Relates: 0ebe516
Relates: #29530, #29864

Signed-off-by: Tam Mach <tam.mach@cilium.io>
sayboras added a commit that referenced this pull request May 15, 2024
[ upstream commit 32b2784 ]

This commit is to re-enable proxy rule installation in native mode if
cilium envoy config or ipsec is enabled. The reason is to handle the
reply packet of hair-pinning traffic in Ingress L7 proxy (i.e. backend
is in the same node).

Relates: 0ebe516
Relates: #29530, #29864

Signed-off-by: Tam Mach <tam.mach@cilium.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/encryption Impacts encryption support such as IPSec, WireGuard, or kTLS. area/proxy Impacts proxy components, including DNS, Kafka, Envoy and/or XDS servers. backport/author The backport will be carried out by the author of the PR. backport-done/1.13 The backport for Cilium 1.13.x for this PR is done. backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. feature/ipsec Relates to Cilium's IPsec feature kind/bug This is a bug in the Cilium logic. kind/regression This functionality worked fine before, but was broken in a newer release of Cilium. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/bug This PR fixes an issue in a previous release of Cilium. sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages.
Projects
1.12.20
Needs backport from main
1.13.13
Backport done to v1.13
1.14.8
Backport done to v1.14
Development

Successfully merging this pull request may close these issues.

None yet

5 participants