Implement traffic control interfaces via OVS #3580

hongliangl · 2022-04-02T11:42:33Z

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

codecov-commenter · 2022-04-02T12:48:11Z

Codecov Report

Merging #3580 (e984e42) into main (00f9d98) will decrease coverage by 0.88%.
The diff coverage is 9.09%.

@@            Coverage Diff             @@
##             main    #3580      +/-   ##
==========================================
- Coverage   64.58%   63.70%   -0.89%     
==========================================
  Files         278      278              
  Lines       39517    39604      +87     
==========================================
- Hits        25523    25229     -294     
- Misses      12011    12428     +417     
+ Partials     1983     1947      -36

Flag	Coverage Δ
e2e-tests	`50.13% <9.09%> (?)`
kind-e2e-tests	`50.51% <9.09%> (-1.75%)`	⬇️
unit-tests	`43.73% <8.08%> (-0.06%)`	⬇️

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

Impacted Files	Coverage Δ
pkg/agent/openflow/framework.go	`87.00% <0.00%> (-2.70%)`	⬇️
pkg/agent/openflow/pod_connectivity.go	`42.26% <5.79%> (-26.06%)`	⬇️
pkg/agent/openflow/client.go	`70.70% <8.33%> (-1.64%)`	⬇️
pkg/agent/openflow/pipeline.go	`72.54% <100.00%> (-2.35%)`	⬇️
pkg/controller/egress/store/egressgroup.go	`1.72% <0.00%> (-54.32%)`	⬇️
pkg/agent/route/route_linux.go	`30.17% <0.00%> (-19.63%)`	⬇️
pkg/agent/util/net.go	`32.44% <0.00%> (-19.12%)`	⬇️
pkg/agent/nodeportlocal/k8s/annotations.go	`84.44% <0.00%> (-8.89%)`	⬇️
pkg/agent/util/ipset/ipset.go	`59.45% <0.00%> (-8.11%)`	⬇️
pkg/agent/openflow/service.go	`78.16% <0.00%> (-5.75%)`	⬇️
... and 22 more

hongliangl · 2022-04-09T05:07:35Z

Before merging this PR, issue #3583 should be fixed.

jianjuns

Please add commit message to describe the change.

pkg/ovs/openflow/interfaces.go

pkg/agent/openflow/pipeline.go

pkg/agent/openflow/client.go

pkg/agent/openflow/pod_connectivity.go

pkg/agent/openflow/client.go

pkg/agent/openflow/fields.go

pkg/agent/openflow/pipeline.go

pkg/agent/openflow/client.go

pkg/agent/openflow/pod_connectivity.go

cmd/antrea-agent/agent.go

tnqn

LGTM overall

pkg/agent/openflow/fields.go

test/integration/agent/openflow_test.go

pkg/agent/openflow/fields.go

wenyingd · 2022-04-28T03:18:36Z

pkg/agent/openflow/pod_connectivity.go

+			Action().OutputToRegField(TrafficControlTargetOFPortField).
+			Done(),
+		// This generates the flow to output the packets to be redirected to the target traffic control port.
+		L2ForwardingOutTable.ofTable.BuildFlow(priorityHigh+1).


Question: A packet is either mirrored or redirected, but not both mirrored and redirected?

A packet should be either mirrored or redirected.

wenyingd · 2022-04-28T03:51:35Z

pkg/agent/openflow/client.go

+	cacheKey := fmt.Sprintf("tc_%s", name)
+	c.replayMutex.RLock()
+	defer c.replayMutex.RUnlock()
+	return c.modifyFlows(c.featurePodConnectivity.tcCachedFlows, cacheKey, flows)


Using modifyFlows, it might delete the original tc flows which are not included in the flows generated by this call. I think your target is to append these flows in the cache but not overwrite?

Since a traffic rule could be updated, the original tc flows should be deleted and new flows should be installed. Flows in the cache should be also overwritten.

So do you mean InstallTrafficControlMarkFlows is called for both add and update case? And UninstallTrafficControlMarkFlows is called only when delete the tc configuration?

