diff mbox

[ovs-dev,branch-v2.5] ofproto-dpif-xlate: Fix duplicate multicast packets

Message ID TU4PR84MB00949EFEA83A738463AFF92FE5BF0@TU4PR84MB0094.NAMPRD84.PROD.OUTLOOK.COM
State Accepted
Headers show

Commit Message

O'Reilly, Darragh Nov. 15, 2016, 10:32 a.m. UTC 
When iterating the list of mrouters, skip any that are not on the same
vlan as the multicast packet to be forwarded. This bug was causing
duplicate packets when more than one mrouter was behind a trunk port.

Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2016-November/042938.html
Signed-off-by: Darragh O'Reilly <darragh.oreilly@hpe.com>

---
 ofproto/ofproto-dpif-xlate.c |  5 +++-
 tests/automake.mk            |  3 +-
 tests/mcast-snooping.at      | 67 ++++++++++++++++++++++++++++++++++++++++++++
 tests/testsuite.at           |  1 +
 4 files changed, 74 insertions(+), 2 deletions(-)
 create mode 100644 tests/mcast-snooping.at

Comments

Simon Horman Nov. 17, 2016, 3:43 p.m. UTC | #1 
On Tue, Nov 15, 2016 at 10:32:49AM +0000, O'Reilly, Darragh wrote:
> When iterating the list of mrouters, skip any that are not on the same
> vlan as the multicast packet to be forwarded. This bug was causing
> duplicate packets when more than one mrouter was behind a trunk port.
> 
> Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2016-November/042938.html
> Signed-off-by: Darragh O'Reilly <darragh.oreilly@hpe.com>

Thanks, applied to branch-2.5.

For the record: a similar patch is already present in the branch-2.6 and
master branches.
diff mbox

