Remove the associated stale conntrack entries when UDP Endpoints are removed #5112

hongliangl · 2023-06-12T09:30:33Z

Previously, when a client accessed a UDP Service through AntreaProxy, if the
selected backend Pod was deleted and no new available backend Pod was automatically
selected by AntreaProxy, the connection would always remain unavailable. This issue
occurred because a conntrack entry was generated through OVS ct action in AntreaProxy
when handling the first packet of the UDP connection. Even if the selected Endpoint
was removed, the conntrack entry's timeout would be flushed upon receiving a packet
from the client in AntreaProxy OVS pipeline. Consequently, the stale conntrack entry
would persist as long as the client continued sending packets to the UDP service,
causing AntreaProxy's OVS pipeline to direct the packets to the IP of the removed
backend Pod.

This PR addresses the issue by removing the stale conntrack entries when the
associated UDP Endpoints are removed. This ensures that a new available backend Pod
can be selected in AntreaProxy's OVS pipeline.

Please note the following:

Currently, this implementation is only available on Linux.
Due to the restriction of the go library 'netlink', there is no API to specify a
target zone. As a result, when deleting a stale conntrack entry with a
destination port (such as NodePort), not only will the conntrack entry whose
destination port is the port added by AntreaProxy be deleted, but also the
conntrack entry that is not added by AntreaProxy will be deleted. This behavior
is unexpected, as only the conntrack entries added by AntreaProxy should be
deleted.

pkg/agent/util/conntrack/conntrack.go

antoninbas · 2023-06-13T01:07:11Z

Left a comment in #5115 (comment)
Do we actually need to use the conntrack program for this, as opposed to a Go library generating the right netlink message?

tnqn · 2023-06-13T11:59:46Z

Left a comment in #5115 (comment) Do we actually need to use the conntrack program for this, as opposed to a Go library generating the right netlink message?

OVS also supports removing conntrack entry via ovs-dpctl flush-conntrack or ovs-appctl dpctl/flush-conntrack.

tnqn · 2023-06-13T12:01:58Z

And do we also need to do the same as https://github.com/kubernetes/kubernetes/blob/232cdf97164f6e3a77ee954bbdd29866232f04dc/pkg/proxy/conntrack/cleanup.go#L36-L39?

antoninbas · 2023-06-13T17:20:50Z

Left a comment in #5115 (comment) Do we actually need to use the conntrack program for this, as opposed to a Go library generating the right netlink message?

OVS also supports removing conntrack entry via ovs-dpctl flush-conntrack or ovs-appctl dpctl/flush-conntrack.

I think that's better than installing something new, but I prefer the library approach if possible.
The OVS tool is useful if we also need to do the same thing on WIndows, but it doesn't appear to be the case?

hongliangl · 2023-06-14T01:23:50Z

And do we also need to do the same as https://github.com/kubernetes/kubernetes/blob/232cdf97164f6e3a77ee954bbdd29866232f04dc/pkg/proxy/conntrack/cleanup.go#L36-L39?

@tnqn We don't need this since we use packet-out to send a reject packet directly to the output interface which will not generate a conntrack entry.

hongliangl · 2023-06-14T04:26:49Z

Left a comment in #5115 (comment) Do we actually need to use the conntrack program for this, as opposed to a Go library generating the right netlink message?

OVS also supports removing conntrack entry via ovs-dpctl flush-conntrack or ovs-appctl dpctl/flush-conntrack.

ovs-dpctl flush-conntrack could be used to flush conntrack, but it can only flush all conntrack entries added by OVS or a specific one with ct-tuple like this ct_nw_src=10.10.0.14,ct_nw_dst=10.96.0.9,ct_nw_proto=17,ct_tp_src=12345,ct_tp_dst=80. ct_nw_src must be set, otherwise it cannot work.

hongliangl · 2023-06-14T09:42:58Z

I have updated this PR and now it uses Go library netlink in this PR to delete stale conntrack entry. @tnqn @antoninbas

