[flexible-ipam] Multiple-VLAN support #3247

gran-vmv · 2022-01-26T09:22:01Z

Support Multiple-VLAN for Antrea FlexibleIPAM.
Update VLAN format from string to integer in CRD.
Traffic from the Pods whose IPPool is configured with VLAN ID will be tagged when leaving Node uplink.
Cross VLAN traffic will be sent to underlay gateway always.
Enable AntreaIPAM will change Antrea DNAT CT zone from 65520 to 4096-8190 (for VLAN 0-4094).

Signed-off-by: gran [email protected]

codecov-commenter · 2022-01-26T11:04:32Z

Codecov Report

Merging #3247 (1c40497) into main (3c88bc2) will decrease coverage by 22.64%.
The diff coverage is 20.19%.

@@             Coverage Diff             @@
##             main    #3247       +/-   ##
===========================================
- Coverage   65.59%   42.95%   -22.65%     
===========================================
  Files         277      209       -68     
  Lines       27264    24803     -2461     
===========================================
- Hits        17884    10653     -7231     
- Misses       7486    13068     +5582     
+ Partials     1894     1082      -812

Flag	Coverage Δ
kind-e2e-tests	`?`
unit-tests	`42.95% <20.19%> (-0.15%)`	⬇️

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

Impacted Files	Coverage Δ
pkg/agent/cniserver/ipam/ipam_delegator.go	`0.00% <0.00%> (-48.84%)`	⬇️
pkg/agent/cniserver/ipam/ipam_service.go	`8.69% <0.00%> (-43.48%)`	⬇️
pkg/agent/cniserver/pod_configuration_linux.go	`0.00% <0.00%> (-66.67%)`	⬇️
pkg/agent/openflow/framework.go	`91.54% <0.00%> (-2.66%)`	⬇️
pkg/ovs/openflow/ofctrl_builder.go	`17.91% <0.00%> (-41.61%)`	⬇️
pkg/ovs/ovsconfig/ovs_client.go	`1.63% <0.00%> (-44.96%)`	⬇️
pkg/agent/openflow/pipeline.go	`24.51% <9.17%> (-52.42%)`	⬇️
pkg/agent/cniserver/pod_configuration.go	`35.11% <31.25%> (-18.41%)`	⬇️
pkg/agent/cniserver/server.go	`46.54% <33.33%> (-19.12%)`	⬇️
pkg/ovs/openflow/ofctrl_action.go	`11.23% <42.10%> (-59.12%)`	⬇️
... and 214 more

pkg/agent/openflow/framework.go

pkg/agent/openflow/pipeline.go

pkg/ovs/openflow/ofctrl_action.go

pkg/agent/openflow/pipeline.go

pkg/agent/openflow/network_policy.go

jianjuns · 2022-03-22T15:36:36Z

pkg/agent/cniserver/ipam/antrea_ipam.go

@@ -143,7 +142,7 @@ func (d *AntreaIPAM) Add(args *invoke.Args, k8sArgs *argtypes.K8sArgs, networkCo

 	klog.V(4).InfoS("IP allocation successful", "IP", ip.String(), "Pod", string(k8sArgs.K8S_POD_NAME))

-	result := IPAMResult{Result: current.Result{CNIVersion: current.ImplementedSpecVersion}, VLANID: parseVLANID(subnetInfo.VLAN)}
+	result := IPAMResult{Result: current.Result{CNIVersion: current.ImplementedSpecVersion}, VLANID: subnetInfo.VLAN & 0xfff}


I feel we should check the VLAN value is valid (<=0xffe) and return an error if not.

Thanks. Added minimum and maximum in CRD.

build/yamls/base/crds.yml

pkg/agent/openflow/pipeline.go

tnqn · 2022-03-23T17:46:45Z

pkg/agent/openflow/pipeline.go

 			Cookie(cookieID).
 			MatchInPort(config.UplinkOFPort).
 			Action().Output(config.BridgeOFPort).
 			Done(),
 		// This generates the flow to forward the packets from bridge local port to uplink port.
-		ClassifierTable.ofTable.BuildFlow(priorityNormal).
+		ClassifierTable.ofTable.BuildFlow(priorityLow).


Is this question addressed? @wenyingd I think you were suggesting to keep priorities added in hostBridgeLocalFlows and podUplinkClassifierFlows unchanged?

tnqn · 2022-03-23T17:52:21Z

pkg/agent/openflow/pipeline.go

 			Cookie(cookieID).
 			MatchInPort(config.BridgeOFPort).
 			Action().Output(config.UplinkOFPort).
 			Done(),
 	}
 }

+// hostBridgeUplinkVLANFlows generates the flows to match VLAN packets from uplink port.
+func (f *featurePodConnectivity) hostBridgeUplinkVLANFlows() []binding.Flow {
+	vlanMask := uint16(0x1000)


what does it mean by using 0x1000 mask? Isn't vlan only using 12bits?

At here, we use it to match packet with any VLAN, this is required for popVLAN action.
OXM_OF_VLAN_VID supports below match cases:

Fully wildcarded Matches any packet, that is, one without an 802.1Q header or with an 802.1Q header with any TCI value. Value 0x0000 (OFPVID_NONE), mask 0xffff (or no mask) Matches only packets without an 802.1Q header. Value 0x1000, mask 0x1000 Matches any packet with an 802.1Q header, regardless of VLAN ID. Value 0x1009, mask 0xffff (or no mask) Match only packets with an 802.1Q header with VLAN ID 9. Value 0x1001, mask 0x1001 Matches only packets that have an 802.1Q header with an odd-numbered VLAN ID. (This is just an example; one can match on any desired VLAN ID bit pattern.)

Using openflow13.OFPVID_PRESENT may be more readable.

Thanks. Fixed.

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

tnqn

LGTM overall

pkg/agent/openflow/fields.go

tnqn · 2022-03-24T06:53:27Z

pkg/agent/openflow/pipeline.go

 			Cookie(cookieID).
 			MatchInPort(config.BridgeOFPort).
 			Action().Output(config.UplinkOFPort).
 			Done(),
 	}
 }

