diff mbox

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

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

Commit Message

O'Reilly, Darragh Nov. 11, 2016, 1:57 p.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>
---
v1->v2 added test

 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. 14, 2016, 12:43 p.m. UTC | #1 
On Fri, Nov 11, 2016 at 01:57:00PM +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>
> ---
> v1->v2 added test

Thanks, I have applied this to the master branch.

Please let me know which if any of branch-2.* you feel it is appropriate for.
O'Reilly, Darragh Nov. 14, 2016, 12:53 p.m. UTC | #2 
Thanks. 

> From: Simon Horman [mailto:simon.horman@netronome.com]
> Sent: 14 November 2016 12:44
> Thanks, I have applied this to the master branch.
> 
> Please let me know which if any of branch-2.* you feel it is appropriate for.

Thanks. The customer is on branch-2.5.
Simon Horman Nov. 15, 2016, 9:11 a.m. UTC | #3 
On Mon, Nov 14, 2016 at 12:53:26PM +0000, O'Reilly, Darragh wrote:
> Thanks. 
> 
> > From: Simon Horman [mailto:simon.horman@netronome.com]
> > Sent: 14 November 2016 12:44
> > Thanks, I have applied this to the master branch.
> > 
> > Please let me know which if any of branch-2.* you feel it is appropriate for.
> 
> Thanks. The customer is on branch-2.5. 

Thanks. I have added this patch to branch-2.6.
Unfortunately the test seems to fail when the patch is applied to
branch-2.5. I am wondering if you could provide an updated patch for that
branch? Perhaps something with the subject-prefix [PATCH branch-v2.5].

Thanks in advance.
O'Reilly, Darragh Nov. 15, 2016, 10:26 a.m. UTC | #4 
> -----Original Message-----
> From: Simon Horman [mailto:simon.horman@netronome.com]
> Sent: 15 November 2016 09:12

> Thanks. I have added this patch to branch-2.6.
> Unfortunately the test seems to fail when the patch is applied to branch-2.5. I
> am wondering if you could provide an updated patch for that branch? Perhaps
> something with the subject-prefix [PATCH branch-v2.5].

Sure will do. It seems the output of dpif/show and the empty udp packet
generated by netdev-dummy/receive has changed a little.
diff mbox

Patch

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index f6391ed..4d10a54 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -2172,11 +2172,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 c170ae7..92380d2 100644
--- a/tests/automake.mk
+++ b/tests/automake.mk
@@ -93,7 +93,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..9cceace
--- /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-internal)
+		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
+01005e5e0101aa55aa550001810006bd08004500001c00000000401180710a000001ef5e010100001f400008e63d
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
diff --git a/tests/testsuite.at b/tests/testsuite.at
index 2123bee..72fa545 100644
--- a/tests/testsuite.at
+++ b/tests/testsuite.at
@@ -75,3 +75,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])