| Message ID | 1571231201-2974-1-git-send-email-dceara@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,ovn,v2] ovn-northd: Fix IP multicast flooding to mrouter. | expand |
On Wed, Oct 16, 2019 at 6:36 PM Dumitru Ceara <dceara@redhat.com> wrote: > OVN logical flow "drop" actions can't be combined with other actions. > Commit 79308138891a created such a scenario if a logical switch has > mcast_snoop=true, mcast_flood_unregistered=false and is connected to a > logical router with mcast_relay=enabled. > > To fix the issue we now explicitly add a drop flow for unregistered IP > multicast traffic in a logical switch if mcast_snoop=true, > mcast_flood_unregistered=false and the switch doesn't have any ports > with mcast_flood=true and isn't connected to a router with > mcast_relay=true. > > Fixes: 79308138891a ("ovn-northd: Add static IP multicast flood > configuration") > Signed-off-by: Dumitru Ceara <dceara@redhat.com> > > Thanks. I applied this to master. Numan > --- > v2: Address Numan's comments (update documentation). > --- > northd/ovn-northd.8.xml | 13 +++++++++++++ > northd/ovn-northd.c | 8 +++++++- > tests/ovn.at | 50 > ++++++++++++++++++++++++++++++++++++++++++++++--- > 3 files changed, 67 insertions(+), 4 deletions(-) > > diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml > index 937702e..b5dfcd1 100644 > --- a/northd/ovn-northd.8.xml > +++ b/northd/ovn-northd.8.xml > @@ -992,6 +992,19 @@ output; > </li> > > <li> > + A priority-80 flow that drops all unregistered IP multicast > traffic > + if <ref column="other_config" table="Logical_Switch"/> > + <code>:mcast_snoop='true'</code> and > + <ref column="other_config" table="Logical_Switch"/> > + <code>:mcast_flood_unregistered='false'</code> and the switch is > + not connected to a logical router that has > + <ref column="options" table="Logical_Router"/> > + <code>:mcast_relay='true'</code> and the switch doesn't have any > + logical port with <ref column="options" > table="Logical_Switch_Port"/> > + <code>:mcast_flood='true'</code>. > + </li> > + > + <li> > A priority-70 flow that outputs all packets with an Ethernet > broadcast > or multicast <code>eth.dst</code> to the <code>MC_FLOOD</code> > multicast group. > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > index e41c9d7..d0844dd 100644 > --- a/northd/ovn-northd.c > +++ b/northd/ovn-northd.c > @@ -5661,7 +5661,13 @@ build_lswitch_flows(struct hmap *datapaths, struct > hmap *ports, > > if (mcast_sw_info->flood_static) { > ds_put_cstr(&actions, "outport =\""MC_STATIC"\"; > output;"); > - } else { > + } > + > + /* Explicitly drop the traffic if relay or static flooding > + * is not configured. > + */ > + if (!mcast_sw_info->flood_relay && > + !mcast_sw_info->flood_static) { > ds_put_cstr(&actions, "drop;"); > } > > diff --git a/tests/ovn.at b/tests/ovn.at > index df00517..d141367 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -16306,7 +16306,7 @@ sleep 1 > OVN_CHECK_PACKETS([hv1/vif3-tx.pcap], [expected]) > OVN_CHECK_PACKETS([hv2/vif3-tx.pcap], [expected]) > > -# Dissable IGMP querier on sw2. > +# Disable IGMP querier on sw2. > ovn-nbctl set Logical_Switch sw2 \ > other_config:mcast_querier="false" > > @@ -16357,6 +16357,50 @@ send_igmp_v3_report hv2-vif3 hv2 \ > 000000000001 $(ip_to_hex 10 0 0 1) f9f8 \ > $(ip_to_hex 239 0 1 68) 04 e9b9 \ > /dev/null > + > +# Check that the IGMP Group is learned by all switches. > +OVS_WAIT_UNTIL([ > + total_entries=`ovn-sbctl find IGMP_Group | grep "239.0.1.68" | wc -l` > + test "${total_entries}" = "2" > +]) > + > +# Send traffic from sw3 and make sure it is relayed by rtr. > +# to ports that joined. > +truncate -s 0 expected_routed_sw1 > +truncate -s 0 expected_routed_sw2 > +truncate -s 0 expected_empty > + > +as hv1 reset_pcap_file hv1-vif1 hv1/vif1 > +as hv1 reset_pcap_file hv1-vif2 hv1/vif2 > +as hv1 reset_pcap_file hv1-vif3 hv1/vif3 > +as hv1 reset_pcap_file hv1-vif4 hv1/vif4 > +as hv2 reset_pcap_file hv2-vif1 hv2/vif1 > +as hv2 reset_pcap_file hv2-vif2 hv2/vif2 > +as hv2 reset_pcap_file hv2-vif3 hv2/vif3 > +as hv2 reset_pcap_file hv2-vif4 hv2/vif4 > + > +send_ip_multicast_pkt hv2-vif4 hv2 \ > + 000000000001 01005e000144 \ > + $(ip_to_hex 10 0 0 42) $(ip_to_hex 239 0 1 68) 1e 20 ca70 11 \ > + e518e518000a3b3a0000 > +store_ip_multicast_pkt \ > + 000000000100 01005e000144 \ > + $(ip_to_hex 10 0 0 42) $(ip_to_hex 239 0 1 68) 1e 1f cb70 11 \ > + e518e518000a3b3a0000 expected_routed_sw1 > +store_ip_multicast_pkt \ > + 000000000200 01005e000144 \ > + $(ip_to_hex 10 0 0 42) $(ip_to_hex 239 0 1 68) 1e 1f cb70 11 \ > + e518e518000a3b3a0000 expected_routed_sw2 > + > +OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [expected_routed_sw1]) > +OVN_CHECK_PACKETS([hv2/vif3-tx.pcap], [expected_routed_sw2]) > +OVN_CHECK_PACKETS([hv1/vif4-tx.pcap], [expected_empty]) > +OVN_CHECK_PACKETS([hv1/vif2-tx.pcap], [expected_empty]) > +OVN_CHECK_PACKETS([hv1/vif3-tx.pcap], [expected_empty]) > +OVN_CHECK_PACKETS([hv2/vif1-tx.pcap], [expected_empty]) > +OVN_CHECK_PACKETS([hv2/vif2-tx.pcap], [expected_empty]) > +OVN_CHECK_PACKETS([hv2/vif4-tx.pcap], [expected_empty]) > + > # Inject IGMP Join for 239.0.1.68 on sw3-p1. > send_igmp_v3_report hv1-vif4 hv1 \ > 000000000001 $(ip_to_hex 10 0 0 1) f9f8 \ > @@ -16369,8 +16413,8 @@ OVS_WAIT_UNTIL([ > test "${total_entries}" = "3" > ]) > > -# Send traffic from sw3 and make sure it is relayed by rtr. > -# and ports that joined. > +# Send traffic from sw3 and make sure it is relayed by rtr > +# to ports that joined. > truncate -s 0 expected_routed_sw1 > truncate -s 0 expected_routed_sw2 > truncate -s 0 expected_switched > -- > 1.8.3.1 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On Wed, Oct 16, 2019 at 4:04 PM Numan Siddique <numans@ovn.org> wrote: > > > > On Wed, Oct 16, 2019 at 6:36 PM Dumitru Ceara <dceara@redhat.com> wrote: >> >> OVN logical flow "drop" actions can't be combined with other actions. >> Commit 79308138891a created such a scenario if a logical switch has >> mcast_snoop=true, mcast_flood_unregistered=false and is connected to a >> logical router with mcast_relay=enabled. >> >> To fix the issue we now explicitly add a drop flow for unregistered IP >> multicast traffic in a logical switch if mcast_snoop=true, >> mcast_flood_unregistered=false and the switch doesn't have any ports >> with mcast_flood=true and isn't connected to a router with >> mcast_relay=true. >> >> Fixes: 79308138891a ("ovn-northd: Add static IP multicast flood configuration") >> Signed-off-by: Dumitru Ceara <dceara@redhat.com> >> > > Thanks. I applied this to master. > > Numan Thanks! > >> >> --- >> v2: Address Numan's comments (update documentation). >> --- >> northd/ovn-northd.8.xml | 13 +++++++++++++ >> northd/ovn-northd.c | 8 +++++++- >> tests/ovn.at | 50 ++++++++++++++++++++++++++++++++++++++++++++++--- >> 3 files changed, 67 insertions(+), 4 deletions(-) >> >> diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml >> index 937702e..b5dfcd1 100644 >> --- a/northd/ovn-northd.8.xml >> +++ b/northd/ovn-northd.8.xml >> @@ -992,6 +992,19 @@ output; >> </li> >> >> <li> >> + A priority-80 flow that drops all unregistered IP multicast traffic >> + if <ref column="other_config" table="Logical_Switch"/> >> + <code>:mcast_snoop='true'</code> and >> + <ref column="other_config" table="Logical_Switch"/> >> + <code>:mcast_flood_unregistered='false'</code> and the switch is >> + not connected to a logical router that has >> + <ref column="options" table="Logical_Router"/> >> + <code>:mcast_relay='true'</code> and the switch doesn't have any >> + logical port with <ref column="options" table="Logical_Switch_Port"/> >> + <code>:mcast_flood='true'</code>. >> + </li> >> + >> + <li> >> A priority-70 flow that outputs all packets with an Ethernet broadcast >> or multicast <code>eth.dst</code> to the <code>MC_FLOOD</code> >> multicast group. >> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c >> index e41c9d7..d0844dd 100644 >> --- a/northd/ovn-northd.c >> +++ b/northd/ovn-northd.c >> @@ -5661,7 +5661,13 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap *ports, >> >> if (mcast_sw_info->flood_static) { >> ds_put_cstr(&actions, "outport =\""MC_STATIC"\"; output;"); >> - } else { >> + } >> + >> + /* Explicitly drop the traffic if relay or static flooding >> + * is not configured. >> + */ >> + if (!mcast_sw_info->flood_relay && >> + !mcast_sw_info->flood_static) { >> ds_put_cstr(&actions, "drop;"); >> } >> >> diff --git a/tests/ovn.at b/tests/ovn.at >> index df00517..d141367 100644 >> --- a/tests/ovn.at >> +++ b/tests/ovn.at >> @@ -16306,7 +16306,7 @@ sleep 1 >> OVN_CHECK_PACKETS([hv1/vif3-tx.pcap], [expected]) >> OVN_CHECK_PACKETS([hv2/vif3-tx.pcap], [expected]) >> >> -# Dissable IGMP querier on sw2. >> +# Disable IGMP querier on sw2. >> ovn-nbctl set Logical_Switch sw2 \ >> other_config:mcast_querier="false" >> >> @@ -16357,6 +16357,50 @@ send_igmp_v3_report hv2-vif3 hv2 \ >> 000000000001 $(ip_to_hex 10 0 0 1) f9f8 \ >> $(ip_to_hex 239 0 1 68) 04 e9b9 \ >> /dev/null >> + >> +# Check that the IGMP Group is learned by all switches. >> +OVS_WAIT_UNTIL([ >> + total_entries=`ovn-sbctl find IGMP_Group | grep "239.0.1.68" | wc -l` >> + test "${total_entries}" = "2" >> +]) >> + >> +# Send traffic from sw3 and make sure it is relayed by rtr. >> +# to ports that joined. >> +truncate -s 0 expected_routed_sw1 >> +truncate -s 0 expected_routed_sw2 >> +truncate -s 0 expected_empty >> + >> +as hv1 reset_pcap_file hv1-vif1 hv1/vif1 >> +as hv1 reset_pcap_file hv1-vif2 hv1/vif2 >> +as hv1 reset_pcap_file hv1-vif3 hv1/vif3 >> +as hv1 reset_pcap_file hv1-vif4 hv1/vif4 >> +as hv2 reset_pcap_file hv2-vif1 hv2/vif1 >> +as hv2 reset_pcap_file hv2-vif2 hv2/vif2 >> +as hv2 reset_pcap_file hv2-vif3 hv2/vif3 >> +as hv2 reset_pcap_file hv2-vif4 hv2/vif4 >> + >> +send_ip_multicast_pkt hv2-vif4 hv2 \ >> + 000000000001 01005e000144 \ >> + $(ip_to_hex 10 0 0 42) $(ip_to_hex 239 0 1 68) 1e 20 ca70 11 \ >> + e518e518000a3b3a0000 >> +store_ip_multicast_pkt \ >> + 000000000100 01005e000144 \ >> + $(ip_to_hex 10 0 0 42) $(ip_to_hex 239 0 1 68) 1e 1f cb70 11 \ >> + e518e518000a3b3a0000 expected_routed_sw1 >> +store_ip_multicast_pkt \ >> + 000000000200 01005e000144 \ >> + $(ip_to_hex 10 0 0 42) $(ip_to_hex 239 0 1 68) 1e 1f cb70 11 \ >> + e518e518000a3b3a0000 expected_routed_sw2 >> + >> +OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [expected_routed_sw1]) >> +OVN_CHECK_PACKETS([hv2/vif3-tx.pcap], [expected_routed_sw2]) >> +OVN_CHECK_PACKETS([hv1/vif4-tx.pcap], [expected_empty]) >> +OVN_CHECK_PACKETS([hv1/vif2-tx.pcap], [expected_empty]) >> +OVN_CHECK_PACKETS([hv1/vif3-tx.pcap], [expected_empty]) >> +OVN_CHECK_PACKETS([hv2/vif1-tx.pcap], [expected_empty]) >> +OVN_CHECK_PACKETS([hv2/vif2-tx.pcap], [expected_empty]) >> +OVN_CHECK_PACKETS([hv2/vif4-tx.pcap], [expected_empty]) >> + >> # Inject IGMP Join for 239.0.1.68 on sw3-p1. >> send_igmp_v3_report hv1-vif4 hv1 \ >> 000000000001 $(ip_to_hex 10 0 0 1) f9f8 \ >> @@ -16369,8 +16413,8 @@ OVS_WAIT_UNTIL([ >> test "${total_entries}" = "3" >> ]) >> >> -# Send traffic from sw3 and make sure it is relayed by rtr. >> -# and ports that joined. >> +# Send traffic from sw3 and make sure it is relayed by rtr >> +# to ports that joined. >> truncate -s 0 expected_routed_sw1 >> truncate -s 0 expected_routed_sw2 >> truncate -s 0 expected_switched >> -- >> 1.8.3.1 >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml index 937702e..b5dfcd1 100644 --- a/northd/ovn-northd.8.xml +++ b/northd/ovn-northd.8.xml @@ -992,6 +992,19 @@ output; </li> <li> + A priority-80 flow that drops all unregistered IP multicast traffic + if <ref column="other_config" table="Logical_Switch"/> + <code>:mcast_snoop='true'</code> and + <ref column="other_config" table="Logical_Switch"/> + <code>:mcast_flood_unregistered='false'</code> and the switch is + not connected to a logical router that has + <ref column="options" table="Logical_Router"/> + <code>:mcast_relay='true'</code> and the switch doesn't have any + logical port with <ref column="options" table="Logical_Switch_Port"/> + <code>:mcast_flood='true'</code>. + </li> + + <li> A priority-70 flow that outputs all packets with an Ethernet broadcast or multicast <code>eth.dst</code> to the <code>MC_FLOOD</code> multicast group. diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index e41c9d7..d0844dd 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -5661,7 +5661,13 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap *ports, if (mcast_sw_info->flood_static) { ds_put_cstr(&actions, "outport =\""MC_STATIC"\"; output;"); - } else { + } + + /* Explicitly drop the traffic if relay or static flooding + * is not configured. + */ + if (!mcast_sw_info->flood_relay && + !mcast_sw_info->flood_static) { ds_put_cstr(&actions, "drop;"); } diff --git a/tests/ovn.at b/tests/ovn.at index df00517..d141367 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -16306,7 +16306,7 @@ sleep 1 OVN_CHECK_PACKETS([hv1/vif3-tx.pcap], [expected]) OVN_CHECK_PACKETS([hv2/vif3-tx.pcap], [expected]) -# Dissable IGMP querier on sw2. +# Disable IGMP querier on sw2. ovn-nbctl set Logical_Switch sw2 \ other_config:mcast_querier="false" @@ -16357,6 +16357,50 @@ send_igmp_v3_report hv2-vif3 hv2 \ 000000000001 $(ip_to_hex 10 0 0 1) f9f8 \ $(ip_to_hex 239 0 1 68) 04 e9b9 \ /dev/null + +# Check that the IGMP Group is learned by all switches. +OVS_WAIT_UNTIL([ + total_entries=`ovn-sbctl find IGMP_Group | grep "239.0.1.68" | wc -l` + test "${total_entries}" = "2" +]) + +# Send traffic from sw3 and make sure it is relayed by rtr. +# to ports that joined. +truncate -s 0 expected_routed_sw1 +truncate -s 0 expected_routed_sw2 +truncate -s 0 expected_empty + +as hv1 reset_pcap_file hv1-vif1 hv1/vif1 +as hv1 reset_pcap_file hv1-vif2 hv1/vif2 +as hv1 reset_pcap_file hv1-vif3 hv1/vif3 +as hv1 reset_pcap_file hv1-vif4 hv1/vif4 +as hv2 reset_pcap_file hv2-vif1 hv2/vif1 +as hv2 reset_pcap_file hv2-vif2 hv2/vif2 +as hv2 reset_pcap_file hv2-vif3 hv2/vif3 +as hv2 reset_pcap_file hv2-vif4 hv2/vif4 + +send_ip_multicast_pkt hv2-vif4 hv2 \ + 000000000001 01005e000144 \ + $(ip_to_hex 10 0 0 42) $(ip_to_hex 239 0 1 68) 1e 20 ca70 11 \ + e518e518000a3b3a0000 +store_ip_multicast_pkt \ + 000000000100 01005e000144 \ + $(ip_to_hex 10 0 0 42) $(ip_to_hex 239 0 1 68) 1e 1f cb70 11 \ + e518e518000a3b3a0000 expected_routed_sw1 +store_ip_multicast_pkt \ + 000000000200 01005e000144 \ + $(ip_to_hex 10 0 0 42) $(ip_to_hex 239 0 1 68) 1e 1f cb70 11 \ + e518e518000a3b3a0000 expected_routed_sw2 + +OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [expected_routed_sw1]) +OVN_CHECK_PACKETS([hv2/vif3-tx.pcap], [expected_routed_sw2]) +OVN_CHECK_PACKETS([hv1/vif4-tx.pcap], [expected_empty]) +OVN_CHECK_PACKETS([hv1/vif2-tx.pcap], [expected_empty]) +OVN_CHECK_PACKETS([hv1/vif3-tx.pcap], [expected_empty]) +OVN_CHECK_PACKETS([hv2/vif1-tx.pcap], [expected_empty]) +OVN_CHECK_PACKETS([hv2/vif2-tx.pcap], [expected_empty]) +OVN_CHECK_PACKETS([hv2/vif4-tx.pcap], [expected_empty]) + # Inject IGMP Join for 239.0.1.68 on sw3-p1. send_igmp_v3_report hv1-vif4 hv1 \ 000000000001 $(ip_to_hex 10 0 0 1) f9f8 \ @@ -16369,8 +16413,8 @@ OVS_WAIT_UNTIL([ test "${total_entries}" = "3" ]) -# Send traffic from sw3 and make sure it is relayed by rtr. -# and ports that joined. +# Send traffic from sw3 and make sure it is relayed by rtr +# to ports that joined. truncate -s 0 expected_routed_sw1 truncate -s 0 expected_routed_sw2 truncate -s 0 expected_switched
OVN logical flow "drop" actions can't be combined with other actions. Commit 79308138891a created such a scenario if a logical switch has mcast_snoop=true, mcast_flood_unregistered=false and is connected to a logical router with mcast_relay=enabled. To fix the issue we now explicitly add a drop flow for unregistered IP multicast traffic in a logical switch if mcast_snoop=true, mcast_flood_unregistered=false and the switch doesn't have any ports with mcast_flood=true and isn't connected to a router with mcast_relay=true. Fixes: 79308138891a ("ovn-northd: Add static IP multicast flood configuration") Signed-off-by: Dumitru Ceara <dceara@redhat.com> --- v2: Address Numan's comments (update documentation). --- northd/ovn-northd.8.xml | 13 +++++++++++++ northd/ovn-northd.c | 8 +++++++- tests/ovn.at | 50 ++++++++++++++++++++++++++++++++++++++++++++++--- 3 files changed, 67 insertions(+), 4 deletions(-)