Ensure full functionality of AntreaProxy with proxyAll enabled when kube-proxy presents #6308

hongliangl · 2024-05-08T09:21:42Z

Depends on #6381

Resolve #6232

To ensure full functionality of AntreaProxy, except for handling ClusterIP from Nodes,
even when kube-proxy in iptables mode is present, certain key changes are implemented
when proxyAll is enabled:

The jump rules for the chains managed by Antrea, ANTREA-PREROUTING and ANTREA-OUTPUT
in nat table, are installed by inserting instead of appending to bypass the chain
KUBE-SERVICES performing Service DNAT managed by kube-proxy. Antrea ensures that
the jump rules take precedence over those managed by kube-proxy.

The iptables rules of nat table chain ANTREA-PREROUTING are like below, and they are
similar in chain ANTREA-OUTPUT.

-A ANTREA-PREROUTING -m comment --comment "Antrea: DNAT external to NodePort packets" -m set --match-set ANTREA-NODEPORT-IP dst,dst -j DNAT --to-destination 169.254.0.252

The rule is to DNAT NodePort traffic, bypassing chain KUBE-SERVICES.

The iptables rules of raw table chains ANTREA-PREROUTING / ANTREA-OUTPUT are like
below:

1. -A ANTREA-PREROUTING -m comment --comment "Antrea: do not track incoming encapsulation packets" -m udp -p udp --dport 6081 -m addrtype --dst-type LOCAL -j NOTRACK
2. -A ANTREA-PREROUTING -m comment --comment "Antrea: drop Pod multicast traffic forwarded via underlay network" -m set --match-set CLUSTER-NODE-IP src -d 224.0.0.0/4 -j DROP
3. -A ANTREA-PREROUTING -m comment --comment "Antrea: do not track request packets destined to external IPs" -m set --match-set ANTREA-EXTERNAL-IP dst -j NOTRACK
4. -A ANTREA-PREROUTING -m comment --comment "Antrea: do not track reply packets sourced from external IPs" -m set --match-set ANTREA-EXTERNAL-IP src -j NOTRACK
5. -A ANTREA-OUTPUT -m comment --comment "Antrea: do not track request packets destined to external IPs" -m set --match-set ANTREA-EXTERNAL-IP dst -j NOTRACK

Rules 1-2 are not new rules.
Rule 3 is to bypass conntrack for packets sourced from external and destined to
externalIPs, which also results in bypassing the chains managed by Antrea Proxy
and kube-proxy in nat table.
Rule 4 is to bypass conntrack for packets sourced from externalIPs, which also
results in bypassing the chains managed by Antrea Proxy and kube-proxy in nat
table.
Rule 5 is to bypass conntrack for packets sourced from local and destined to
externalIPs, which also results in bypassing the chains managed by Antrea Proxy
and kube-proxy in nat table.

The following are the benchmark results of a LoadBalancer Service configured with DSR mode.
The results of TCP_STREAM and TCP_RR (single TCP connection) are almost the same as that
before. The result of TCP_CRR (multiple TCP connections) performs better than before. One
reason should be that conntrack is skipped for LoadBalancer Services.

Test           v2.0 proxyAll     Dev proxyAll    Delta
TCP_STREAM     4933.97           4918.35         -0.32%
TCP_RR         8095.49           8032.4         -0.78%
TCP_CRR        1645.66           1888.93         +14.79%

TODO:

In iptables nat table, ensure that Antrea iptables chain ANTREA-PREROUTING and ANTREA-OUTPUT are in the first rule of corresponding default iptables chains to skip kube-proxy chains.
Add unit tests
Add e2e tests? Whether to add a new Kind test [proxyAll=true, LoadBalancerMode=DSR, NodeIPAM=true, kube-proxy presents]
Update related document.
Update manifests
PR / commit description
Benchmark of LoadBalancer

hongliangl · 2024-05-22T02:25:04Z

/test-all

tnqn · 2024-07-04T14:51:09Z

pkg/agent/route/route_linux.go

