[Multicast] Multicast group auto-discovery #2652

wenyingd · 2021-08-26T11:26:35Z

Use packetIn/packetOut to collect IGMP membership report and detect
Multicast group.
Add an OpenFlow entry for each Multicast group in which local Pods
have joined to forward the packet normally and output it to
antrea-gw0.
Output the Multicast traffic to antrea-gw0 if no local Pods have
joined the target Multicast group.

Signed-off-by: wenyingd wenyingd@vmware.com

lgtm-com · 2021-08-26T12:24:11Z

This pull request introduces 1 alert when merging 8ac0b68 into f40b4ec - view on LGTM.com

new alerts:

1 for Missing error check

codecov-commenter · 2021-08-26T12:29:24Z

Codecov Report

Merging #2652 (54d9b75) into main (2fcec4a) will decrease coverage by 0.41%.
The diff coverage is 37.10%.

@@            Coverage Diff             @@
##             main    #2652      +/-   ##
==========================================
- Coverage   60.85%   60.43%   -0.42%     
==========================================
  Files         295      297       +2     
  Lines       24897    25279     +382     
==========================================
+ Hits        15150    15278     +128     
- Misses       8096     8330     +234     
- Partials     1651     1671      +20

Flag	Coverage Δ
kind-e2e-tests	`46.97% <3.68%> (-0.70%)`	⬇️
unit-tests	`40.30% <37.22%> (-0.07%)`	⬇️

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

Impacted Files	Coverage Δ
pkg/agent/interfacestore/types.go	`72.72% <ø> (ø)`
pkg/agent/openflow/client.go	`54.65% <0.00%> (-3.11%)`	⬇️
pkg/apiserver/handlers/featuregates/handler.go	`76.00% <ø> (ø)`
pkg/features/antrea_features.go	`16.66% <ø> (ø)`
pkg/ovs/openflow/ofctrl_builder.go	`59.21% <0.00%> (-0.55%)`	⬇️
pkg/ovs/openflow/ofctrl_packetout.go	`79.60% <0.00%> (-6.05%)`	⬇️
pkg/agent/multicast/mcast_discovery.go	`14.06% <14.06%> (ø)`
pkg/agent/interfacestore/interface_cache.go	`75.43% <25.00%> (-5.94%)`	⬇️
pkg/agent/openflow/pipeline.go	`72.91% <25.64%> (-1.45%)`	⬇️
pkg/agent/multicast/mcast_controller.go	`71.24% <71.24%> (ø)`
... and 13 more

lgtm-com · 2021-08-27T09:54:46Z

This pull request introduces 1 alert when merging 9f4a43e into 04619b6 - view on LGTM.com

new alerts:

1 for Missing error check

lgtm-com · 2021-08-30T03:19:44Z

This pull request introduces 1 alert when merging 7742ef9 into d8d70ac - view on LGTM.com

new alerts:

1 for Missing error check

lgtm-com · 2021-09-01T09:05:11Z

This pull request introduces 1 alert when merging b21ade3 into bfa1bc4 - view on LGTM.com

new alerts:

1 for Missing error check

pkg/agent/controller/multicast/mcast_controller.go

lgtm-com · 2021-09-02T07:33:40Z

This pull request introduces 1 alert when merging dc64ca9 into dded211 - view on LGTM.com

new alerts:

1 for Missing error check

pkg/agent/controller/multicast/mcast_controller.go

ceclinux · 2021-09-02T16:10:29Z

pkg/agent/controller/multicast/mcast_controller.go