wenyingd · 2022-04-28T03:55:15Z

pkg/agent/openflow/pod_connectivity.go

+		Cookie(f.cookieAllocator.Request(f.category).Raw()).
+		MatchInPort(returnOFPort).
+		Action().LoadRegMark(FromTCReturnRegMark).
+		Action().GotoStage(stageRouting).


Why not go to stageSwitching since the dst MAC in the packet is set before output to the redirected port?

I'll update the comments here

// trafficControlReturnClassifierFlow generates the flow to mark the packets from traffic control return port and forward // the packets to stageRouting directly. Note that, for the packets which are originally to be output to a tunnel port, // value of NXM_NX_TUN_IPV4_DST for the returned packets needs to be loaded in stageRouting.

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

hongliangl · 2022-04-28T11:43:27Z

/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

tnqn

LGTM, thanks

hongliangl · 2022-04-28T14:06:10Z

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

hongliangl · 2022-04-28T15:10:09Z

/test-ipv6-only-conformance

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

/test-ipv6-e2e

hongliangl · 2022-04-29T00:43:25Z

@jianjuns @wenyingd Do you have any further comments? Thanks.

jianjuns · 2022-04-29T00:44:21Z

@jianjuns @wenyingd Do you have any further comments? Thanks.
Not from me (I did go through one more time).

tnqn · 2022-05-05T16:31:12Z

tested the PR with a network analyzer and both mirroring and redirecting worked as expected, merging it.

hongliangl · 2022-05-05T23:29:56Z

Thanks for testing and merging this.

hongliangl changed the title ~~Add IDS/IPS support in data path.~~ [WIP] Add IDS/IPS support in data path. Apr 2, 2022

hongliangl requested review from wenqiq, wenyingd and tnqn April 2, 2022 11:43

hongliangl changed the title ~~[WIP] Add IDS/IPS support in data path.~~ [WIP] Add IDS/IPS support in data path Apr 2, 2022

hongliangl force-pushed the antrea-l7-datapath branch 3 times, most recently from 8a55b2f to 9f6ca48 Compare April 6, 2022 23:57

hongliangl added this to the Antrea v1.7 release milestone Apr 8, 2022

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

hongliangl changed the title ~~[WIP] Add IDS/IPS support in data path~~ [WIP] Add third-party engine traffic scan support in data path Apr 8, 2022

hongliangl changed the title ~~[WIP] Add third-party engine traffic scan support in data path~~ [WIP] Implement traffic control interfaces via OVS Apr 8, 2022

hongliangl force-pushed the antrea-l7-datapath branch from 9f6ca48 to edea083 Compare April 9, 2022 05:02

hongliangl marked this pull request as ready for review April 9, 2022 05:07

hongliangl changed the title ~~[WIP] Implement traffic control interfaces via OVS~~ Implement traffic control interfaces via OVS Apr 9, 2022

hongliangl requested a review from jianjuns April 9, 2022 05:07

hongliangl force-pushed the antrea-l7-datapath branch from edea083 to 96d01ca Compare April 11, 2022 02:18

jianjuns reviewed Apr 11, 2022

View reviewed changes

hongliangl requested a review from jianjuns April 12, 2022 02:05

hongliangl force-pushed the antrea-l7-datapath branch 3 times, most recently from 2f28158 to b8c30d6 Compare April 12, 2022 04:51

wenqiq reviewed Apr 12, 2022

View reviewed changes

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

hongliangl force-pushed the antrea-l7-datapath branch 2 times, most recently from 1333d66 to e630e13 Compare April 12, 2022 09:52

wenyingd reviewed Apr 12, 2022

View reviewed changes

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

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

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

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

hongliangl force-pushed the antrea-l7-datapath branch 2 times, most recently from 365f341 to db3e5f6 Compare April 12, 2022 16:20

tnqn reviewed Apr 27, 2022

View reviewed changes

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

tnqn reviewed Apr 27, 2022

View reviewed changes

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

hongliangl force-pushed the antrea-l7-datapath branch from 844ae80 to 177c38d Compare April 27, 2022 13:08