+				MatchCIDRDst(c.kubeServiceHost).
+				MatchTransProtocol(iptables.ProtocolTCP).
+				MatchPortDst(&c.kubeServicePort, nil).
+				SetTarget(kubeProxyServiceChain).


This makes strong assumption on kube-proxy runs in iptables mode but it could be ipvs or nftables mode. I'm not sure if the current implementation can work with the other two modes, but there seems no reason why it should jump to the service chain directly here, could it just return from this chain?

Another possible approach is, handle traffic to external ips and nodeports only in this patch, given there seems no obvious benefit to redirect clusterIP traffic to OVS at the moment, but it takes some code to specially handle kubernetes service's cluster IP. In theory, there should be no "external to ClusterIP" traffic.

Another possible approach is, handle traffic to external ips and nodeports only in this patch, given there seems no obvious benefit to redirect clusterIP traffic to OVS at the moment, but it takes some code to specially handle kubernetes service's cluster IP. In theory, there should be no "external to ClusterIP" traffic.

I'm fine with this, and the code will be simpler. As for ClusterIP, maybe we could handle that in the future if there are users that need external to ClusterIP.

pkg/agent/route/route_linux.go

docs/antrea-proxy.md

tnqn

Thanks for making the changes, this is now easier to review

pkg/agent/route/route_linux.go

tnqn · 2024-07-08T15:03:27Z

pkg/agent/route/route_linux.go

@@ -829,6 +929,12 @@ func (c *Client) restoreIptablesData(podCIDR *net.IPNet,
 			"-j", iptables.DNATTarget,
 			"--to-destination", nodePortDNATVirtualIP.String(),
 		}...)
