Re: [ovs-dev] [PATCH ovn 4/4] multicast: Properly flood IGMP queries and reports.
On 7/19/22 19:19, Numan Siddique wrote: > On Thu, Jul 7, 2022 at 6:27 AM Lucas Alvares Gomes > wrote: >> >> On Thu, Jul 7, 2022 at 12:24 PM Lucas Alvares Gomes >> wrote: >>> >>> Hi, >>> >>> I tested this series of patches with OpenStack upstream [0] since we >>> have some automated tests for IGMP and it seems good: >>> >>> test_multicast_between_vms_on_same_network[id-113486fc-24c9-4be4-8361-03b1c9892867] >>> pass (link: >>> https://98794ab76263ee253bc7-6b6bf6aa2495d781bdba3c8c61916451.ssl.cf1.rackcdn.com/848934/2/check/neutron-tempest-plugin-ovn/b3191bf/testr_results.html, >>> this will expire at some point) >>> >>> What I did was to change ML2/OVN to set mcast_flood to False on the >>> LSPs, apply the series of patches in OVN and then run the tests. >>> >>> Tested-By: >>> https://review.opendev.org/c/openstack/neutron-tempest-plugin/+/848934/ >>> >> >> Tested-By: Lucas Alvares Gomes > > Acked-by: Numan Siddique Thanks, I included your ack in v2 (no change to this patch): https://patchwork.ozlabs.org/project/ovn/list/?series=310294=* > > It would be great if someone else also reviews this patch. > I agree, the more eyes on this change the better! > Numan Thanks, Dumitru ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn 4/4] multicast: Properly flood IGMP queries and reports.
On Thu, Jul 7, 2022 at 6:27 AM Lucas Alvares Gomes wrote: > > On Thu, Jul 7, 2022 at 12:24 PM Lucas Alvares Gomes > wrote: > > > > Hi, > > > > I tested this series of patches with OpenStack upstream [0] since we > > have some automated tests for IGMP and it seems good: > > > > test_multicast_between_vms_on_same_network[id-113486fc-24c9-4be4-8361-03b1c9892867] > > pass (link: > > https://98794ab76263ee253bc7-6b6bf6aa2495d781bdba3c8c61916451.ssl.cf1.rackcdn.com/848934/2/check/neutron-tempest-plugin-ovn/b3191bf/testr_results.html, > > this will expire at some point) > > > > What I did was to change ML2/OVN to set mcast_flood to False on the > > LSPs, apply the series of patches in OVN and then run the tests. > > > > Tested-By: > > https://review.opendev.org/c/openstack/neutron-tempest-plugin/+/848934/ > > > > Tested-By: Lucas Alvares Gomes Acked-by: Numan Siddique It would be great if someone else also reviews this patch. Numan > > > [0] https://review.opendev.org/q/topic:test-igmp > > > > Cheers, > > Lucas > > > > On Tue, Jul 5, 2022 at 1:13 PM Dumitru Ceara wrote: > > > > > > RFC 4541 "Considerations for Internet Group Management Protocol (IGMP) > > > and Multicast Listener Discovery (MLD) Snooping Switches" [0] states: > > > > > > For IGMP packets: > > > 2.1.1. IGMP Forwarding Rules > > >1) A snooping switch should forward IGMP Membership Reports only to > > > those ports where multicast routers are attached. > > > > > > Alternatively stated: a snooping switch should not forward IGMP > > > Membership Reports to ports on which only hosts are attached. > > > An > > > administrative control may be provided to override this > > > restriction, allowing the report messages to be flooded to other > > > ports. > > > > > > This is the main IGMP snooping functionality for the control > > > path. > > > > > > Sending membership reports to other hosts can result, for IGMPv1 > > > and IGMPv2, in unintentionally preventing a host from joining a > > > specific multicast group. > > > > > > When an IGMPv1 or IGMPv2 host receives a membership report for a > > > group address that it intends to join, the host will suppress > > > its > > > own membership report for the same group. This join or message > > > suppression is a requirement for IGMPv1 and IGMPv2 hosts. > > > However, if a switch does not receive a membership report from > > > the > > > host it will not forward multicast data to it. > > > > > > In OVN this translates to forwarding reports only on: > > > a. ports where mrouters have been learned (IGMP queries were received on > > >those ports) > > > b. ports connecting a multicast enabled logical switch to a multicast > > >relay enabled logical router (OVN mrouter) > > > c. ports configured with mcast_flood_reports=true > > > > > > 2.1.2. Data Forwarding Rules > > > > > >1) Packets with a destination IP address outside 224.0.0.X which > > > are > > > not IGMP should be forwarded according to group-based port > > > membership tables and must also be forwarded on router ports. > > > > > > In OVN this translates to forwarding traffic for a group G to: > > > a. all ports where G was learned > > > b. all ports where an (external or OVN) mrouter was learned. > > > c. all ports configured with mcast_flood=true > > > > > > IGMP queries are already snooped by ovn-controller. Just propagate the > > > information about where mrouters are to the Southbound DB through means > > > of a custom IGMP_Group (name="mrouters"). > > > > > > Snooped queries are then re-injected in the OVS pipeline with outport > > > set to MC_FLOOD_L2 (only flood them in the L2 domain). > > > > > > Snooped reports are re-injected in the OVS pipeline with outport set to > > > MC_MROUTER_FLOOD (flood them to snooped mrouters and OVN mrouters). > > > > > > The same behavior applies to IPv6 too (MLD). > > > > > > [0] https://datatracker.ietf.org/doc/html/rfc4541 > > > > > > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1933990 > > > Reported-at: https://github.com/ovn-org/ovn/issues/126 > > > Signed-off-by: Dumitru Ceara > > > --- > > > controller/ip-mcast.c | 129 - > > > controller/ip-mcast.h | 14 > > > controller/pinctrl.c| 129 + > > > lib/ip-mcast-index.h|5 + > > > lib/mcast-group-index.h |4 - > > > northd/northd.c | 54 +++ > > > northd/ovn-northd.8.xml |6 -- > > > tests/ovn-macros.at | 25 +++ > > > tests/ovn.at| 164 > > > ++- > > > 9 files changed, 472 insertions(+), 58 deletions(-) > > > > > > diff --git a/controller/ip-mcast.c b/controller/ip-mcast.c > > > index 9b0b4465a..a870fb29e 100644 > > > ---
Re: [ovs-dev] [PATCH ovn 4/4] multicast: Properly flood IGMP queries and reports.
On Thu, Jul 7, 2022 at 12:24 PM Lucas Alvares Gomes
wrote:
>
> Hi,
>
> I tested this series of patches with OpenStack upstream [0] since we
> have some automated tests for IGMP and it seems good:
>
> test_multicast_between_vms_on_same_network[id-113486fc-24c9-4be4-8361-03b1c9892867]
> pass (link:
> https://98794ab76263ee253bc7-6b6bf6aa2495d781bdba3c8c61916451.ssl.cf1.rackcdn.com/848934/2/check/neutron-tempest-plugin-ovn/b3191bf/testr_results.html,
> this will expire at some point)
>
> What I did was to change ML2/OVN to set mcast_flood to False on the
> LSPs, apply the series of patches in OVN and then run the tests.
>
> Tested-By:
> https://review.opendev.org/c/openstack/neutron-tempest-plugin/+/848934/
>
Tested-By: Lucas Alvares Gomes
> [0] https://review.opendev.org/q/topic:test-igmp
>
> Cheers,
> Lucas
>
> On Tue, Jul 5, 2022 at 1:13 PM Dumitru Ceara wrote:
> >
> > RFC 4541 "Considerations for Internet Group Management Protocol (IGMP)
> > and Multicast Listener Discovery (MLD) Snooping Switches" [0] states:
> >
> > For IGMP packets:
> > 2.1.1. IGMP Forwarding Rules
> >1) A snooping switch should forward IGMP Membership Reports only to
> > those ports where multicast routers are attached.
> >
> > Alternatively stated: a snooping switch should not forward IGMP
> > Membership Reports to ports on which only hosts are attached. An
> > administrative control may be provided to override this
> > restriction, allowing the report messages to be flooded to other
> > ports.
> >
> > This is the main IGMP snooping functionality for the control path.
> >
> > Sending membership reports to other hosts can result, for IGMPv1
> > and IGMPv2, in unintentionally preventing a host from joining a
> > specific multicast group.
> >
> > When an IGMPv1 or IGMPv2 host receives a membership report for a
> > group address that it intends to join, the host will suppress its
> > own membership report for the same group. This join or message
> > suppression is a requirement for IGMPv1 and IGMPv2 hosts.
> > However, if a switch does not receive a membership report from the
> > host it will not forward multicast data to it.
> >
> > In OVN this translates to forwarding reports only on:
> > a. ports where mrouters have been learned (IGMP queries were received on
> >those ports)
> > b. ports connecting a multicast enabled logical switch to a multicast
> >relay enabled logical router (OVN mrouter)
> > c. ports configured with mcast_flood_reports=true
> >
> > 2.1.2. Data Forwarding Rules
> >
> >1) Packets with a destination IP address outside 224.0.0.X which are
> > not IGMP should be forwarded according to group-based port
> > membership tables and must also be forwarded on router ports.
> >
> > In OVN this translates to forwarding traffic for a group G to:
> > a. all ports where G was learned
> > b. all ports where an (external or OVN) mrouter was learned.
> > c. all ports configured with mcast_flood=true
> >
> > IGMP queries are already snooped by ovn-controller. Just propagate the
> > information about where mrouters are to the Southbound DB through means
> > of a custom IGMP_Group (name="mrouters").
> >
> > Snooped queries are then re-injected in the OVS pipeline with outport
> > set to MC_FLOOD_L2 (only flood them in the L2 domain).
> >
> > Snooped reports are re-injected in the OVS pipeline with outport set to
> > MC_MROUTER_FLOOD (flood them to snooped mrouters and OVN mrouters).
> >
> > The same behavior applies to IPv6 too (MLD).
> >
> > [0] https://datatracker.ietf.org/doc/html/rfc4541
> >
> > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1933990
> > Reported-at: https://github.com/ovn-org/ovn/issues/126
> > Signed-off-by: Dumitru Ceara
> > ---
> > controller/ip-mcast.c | 129 -
> > controller/ip-mcast.h | 14
> > controller/pinctrl.c| 129 +
> > lib/ip-mcast-index.h|5 +
> > lib/mcast-group-index.h |4 -
> > northd/northd.c | 54 +++
> > northd/ovn-northd.8.xml |6 --
> > tests/ovn-macros.at | 25 +++
> > tests/ovn.at| 164
> > ++-
> > 9 files changed, 472 insertions(+), 58 deletions(-)
> >
> > diff --git a/controller/ip-mcast.c b/controller/ip-mcast.c
> > index 9b0b4465a..a870fb29e 100644
> > --- a/controller/ip-mcast.c
> > +++ b/controller/ip-mcast.c
> > @@ -16,6 +16,7 @@
> > #include
> >
> > #include "ip-mcast.h"
> > +#include "ip-mcast-index.h"
> > #include "lport.h"
> > #include "lib/ovn-sb-idl.h"
> >
> > @@ -27,6 +28,18 @@ struct igmp_group_port {
> > const struct sbrec_port_binding *port;
> > };
> >
> > +static const struct sbrec_igmp_group *
> > +igmp_group_lookup_(struct
Re: [ovs-dev] [PATCH ovn 4/4] multicast: Properly flood IGMP queries and reports.
Hi,
I tested this series of patches with OpenStack upstream [0] since we
have some automated tests for IGMP and it seems good:
test_multicast_between_vms_on_same_network[id-113486fc-24c9-4be4-8361-03b1c9892867]
pass (link:
https://98794ab76263ee253bc7-6b6bf6aa2495d781bdba3c8c61916451.ssl.cf1.rackcdn.com/848934/2/check/neutron-tempest-plugin-ovn/b3191bf/testr_results.html,
this will expire at some point)
What I did was to change ML2/OVN to set mcast_flood to False on the
LSPs, apply the series of patches in OVN and then run the tests.
Tested-By:
https://review.opendev.org/c/openstack/neutron-tempest-plugin/+/848934/
[0] https://review.opendev.org/q/topic:test-igmp
Cheers,
Lucas
On Tue, Jul 5, 2022 at 1:13 PM Dumitru Ceara wrote:
>
> RFC 4541 "Considerations for Internet Group Management Protocol (IGMP)
> and Multicast Listener Discovery (MLD) Snooping Switches" [0] states:
>
> For IGMP packets:
> 2.1.1. IGMP Forwarding Rules
>1) A snooping switch should forward IGMP Membership Reports only to
> those ports where multicast routers are attached.
>
> Alternatively stated: a snooping switch should not forward IGMP
> Membership Reports to ports on which only hosts are attached. An
> administrative control may be provided to override this
> restriction, allowing the report messages to be flooded to other
> ports.
>
> This is the main IGMP snooping functionality for the control path.
>
> Sending membership reports to other hosts can result, for IGMPv1
> and IGMPv2, in unintentionally preventing a host from joining a
> specific multicast group.
>
> When an IGMPv1 or IGMPv2 host receives a membership report for a
> group address that it intends to join, the host will suppress its
> own membership report for the same group. This join or message
> suppression is a requirement for IGMPv1 and IGMPv2 hosts.
> However, if a switch does not receive a membership report from the
> host it will not forward multicast data to it.
>
> In OVN this translates to forwarding reports only on:
> a. ports where mrouters have been learned (IGMP queries were received on
>those ports)
> b. ports connecting a multicast enabled logical switch to a multicast
>relay enabled logical router (OVN mrouter)
> c. ports configured with mcast_flood_reports=true
>
> 2.1.2. Data Forwarding Rules
>
>1) Packets with a destination IP address outside 224.0.0.X which are
> not IGMP should be forwarded according to group-based port
> membership tables and must also be forwarded on router ports.
>
> In OVN this translates to forwarding traffic for a group G to:
> a. all ports where G was learned
> b. all ports where an (external or OVN) mrouter was learned.
> c. all ports configured with mcast_flood=true
>
> IGMP queries are already snooped by ovn-controller. Just propagate the
> information about where mrouters are to the Southbound DB through means
> of a custom IGMP_Group (name="mrouters").
>
> Snooped queries are then re-injected in the OVS pipeline with outport
> set to MC_FLOOD_L2 (only flood them in the L2 domain).
>
> Snooped reports are re-injected in the OVS pipeline with outport set to
> MC_MROUTER_FLOOD (flood them to snooped mrouters and OVN mrouters).
>
> The same behavior applies to IPv6 too (MLD).
>
> [0] https://datatracker.ietf.org/doc/html/rfc4541
>
> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1933990
> Reported-at: https://github.com/ovn-org/ovn/issues/126
> Signed-off-by: Dumitru Ceara
> ---
> controller/ip-mcast.c | 129 -
> controller/ip-mcast.h | 14
> controller/pinctrl.c| 129 +
> lib/ip-mcast-index.h|5 +
> lib/mcast-group-index.h |4 -
> northd/northd.c | 54 +++
> northd/ovn-northd.8.xml |6 --
> tests/ovn-macros.at | 25 +++
> tests/ovn.at| 164
> ++-
> 9 files changed, 472 insertions(+), 58 deletions(-)
>
> diff --git a/controller/ip-mcast.c b/controller/ip-mcast.c
> index 9b0b4465a..a870fb29e 100644
> --- a/controller/ip-mcast.c
> +++ b/controller/ip-mcast.c
> @@ -16,6 +16,7 @@
> #include
>
> #include "ip-mcast.h"
> +#include "ip-mcast-index.h"
> #include "lport.h"
> #include "lib/ovn-sb-idl.h"
>
> @@ -27,6 +28,18 @@ struct igmp_group_port {
> const struct sbrec_port_binding *port;
> };
>
> +static const struct sbrec_igmp_group *
> +igmp_group_lookup_(struct ovsdb_idl_index *igmp_groups,
> + const char *addr_str,
> + const struct sbrec_datapath_binding *datapath,
> + const struct sbrec_chassis *chassis);
> +
> +static struct sbrec_igmp_group *
> +igmp_group_create_(struct ovsdb_idl_txn *idl_txn,
> + const char
[ovs-dev] [PATCH ovn 4/4] multicast: Properly flood IGMP queries and reports.
RFC 4541 "Considerations for Internet Group Management Protocol (IGMP)
and Multicast Listener Discovery (MLD) Snooping Switches" [0] states:
For IGMP packets:
2.1.1. IGMP Forwarding Rules
1) A snooping switch should forward IGMP Membership Reports only to
those ports where multicast routers are attached.
Alternatively stated: a snooping switch should not forward IGMP
Membership Reports to ports on which only hosts are attached. An
administrative control may be provided to override this
restriction, allowing the report messages to be flooded to other
ports.
This is the main IGMP snooping functionality for the control path.
Sending membership reports to other hosts can result, for IGMPv1
and IGMPv2, in unintentionally preventing a host from joining a
specific multicast group.
When an IGMPv1 or IGMPv2 host receives a membership report for a
group address that it intends to join, the host will suppress its
own membership report for the same group. This join or message
suppression is a requirement for IGMPv1 and IGMPv2 hosts.
However, if a switch does not receive a membership report from the
host it will not forward multicast data to it.
In OVN this translates to forwarding reports only on:
a. ports where mrouters have been learned (IGMP queries were received on
those ports)
b. ports connecting a multicast enabled logical switch to a multicast
relay enabled logical router (OVN mrouter)
c. ports configured with mcast_flood_reports=true
2.1.2. Data Forwarding Rules
1) Packets with a destination IP address outside 224.0.0.X which are
not IGMP should be forwarded according to group-based port
membership tables and must also be forwarded on router ports.
In OVN this translates to forwarding traffic for a group G to:
a. all ports where G was learned
b. all ports where an (external or OVN) mrouter was learned.
c. all ports configured with mcast_flood=true
IGMP queries are already snooped by ovn-controller. Just propagate the
information about where mrouters are to the Southbound DB through means
of a custom IGMP_Group (name="mrouters").
Snooped queries are then re-injected in the OVS pipeline with outport
set to MC_FLOOD_L2 (only flood them in the L2 domain).
Snooped reports are re-injected in the OVS pipeline with outport set to
MC_MROUTER_FLOOD (flood them to snooped mrouters and OVN mrouters).
The same behavior applies to IPv6 too (MLD).
[0] https://datatracker.ietf.org/doc/html/rfc4541
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1933990
Reported-at: https://github.com/ovn-org/ovn/issues/126
Signed-off-by: Dumitru Ceara
---
controller/ip-mcast.c | 129 -
controller/ip-mcast.h | 14
controller/pinctrl.c| 129 +
lib/ip-mcast-index.h|5 +
lib/mcast-group-index.h |4 -
northd/northd.c | 54 +++
northd/ovn-northd.8.xml |6 --
tests/ovn-macros.at | 25 +++
tests/ovn.at| 164 ++-
9 files changed, 472 insertions(+), 58 deletions(-)
diff --git a/controller/ip-mcast.c b/controller/ip-mcast.c
index 9b0b4465a..a870fb29e 100644
--- a/controller/ip-mcast.c
+++ b/controller/ip-mcast.c
@@ -16,6 +16,7 @@
#include
#include "ip-mcast.h"
+#include "ip-mcast-index.h"
#include "lport.h"
#include "lib/ovn-sb-idl.h"
@@ -27,6 +28,18 @@ struct igmp_group_port {
const struct sbrec_port_binding *port;
};
+static const struct sbrec_igmp_group *
+igmp_group_lookup_(struct ovsdb_idl_index *igmp_groups,
+ const char *addr_str,
+ const struct sbrec_datapath_binding *datapath,
+ const struct sbrec_chassis *chassis);
+
+static struct sbrec_igmp_group *
+igmp_group_create_(struct ovsdb_idl_txn *idl_txn,
+ const char *addr_str,
+ const struct sbrec_datapath_binding *datapath,
+ const struct sbrec_chassis *chassis);
+
struct ovsdb_idl_index *
igmp_group_index_create(struct ovsdb_idl *idl)
{
@@ -54,17 +67,16 @@ igmp_group_lookup(struct ovsdb_idl_index *igmp_groups,
return NULL;
}
-struct sbrec_igmp_group *target =
-sbrec_igmp_group_index_init_row(igmp_groups);
-
-sbrec_igmp_group_index_set_address(target, addr_str);
-sbrec_igmp_group_index_set_datapath(target, datapath);
-sbrec_igmp_group_index_set_chassis(target, chassis);
+return igmp_group_lookup_(igmp_groups, addr_str, datapath, chassis);
+}
-const struct sbrec_igmp_group *g =
-sbrec_igmp_group_index_find(igmp_groups, target);
-sbrec_igmp_group_index_destroy_row(target);
-return g;
+const struct sbrec_igmp_group *
+igmp_mrouter_lookup(struct ovsdb_idl_index *igmp_groups,
+