hongliangl requested a review from tnqn April 27, 2022 13:08

hongliangl force-pushed the antrea-l7-datapath branch 3 times, most recently from 04d3598 to bde339f Compare April 27, 2022 13:14

hongliangl requested a review from jianjuns April 27, 2022 13:15

tnqn reviewed Apr 27, 2022

View reviewed changes

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

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

cmd/antrea-agent/agent.go Outdated Show resolved Hide resolved

hongliangl force-pushed the antrea-l7-datapath branch 2 times, most recently from fd7c42c to 276ffde Compare April 27, 2022 15:51

hongliangl requested a review from tnqn April 27, 2022 15:53

tnqn reviewed Apr 27, 2022

View reviewed changes

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

test/integration/agent/openflow_test.go Show resolved Hide resolved

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

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

hongliangl force-pushed the antrea-l7-datapath branch from 276ffde to 2d6c412 Compare April 28, 2022 02:26

hongliangl requested a review from tnqn April 28, 2022 02:28

wenyingd reviewed Apr 28, 2022

View reviewed changes

Implement traffic control interfaces via OVS

e984e42

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

hongliangl force-pushed the antrea-l7-datapath branch from 2d6c412 to e984e42 Compare April 28, 2022 04:21

hongliangl requested a review from wenyingd April 28, 2022 04:22

tnqn approved these changes Apr 28, 2022

View reviewed changes

tnqn merged commit 0d9a4b1 into antrea-io:main May 5, 2022

hongliangl deleted the antrea-l7-datapath branch May 5, 2022 23:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement traffic control interfaces via OVS #3580

Implement traffic control interfaces via OVS #3580

hongliangl commented Apr 2, 2022 •

edited by tnqn

codecov-commenter commented Apr 2, 2022 •

edited

hongliangl commented Apr 9, 2022

jianjuns left a comment

tnqn left a comment

wenyingd Apr 28, 2022

hongliangl Apr 28, 2022

wenyingd Apr 28, 2022 •

edited

hongliangl Apr 28, 2022

wenyingd Apr 28, 2022

hongliangl Apr 28, 2022

wenyingd Apr 28, 2022

hongliangl Apr 28, 2022

hongliangl commented Apr 28, 2022

tnqn left a comment

hongliangl commented Apr 28, 2022

hongliangl commented Apr 28, 2022

hongliangl commented Apr 29, 2022

hongliangl commented Apr 29, 2022

jianjuns commented Apr 29, 2022

tnqn commented May 5, 2022

hongliangl commented May 5, 2022

Implement traffic control interfaces via OVS #3580

Implement traffic control interfaces via OVS #3580

Conversation

hongliangl commented Apr 2, 2022 • edited by tnqn

codecov-commenter commented Apr 2, 2022 • edited

Codecov Report

hongliangl commented Apr 9, 2022

jianjuns left a comment

Choose a reason for hiding this comment

tnqn left a comment

Choose a reason for hiding this comment

wenyingd Apr 28, 2022

Choose a reason for hiding this comment

hongliangl Apr 28, 2022

Choose a reason for hiding this comment

wenyingd Apr 28, 2022 • edited

Choose a reason for hiding this comment

hongliangl Apr 28, 2022

Choose a reason for hiding this comment

wenyingd Apr 28, 2022

Choose a reason for hiding this comment

hongliangl Apr 28, 2022

Choose a reason for hiding this comment

wenyingd Apr 28, 2022

Choose a reason for hiding this comment

hongliangl Apr 28, 2022

Choose a reason for hiding this comment

hongliangl commented Apr 28, 2022

tnqn left a comment

Choose a reason for hiding this comment

hongliangl commented Apr 28, 2022

hongliangl commented Apr 28, 2022

hongliangl commented Apr 29, 2022

hongliangl commented Apr 29, 2022

jianjuns commented Apr 29, 2022

tnqn commented May 5, 2022

hongliangl commented May 5, 2022

hongliangl commented Apr 2, 2022 •

edited by tnqn

codecov-commenter commented Apr 2, 2022 •

edited

wenyingd Apr 28, 2022 •

edited