+		writeLine(iptablesData, []string{
+			"-A", antreaPreRoutingChain,
+			"-m", "comment", "--comment", `"Antrea: accept external to external IP packets"`,


this is nat table, why accepting packets here?

I have removed this. According to the document :

NAT
This is the realm of the `nat' table, which is fed packets from two netfilter hooks: for non-local packets, the NF_IP_PRE_ROUTING and NF_IP_POST_ROUTING hooks are perfect for destination and source alterations respectively. If CONFIG_IP_NF_NAT_LOCAL is defined, the hooks NF_IP_LOCAL_OUT and NF_IP_LOCAL_IN are used for altering the destination of local packets.

This table is slightly different from the `filter' table, in that only the first packet of a new connection will traverse the table: the result of this traversal is then applied to all future packets in the same connection.

The IPs matched by the ipset has been enforced with target NOTRACK, and the corresponding packets will be processed in nat table.

pkg/agent/route/route_linux.go

tnqn · 2024-07-08T15:09:24Z

test/e2e/proxy_test.go

@@ -744,6 +744,7 @@ func testProxyHairpin(t *testing.T, isIPv6 bool) {
 	agnhostHost := fmt.Sprintf("agnhost-host-%v", isIPv6)
 	createAgnhostPod(t, data, agnhostHost, node, true)
 	t.Run("HostNetwork Endpoints", func(t *testing.T) {
+		skipIfKubeProxyEnabled(t, data)


why this is needed?

Previously, this test is ran when kube-proxy is removed and Antrea Proxy handles ClusterIP traffic. Now, as the kube-proxy presents and handles ClusterIP traffic, the result of the test won't be the expected result.

jianjuns · 2024-07-08T18:36:17Z

docs/antrea-proxy.md

+handle all Service traffic.
+
+Starting with Antrea v2.1, when only `proxyAll` is enabled, AntreaProxy is capable
+of handing all kinds of Service traffic except for ClusterIP, even if kube-proxy is


except for -> ", not just ClusterIP Service traffic"?

It should be except. LoadBalancer, NodePort and externalIP traffic except ClusterIP.

tnqn

LGTM overall, some nits

docs/antrea-proxy.md

pkg/agent/route/route_linux.go

pkg/agent/route/route_windows.go

tnqn · 2024-07-09T14:24:52Z

test/e2e/proxy_test.go

@@ -745,6 +745,9 @@ func testProxyHairpin(t *testing.T, isIPv6 bool) {
 	createAgnhostPod(t, data, agnhostHost, node, true)
 	t.Run("HostNetwork Endpoints", func(t *testing.T) {
 		skipIfProxyAllDisabled(t, data)
+		// AntreProxy with proxyAll enabled doesn't handle ClusterIP traffic sourced from local Node when kube-proxy


Suggested change

// AntreProxy with proxyAll enabled doesn't handle ClusterIP traffic sourced from local Node when kube-proxy

// AntreaProxy with proxyAll enabled doesn't handle ClusterIP traffic sourced from local Node when kube-proxy

To confirm, when proxyAll is enabled and kube-proxy is running, such hairpin case can still work, it's just the client IP is not the virtual IP, right? If yes, I think the test should just change the expected IP, instead of not validating this case.

Yes, it still can work, but the expected IP is not the virtual IP. I will update the test.

tnqn · 2024-07-10T15:13:05Z

docs/antrea-proxy.md

+Starting with Antrea v2.1, when `proxyAll` is enabled, AntreaProxy is capable
+of handling all kinds of Service traffic except ClusterIP, even if kube-proxy in
+iptables mode is present. This is accomplished by prioritizing the rules installed
+by Antrea Proxy redirecting Service traffic to OVS over those installed by kube-proxy.


Consider this:

Starting with Antrea v2.1, when `proxyAll` is enabled, AntreaProxy will handle Service traffic destined to NodePort, LoadBalancerIP and ExternalIP, even if kube-proxy is present. This benefits users who want to take advantage of AntreaProxy's advanced features, such as Direct Server Return (DSR) mode, but lack control over kube-proxy's installation. This is accomplished by prioritizing the rules installed by AntreaProxy over those installed by kube-proxy, thus it works only with kube-proxy iptables mode. Support for other kube-proxy modes may be added in the future.

cc @jianjuns

Updated the document.

…ube-proxy presents To ensure full functionality of AntreaProxy, except for handling ClusterIP from Nodes, even when kube-proxy in iptables mode is present, certain key changes are implemented when proxyAll is enabled: The jump rules for the chains managed by Antrea, `ANTREA-PREROUTING` and `ANTREA-OUTPUT` in nat table, are installed by inserting instead of appending to bypass the chain `KUBE-SERVICES` performing Service DNAT managed by kube-proxy. Antrea ensures that the jump rules take precedence over those managed by kube-proxy. The iptables rules of nat table chain `ANTREA-PREROUTING` are like below, and they are similar in chain `ANTREA-OUTPUT`. ``` -A ANTREA-PREROUTING -m comment --comment "Antrea: DNAT external to NodePort packets" -m set --match-set ANTREA-NODEPORT-IP dst,dst -j DNAT --to-destination 169.254.0.252 ``` The rule is to DNAT NodePort traffic, bypassing chain `KUBE-SERVICES`. The iptables rules of raw table chains ANTREA-PREROUTING / ANTREA-OUTPUT are like below: ``` 1. -A ANTREA-PREROUTING -m comment --comment "Antrea: do not track incoming encapsulation packets" -m udp -p udp --dport 6081 -m addrtype --dst-type LOCAL -j NOTRACK 2. -A ANTREA-PREROUTING -m comment --comment "Antrea: drop Pod multicast traffic forwarded via underlay network" -m set --match-set CLUSTER-NODE-IP src -d 224.0.0.0/4 -j DROP 3. -A ANTREA-PREROUTING -m comment --comment "Antrea: do not track request packets destined to external IPs" -m set --match-set ANTREA-EXTERNAL-IP dst -j NOTRACK 4. -A ANTREA-PREROUTING -m comment --comment "Antrea: do not track reply packets sourced from external IPs" -m set --match-set ANTREA-EXTERNAL-IP src -j NOTRACK 5. -A ANTREA-OUTPUT -m comment --comment "Antrea: do not track request packets destined to external IPs" -m set --match-set ANTREA-EXTERNAL-IP dst -j NOTRACK ``` - Rules 1-2 are not new rules. - Rule 3 is to bypass conntrack for packets sourced from external and destined to externalIPs, which also results in bypassing the chains managed by Antrea Proxy and kube-proxy in nat table. - Rule 4 is to bypass conntrack for packets sourced from externalIPs, which also results in bypassing the chains managed by Antrea Proxy and kube-proxy in nat table. - Rule 5 is to bypass conntrack for packets sourced from local and destined to externalIPs, which also results in bypassing the chains managed by Antrea Proxy and kube-proxy in nat table. The following are the benchmark results of a LoadBalancer Service configured with DSR mode. The results of TCP_STREAM and TCP_RR (single TCP connection) are almost the same as that before. The result of TCP_CRR (multiple TCP connections) performs better than before. One reason should be that conntrack is skipped for LoadBalancer Services. ``` Test v2.0 proxyAll Dev proxyAll Delta TCP_STREAM 4933.97 4918.35 -0.32% TCP_RR 8095.49 8032.4 -0.78% TCP_CRR 1645.66 1888.93 +14.79% ``` Signed-off-by: Hongliang Liu <lhongliang@vmware.com>

tnqn

LGTM

tnqn · 2024-07-11T03:04:18Z

/test-all
/test-ipv6-all
/test-ipv6-only-all

luolanzone

Thanks a lot for the efforts!

one nit: since you updated all AntreaProxy to Antrea Proxy in another PR, better to update here in antrea-proxy.md as well to avoid any dependency and another round of updates.

hongliangl · 2024-07-11T07:00:36Z

Thanks a lot for the efforts!

one nit: since you updated all AntreaProxy to Antrea Proxy in another PR, better to update here in antrea-proxy.md as well to avoid any dependency and another round of updates.

I can update document antrea-proxy.md in that PR.

tnqn · 2024-07-11T08:25:26Z

/test-kind-ipv6-only-all
/test-kind-ipv6-all

hongliangl requested review from tnqn and antoninbas May 8, 2024 09:22

hongliangl force-pushed the 20240429-bypass-kube-proxy branch from 90b6c82 to 7b1ec7e Compare May 8, 2024 09:32

hongliangl added area/proxy Issues or PRs related to proxy functions in Antrea action/release-note Indicates a PR that should be included in release notes. area/OS/linux Issues or PRs related to the Linux operating system. labels May 8, 2024

hongliangl added this to the Antrea v2.1 release milestone May 8, 2024

hongliangl force-pushed the 20240429-bypass-kube-proxy branch from 7b1ec7e to 559dde5 Compare May 8, 2024 12:22

hongliangl removed request for antoninbas and tnqn May 8, 2024 16:06

hongliangl force-pushed the 20240429-bypass-kube-proxy branch 2 times, most recently from ece78aa to e5a6949 Compare May 9, 2024 01:38

hongliangl requested review from tnqn and antoninbas May 9, 2024 09:43

hongliangl marked this pull request as ready for review May 9, 2024 09:44

hongliangl marked this pull request as draft May 17, 2024 05:48

hongliangl force-pushed the 20240429-bypass-kube-proxy branch 2 times, most recently from e4b5ccb to e8b24b1 Compare May 20, 2024 11:23

hongliangl marked this pull request as ready for review May 20, 2024 11:27

hongliangl force-pushed the 20240429-bypass-kube-proxy branch 2 times, most recently from 7a890bc to 1b705a5 Compare May 21, 2024 08:50

hongliangl force-pushed the 20240429-bypass-kube-proxy branch 3 times, most recently from 2254f1d to 716b05a Compare May 30, 2024 03:20

hongliangl mentioned this pull request May 30, 2024

Update iptables builder #6381

Merged

hongliangl force-pushed the 20240429-bypass-kube-proxy branch 3 times, most recently from 99916ca to cad061b Compare June 5, 2024 08:37

hongliangl force-pushed the 20240429-bypass-kube-proxy branch from cad061b to cda84bd Compare June 6, 2024 12:44

hongliangl requested a review from tnqn July 3, 2024 15:27

hongliangl force-pushed the 20240429-bypass-kube-proxy branch from a61ff64 to 54e5429 Compare July 4, 2024 06:22

tnqn reviewed Jul 4, 2024

View reviewed changes

jianjuns reviewed Jul 5, 2024

View reviewed changes

docs/antrea-proxy.md Outdated Show resolved Hide resolved

docs/antrea-proxy.md Outdated Show resolved Hide resolved

docs/antrea-proxy.md Outdated Show resolved Hide resolved

docs/antrea-proxy.md Show resolved Hide resolved

hongliangl force-pushed the 20240429-bypass-kube-proxy branch 2 times, most recently from 6f666b7 to bcd4544 Compare July 8, 2024 06:54

hongliangl requested review from tnqn and jianjuns July 8, 2024 06:57

hongliangl force-pushed the 20240429-bypass-kube-proxy branch from bcd4544 to 61a12f3 Compare July 8, 2024 08:17

tnqn reviewed Jul 8, 2024

View reviewed changes

jianjuns reviewed Jul 8, 2024

View reviewed changes

hongliangl force-pushed the 20240429-bypass-kube-proxy branch 3 times, most recently from 1041cd6 to 5d5faf7 Compare July 9, 2024 07:03

hongliangl requested a review from tnqn July 9, 2024 08:10

tnqn reviewed Jul 9, 2024

View reviewed changes

hongliangl force-pushed the 20240429-bypass-kube-proxy branch from 5d5faf7 to ba9648b Compare July 10, 2024 04:01

tnqn reviewed Jul 10, 2024

View reviewed changes

hongliangl force-pushed the 20240429-bypass-kube-proxy branch 2 times, most recently from 1f3f1ce to 8f6410e Compare July 11, 2024 02:36

hongliangl force-pushed the 20240429-bypass-kube-proxy branch from 8f6410e to 48b8f7d Compare July 11, 2024 02:43

tnqn approved these changes Jul 11, 2024

View reviewed changes

luolanzone reviewed Jul 11, 2024

View reviewed changes

tnqn merged commit 5cee770 into antrea-io:main Jul 11, 2024
57 of 68 checks passed

hongliangl deleted the 20240429-bypass-kube-proxy branch July 11, 2024 14:36

hongliangl mentioned this pull request Jul 18, 2024

Update documents to replace AntreaProxy with Antrea Proxy #6515

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ensure full functionality of AntreaProxy with proxyAll enabled when kube-proxy presents #6308

Ensure full functionality of AntreaProxy with proxyAll enabled when kube-proxy presents #6308

hongliangl commented May 8, 2024 •

edited

Loading

hongliangl commented May 22, 2024

tnqn Jul 4, 2024

hongliangl Jul 5, 2024

tnqn left a comment

tnqn Jul 8, 2024

hongliangl Jul 9, 2024

tnqn Jul 8, 2024

hongliangl Jul 9, 2024

jianjuns Jul 8, 2024

hongliangl Jul 9, 2024

tnqn left a comment

tnqn Jul 9, 2024

tnqn Jul 9, 2024

hongliangl Jul 10, 2024

tnqn Jul 10, 2024 •

edited

Loading

jianjuns Jul 10, 2024

hongliangl Jul 11, 2024

tnqn left a comment

tnqn commented Jul 11, 2024

luolanzone left a comment

hongliangl commented Jul 11, 2024

tnqn commented Jul 11, 2024

	// AntreProxy with proxyAll enabled doesn't handle ClusterIP traffic sourced from local Node when kube-proxy
	// AntreaProxy with proxyAll enabled doesn't handle ClusterIP traffic sourced from local Node when kube-proxy

Ensure full functionality of AntreaProxy with proxyAll enabled when kube-proxy presents #6308

Ensure full functionality of AntreaProxy with proxyAll enabled when kube-proxy presents #6308

Conversation

hongliangl commented May 8, 2024 • edited Loading

hongliangl commented May 22, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tnqn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tnqn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tnqn Jul 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tnqn left a comment

Choose a reason for hiding this comment

tnqn commented Jul 11, 2024

luolanzone left a comment

Choose a reason for hiding this comment

hongliangl commented Jul 11, 2024

tnqn commented Jul 11, 2024

hongliangl commented May 8, 2024 •

edited

Loading

tnqn Jul 10, 2024 •

edited

Loading