+const (
+	groupJoin eventType = iota
+	groupLeave
+


Any reason why the query version is 3?

I think IGMP3 is now the main version for IGMP, so I use it as the default setting. Do you think we should send IGMP messages with all versions?

pkg/agent/controller/multicast/mcast_discovery.go

wenyingd · 2021-12-08T02:18:11Z

/test-all
/test-windows-all
/test-ipv6-all
/test-ipv6-only-all

tnqn

LGTM

tnqn · 2021-12-08T03:15:50Z

/test-integration
/test-conformance

tnqn · 2021-12-08T08:42:00Z

@jianjuns do you have more comments?

pkg/ovs/openflow/ofctrl_packetout.go

pkg/agent/openflow/pipeline.go

ceclinux · 2021-12-08T09:23:04Z

pkg/agent/multicast/mcast_controller.go

+	}
+	status := obj.(*GroupMemberStatus)
+	if c.groupHasInstalled(groupKey) {
+		status.mutex.Lock()


Would mind explaining why you need lock and unlock here? I don't think it is necessary

There are two goroutines to operate the groupCache, one is this one to install/uninstall flows or routing entries, and the other is to receive the GroupMemberEvent. If not lock the entry, a race might exist that we find a group is stale and before removing the flows/routing entries in worker, the other goroutine get a new membership report, but worker deletes the entry.

thanks for your explaination

wenyingd · 2021-12-10T01:10:46Z

/test-all

ceclinux · 2021-12-10T05:39:42Z

LGTM

wenyingd · 2021-12-13T02:13:12Z

/test-e2e
/skip-windows-all
/skip-windows-proxyall-e2e
/skip-ipv6-all

wenyingd · 2021-12-13T02:23:45Z

/test-e2e

XinShuYang · 2021-12-13T06:28:40Z

/test-e2e

pkg/ovs/openflow/ofctrl_packetout.go

1. Use packetIn/packetOut to collect IGMP membership report and detect Multicast group. 2. Add an OpenFlow entry for each Multicast group in which local Pods have joined to forward the packet normally and output it to antrea-gw0. 3. Output the Multicast traffic to antrea-gw0 if no local Pods have joined the target Multicast group. Signed-off-by: wenyingd <wenyingd@vmware.com>

wenyingd · 2021-12-14T00:35:30Z

/test-integration
/test-all

wenyingd · 2021-12-14T00:36:54Z

/test-windows-all
/test-ipv6-only-all
/test-ipv6-all

tnqn

LGTM

tnqn · 2022-01-05T10:17:37Z

Due to jenkins testbed issue, I have ran the required checks manually and the results are good.

1. Use packetIn/packetOut to collect IGMP membership report and detect Multicast group. 2. Add an OpenFlow entry for each Multicast group in which local Pods have joined to forward the packet normally and output it to antrea-gw0. 3. Output the Multicast traffic to antrea-gw0 if no local Pods have joined the target Multicast group. Signed-off-by: wenyingd <wenyingd@vmware.com>

wenyingd force-pushed the multicast_autodiscovery branch from 8f9708f to 9cce75d Compare August 26, 2021 11:41

wenyingd changed the title ~~[WIP][Multicast] Multicast client auto-discovery~~ [Multicast] Multicast group auto-discovery Aug 26, 2021

wenyingd force-pushed the multicast_autodiscovery branch from 9cce75d to 8ac0b68 Compare August 26, 2021 11:52

wenyingd force-pushed the multicast_autodiscovery branch from 8ac0b68 to 9f4a43e Compare August 27, 2021 09:28

wenyingd force-pushed the multicast_autodiscovery branch 2 times, most recently from 8ce22df to 7742ef9 Compare August 30, 2021 02:54

wenyingd requested a review from ceclinux August 30, 2021 07:21

wenyingd mentioned this pull request Aug 30, 2021

Multicast Support in Antrea #2251

Closed

12 tasks

wenyingd force-pushed the multicast_autodiscovery branch from 7742ef9 to b21ade3 Compare September 1, 2021 08:36

weiqiangt reviewed Sep 2, 2021

View reviewed changes

pkg/agent/controller/multicast/mcast_controller.go Outdated Show resolved Hide resolved

wenyingd force-pushed the multicast_autodiscovery branch from b21ade3 to dc64ca9 Compare September 2, 2021 07:05

wenyingd force-pushed the multicast_autodiscovery branch 3 times, most recently from 38fc4c6 to dccac0e Compare September 2, 2021 09:16

ceclinux reviewed Sep 3, 2021

View reviewed changes

wenyingd force-pushed the multicast_autodiscovery branch from dccac0e to 13c8a50 Compare September 7, 2021 09:31

wenyingd force-pushed the multicast_autodiscovery branch 5 times, most recently from d7b5c56 to dc14722 Compare September 17, 2021 11:47

wenyingd force-pushed the multicast_autodiscovery branch from dc14722 to 2a42595 Compare October 8, 2021 13:03

wenyingd requested review from antoninbas and jianjuns October 11, 2021 01:20

wenyingd force-pushed the multicast_autodiscovery branch from 60e0883 to 85422c7 Compare December 8, 2021 02:05

tnqn previously approved these changes Dec 8, 2021

View reviewed changes

ceclinux reviewed Dec 8, 2021

View reviewed changes

pkg/ovs/openflow/ofctrl_packetout.go Outdated Show resolved Hide resolved

ceclinux reviewed Dec 8, 2021

View reviewed changes

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

ceclinux reviewed Dec 8, 2021

View reviewed changes

wenyingd dismissed tnqn’s stale review via dfb0cd6 December 10, 2021 00:41

wenyingd force-pushed the multicast_autodiscovery branch from 85422c7 to dfb0cd6 Compare December 10, 2021 00:41

wenyingd requested review from ceclinux and tnqn December 10, 2021 00:52

tnqn reviewed Dec 13, 2021

View reviewed changes

pkg/ovs/openflow/ofctrl_packetout.go Outdated Show resolved Hide resolved

wenyingd force-pushed the multicast_autodiscovery branch from dfb0cd6 to 54d9b75 Compare December 14, 2021 00:02

wenyingd requested a review from tnqn December 14, 2021 00:12

tnqn approved these changes Dec 14, 2021

View reviewed changes

tnqn added this to the Antrea v1.5 release milestone Jan 4, 2022

tnqn merged commit 6903eea into antrea-io:main Jan 5, 2022

ceclinux mentioned this pull request Apr 1, 2022

[WIP Multicast] Add IPv6 support for Multicast #3572

Closed

3 tasks

wenyingd deleted the multicast_autodiscovery branch June 13, 2022 08:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Multicast] Multicast group auto-discovery #2652

