| Message ID | 20240315105606.107757-1-mheib@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,1/2] northd: Don't skip transit switch LSP when creating mcast groups. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
| ovsrobot/github-robot-_Build_and_Test | fail | github build: failed |
On 3/15/24 11:56, Mohammad Heib wrote: > Currently when we enable IGMP on OVN-IC cluster with two or more AZs > and one vm from AZ1 send IGMP report, northd will create the following > multicast_group on each AZ: > > AZ1: > 1. multicast_group that forward the mcast traffic from LS1 to the VM. > 2. multicast_group that forward the mcast traffic from LR1 to the LS1. > > AZ2: > 1. multicast_group that forward the mcast traffic from TS to LR1 in > AZ1. > > This design works fine if we have one logical network only on each AZ, > but if we have two or more logical network on the same AZ that separated > from each other and only connected via transit switch and both join the > same mcast network, the traffic will be delivered between those two > networks because ovn floods it to all routers that connected to the > transit switch (see logical flow table 27). > > The above design is not the right way to handle such mcast traffic > because future changes for ovn that make it match explicitly on the igmp > group address and not forward traffic to routers can break the mcast > traffic in ovn-ic. > > This patch updates the above design by adding the router port that > connects to the transit switch to the multicast_group even if the peer > port have mcast_flood enabled. > > Signed-off-by: Mohammad Heib <mheib@redhat.com> > --- Thanks, Mohammad! Applied to main and backported all the way to 23.06. Regards, Dumitru
diff --git a/northd/northd.c b/northd/northd.c index 1839b7d8b..98c837a20 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -5377,11 +5377,13 @@ ovn_igmp_group_get_ports(const struct sbrec_igmp_group *sb_igmp_group, continue; } - /* If this is already a port of a router on which relay is enabled, - * skip it for the group. Traffic is flooded there anyway. + /* If this is already a port of a router on which relay is enabled + * and it's not a transit switch to router port,skip it for the group. + * Traffic is flooded there anyway. */ if (port->peer && port->peer->od && - port->peer->od->mcast_info.rtr.relay) { + port->peer->od->mcast_info.rtr.relay && + !ovn_datapath_is_transit_switch(port->od)) { continue; } diff --git a/northd/northd.h b/northd/northd.h index 3f1cd8341..5e9fa4745 100644 --- a/northd/northd.h +++ b/northd/northd.h @@ -362,6 +362,12 @@ ovn_datapath_is_stale(const struct ovn_datapath *od) return !od->nbr && !od->nbs; }; +static inline bool +ovn_datapath_is_transit_switch(const struct ovn_datapath *od) +{ + return od->tunnel_key >= OVN_MIN_DP_KEY_GLOBAL; +} + /* Pipeline stages. */ /* The two purposes for which ovn-northd uses OVN logical datapaths. */ diff --git a/tests/ovn.at b/tests/ovn.at index 438c7690a..4a9e433b2 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -26903,10 +26903,20 @@ wait_row_count IGMP_Group 2 address=239.0.1.68 wait_row_count IGMP_Group 2 address='"ff0a:dead:beef::1"' check ovn-nbctl --wait=hv sync +#Validate that Multicast Group contains all registered ports for +# specific igmp group. +ts_dp=$(fetch_column datapath_binding _uuid external_ids:name=ts) +ports=$(fetch_column multicast_group ports name="239.0.1.68" datapath=$ts_dp) +check test X2 = X$(echo $ports | wc -w) + + ovn_as az2 wait_row_count IGMP_Group 2 address=239.0.1.68 wait_row_count IGMP_Group 2 address='"ff0a:dead:beef::1"' check ovn-nbctl --wait=hv sync +ts_dp=$(fetch_column datapath_binding _uuid external_ids:name=ts) +ports=$(fetch_column multicast_group ports name="239.0.1.68" datapath=$ts_dp) +check test X2 = X$(echo $ports | wc -w) # Send an IP multicast packet from LSP2, it should be forwarded # to lsp1 and lsp3.
Currently when we enable IGMP on OVN-IC cluster with two or more AZs and one vm from AZ1 send IGMP report, northd will create the following multicast_group on each AZ: AZ1: 1. multicast_group that forward the mcast traffic from LS1 to the VM. 2. multicast_group that forward the mcast traffic from LR1 to the LS1. AZ2: 1. multicast_group that forward the mcast traffic from TS to LR1 in AZ1. This design works fine if we have one logical network only on each AZ, but if we have two or more logical network on the same AZ that separated from each other and only connected via transit switch and both join the same mcast network, the traffic will be delivered between those two networks because ovn floods it to all routers that connected to the transit switch (see logical flow table 27). The above design is not the right way to handle such mcast traffic because future changes for ovn that make it match explicitly on the igmp group address and not forward traffic to routers can break the mcast traffic in ovn-ic. This patch updates the above design by adding the router port that connects to the transit switch to the multicast_group even if the peer port have mcast_flood enabled. Signed-off-by: Mohammad Heib <mheib@redhat.com> --- northd/northd.c | 8 +++++--- northd/northd.h | 6 ++++++ tests/ovn.at | 10 ++++++++++ 3 files changed, 21 insertions(+), 3 deletions(-)