tnqn

LGTM overall

pkg/agent/proxy/proxier.go

tnqn · 2023-07-12T05:32:50Z

pkg/agent/proxy/proxier.go

+			if needClearConntrackEntries(svcInfo.OFProtocol) {
+				if !p.removeStaleEndpointConntrackEntries(svcPortName, svcInfo, staleEndpoints) {
+					continue
+				}
+			}


could it be moved to removeStaleEndpoints? it seems a fair sub function of it.

It could be moved to removeStaleEndpoints, and it is more sensible to read and understand. However, it will be called redundantly once when deleting a Service. For example,func (p *proxier) removeStaleServices() is called when deleting a Service. Within the function, the conntrack items related with the Service will be deleted in

if !p.removeServiceFlows(svcInfo) { continue }

As a result, it is no need to delete the contrack items in

if !p.removeStaleEndpoints(svcPortName, svcInfo, endpoints) { continue }

I took another look, it seems we shouldn't even call removeStaleEndpointConntrackEntries in removeServiceFlows, as it would lead to connection disrupted when Services are updated.

We didn't call removeStaleEndpointConntrackEntries in removeServiceFlows, do you meant that we call ClearConntrackEntryForService in removeServiceFlows?

pkg/agent/route/interfaces.go

tnqn · 2023-07-12T05:39:03Z

Due to the restriction of the go library 'netlink', there is no API to specify a
target zone. As a result, when deleting a stale conntrack entry with a
destination port (such as NodePort), not only will the conntrack entry whose
destination port is the port added by AntreaProxy be deleted, but also the
conntrack entry that is not added by AntreaProxy will be deleted. This behavior
is unexpected, as only the conntrack entries added by AntreaProxy should be
deleted.

Could it incur any potential issues? If yes, we should perhaps have a feature gate for it, in case it causes problems to production clusters, otherwise it would be effective without any way to disable it.

hongliangl · 2023-07-12T06:52:05Z

Could it incur any potential issues? If yes, we should perhaps have a feature gate for it, in case it causes problems to production clusters, otherwise it would be effective without any way to disable it.

The potential risk is that it could delete the UDP conntrack items introduced by kube-proxy. For example, assuming that Antrea is deployed with kube-proxy, and a UDP NodePort Service is created.

First, an external client accesses to the Service, conntrack items containing the NodePort port will be created by kube-proxy in zone 0.
Then a Pod client accesses to the Service, conntrack items containing the NodePort port will be created by Antrea with OVS ct in zone 0xfff0 (IPv4) or zone 0xffe6 (IPv6).
When deleting the Service, Antrea will try to delete the conntrack items matching the NodePort port from all zones (including zone 0), and kube-proxy will also try to delete the conntrack items in zone 0.

tnqn

There seems to be some problems after second look.

tnqn · 2023-07-12T06:54:55Z

pkg/agent/proxy/proxier.go

+			if needClearConntrackEntries(svcInfo.OFProtocol) {
+				if !p.removeStaleEndpointConntrackEntries(svcPortName, svcInfo, staleEndpoints) {
+					continue
+				}
+			}


I took another look, it seems we shouldn't even call removeStaleEndpointConntrackEntries in removeServiceFlows, as it would lead to connection disrupted when Services are updated.

tnqn · 2023-07-12T06:56:29Z

pkg/agent/proxy/proxier.go

