Re: [ovs-dev] [PATCH ovn] ovn-northd: Fix IP local multicast flooding.

2020-02-27 Thread Dumitru Ceara
On 2/27/20 8:52 AM, Numan Siddique wrote:
> On Thu, Feb 27, 2020 at 2:24 AM Mark Michelson  wrote:
>>
>> Acked-by: Mark Michelson 
> 
> Thanks Dumitru and Mark.
> 
> I applied this patch to master and branch-20.03
> 
> Thanks
> Numan
> 

Thanks Mark and Numan!

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH ovn] ovn-northd: Fix IP local multicast flooding.

2020-02-26 Thread Numan Siddique
On Thu, Feb 27, 2020 at 2:24 AM Mark Michelson  wrote:
>
> Acked-by: Mark Michelson 

Thanks Dumitru and Mark.

I applied this patch to master and branch-20.03

Thanks
Numan

>
> On 2/14/20 5:42 AM, Dumitru Ceara wrote:
> > Skip IGMP and MLD entries learned for local multicast groups when
> > generating logical flows. We still allow ovn-controller to learn them as
> > it might be useful information for administrators to see that hosts
> > register for the groups even though they are not expected to send JOIN
> > messages for this range.
> >
> > Fixes: ddc64665b678 ("OVN: Add ovn-northd IGMP support")
> > Reported-by: Lucas Alvares Gomes 
> > Reported-at: https://bugzilla.redhat.com/1803008
> > Signed-off-by: Dumitru Ceara 
> > ---
> >   northd/ovn-northd.c |  15 +++
> >   tests/ovn.at| 119 
> > +---
> >   2 files changed, 109 insertions(+), 25 deletions(-)
> >
> > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> > index 46521b5..6d95ee2 100644
> > --- a/northd/ovn-northd.c
> > +++ b/northd/ovn-northd.c
> > @@ -6608,6 +6608,15 @@ build_lswitch_flows(struct hmap *datapaths, struct 
> > hmap *ports,
> >   &igmp_group->datapath->mcast_info.sw;
> >
> >   if (IN6_IS_ADDR_V4MAPPED(&igmp_group->address)) {
> > +/* RFC 4541, section 2.1.2, item 2: Skip groups in the 
> > 224.0.0.X
> > + * range.
> > + */
> > +ovs_be32 group_address =
> > +in6_addr_get_mapped_ipv4(&igmp_group->address);
> > +if (ip_is_local_multicast(group_address)) {
> > +continue;
> > +}
> > +
> >   if (mcast_sw_info->active_v4_flows >= 
> > mcast_sw_info->table_size) {
> >   continue;
> >   }
> > @@ -6615,6 +6624,12 @@ build_lswitch_flows(struct hmap *datapaths, struct 
> > hmap *ports,
> >   ds_put_format(&match, "eth.mcast && ip4 && ip4.dst == %s ",
> > igmp_group->mcgroup.name);
> >   } else {
> > +/* RFC 4291, section 2.7.1: Skip groups that correspond to all
> > + * hosts.
> > + */
> > +if (ipv6_is_all_hosts(&igmp_group->address)) {
> > +continue;
> > +}
> >   if (mcast_sw_info->active_v6_flows >= 
> > mcast_sw_info->table_size) {
> >   continue;
> >   }
> > diff --git a/tests/ovn.at b/tests/ovn.at
> > index ff9a732..d2f1101 100644
> > --- a/tests/ovn.at
> > +++ b/tests/ovn.at
> > @@ -15434,7 +15434,7 @@ ovn-nbctl set Logical_Switch sw1   \
> >   other_config:mcast_snoop="true"
> >
> >   # No IGMP query should be generated by sw1 (mcast_querier="false").
> > -truncate -s 0 expected
> > +> expected
> >   OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [expected])
> >   OVN_CHECK_PACKETS([hv1/vif2-tx.pcap], [expected])
> >   OVN_CHECK_PACKETS([hv2/vif1-tx.pcap], [expected])
> > @@ -15454,14 +15454,14 @@ send_igmp_v3_report hv2-vif1 hv2 0002 
> > $(ip_to_hex 10 0 0 2) f9f9 \
> >
> >   # Check that the IGMP Group is learned on both hv.
> >   OVS_WAIT_UNTIL([
> > -total_entries=`ovn-sbctl find IGMP_Group | grep "239.0.1.68" | wc -l`
> > +total_entries=`ovn-sbctl find IGMP_Group | grep "239.0.1.68" -c`
> >   test "${total_entries}" = "2"
> >   ])
> >
> >   # Send traffic and make sure it gets forwarded only on the two ports that
> >   # joined.
> > -truncate -s 0 expected
> > -truncate -s 0 expected_empty
> > +> expected
> > +> expected_empty
> >   send_ip_multicast_pkt hv1-vif2 hv1 \
> >   0001 01005e000144 \
> >   $(ip_to_hex 10 0 0 42) $(ip_to_hex 239 0 1 68) 1e 20 ca70 11 \
> > @@ -15486,15 +15486,15 @@ send_igmp_v3_report hv1-vif1 hv1 \
> >
> >   # Check IGMP_Group table on both HV.
> >   OVS_WAIT_UNTIL([
> > -total_entries=`ovn-sbctl find IGMP_Group | grep "239.0.1.68" | wc -l`
> > +total_entries=`ovn-sbctl find IGMP_Group | grep "239.0.1.68" -c`
> >   test "${total_entries}" = "1"
> >   ])
> >
> >   # Send traffic and make sure it gets forwarded only on the port that 
> > joined.
> >   as hv1 reset_pcap_file hv1-vif1 hv1/vif1
> >   as hv2 reset_pcap_file hv2-vif1 hv2/vif1
> > -truncate -s 0 expected
> > -truncate -s 0 expected_empty
> > +> expected
> > +> expected_empty
> >   send_ip_multicast_pkt hv1-vif2 hv1 \
> >   0001 01005e000144 \
> >   $(ip_to_hex 10 0 0 42) $(ip_to_hex 239 0 1 68) 1e 20 ca70 11 \
> > @@ -15514,10 +15514,43 @@ OVN_CHECK_PACKETS([hv2/vif3-tx.pcap], 
> > [expected_empty])
> >   # Flush IGMP groups.
> >   ovn-sbctl ip-multicast-flush sw1
> >   OVS_WAIT_UNTIL([
> > -total_entries=`ovn-sbctl find IGMP_Group | grep "239.0.1.68" | wc -l`
> > +total_entries=`ovn-sbctl find IGMP_Group | grep "239.0.1.68" -c`
> >   test "${total_entries}" = "0"
> >   ])
> >
> > +# Check that traffic for 224.0.0.X is flooded even if some hosts register 
> > for
> > +# it.
> > +# Inj

Re: [ovs-dev] [PATCH ovn] ovn-northd: Fix IP local multicast flooding.

2020-02-26 Thread Mark Michelson

Acked-by: Mark Michelson 

On 2/14/20 5:42 AM, Dumitru Ceara wrote:

Skip IGMP and MLD entries learned for local multicast groups when
generating logical flows. We still allow ovn-controller to learn them as
it might be useful information for administrators to see that hosts
register for the groups even though they are not expected to send JOIN
messages for this range.

Fixes: ddc64665b678 ("OVN: Add ovn-northd IGMP support")
Reported-by: Lucas Alvares Gomes 
Reported-at: https://bugzilla.redhat.com/1803008
Signed-off-by: Dumitru Ceara 
---
  northd/ovn-northd.c |  15 +++
  tests/ovn.at| 119 +---
  2 files changed, 109 insertions(+), 25 deletions(-)

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 46521b5..6d95ee2 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -6608,6 +6608,15 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap 
*ports,
  &igmp_group->datapath->mcast_info.sw;
  
  if (IN6_IS_ADDR_V4MAPPED(&igmp_group->address)) {

+/* RFC 4541, section 2.1.2, item 2: Skip groups in the 224.0.0.X
+ * range.
+ */
+ovs_be32 group_address =
+in6_addr_get_mapped_ipv4(&igmp_group->address);
+if (ip_is_local_multicast(group_address)) {
+continue;
+}
+
  if (mcast_sw_info->active_v4_flows >= mcast_sw_info->table_size) {
  continue;
  }
@@ -6615,6 +6624,12 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap 
*ports,
  ds_put_format(&match, "eth.mcast && ip4 && ip4.dst == %s ",
igmp_group->mcgroup.name);
  } else {
+/* RFC 4291, section 2.7.1: Skip groups that correspond to all
+ * hosts.
+ */
+if (ipv6_is_all_hosts(&igmp_group->address)) {
+continue;
+}
  if (mcast_sw_info->active_v6_flows >= mcast_sw_info->table_size) {
  continue;
  }
diff --git a/tests/ovn.at b/tests/ovn.at
index ff9a732..d2f1101 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -15434,7 +15434,7 @@ ovn-nbctl set Logical_Switch sw1   \
  other_config:mcast_snoop="true"
  
  # No IGMP query should be generated by sw1 (mcast_querier="false").

-truncate -s 0 expected
+> expected
  OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [expected])
  OVN_CHECK_PACKETS([hv1/vif2-tx.pcap], [expected])
  OVN_CHECK_PACKETS([hv2/vif1-tx.pcap], [expected])
@@ -15454,14 +15454,14 @@ send_igmp_v3_report hv2-vif1 hv2 0002 
$(ip_to_hex 10 0 0 2) f9f9 \
  
  # Check that the IGMP Group is learned on both hv.

  OVS_WAIT_UNTIL([
-total_entries=`ovn-sbctl find IGMP_Group | grep "239.0.1.68" | wc -l`
+total_entries=`ovn-sbctl find IGMP_Group | grep "239.0.1.68" -c`
  test "${total_entries}" = "2"
  ])
  
  # Send traffic and make sure it gets forwarded only on the two ports that

  # joined.
-truncate -s 0 expected
-truncate -s 0 expected_empty
+> expected
+> expected_empty
  send_ip_multicast_pkt hv1-vif2 hv1 \
  0001 01005e000144 \
  $(ip_to_hex 10 0 0 42) $(ip_to_hex 239 0 1 68) 1e 20 ca70 11 \
@@ -15486,15 +15486,15 @@ send_igmp_v3_report hv1-vif1 hv1 \
  
  # Check IGMP_Group table on both HV.

  OVS_WAIT_UNTIL([
-total_entries=`ovn-sbctl find IGMP_Group | grep "239.0.1.68" | wc -l`
+total_entries=`ovn-sbctl find IGMP_Group | grep "239.0.1.68" -c`
  test "${total_entries}" = "1"
  ])
  
  # Send traffic and make sure it gets forwarded only on the port that joined.

  as hv1 reset_pcap_file hv1-vif1 hv1/vif1
  as hv2 reset_pcap_file hv2-vif1 hv2/vif1
-truncate -s 0 expected
-truncate -s 0 expected_empty
+> expected
+> expected_empty
  send_ip_multicast_pkt hv1-vif2 hv1 \
  0001 01005e000144 \
  $(ip_to_hex 10 0 0 42) $(ip_to_hex 239 0 1 68) 1e 20 ca70 11 \
@@ -15514,10 +15514,43 @@ OVN_CHECK_PACKETS([hv2/vif3-tx.pcap], 
[expected_empty])
  # Flush IGMP groups.
  ovn-sbctl ip-multicast-flush sw1
  OVS_WAIT_UNTIL([
-total_entries=`ovn-sbctl find IGMP_Group | grep "239.0.1.68" | wc -l`
+total_entries=`ovn-sbctl find IGMP_Group | grep "239.0.1.68" -c`
  test "${total_entries}" = "0"
  ])
  
+# Check that traffic for 224.0.0.X is flooded even if some hosts register for

+# it.
+# Inject IGMP Join for 224.0.0.42 on sw1-p11.
+send_igmp_v3_report hv1-vif1 hv1 \
+0001 $(ip_to_hex 10 0 0 1) f9f8 \
+$(ip_to_hex 224 0 0 42) 04 f9d3 \
+/dev/null
+
+# Check that the IGMP Group is learned.
+OVS_WAIT_UNTIL([
+total_entries=`ovn-sbctl find IGMP_Group | grep "224.0.0.42" -c`
+test "${total_entries}" = "1"
+])
+
+# Send traffic and make sure it gets flooded to all ports.
+as hv1 reset_pcap_file hv1-vif1 hv1/vif1
+as hv1 reset_pcap_file hv1-vif2 hv1/vif2
+as hv2 reset_pcap_file hv2-vif1 hv2/vif1
+as hv2 reset_pcap_file hv2-vif2 hv2/vif2
+> expected
+send_ip_multica

Re: [ovs-dev] [PATCH ovn] ovn-northd: Fix IP local multicast flooding.

2020-02-14 Thread Dumitru Ceara
On 2/14/20 11:42 AM, Dumitru Ceara wrote:
> Skip IGMP and MLD entries learned for local multicast groups when
> generating logical flows. We still allow ovn-controller to learn them as
> it might be useful information for administrators to see that hosts
> register for the groups even though they are not expected to send JOIN
> messages for this range.
> 
> Fixes: ddc64665b678 ("OVN: Add ovn-northd IGMP support")
> Reported-by: Lucas Alvares Gomes 
> Reported-at: https://bugzilla.redhat.com/1803008
> Signed-off-by: Dumitru Ceara 

Please also backport this to branch-20.03.

Thanks,
Dumitru

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH ovn] ovn-northd: Fix IP local multicast flooding.

2020-02-14 Thread Dumitru Ceara
Skip IGMP and MLD entries learned for local multicast groups when
generating logical flows. We still allow ovn-controller to learn them as
it might be useful information for administrators to see that hosts
register for the groups even though they are not expected to send JOIN
messages for this range.

Fixes: ddc64665b678 ("OVN: Add ovn-northd IGMP support")
Reported-by: Lucas Alvares Gomes 
Reported-at: https://bugzilla.redhat.com/1803008
Signed-off-by: Dumitru Ceara 
---
 northd/ovn-northd.c |  15 +++
 tests/ovn.at| 119 +---
 2 files changed, 109 insertions(+), 25 deletions(-)

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 46521b5..6d95ee2 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -6608,6 +6608,15 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap 
*ports,
 &igmp_group->datapath->mcast_info.sw;
 
 if (IN6_IS_ADDR_V4MAPPED(&igmp_group->address)) {
+/* RFC 4541, section 2.1.2, item 2: Skip groups in the 224.0.0.X
+ * range.
+ */
+ovs_be32 group_address =
+in6_addr_get_mapped_ipv4(&igmp_group->address);
+if (ip_is_local_multicast(group_address)) {
+continue;
+}
+
 if (mcast_sw_info->active_v4_flows >= mcast_sw_info->table_size) {
 continue;
 }
@@ -6615,6 +6624,12 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap 
*ports,
 ds_put_format(&match, "eth.mcast && ip4 && ip4.dst == %s ",
   igmp_group->mcgroup.name);
 } else {
+/* RFC 4291, section 2.7.1: Skip groups that correspond to all
+ * hosts.
+ */
+if (ipv6_is_all_hosts(&igmp_group->address)) {
+continue;
+}
 if (mcast_sw_info->active_v6_flows >= mcast_sw_info->table_size) {
 continue;
 }
diff --git a/tests/ovn.at b/tests/ovn.at
index ff9a732..d2f1101 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -15434,7 +15434,7 @@ ovn-nbctl set Logical_Switch sw1   \
 other_config:mcast_snoop="true"
 
 # No IGMP query should be generated by sw1 (mcast_querier="false").
-truncate -s 0 expected
+> expected
 OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [expected])
 OVN_CHECK_PACKETS([hv1/vif2-tx.pcap], [expected])
 OVN_CHECK_PACKETS([hv2/vif1-tx.pcap], [expected])
@@ -15454,14 +15454,14 @@ send_igmp_v3_report hv2-vif1 hv2 0002 
$(ip_to_hex 10 0 0 2) f9f9 \
 
 # Check that the IGMP Group is learned on both hv.
 OVS_WAIT_UNTIL([
-total_entries=`ovn-sbctl find IGMP_Group | grep "239.0.1.68" | wc -l`
+total_entries=`ovn-sbctl find IGMP_Group | grep "239.0.1.68" -c`
 test "${total_entries}" = "2"
 ])
 
 # Send traffic and make sure it gets forwarded only on the two ports that
 # joined.
-truncate -s 0 expected
-truncate -s 0 expected_empty
+> expected
+> expected_empty
 send_ip_multicast_pkt hv1-vif2 hv1 \
 0001 01005e000144 \
 $(ip_to_hex 10 0 0 42) $(ip_to_hex 239 0 1 68) 1e 20 ca70 11 \
@@ -15486,15 +15486,15 @@ send_igmp_v3_report hv1-vif1 hv1 \
 
 # Check IGMP_Group table on both HV.
 OVS_WAIT_UNTIL([
-total_entries=`ovn-sbctl find IGMP_Group | grep "239.0.1.68" | wc -l`
+total_entries=`ovn-sbctl find IGMP_Group | grep "239.0.1.68" -c`
 test "${total_entries}" = "1"
 ])
 
 # Send traffic and make sure it gets forwarded only on the port that joined.
 as hv1 reset_pcap_file hv1-vif1 hv1/vif1
 as hv2 reset_pcap_file hv2-vif1 hv2/vif1
-truncate -s 0 expected
-truncate -s 0 expected_empty
+> expected
+> expected_empty
 send_ip_multicast_pkt hv1-vif2 hv1 \
 0001 01005e000144 \
 $(ip_to_hex 10 0 0 42) $(ip_to_hex 239 0 1 68) 1e 20 ca70 11 \
@@ -15514,10 +15514,43 @@ OVN_CHECK_PACKETS([hv2/vif3-tx.pcap], 
[expected_empty])
 # Flush IGMP groups.
 ovn-sbctl ip-multicast-flush sw1
 OVS_WAIT_UNTIL([
-total_entries=`ovn-sbctl find IGMP_Group | grep "239.0.1.68" | wc -l`
+total_entries=`ovn-sbctl find IGMP_Group | grep "239.0.1.68" -c`
 test "${total_entries}" = "0"
 ])
 
+# Check that traffic for 224.0.0.X is flooded even if some hosts register for
+# it.
+# Inject IGMP Join for 224.0.0.42 on sw1-p11.
+send_igmp_v3_report hv1-vif1 hv1 \
+0001 $(ip_to_hex 10 0 0 1) f9f8 \
+$(ip_to_hex 224 0 0 42) 04 f9d3 \
+/dev/null
+
+# Check that the IGMP Group is learned.
+OVS_WAIT_UNTIL([
+total_entries=`ovn-sbctl find IGMP_Group | grep "224.0.0.42" -c`
+test "${total_entries}" = "1"
+])
+
+# Send traffic and make sure it gets flooded to all ports.
+as hv1 reset_pcap_file hv1-vif1 hv1/vif1
+as hv1 reset_pcap_file hv1-vif2 hv1/vif2
+as hv2 reset_pcap_file hv2-vif1 hv2/vif1
+as hv2 reset_pcap_file hv2-vif2 hv2/vif2
+> expected
+send_ip_multicast_pkt hv1-vif2 hv1 \
+0001 01005e000144 \
+$(ip_to_hex 10 0 0 42) $(ip_to_hex 224 0 0 42) 1e 01 f989 11 \
+