Fix the issue of local probe bypassing flows on Windows #3510

hongliangl · 2022-03-23T09:12:29Z

When proxyAll is enabled, kube-proxy can be replaced by AntreaProxy, then
Service traffic and non-Service traffic can be distinguished by ServiceCTMark
and NotServiceCTMark. Service traffic with ServiceCTMark should not bypass
Network Policies, and non-Service traffic generated by kubelet with
NotServiceCTMark should bypass Network Policies.

This PR also fixes the issue that the reply packets of Pod -> local Antrea gateway
bypasses EgressMetricTable.

Signed-off-by: Hongliang Liu lhongliang@vmware.com

pkg/agent/openflow/pipeline.go

codecov-commenter · 2022-03-24T00:24:02Z

Codecov Report

Merging #3510 (5fc6a24) into main (5905e98) will decrease coverage by 7.24%.
The diff coverage is 61.53%.

@@            Coverage Diff             @@
##             main    #3510      +/-   ##
==========================================
- Coverage   64.66%   57.41%   -7.25%     
==========================================
  Files         278      392     +114     
  Lines       39363    54837   +15474     
==========================================
+ Hits        25454    31485    +6031     
- Misses      11939    20906    +8967     
- Partials     1970     2446     +476

Flag	Coverage Δ
e2e-tests	`50.25% <61.53%> (?)`
integration-tests	`38.45% <ø> (?)`
kind-e2e-tests	`52.31% <61.53%> (-0.03%)`	⬇️
unit-tests	`43.81% <15.38%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
pkg/ovs/openflow/ofctrl_builder.go	`59.26% <0.00%> (-0.41%)`	⬇️
pkg/agent/openflow/pipeline.go	`75.11% <61.11%> (-0.40%)`	⬇️
pkg/agent/openflow/client.go	`73.13% <100.00%> (+0.99%)`	⬆️
pkg/agent/openflow/pod_connectivity.go	`68.62% <100.00%> (+0.31%)`	⬆️
pkg/agent/controller/networkpolicy/fqdn.go	`75.51% <0.00%> (-2.60%)`	⬇️
pkg/agent/route/route_linux.go	`48.22% <0.00%> (-1.58%)`	⬇️
pkg/ipam/poolallocator/allocator.go	`53.49% <0.00%> (-0.97%)`	⬇️
...gent/controller/networkpolicy/status_controller.go	`81.51% <0.00%> (-0.85%)`	⬇️
pkg/ovs/ovsctl/appctl.go	`41.86% <0.00%> (-0.78%)`	⬇️
pkg/agent/controller/egress/egress_controller.go	`74.35% <0.00%> (-0.52%)`	⬇️
... and 131 more

wenyingd · 2022-03-24T00:36:47Z

pkg/agent/openflow/pipeline.go

 				Cookie(cookieID).
 				MatchProtocol(ipProtocol).
-				MatchSrcIP(gatewayIP).
-				Action().GotoStage(stageConntrack).
+				MatchSrcIP(gatewayIP)


From my side, I would prefer to match gw port as the in_port or match the fromSource reg instead of gatewayIP, because ingress stage is after post routing stage, I am not sure if SNAT is taken for no Service IP in future. Matching in_port or source reg is more compatible.

For request packets of non-Service connections, FromGatewayRegMark && NotServiceCTMark -> stageConntrack

✅ client is the local Node, destination is a local Pod

❎ client is a remote Pod or remote Node, destination is a local Pod (Windows noEncap mode, remote traffic from remote Pod CIDR is sent to local Pod CIDR via Antrea gateway, not uplink)

I think the packets of the second case should not bypass ingress rules.

Make sense.

You remind me, we should add state -rpl+trk to avoid impact on reply packets , since reply packets are possible to goto Metric table for statistics collection.

Thanks, updated.

wenyingd

LGTM

hongliangl · 2022-03-27T08:25:42Z

/test-all-features-conformance
/test-conformance
/test-e2e
/test-flexible-ipam-e2e
/test-ipv6-conformance
/test-ipv6-e2e
/test-ipv6-networkpolicy
/test-ipv6-only-conformance
/test-ipv6-only-e2e
/test-ipv6-only-networkpolicy
/test-multicluster-e2e
/test-networkpolicy
/test-windows-conformance
/test-windows-e2e
/test-windows-networkpolicy
/test-windows-proxyall-e2e
/test-integration

hongliangl · 2022-03-30T13:46:20Z

/test-windows-conformance
/test-ipv6-only-e2e
/test-integration
/test-ipv6-only-e2e

hongliangl · 2022-03-30T13:46:32Z

/test-multicluster-e2e

hongliangl · 2022-04-14T17:52:28Z

/test-all-features-conformance
/test-conformance
/test-e2e
/test-flexible-ipam-e2e
/test-multicluster-e2e
/test-networkpolicy
/test-windows-conformance
/test-windows-e2e
/test-windows-networkpolicy
/test-windows-proxyall-e2e

hongliangl · 2022-04-15T00:57:28Z

/test-integration
/test-ipv6-conformance
/test-ipv6-e2e
/test-ipv6-networkpolicy
/test-ipv6-only-conformance
/test-ipv6-only-e2e
/test-ipv6-only-networkpolicy
/test-windows-networkpolicy
/test-multicluster-e2e

hongliangl · 2022-04-15T18:15:26Z

/test-integration
/test-ipv6-conformance
/test-ipv6-e2e
/test-windows-networkpolicy
/test-multicluster-e2e

hongliangl · 2022-04-15T18:15:44Z

/test-flexible-ipam-e2e

hongliangl · 2022-04-15T19:37:52Z

/test-integration

jianjuns · 2022-04-15T21:07:36Z

/test-integration

jianjuns · 2022-04-15T23:51:21Z

You need to fix the integration test:

table=IngressSecurityClassifier, priority=210,ct_state=-rpl+trk,ip,nw_src=192.168.1.1 actions=goto_table:ConntrackCommit

hongliangl · 2022-04-16T00:29:40Z

/test-multicluster-e2e

hongliangl · 2022-04-16T00:30:01Z

/test-ipv6-only-e2e
/test-ipv6-e2e

hongliangl · 2022-04-16T00:35:49Z

/test-ipv6-e2e

hongliangl · 2022-04-16T00:38:30Z

You need to fix the integration test:

table=IngressSecurityClassifier, priority=210,ct_state=-rpl+trk,ip,nw_src=192.168.1.1 actions=goto_table:ConntrackCommit

Fixed, and I think there is something wrong with test ipv6-e2e and multicluster-e2e since other PRs always get failed for these two tests.

jianjuns · 2022-04-18T22:43:22Z

/test-e2e

hongliangl · 2022-04-19T01:51:12Z

/test-networkpolicy

wenyingd

LGTM

jianjuns · 2022-04-19T16:22:44Z

/test-e2e

hongliangl · 2022-04-19T22:47:26Z

/test-e2e

jianjuns · 2022-04-20T21:49:42Z

/test-e2e
/test-networkpolicy

When proxyAll is enabled, kube-proxy can be replaced by AntreaProxy, then Service traffic and non-Service traffic can be distinguished by ServiceCTMark and NotServiceCTMark. Service traffic with ServiceCTMark should not bypass Network Policies, and non-Service traffic generated by kubelet with NotServiceCTMark should bypass Network Policies. Signed-off-by: Hongliang Liu <lhongliang@vmware.com>

hongliangl · 2022-04-20T23:53:11Z

/test-all-features-conformance
/test-conformance
/test-e2e
/test-flexible-ipam-e2e
/test-ipv6-conformance
/test-ipv6-e2e
/test-ipv6-networkpolicy
/test-ipv6-only-conformance
/test-ipv6-only-e2e
/test-ipv6-only-networkpolicy
/test-multicluster-e2e
/test-networkpolicy
/test-windows-conformance
/test-windows-e2e
/test-windows-networkpolicy
/test-windows-proxyall-e2e
/test-integration

hongliangl · 2022-04-21T06:46:36Z

/test-all-features-conformance
/test-ipv6-e2e
/test-networkpolicy
/test-windows-networkpolicy to
t /test-windows-proxyall-e2e to re-tr

hongliangl · 2022-04-21T06:47:09Z

/test-hw-offload

hongliangl · 2022-04-21T06:47:18Z

/test-flexible-ipam-e2e

hongliangl · 2022-04-21T09:58:50Z

/test-windows-networkpolicy
/test-ipv6-e2e
/test-all-features-conformance

hongliangl · 2022-04-21T13:01:25Z

/test-networkpolicy
/test-windows-networkpolicy
/test-all-features-conformance

jianjuns · 2022-04-21T16:33:11Z

/test-networkpolicy

hongliangl · 2022-04-21T20:40:13Z

/test-networkpolicy

hongliangl · 2023-01-31T07:05:42Z

pkg/agent/openflow/pipeline.go

-// a local gateway IP to bypass Network Policies. See https://github.com/antrea-io/antrea/issues/280.
-// TODO: Fix it after replacing kube-proxy with AntreaProxy.
-func (f *featurePodConnectivity) localProbeFlow() []binding.Flow {
+// When proxyAll is disabled, the probe packets are identified by matching the source IP is the Antrea gateway IP;


hongliangl requested review from tnqn and jianjuns March 23, 2022 09:12

jianjuns requested a review from wenyingd March 23, 2022 18:45

jianjuns reviewed Mar 23, 2022

View reviewed changes

pkg/agent/openflow/pipeline.go Outdated Show resolved Hide resolved

pkg/agent/openflow/pipeline.go Outdated Show resolved Hide resolved

hongliangl force-pushed the fix-bypassing-local-probe-flows branch from 3cb947c to 863b6ba Compare March 24, 2022 00:15

hongliangl requested a review from jianjuns March 24, 2022 00:16

wenyingd reviewed Mar 24, 2022

View reviewed changes

hongliangl force-pushed the fix-bypassing-local-probe-flows branch from 863b6ba to a43fdc9 Compare March 25, 2022 02:32

hongliangl requested a review from wenyingd March 25, 2022 02:34

wenyingd previously approved these changes Mar 25, 2022

View reviewed changes

hongliangl dismissed wenyingd’s stale review via 8ccc892 April 14, 2022 17:50

hongliangl force-pushed the fix-bypassing-local-probe-flows branch from a43fdc9 to 8ccc892 Compare April 14, 2022 17:50

jianjuns previously approved these changes Apr 14, 2022

View reviewed changes

hongliangl dismissed jianjuns’s stale review via 9c6558f April 16, 2022 00:25

hongliangl force-pushed the fix-bypassing-local-probe-flows branch from 8ccc892 to 9c6558f Compare April 16, 2022 00:25

jianjuns previously approved these changes Apr 18, 2022

View reviewed changes

wenyingd previously approved these changes Apr 19, 2022

View reviewed changes

hongliangl added the action/release-note Indicates a PR that should be included in release notes. label Apr 20, 2022

hongliangl dismissed stale reviews from wenyingd and jianjuns via 23560c1 April 20, 2022 23:41

hongliangl force-pushed the fix-bypassing-local-probe-flows branch from 3153e42 to 23560c1 Compare April 20, 2022 23:41

hongliangl force-pushed the fix-bypassing-local-probe-flows branch from 23560c1 to 5fc6a24 Compare April 20, 2022 23:51

hongliangl requested a review from jianjuns April 21, 2022 13:01

jianjuns approved these changes Apr 21, 2022

View reviewed changes

jianjuns merged commit b93b566 into antrea-io:main Apr 22, 2022

hongliangl deleted the fix-bypassing-local-probe-flows branch April 23, 2022 11:59

Dyanngg mentioned this pull request Apr 29, 2022

Antrea wildcard fqdn netpolicy not working #3680

Closed

tnqn mentioned this pull request May 10, 2022

Automated cherry pick of #3510: Fix the issue of local probe bypassing flows on Windows #3715

Closed

hongliangl commented Jan 31, 2023

View reviewed changes

Fix the issue of local probe bypassing flows on Windows #3510

Fix the issue of local probe bypassing flows on Windows #3510

Conversation

hongliangl commented Mar 23, 2022 • edited

codecov-commenter commented Mar 24, 2022 • edited

Codecov Report

wenyingd Mar 24, 2022

Choose a reason for hiding this comment

hongliangl Mar 24, 2022 • edited

Choose a reason for hiding this comment

wenyingd Mar 25, 2022

Choose a reason for hiding this comment

wenyingd Mar 25, 2022 • edited by hongliangl

Choose a reason for hiding this comment

hongliangl Mar 25, 2022

Choose a reason for hiding this comment

wenyingd left a comment

Choose a reason for hiding this comment

hongliangl commented Mar 27, 2022

hongliangl commented Mar 30, 2022

hongliangl commented Mar 30, 2022

hongliangl commented Apr 14, 2022

hongliangl commented Apr 15, 2022

hongliangl commented Apr 15, 2022

hongliangl commented Apr 15, 2022

hongliangl commented Apr 15, 2022

jianjuns commented Apr 15, 2022

jianjuns commented Apr 15, 2022

hongliangl commented Apr 16, 2022

hongliangl commented Apr 16, 2022

hongliangl commented Apr 16, 2022

hongliangl commented Apr 16, 2022

jianjuns commented Apr 18, 2022

hongliangl commented Apr 19, 2022

wenyingd left a comment

Choose a reason for hiding this comment

jianjuns commented Apr 19, 2022

hongliangl commented Apr 19, 2022

jianjuns commented Apr 20, 2022

hongliangl commented Apr 20, 2022

hongliangl commented Apr 21, 2022

hongliangl commented Apr 21, 2022

hongliangl commented Apr 21, 2022

hongliangl commented Apr 21, 2022

hongliangl commented Apr 21, 2022

jianjuns commented Apr 21, 2022

hongliangl commented Apr 21, 2022

hongliangl Jan 31, 2023

Choose a reason for hiding this comment

hongliangl commented Mar 23, 2022 •

edited

codecov-commenter commented Mar 24, 2022 •

edited

hongliangl Mar 24, 2022 •

edited

wenyingd Mar 25, 2022 •

edited by hongliangl