Add ICMP support in ACNP and ANP #3472

GraysonWu · 2022-03-18T00:33:43Z

This PR added ICMP support in Antrea-native policy. User could define a policy only enfored on ICMP traffic or ICMP traffic with specific ICMP type or ICMP code.

Added a new field called protocols which contains protocols that are not supported by ports. Currently, only ICMP protocol is in this field.
On the controller side,

Both ports and protocols will be translate into Service of internalNP
Added ICMPType and ICMPCode to Service

On the agent side,

Added a new struct matchPair which contains one matchKey and one matchValue
Added some new MatchKey: MatchICMPType, MatchICMPCode, MatchICMPv6Type and MatchICMPv6Code
Change conjunctiveMatch from contains only one matchKey-matchValue pair to contains a list of matchPair, in order to support flows with multipul match conditions like this: icmp,icmp_type=8,icmp_code=0,action=conjunction(2,3/3)

Provide some examples here:

apiVersion: crd.antrea.io/v1alpha1
kind: ClusterNetworkPolicy
metadata:
  name: acnp-icmp
spec:
    priority: 5
    tier: SecurityOps
    appliedTo:
      - podSelector:
          matchLabels:
            app: client
    egress:
      - action: Drop
        to:
          - podSelector:
              matchLabels:
                app: server
        protocols:
          - icmp:
              icmpType: 8
              icmpCode: 0

With the policy above, these corresponding flows will be installed on the OVS:

table=AntreaPolicyEgressRule, n_packets=0, n_bytes=0, priority=44900,conj_id=2 actions=load:0x2->NXM_NX_REG3[],load:0x1->NXM_NX_REG0[10],resubmit(,EgressMetric)
table=AntreaPolicyEgressRule, n_packets=0, n_bytes=0, priority=44900,ip,nw_src=x.x.x.x actions=conjunction(2,1/3)
table=AntreaPolicyEgressRule, n_packets=0, n_bytes=0, priority=44900,ip,nw_dst=x.x.x.x actions=conjunction(2,2/3)
table=AntreaPolicyEgressRule, n_packets=0, n_bytes=0, priority=44900,icmp,icmp_type=8,icmp_code=0 actions=conjunction(2,3/3)

Signed-off-by: wgrayson [email protected]

GraysonWu · 2022-03-18T00:39:56Z

E2E tests and doc changes are in PR #3635.

lgtm-com · 2022-03-18T01:10:03Z

This pull request introduces 1 alert when merging 1d55c1c into 4db3c18 - view on LGTM.com

new alerts:

1 for Log entries created from user input

lgtm-com · 2022-03-21T19:44:45Z

This pull request introduces 1 alert when merging 17b2cba into 53a5a8d - view on LGTM.com

new alerts:

1 for Log entries created from user input

codecov-commenter · 2022-03-21T20:44:06Z

Codecov Report

Merging #3472 (246f1c5) into main (3aeb404) will decrease coverage by 6.97%.
The diff coverage is 66.66%.

@@            Coverage Diff             @@
##             main    #3472      +/-   ##
==========================================
- Coverage   64.59%   57.62%   -6.98%     
==========================================
  Files         278      392     +114     
  Lines       39363    55339   +15976     
==========================================
+ Hits        25427    31887    +6460     
- Misses      11957    21012    +9055     
- Partials     1979     2440     +461

Flag	Coverage Δ
e2e-tests	`50.30% <39.71%> (?)`
integration-tests	`38.27% <ø> (?)`
kind-e2e-tests	`52.30% <38.46%> (+<0.01%)`	⬆️
unit-tests	`43.86% <65.73%> (+0.05%)`	⬆️

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

