Support Antrea NetworkPolicy in Traceflow #1361

gran-vmv · 2020-10-12T09:23:40Z

This PR added 3 major changes:

Mirror existing Antrea NetworkPolicy drop flows with action=controller when running TF
Collect drop table and rule when PacketIn
Add dropFlag into ofFlow struct to identify if the flow contains drop action

This PR closes #1225

antrea-bot · 2020-10-12T09:23:56Z

Thanks for your PR.
Unit tests and code linters are run automatically every time the PR is updated.
E2e, conformance and network policy tests can only be triggered by a member of the vmware-tanzu organization. Regular contributors to the project should join the org.

The following commands are available:

/test-e2e: to trigger e2e tests.
/skip-e2e: to skip e2e tests.
/test-conformance: to trigger conformance tests.
/skip-conformance: to skip conformance tests.
/test-whole-conformance: to trigger all conformance tests on linux.
/skip-whole-conformance: to skip all conformance tests on linux.
/test-networkpolicy: to trigger networkpolicy tests.
/skip-networkpolicy: to skip networkpolicy tests.
/test-windows-conformance: to trigger windows conformance tests.
/skip-windows-conformance: to skip windows conformance tests.
/test-windows-networkpolicy: to trigger windows networkpolicy tests.
/skip-windows-networkpolicy: to skip windows networkpolicy tests.
/test-hw-offload: to trigger ovs hardware offload test.
/skip-hw-offload: to skip ovs hardware offload test.
/test-all: to trigger all tests (except whole conformance).
/skip-all: to skip all tests (except whole conformance).

codecov-io · 2020-10-12T09:28:15Z

Codecov Report

Merging #1361 (d3ec61b) into master (d1e0624) will decrease coverage by 13.01%.
The diff coverage is 71.42%.

@@             Coverage Diff             @@
##           master    #1361       +/-   ##
===========================================
- Coverage   67.58%   54.56%   -13.02%     
===========================================
  Files         169      135       -34     
  Lines       13465    12508      -957     
===========================================
- Hits         9100     6825     -2275     
- Misses       3420     5046     +1626     
+ Partials      945      637      -308

Flag	Coverage Δ
integration-tests	`45.64% <31.25%> (-0.09%)`	⬇️
kind-e2e-tests	`?`
unit-tests	`41.38% <63.26%> (-0.37%)`	⬇️

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

Impacted Files	Coverage Δ
pkg/agent/controller/traceflow/packetin.go	`12.33% <57.57%> (-51.85%)`	⬇️
pkg/agent/openflow/client.go	`52.09% <100.00%> (-16.01%)`	⬇️
pkg/ovs/openflow/ofctrl_action.go	`91.30% <100.00%> (+0.08%)`	⬆️
pkg/ovs/openflow/ofctrl_flow.go	`75.00% <100.00%> (+5.18%)`	⬆️
pkg/agent/agent_linux.go	`0.00% <0.00%> (-100.00%)`	⬇️
pkg/apis/controlplane/helper.go	`0.00% <0.00%> (-100.00%)`	⬇️
pkg/apis/controlplane/v1beta2/helper.go	`0.00% <0.00%> (-100.00%)`	⬇️
pkg/agent/proxy/types/groupcounter.go	`0.00% <0.00%> (-95.00%)`	⬇️
pkg/controller/networkpolicy/tier.go	`0.00% <0.00%> (-90.00%)`	⬇️
pkg/apis/controlplane/register.go	`0.00% <0.00%> (-85.72%)`	⬇️
... and 108 more

pkg/agent/controller/traceflow/packetin.go

pkg/agent/openflow/client.go

jianjuns

LGTM

pkg/agent/openflow/client.go

jianjuns

The change LGTM. But hope @wenyingd can review too.

pkg/agent/openflow/pipeline.go

wenyingd · 2020-10-30T08:30:57Z

pkg/agent/openflow/client.go