Patch

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 9537131..c9c614c 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -2239,11 +2239,14 @@  xlate_normal_mcast_send_mrouters(struct xlate_ctx *ctx,
     xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp);
     LIST_FOR_EACH(mrouter, mrouter_node, &ms->mrouter_lru) {
         mcast_xbundle = xbundle_lookup(xcfg, mrouter->port);
-        if (mcast_xbundle && mcast_xbundle != in_xbundle) {
+        if (mcast_xbundle && mcast_xbundle != in_xbundle
+            && mrouter->vlan == vlan) {
             xlate_report(ctx, "forwarding to mcast router port");
             output_normal(ctx, mcast_xbundle, vlan);
         } else if (!mcast_xbundle) {
             xlate_report(ctx, "mcast router port is unknown, dropping");
+        } else if (mrouter->vlan != vlan) {
+            xlate_report(ctx, "mcast router is on another vlan, dropping");
         } else {
             xlate_report(ctx, "mcast router port is input port, dropping");
         }
diff --git a/tests/automake.mk b/tests/automake.mk
index 4dd6598..fa6b2ee 100644
--- a/tests/automake.mk
+++ b/tests/automake.mk
@@ -92,7 +92,8 @@  TESTSUITE_AT = \
 	tests/ovn-nbctl.at \
 	tests/ovn-sbctl.at \
 	tests/ovn-controller.at \
-	tests/ovn-controller-vtep.at
+	tests/ovn-controller-vtep.at \
+	tests/mcast-snooping.at
 
 SYSTEM_KMOD_TESTSUITE_AT = \
 	tests/system-common-macros.at \
diff --git a/tests/mcast-snooping.at b/tests/mcast-snooping.at
new file mode 100644
index 0000000..85b4339
--- /dev/null
+++ b/tests/mcast-snooping.at
@@ -0,0 +1,67 @@ 
+AT_BANNER([mcast snooping])
+
+AT_SETUP([mcast - check multicasts to trunk ports are not duplicated])
+
+OVS_VSWITCHD_START([])
+
+AT_CHECK([
+    ovs-vsctl set bridge br0 \
+    datapath_type=dummy \
+    mcast_snooping_enable=true \
+    other-config:mcast-snooping-disable-flood-unregistered=true
+], [0])
+
+AT_CHECK([ovs-ofctl add-flow br0 action=normal])
+
+# Create an access port p1 on vlan 1725, and a trunk port p2.
+AT_CHECK([
+    ovs-vsctl add-port br0 p1 tag=1725 -- set Interface p1 type=dummy \
+    other-config:hwaddr=aa:55:aa:55:00:01 ofport_request=1 \
+    -- add-port br0 p2 -- set Interface p2 type=dummy \
+    other-config:hwaddr=aa:55:aa:55:00:02 ofport_request=2
+], [0])
+
+AT_CHECK([ovs-appctl dpif/show], [0], [dnl
+dummy@ovs-dummy: hit:0 missed:0
+	br0:
+		br0 65534/100: (dummy)
+		p1 1/1: (dummy)
+		p2 2/2: (dummy)
+])
+
+# Send IGMPv3 query on p2 with vlan 1725
+# 5c:8a:38:55:25:52 > 01:00:5e:00:00:01, ethertype 802.1Q (0x8100), length 64: vlan 1725, p 0, ethertype IPv4,
+# 172.17.25.1 > 224.0.0.1: igmp query v3
+AT_CHECK([ovs-appctl netdev-dummy/receive p2 \
+'01005e0000015c8a38552552810006bd080046c000240000000001027f00ac111901e0000001940400001164ec1e00000000027d000000000000000000000000'])
+
+# Send IGMPv3 query on p2 with vlan 1728
+# 5c:8a:38:55:25:52 > 01:00:5e:00:00:01, ethertype 802.1Q (0x8100), length 64: vlan 1728, p 0, ethertype IPv4,
+# 172.17.28.1 > 224.0.0.1: igmp query v3
+AT_CHECK([ovs-appctl netdev-dummy/receive p2 \
+'01005e0000015c8a38552552810006c0080046c000240000000001027c00ac111c01e0000001940400001164ec1e00000000027d000000000000000000000000'])
+
+AT_CHECK([ovs-appctl mdb/show br0], [0], [dnl
+ port  VLAN  GROUP                Age
+    2  1725  querier               0
+    2  1728  querier               0
+])
+
+AT_CHECK([ovs-vsctl set Interface p2 options:tx_pcap=p2.pcap])
+
+# Send a multicast packet on p1
+AT_CHECK([
+    ovs-appctl netdev-dummy/receive p1 \
+    'in_port(1),eth(src=aa:55:aa:55:00:01,dst=01:00:5e:5e:01:01),eth_type(0x0800),ipv4(src=10.0.0.1,dst=239.94.1.1,proto=17,tos=0,ttl=64,frag=no),udp(src=0,dst=8000)'
+])
+
+# Check this packet was forwarded exactly once to p2 and has vlan tag 1725
+# aa:55:aa:55:00:01 > 01:00:5e:5e:01:01, ethertype 802.1Q (0x8100), length 46: vlan 1725, p 0, ethertype IPv4,
+# 10.0.0.1.0 > 239.94.1.1.8000: UDP, length 0
+AT_CHECK([ovs-pcap p2.pcap > p2.pcap.txt 2>&1])
+AT_CHECK([cat p2.pcap.txt], [0], [dnl
+01005e5e0101aa55aa550001810006bd08004500001c00000000401180710a000001ef5e010100001f4000000000000000000000000000000000000000000000
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
diff --git a/tests/testsuite.at b/tests/testsuite.at
index 28f2295..6fd2dbd 100644
--- a/tests/testsuite.at
+++ b/tests/testsuite.at
@@ -74,3 +74,4 @@  m4_include([tests/ovn-nbctl.at])
 m4_include([tests/ovn-sbctl.at])
 m4_include([tests/ovn-controller.at])
 m4_include([tests/ovn-controller-vtep.at])
+m4_include([tests/mcast-snooping.at])