Impacted Files	Coverage Δ
pkg/agent/types/networkpolicy.go	`81.08% <ø> (ø)`
pkg/apis/controlplane/types.go	`0.00% <ø> (ø)`
pkg/ovs/openflow/ofctrl_builder.go	`64.19% <7.14%> (+4.51%)`	⬆️
...g/controller/networkpolicy/clusternetworkpolicy.go	`69.21% <50.00%> (ø)`
pkg/agent/openflow/pipeline.go	`77.06% <66.66%> (+1.74%)`	⬆️
pkg/agent/openflow/network_policy.go	`84.88% <70.93%> (-1.03%)`	⬇️
...kg/controller/networkpolicy/antreanetworkpolicy.go	`83.33% <100.00%> (ø)`
pkg/controller/networkpolicy/crd_utils.go	`92.20% <100.00%> (+0.48%)`	⬆️
pkg/controller/networkpolicy/validate.go	`50.62% <100.00%> (ø)`
pkg/controller/stats/aggregator.go	`81.05% <100.00%> (ø)`
... and 139 more

build/yamls/base/crds.yml

pkg/apis/crd/v1alpha1/types.go

jianjuns · 2022-04-08T00:48:13Z

pkg/apis/crd/v1alpha1/types.go

@@ -600,3 +605,18 @@ type NamespacedName struct {
 	Name      string `json:"name,omitempty"`
 	Namespace string `json:"namespace,omitempty"`
 }
+
+// NetworkPolicyProtocol includes all protocols that `ports` can't support. All
+// fields should be used as a stand-alone field. To match all traffic with a


standalone field

What does "standalone field" mean?

It means all fields cannot be used with each other.

pkg/apis/crd/v1alpha1/types.go

tnqn

LGTM overall, @wenyingd could you take a look at openflow change?

pkg/apis/crd/v1alpha1/types.go

pkg/agent/openflow/network_policy.go

tnqn · 2022-04-08T17:10:09Z

@GraysonWu could you add more information to PR description and commit message, e.g. what change is made to API, an usage example?

Add E2E tests for ICMP support PR antrea-io#3472 Signed-off-by: wgrayson <[email protected]>

Add E2E tests and related content in doc for ICMP support PR antrea-io#3472 Signed-off-by: wgrayson <[email protected]>

pkg/agent/openflow/network_policy.go

pkg/apis/controlplane/v1beta2/types.go

pkg/agent/openflow/network_policy.go

Add E2E tests and related content in doc for ICMP support PR antrea-io#3472 Signed-off-by: wgrayson <[email protected]>

tnqn

LGTM, @GraysonWu could you rebase on main so we can merge?

This PR added ICMP support in Antrea-native policy. User could define a policy only enfored on ICMP traffic or ICMP traffic with specific ICMP type or ICMP code. 1. Added a new field called `protocols` which contains protocols that are not supported by `ports`. Currently, only ICMP protocol is in this field. 2. On the controller side, * Both `ports` and `protocols` will be translate into `Service` of internalNP * Added `ICMPType` and `ICMPCode` to `Service` 3. On the agent side, * Added a new struct `matchPair` which contains one matchKey and one matchValue * Added some new `MatchKey`: `MatchICMPType`, `MatchICMPCode`, `MatchICMPv6Type` and `MatchICMPv6Code` * Change `conjunctiveMatch` from contains only one matchKey-matchValue pair to contains a list of `matchPair`, in order to support flows with multipul match conditions like this: `icmp,icmp_type=8,icmp_code=0,action=conjunction(2,3/3)` Signed-off-by: wgrayson <[email protected]>

Add E2E tests and related content in doc for ICMP support PR antrea-io#3472 Signed-off-by: wgrayson <[email protected]>

GraysonWu · 2022-04-19T21:53:53Z

LGTM, @GraysonWu could you rebase on main so we can merge?

Done. Just a friendly reminder: e2e tests and doc update are in PR #3635

pkg/agent/openflow/network_policy.go

pkg/controller/networkpolicy/crd_utils.go

Signed-off-by: wgrayson <[email protected]>

Add E2E tests and related content in doc for ICMP support PR antrea-io#3472 Signed-off-by: wgrayson <[email protected]>

tnqn · 2022-04-21T04:09:35Z

/test-all

GraysonWu · 2022-04-21T21:08:30Z

/test-networkpolicy
/test-hw-offload
/test-e2e
/test-all-features-conformance

tnqn · 2022-04-25T12:37:48Z