@@ -738,6 +738,7 @@ func (c *client) InstallTraceflowFlows(dataplaneTag uint8) error {
 	flows := []binding.Flow{}
 	c.conjMatchFlowLock.Lock()
 	defer c.conjMatchFlowLock.Unlock()
+	// Copy default drop rules
 	for _, ctx := range c.globalConjMatchFlowCache {
 		if ctx.dropFlow != nil {
 			flows = append(


Having discussion with @weiqiangt , after metric flow is introduced, the actual "drop" action should happen in the XgressMetricTable, so I think we could add traceflow only in XgressMetricTable with the flow having "dropFlag". Otherwise, the flow in the default drop table is useless.

Think over, as metricflow doesn't record from which policy table the packet is resubmitted in, does it affect the result of Traceflow? I mean we actually want to report that the packet is dropped by K8s Policy Rule or Antrea Policy Rule, but it seems the current implementation would report the packet is dropped by MetricTable

Will the K8s NetworkPolicy use default drop table?
I think Traceflow need to mock all NetworkPolicy flows with "drop" action, including default drop flow for K8s NetworkPolicy and XgressMetricTable for Antrea NetworkPolicy.

We can identify this by the prefix in networkPolicy property. This function is added by @tnqn
Here is my test result:

status: phase: Succeeded results: - node: gran-k8s0-2 observations: - action: Forwarded component: SpoofGuard - action: Forwarded component: NetworkPolicy componentInfo: EgressRule networkPolicy: AntreaNetworkPolicy:default/test-anp - action: Dropped component: NetworkPolicy componentInfo: IngressMetric networkPolicy: AntreaNetworkPolicy:default/test-anp timestamp: 1603965404

pkg/agent/controller/traceflow/packetin_test.go

pkg/ovs/openflow/ofctrl_flow.go

abhiraut · 2020-11-04T22:23:30Z

can we target this for 0.11 release?

gran-vmv · 2020-11-06T01:47:52Z

can we target this for 0.11 release?

Yes, this is for 0.11 release.

antrea-bot · 2020-11-06T09:34:40Z

Thanks for your PR.
Unit tests and code linters are run automatically every time the PR is updated.
E2e, conformance and network policy tests can only be triggered by a member of the vmware-tanzu organization. Regular contributors to the project should join the org.

The following commands are available:

/test-e2e: to trigger e2e tests.
/skip-e2e: to skip e2e tests.
/test-conformance: to trigger conformance tests.
/skip-conformance: to skip conformance tests.
/test-all-features-conformance: to trigger conformance tests with all alpha features enabled.
/skip-all-features-conformance: to skip conformance tests with all alpha features enabled.
/test-whole-conformance: to trigger all conformance tests on linux.
/skip-whole-conformance: to skip all conformance tests on linux.
/test-networkpolicy: to trigger networkpolicy tests.
/skip-networkpolicy: to skip networkpolicy tests.
/test-windows-conformance: to trigger windows conformance tests.
/skip-windows-conformance: to skip windows conformance tests.
/test-windows-networkpolicy: to trigger windows networkpolicy tests.
/skip-windows-networkpolicy: to skip windows networkpolicy tests.
/test-hw-offload: to trigger ovs hardware offload test.
/skip-hw-offload: to skip ovs hardware offload test.
/test-all: to trigger all tests (except whole conformance).
/skip-all: to skip all tests (except whole conformance).

gran-vmv · 2020-11-11T01:50:55Z

/test-all

gran-vmv requested review from wenyingd, tnqn and jianjuns October 12, 2020 09:23

vmwclabot added the cla-not-required label Oct 12, 2020

gran-vmv added the status/WIP Work in progress label Oct 12, 2020

gran-vmv force-pushed the tf-anp branch 5 times, most recently from 6df090b to 31f5a8a Compare October 14, 2020 09:54

jianjuns reviewed Oct 15, 2020

View reviewed changes

pkg/agent/controller/traceflow/packetin.go Outdated Show resolved Hide resolved

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

gran-vmv force-pushed the tf-anp branch from 31f5a8a to 74ddb5d Compare October 15, 2020 02:11

jianjuns reviewed Oct 15, 2020

View reviewed changes

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

gran-vmv force-pushed the tf-anp branch 3 times, most recently from f89af55 to 3598594 Compare October 16, 2020 06:27

gran-vmv requested review from jianjuns and antoninbas October 19, 2020 03:54

gran-vmv removed the status/WIP Work in progress label Oct 20, 2020

gran-vmv force-pushed the tf-anp branch from 3598594 to 1c01858 Compare October 29, 2020 09:54

jianjuns previously approved these changes Oct 29, 2020

View reviewed changes

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

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

gran-vmv dismissed jianjuns’s stale review via 9a0806b October 30, 2020 02:05

gran-vmv force-pushed the tf-anp branch 2 times, most recently from 9a0806b to b81e642 Compare October 30, 2020 08:29

wenyingd reviewed Oct 30, 2020

View reviewed changes

tnqn reviewed Nov 2, 2020

View reviewed changes

pkg/agent/controller/traceflow/packetin_test.go Show resolved Hide resolved

pkg/ovs/openflow/ofctrl_flow.go Show resolved Hide resolved

gran-vmv force-pushed the tf-anp branch 2 times, most recently from 9ee4873 to 0bdc543 Compare November 3, 2020 01:44

gran-vmv requested review from wenyingd, tnqn and jianjuns November 4, 2020 09:40

gran-vmv added this to the Antrea v0.11.0 release milestone Nov 5, 2020

gran-vmv force-pushed the tf-anp branch from 0bdc543 to c7d9ba7 Compare November 6, 2020 09:34

gran-vmv force-pushed the tf-anp branch 3 times, most recently from d8065ef to a653390 Compare November 11, 2020 01:33

Support Antrea NetworkPolicy in Traceflow

d3ec61b

gran-vmv force-pushed the tf-anp branch from a653390 to d3ec61b Compare November 11, 2020 01:41

jianjuns approved these changes Nov 11, 2020

View reviewed changes

gran-vmv merged commit 6c1a913 into antrea-io:master Nov 11, 2020

gran-vmv mentioned this pull request Dec 23, 2020

Traceflow Support CNP Egress/Ingress Tables #1092

Closed

gran-vmv deleted the tf-anp branch January 21, 2021 09:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support Antrea NetworkPolicy in Traceflow #1361

Support Antrea NetworkPolicy in Traceflow #1361

gran-vmv commented Oct 12, 2020

antrea-bot commented Oct 12, 2020

codecov-io commented Oct 12, 2020 •

edited

Loading

jianjuns left a comment

jianjuns left a comment

wenyingd Oct 30, 2020

wenyingd Oct 30, 2020

gran-vmv Oct 30, 2020

gran-vmv Oct 30, 2020

abhiraut commented Nov 4, 2020

gran-vmv commented Nov 6, 2020

antrea-bot commented Nov 6, 2020

gran-vmv commented Nov 11, 2020

Support Antrea NetworkPolicy in Traceflow #1361

Support Antrea NetworkPolicy in Traceflow #1361

Conversation

gran-vmv commented Oct 12, 2020

antrea-bot commented Oct 12, 2020

codecov-io commented Oct 12, 2020 • edited Loading

Codecov Report

jianjuns left a comment

Choose a reason for hiding this comment

jianjuns left a comment

Choose a reason for hiding this comment

wenyingd Oct 30, 2020

Choose a reason for hiding this comment

wenyingd Oct 30, 2020

Choose a reason for hiding this comment

gran-vmv Oct 30, 2020

Choose a reason for hiding this comment

gran-vmv Oct 30, 2020

Choose a reason for hiding this comment

abhiraut commented Nov 4, 2020

gran-vmv commented Nov 6, 2020

antrea-bot commented Nov 6, 2020

gran-vmv commented Nov 11, 2020

codecov-io commented Oct 12, 2020 •

edited

Loading