@@ -397,6 +442,12 @@ func (p *proxier) uninstallNodePortService(svcPort uint16, protocol binding.Prot
 	if err := p.routeClient.DeleteNodePort(p.nodePortAddresses, svcPort, protocol); err != nil {
 		return fmt.Errorf("failed to remove NodePort traffic redirecting rules: %w", err)
 	}
+	if needClearConntrackEntries(protocol) {
+		// Remove NodePort conntrack entries when protocol is UDP.
+		if err := p.routeClient.ClearConntrackEntryForService(nil, svcPort, nil, protocol); err != nil {


If I'm not mistaken, this could clear all conntrack entries on the Node using that port, even it's not even service traffic, for example, when pod A accesses pod B's this port.

Node IPs and NodePort port are used to match the conntrack items.

but where is Node IP matched?

Sorry, I missed that. I'll Node IPs to match conntrack items for NodePort.

…n zone 0 This is a subsequent PR for antrea-io#5112. As mentioned in antrea-io#5112: > Due to the restriction of the go library 'netlink', there is no API to specify a target zone. As a result, when deleting a stale conntrack entry with a destination port (such as NodePort), not only will the conntrack entry whose destination port is the port added by AntreaProxy be deleted, but also the conntrack entry that is not added by AntreaProxy will be deleted. This behavior is unexpected, as only the conntrack entries added by AntreaProxy should be deleted. This PR is to fix the issue by incorporating a ct zone filter, which is provided by the latest go libray `netlink`. Signed-off-by: Hongliang Liu <lhongliang@vmware.com>

This is a subsequent PR for antrea-io#5112. It fixes the issue that AntreaProxy could unintentionally delete conntrack entries in zone 0 As mentioned in antrea-io#5112: > Due to the restriction of the go library 'netlink', there is no API to specify a target zone. As a result, when deleting a stale conntrack entry with a destination port (such as NodePort), not only will the conntrack entry whose destination port is the port added by AntreaProxy be deleted, but also the conntrack entry that is not added by AntreaProxy will be deleted. This behavior is unexpected, as only the conntrack entries added by AntreaProxy should be deleted. This PR resolves the issue by integrating a CT zone filter, now available in the latest Go library `netlink`. Leveraging this feature, AntreaProxy can accurately delete stale UDP conntrack entries. Consequently, we are promoting the feature gate `CleanupStaleUDPSvcConntrack` to Beta. Signed-off-by: Hongliang Liu <lhongliang@vmware.com>

…n zone 0 This is a subsequent PR for antrea-io#5112. As mentioned in antrea-io#5112: > Due to the restriction of the go library 'netlink', there is no API to specify a target zone. As a result, when deleting a stale conntrack entry with a destination port (such as NodePort), not only will the conntrack entry whose destination port is the port added by AntreaProxy be deleted, but also the conntrack entry that is not added by AntreaProxy will be deleted. This behavior is unexpected, as only the conntrack entries added by AntreaProxy should be deleted. This PR resolves the issue by integrating a CT zone filter, now available in the latest Go library `netlink`. Leveraging this feature, AntreaProxy can accurately delete stale UDP conntrack entries. Signed-off-by: Hongliang Liu <lhongliang@vmware.com>

…n zone 0 (#6193) This is a subsequent PR for #5112. As mentioned in #5112: > Due to the restriction of the go library 'netlink', there is no API to specify a target zone. As a result, when deleting a stale conntrack entry with a destination port (such as NodePort), not only will the conntrack entry whose destination port is the port added by AntreaProxy be deleted, but also the conntrack entry that is not added by AntreaProxy will be deleted. This behavior is unexpected, as only the conntrack entries added by AntreaProxy should be deleted. This PR resolves the issue by integrating a CT zone filter, now available in the latest Go library `netlink`. Leveraging this feature, AntreaProxy can accurately delete stale UDP conntrack entries. Signed-off-by: Hongliang Liu <lhongliang@vmware.com>

In antrea-io#5112, due to the limitations of the Go netlink library, AntreaProxy would unconditionally delete conntrack entries added by kube-proxy in conntrack zone 0. AntreaProxy was supposed to only delete its own entries in conntrack zones 65520 or 65521. To address this, a feature was added to isolate the relevant code. After the merge of antrea-io#6193, the netlink library was updated, allowing AntreaProxy to precisely delete conntrack entries in zones 65520 or 65521. It is now safe to enable the corresponding code by default. Signed-off-by: Hongliang Liu <lhongliang@vmware.com>

…n zone 0 (antrea-io#6193) This is a subsequent PR for antrea-io#5112. As mentioned in antrea-io#5112: > Due to the restriction of the go library 'netlink', there is no API to specify a target zone. As a result, when deleting a stale conntrack entry with a destination port (such as NodePort), not only will the conntrack entry whose destination port is the port added by AntreaProxy be deleted, but also the conntrack entry that is not added by AntreaProxy will be deleted. This behavior is unexpected, as only the conntrack entries added by AntreaProxy should be deleted. This PR resolves the issue by integrating a CT zone filter, now available in the latest Go library `netlink`. Leveraging this feature, AntreaProxy can accurately delete stale UDP conntrack entries. Signed-off-by: Hongliang Liu <lhongliang@vmware.com>

…n zone 0 (#6193) (#6406) This is a subsequent PR for #5112. As mentioned in #5112: > Due to the restriction of the go library 'netlink', there is no API to specify a target zone. As a result, when deleting a stale conntrack entry with a destination port (such as NodePort), not only will the conntrack entry whose destination port is the port added by AntreaProxy be deleted, but also the conntrack entry that is not added by AntreaProxy will be deleted. This behavior is unexpected, as only the conntrack entries added by AntreaProxy should be deleted. This PR resolves the issue by integrating a CT zone filter, now available in the latest Go library `netlink`. Leveraging this feature, AntreaProxy can accurately delete stale UDP conntrack entries. Signed-off-by: Hongliang Liu <lhongliang@vmware.com>

…n zone 0 (#6193) (#6404) This is a subsequent PR for #5112. As mentioned in #5112: > Due to the restriction of the go library 'netlink', there is no API to specify a target zone. As a result, when deleting a stale conntrack entry with a destination port (such as NodePort), not only will the conntrack entry whose destination port is the port added by AntreaProxy be deleted, but also the conntrack entry that is not added by AntreaProxy will be deleted. This behavior is unexpected, as only the conntrack entries added by AntreaProxy should be deleted. This PR resolves the issue by integrating a CT zone filter, now available in the latest Go library `netlink`. Leveraging this feature, AntreaProxy can accurately delete stale UDP conntrack entries. Signed-off-by: Hongliang Liu <lhongliang@vmware.com>

…n zone 0 (#6193) (#6405) This is a subsequent PR for #5112. As mentioned in #5112: > Due to the restriction of the go library 'netlink', there is no API to specify a target zone. As a result, when deleting a stale conntrack entry with a destination port (such as NodePort), not only will the conntrack entry whose destination port is the port added by AntreaProxy be deleted, but also the conntrack entry that is not added by AntreaProxy will be deleted. This behavior is unexpected, as only the conntrack entries added by AntreaProxy should be deleted. This PR resolves the issue by integrating a CT zone filter, now available in the latest Go library `netlink`. Leveraging this feature, AntreaProxy can accurately delete stale UDP conntrack entries. Signed-off-by: Hongliang Liu <lhongliang@vmware.com>

In antrea-io#5112, due to the limitations of the Go netlink library, AntreaProxy would unconditionally delete conntrack entries added by kube-proxy in conntrack zone 0. AntreaProxy was supposed to only delete its own entries in conntrack zones 65520 or 65521. To address this, a feature was added to isolate the relevant code. After the merge of antrea-io#6193, the netlink library was updated, allowing AntreaProxy to precisely delete conntrack entries in zones 65520 or 65521. It is now safe to enable the corresponding code by default. Signed-off-by: Hongliang Liu <lhongliang@vmware.com>

In #5112, due to the limitations of the Go netlink library, AntreaProxy would unconditionally delete conntrack entries added by kube-proxy in conntrack zone 0. AntreaProxy was supposed to only delete its own entries in conntrack zones 65520 or 65521. To address this, a feature was added to isolate the relevant code. After the merge of #6193, the netlink library was updated, allowing AntreaProxy to precisely delete conntrack entries in zones 65520 or 65521. It is now safe to enable the corresponding code by default. Signed-off-by: Hongliang Liu <lhongliang@vmware.com>

hongliangl requested review from tnqn, wenyingd and jianjuns June 12, 2023 09:31

hongliangl added this to the Antrea v1.13 release milestone Jun 12, 2023

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. labels Jun 12, 2023

jianjuns reviewed Jun 12, 2023

View reviewed changes

pkg/agent/util/conntrack/conntrack.go Outdated Show resolved Hide resolved

pkg/agent/util/conntrack/conntrack.go Outdated Show resolved Hide resolved

pkg/agent/util/conntrack/conntrack.go Outdated Show resolved Hide resolved

jianjuns reviewed Jun 12, 2023

View reviewed changes

pkg/agent/util/conntrack/conntrack.go Outdated Show resolved Hide resolved

hongliangl mentioned this pull request Jun 13, 2023

Add conntrack tool in Antrea base image #5115

Closed

hongliangl force-pushed the 20230607-udp-preserve-connection branch 2 times, most recently from 1f10a30 to 910de25 Compare June 14, 2023 09:35

hongliangl force-pushed the 20230607-udp-preserve-connection branch 2 times, most recently from 6d6ffeb to 250ba4b Compare June 14, 2023 10:53

hongliangl force-pushed the 20230607-udp-preserve-connection branch from 250ba4b to b34f1cb Compare June 28, 2023 08:44

hongliangl requested a review from jianjuns June 28, 2023 08:44

hongliangl force-pushed the 20230607-udp-preserve-connection branch from b34f1cb to e441042 Compare June 28, 2023 09:20

hongliangl force-pushed the 20230607-udp-preserve-connection branch from e441042 to 2c9692c Compare July 12, 2023 05:23

tnqn reviewed Jul 12, 2023

View reviewed changes

hongliangl force-pushed the 20230607-udp-preserve-connection branch 2 times, most recently from 0b5cc4d to 1ff376a Compare July 13, 2023 15:20

hongliangl mentioned this pull request May 28, 2024

Promote feature CleanupStaleUDPSvcConntrack from Alpha to Beta #6372

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove the associated stale conntrack entries when UDP Endpoints are removed #5112

Remove the associated stale conntrack entries when UDP Endpoints are removed #5112

hongliangl commented Jun 12, 2023 •

edited

Loading

antoninbas commented Jun 13, 2023

tnqn commented Jun 13, 2023

tnqn commented Jun 13, 2023

antoninbas commented Jun 13, 2023

hongliangl commented Jun 14, 2023

hongliangl commented Jun 14, 2023

hongliangl commented Jun 14, 2023

tnqn left a comment

tnqn Jul 12, 2023

hongliangl Jul 12, 2023

tnqn Jul 12, 2023

hongliangl Jul 12, 2023

tnqn commented Jul 12, 2023

hongliangl commented Jul 12, 2023

tnqn left a comment

tnqn Jul 12, 2023

tnqn Jul 12, 2023

hongliangl Jul 12, 2023

tnqn Jul 12, 2023

hongliangl Jul 12, 2023

Remove the associated stale conntrack entries when UDP Endpoints are removed #5112

Remove the associated stale conntrack entries when UDP Endpoints are removed #5112

Conversation

hongliangl commented Jun 12, 2023 • edited Loading

antoninbas commented Jun 13, 2023

tnqn commented Jun 13, 2023

tnqn commented Jun 13, 2023

antoninbas commented Jun 13, 2023

hongliangl commented Jun 14, 2023

hongliangl commented Jun 14, 2023

hongliangl commented Jun 14, 2023

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

tnqn commented Jul 12, 2023

hongliangl commented Jul 12, 2023

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

hongliangl commented Jun 12, 2023 •

edited

Loading