/test-networkpolicy
/test-e2e

XinShuYang · 2022-04-25T14:29:02Z

/test-networkpolicy
/test-e2e

Add E2E tests and related content in doc for ICMP support PR antrea-io#3472 Signed-off-by: wgrayson <[email protected]>

Add E2E tests and related content in doc for ICMP support PR #3472 Signed-off-by: wgrayson <[email protected]>

GraysonWu force-pushed the icmp-support branch from d80fdea to 1d55c1c Compare March 18, 2022 00:41

GraysonWu force-pushed the icmp-support branch from 1d55c1c to 17b2cba Compare March 21, 2022 19:14

GraysonWu force-pushed the icmp-support branch from 17b2cba to feb334f Compare March 21, 2022 20:49

Dyanngg reviewed Mar 22, 2022

View reviewed changes

build/yamls/base/crds.yml Outdated Show resolved Hide resolved

GraysonWu force-pushed the icmp-support branch 2 times, most recently from 469861a to c0979a0 Compare March 25, 2022 22:58

GraysonWu changed the title ~~Add ICMP support in ACNP and ANP~~ [WIP]Add ICMP support in ACNP and ANP Mar 31, 2022

GraysonWu force-pushed the icmp-support branch 4 times, most recently from 5ea7717 to ad9e549 Compare April 5, 2022 20:16

GraysonWu changed the title ~~[WIP]Add ICMP support in ACNP and ANP~~ Add ICMP support in ACNP and ANP Apr 5, 2022

GraysonWu force-pushed the icmp-support branch from ad9e549 to 45d9048 Compare April 7, 2022 21:34

GraysonWu requested review from tnqn, antoninbas, Dyanngg and jianjuns April 8, 2022 00:15

jianjuns reviewed Apr 8, 2022

View reviewed changes

GraysonWu force-pushed the icmp-support branch from 45d9048 to ce2851a Compare April 8, 2022 01:38

tnqn reviewed Apr 8, 2022

View reviewed changes

pkg/apis/crd/v1alpha1/types.go Outdated Show resolved Hide resolved

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

tnqn added api-review Categorizes an issue or PR as actively needing an API review. kind/feature Categorizes issue or PR as related to a new feature. action/release-note Indicates a PR that should be included in release notes. labels Apr 8, 2022

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

GraysonWu force-pushed the icmp-support branch from ce2851a to fcc1a9f Compare April 9, 2022 01:23

GraysonWu added a commit to GraysonWu/antrea that referenced this pull request Apr 13, 2022

ICMP support e2e test

9563f19

Add E2E tests for ICMP support PR antrea-io#3472 Signed-off-by: wgrayson <[email protected]>

GraysonWu mentioned this pull request Apr 13, 2022

E2E test of Antrea native policy ICMP support #3635

Merged

GraysonWu added a commit to GraysonWu/antrea that referenced this pull request Apr 14, 2022

ICMP support e2e test

fe8a8f5

Add E2E tests and related content in doc for ICMP support PR antrea-io#3472 Signed-off-by: wgrayson <[email protected]>

wenyingd reviewed Apr 14, 2022

View reviewed changes

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

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

pkg/apis/controlplane/v1beta2/types.go Show resolved Hide resolved

GraysonWu dismissed tnqn’s stale review via 0deda52 April 14, 2022 18:56

GraysonWu force-pushed the icmp-support branch from 177bb9d to 0deda52 Compare April 14, 2022 18:56

tnqn reviewed Apr 15, 2022

View reviewed changes

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

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

GraysonWu force-pushed the icmp-support branch from 0deda52 to 7813aa6 Compare April 15, 2022 21:00

GraysonWu added a commit to GraysonWu/antrea that referenced this pull request Apr 15, 2022

E2E test of Antrea native policy ICMP support

574abfd

Add E2E tests and related content in doc for ICMP support PR antrea-io#3472 Signed-off-by: wgrayson <[email protected]>

GraysonWu added a commit to GraysonWu/antrea that referenced this pull request Apr 15, 2022