[Multicast] Multicast group auto-discovery #2652

wenyingd commented Aug 26, 2021 •

edited

Loading

lgtm-com bot commented Aug 26, 2021

codecov-commenter commented Aug 26, 2021 •

edited

Loading

lgtm-com bot commented Aug 27, 2021

lgtm-com bot commented Aug 30, 2021

lgtm-com bot commented Sep 1, 2021

lgtm-com bot commented Sep 2, 2021

ceclinux Sep 2, 2021

wenyingd Oct 11, 2021

wenyingd commented Dec 8, 2021

tnqn left a comment

tnqn commented Dec 8, 2021

tnqn commented Dec 8, 2021

ceclinux Dec 8, 2021

wenyingd Dec 10, 2021

ceclinux Dec 10, 2021

wenyingd commented Dec 10, 2021

ceclinux commented Dec 10, 2021

wenyingd commented Dec 13, 2021

wenyingd commented Dec 13, 2021

XinShuYang commented Dec 13, 2021

wenyingd commented Dec 14, 2021

wenyingd commented Dec 14, 2021

tnqn left a comment

tnqn commented Jan 5, 2022

[Multicast] Multicast group auto-discovery #2652

[Multicast] Multicast group auto-discovery #2652

Conversation

wenyingd commented Aug 26, 2021 • edited Loading

lgtm-com bot commented Aug 26, 2021

codecov-commenter commented Aug 26, 2021 • edited Loading

Codecov Report

lgtm-com bot commented Aug 27, 2021

lgtm-com bot commented Aug 30, 2021

lgtm-com bot commented Sep 1, 2021

lgtm-com bot commented Sep 2, 2021

ceclinux Sep 2, 2021

Choose a reason for hiding this comment

wenyingd Oct 11, 2021

Choose a reason for hiding this comment

wenyingd commented Dec 8, 2021

tnqn left a comment

Choose a reason for hiding this comment

tnqn commented Dec 8, 2021

tnqn commented Dec 8, 2021

ceclinux Dec 8, 2021

Choose a reason for hiding this comment

wenyingd Dec 10, 2021

Choose a reason for hiding this comment

ceclinux Dec 10, 2021

Choose a reason for hiding this comment

wenyingd commented Dec 10, 2021

ceclinux commented Dec 10, 2021

wenyingd commented Dec 13, 2021

wenyingd commented Dec 13, 2021

XinShuYang commented Dec 13, 2021

wenyingd commented Dec 14, 2021

wenyingd commented Dec 14, 2021

tnqn left a comment

Choose a reason for hiding this comment

tnqn commented Jan 5, 2022

wenyingd commented Aug 26, 2021 •

edited

Loading

codecov-commenter commented Aug 26, 2021 •

edited

Loading