+// hostBridgeUplinkVLANFlows generates the flows to match VLAN packets from uplink port.
+func (f *featurePodConnectivity) hostBridgeUplinkVLANFlows() []binding.Flow {
+	vlanMask := uint16(0x1000)


Using openflow13.OFPVID_PRESENT may be more readable.

pkg/ovs/openflow/ofctrl_builder.go

test/e2e/connectivity_test.go

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

tnqn

LGTM, please squash the commits and include proper description in the commit message.

gran-vmv · 2022-03-24T11:11:15Z

LGTM, please squash the commits and include proper description in the commit message.

Hi Quan,
Updated PR description.
I think you can squash all commits when merging this PR, is it correct?

tnqn · 2022-03-24T12:56:55Z

LGTM, please squash the commits and include proper description in the commit message.

Hi Quan, Updated PR description. I think you can squash all commits when merging this PR, is it correct?

Sure, I can do it for this PR, but it would be better if you could include a proper description in the first commit in the begining and format it following the convention in the future.

gran-vmv · 2022-03-24T14:11:37Z

/test-ipv6-only-networkpolicy

gran-vmv self-assigned this Jan 26, 2022

gran-vmv marked this pull request as draft January 26, 2022 09:22

gran-vmv force-pushed the ipam-vlan branch 2 times, most recently from de3f951 to b1ac1e6 Compare January 26, 2022 09:57

gran-vmv force-pushed the ipam-vlan branch 11 times, most recently from a82b7e9 to 0e42608 Compare January 29, 2022 09:47

gran-vmv force-pushed the ipam-vlan branch 3 times, most recently from 9e81683 to bd62b58 Compare February 10, 2022 02:37

gran-vmv force-pushed the ipam-vlan branch 11 times, most recently from 2d67fd0 to 7738f1a Compare February 22, 2022 03:49

gran-vmv force-pushed the ipam-vlan branch 3 times, most recently from cd88ae1 to ce1eccb Compare March 21, 2022 04:09

hongliangl reviewed Mar 21, 2022

View reviewed changes

gran-vmv force-pushed the ipam-vlan branch 5 times, most recently from 7a165f5 to 3007fc2 Compare March 22, 2022 03:09

jianjuns reviewed Mar 22, 2022

View reviewed changes

gran-vmv force-pushed the ipam-vlan branch 2 times, most recently from 09915d1 to 4f8e755 Compare March 23, 2022 01:59

tnqn reviewed Mar 23, 2022

View reviewed changes

gran-vmv added 2 commits March 24, 2022 10:20

[flexible-ipam] Multiple-VLAN support

d506bc8

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

[flexible-ipam] Use uint16 for VLAN in IPPool

69ab1f6

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

gran-vmv force-pushed the ipam-vlan branch 3 times, most recently from ef1f0fd to b4cad23 Compare March 24, 2022 04:07

tnqn reviewed Mar 24, 2022

View reviewed changes

gran-vmv added 2 commits March 24, 2022 15:23

Address comments

38c9e0e

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

Restore CtZone for AntreaIPAM disabled

6bdb471

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

gran-vmv force-pushed the ipam-vlan branch from b4cad23 to 22e80a5 Compare March 24, 2022 07:37

Fix FlowAggregator

1c40497

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

gran-vmv force-pushed the ipam-vlan branch from 22e80a5 to 1c40497 Compare March 24, 2022 08:28

tnqn approved these changes Mar 24, 2022

View reviewed changes

tnqn merged commit 0e5ca83 into antrea-io:main Mar 24, 2022

tnqn added the action/release-note Indicates a PR that should be included in release notes. label Mar 24, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[flexible-ipam] Multiple-VLAN support #3247

[flexible-ipam] Multiple-VLAN support #3247

gran-vmv commented Jan 26, 2022 •

edited

Loading

codecov-commenter commented Jan 26, 2022 •

edited

Loading

jianjuns Mar 22, 2022

gran-vmv Mar 23, 2022

tnqn Mar 23, 2022

tnqn Mar 23, 2022

gran-vmv Mar 24, 2022

tnqn Mar 24, 2022

gran-vmv Mar 24, 2022

tnqn left a comment

tnqn Mar 24, 2022

tnqn left a comment

gran-vmv commented Mar 24, 2022

tnqn commented Mar 24, 2022

gran-vmv commented Mar 24, 2022

[flexible-ipam] Multiple-VLAN support #3247

[flexible-ipam] Multiple-VLAN support #3247

Conversation

gran-vmv commented Jan 26, 2022 • edited Loading

codecov-commenter commented Jan 26, 2022 • edited Loading

Codecov Report

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

Choose a reason for hiding this comment

tnqn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tnqn left a comment

Choose a reason for hiding this comment

gran-vmv commented Mar 24, 2022

tnqn commented Mar 24, 2022

gran-vmv commented Mar 24, 2022

gran-vmv commented Jan 26, 2022 •

edited

Loading

codecov-commenter commented Jan 26, 2022 •

edited

Loading