E2E test of Antrea native policy ICMP support

cfe8f3f

Add E2E tests and related content in doc for ICMP support PR antrea-io#3472 Signed-off-by: wgrayson <[email protected]>

tnqn previously approved these changes Apr 19, 2022

View reviewed changes

GraysonWu dismissed tnqn’s stale review via eb1a554 April 19, 2022 21:50

GraysonWu force-pushed the icmp-support branch from 7813aa6 to eb1a554 Compare April 19, 2022 21:50

GraysonWu added a commit to GraysonWu/antrea that referenced this pull request Apr 19, 2022

E2E test of Antrea native policy ICMP support

fcb9bed

Add E2E tests and related content in doc for ICMP support PR antrea-io#3472 Signed-off-by: wgrayson <[email protected]>

ceclinux reviewed Apr 20, 2022

View reviewed changes

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

ceclinux reviewed Apr 20, 2022

View reviewed changes

pkg/controller/networkpolicy/crd_utils.go Show resolved Hide resolved

Address comments

246f1c5

Signed-off-by: wgrayson <[email protected]>

GraysonWu added a commit to GraysonWu/antrea that referenced this pull request Apr 20, 2022

E2E test of Antrea native policy ICMP support

047c8c9

Add E2E tests and related content in doc for ICMP support PR antrea-io#3472 Signed-off-by: wgrayson <[email protected]>

GraysonWu requested review from ceclinux and tnqn April 20, 2022 21:10

tnqn approved these changes Apr 21, 2022

View reviewed changes

tnqn merged commit 2921271 into antrea-io:main Apr 25, 2022

GraysonWu added a commit to GraysonWu/antrea that referenced this pull request Apr 25, 2022

E2E test of Antrea native policy ICMP support

d5025f0

Add E2E tests and related content in doc for ICMP support PR antrea-io#3472 Signed-off-by: wgrayson <[email protected]>

tnqn pushed a commit that referenced this pull request May 10, 2022

E2E test of Antrea native policy ICMP support (#3635)

ac44d6b

Add E2E tests and related content in doc for ICMP support PR #3472 Signed-off-by: wgrayson <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ICMP support in ACNP and ANP #3472

Add ICMP support in ACNP and ANP #3472

GraysonWu commented Mar 18, 2022 •

edited

Loading

GraysonWu commented Mar 18, 2022 •

edited

Loading

lgtm-com bot commented Mar 18, 2022

lgtm-com bot commented Mar 21, 2022

codecov-commenter commented Mar 21, 2022 •

edited

Loading

jianjuns Apr 8, 2022

GraysonWu Apr 8, 2022

tnqn left a comment

tnqn commented Apr 8, 2022

tnqn left a comment

GraysonWu commented Apr 19, 2022

tnqn commented Apr 21, 2022

GraysonWu commented Apr 21, 2022

tnqn commented Apr 25, 2022

XinShuYang commented Apr 25, 2022

Add ICMP support in ACNP and ANP #3472

Add ICMP support in ACNP and ANP #3472

Conversation

GraysonWu commented Mar 18, 2022 • edited Loading

GraysonWu commented Mar 18, 2022 • edited Loading

lgtm-com bot commented Mar 18, 2022

lgtm-com bot commented Mar 21, 2022

codecov-commenter commented Mar 21, 2022 • edited Loading

Codecov Report

jianjuns Apr 8, 2022

Choose a reason for hiding this comment

GraysonWu Apr 8, 2022

Choose a reason for hiding this comment

tnqn left a comment

Choose a reason for hiding this comment

tnqn commented Apr 8, 2022

tnqn left a comment

Choose a reason for hiding this comment

GraysonWu commented Apr 19, 2022

tnqn commented Apr 21, 2022

GraysonWu commented Apr 21, 2022

tnqn commented Apr 25, 2022

XinShuYang commented Apr 25, 2022

GraysonWu commented Mar 18, 2022 •

edited

Loading

GraysonWu commented Mar 18, 2022 •

edited

Loading

codecov-commenter commented Mar 21, 2022 •

edited

Loading