diff mbox series

[ovs-dev,2/2] ovn-ic: physical: Support multicast_group flooding on IC transit switches.

Message ID 20220119133939.25792.84110.stgit@dceara.remote.csb
State Accepted
Headers show
Series Support static IP multicast forwarding with OVN-IC. | expand

Checks

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

Commit Message

Dumitru Ceara Jan. 19, 2022, 1:39 p.m. UTC
Forwarding on logical patch ports that are part of a multicast group
normally happens on the local chassis.  With interconnect transit
switch, however, this doesn't work properly because patch ports in
the local AZ might not correspond to patch ports in the remote AZ.

This allows forwarding IP multicast traffic across different AZs when
the gateway routers in each AZ are configured to relay multicast and to
unconditionally flood on the gateway ports connected to the transit
switch.  This patch also adds a unit test to cover this scenario.

This is not really a feature, just a case that was missed due to the
fact that OVN-IC [0] and static IP multicast flood configuration [1] were
developed more or less at the same time.

[0] a3e4436c232 ("ovn-ic: Interconnection port controller.")
[1] 79308138891a ("ovn-northd: Add static IP multicast flood configuration")

Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2028981
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
---
 controller/local_data.c |    1 
 controller/local_data.h |    1 
 controller/physical.c   |   20 +++++
 lib/ovn-util.c          |    6 ++
 lib/ovn-util.h          |    1 
 northd/northd.c         |    6 ++
 tests/ovn.at            |  174 +++++++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 206 insertions(+), 3 deletions(-)
diff mbox series

Patch

diff --git a/controller/local_data.c b/controller/local_data.c
index 86050f335..1857bfd1b 100644
--- a/controller/local_data.c
+++ b/controller/local_data.c
@@ -67,6 +67,7 @@  local_datapath_alloc(const struct sbrec_datapath_binding *dp)
     struct local_datapath *ld = xzalloc(sizeof *ld);
     ld->datapath = dp;
     ld->is_switch = datapath_is_switch(dp);
+    ld->is_transit_switch = datapath_is_transit_switch(dp);
     shash_init(&ld->external_ports);
     /* memory accounting - common part. */
     local_datapath_usage += sizeof *ld;
diff --git a/controller/local_data.h b/controller/local_data.h
index 287f95af0..9306ddf15 100644
--- a/controller/local_data.h
+++ b/controller/local_data.h
@@ -44,6 +44,7 @@  struct local_datapath {
     struct hmap_node hmap_node;
     const struct sbrec_datapath_binding *datapath;
     bool is_switch;
+    bool is_transit_switch;
 
     /* The localnet port in this datapath, if any (at most one is allowed). */
     const struct sbrec_port_binding *localnet_port;
diff --git a/controller/physical.c b/controller/physical.c
index aa651b876..6bfa2304d 100644
--- a/controller/physical.c
+++ b/controller/physical.c
@@ -1421,7 +1421,8 @@  consider_mc_group(struct ovsdb_idl_index *sbrec_port_binding_by_name,
                   struct ovn_desired_flow_table *flow_table)
 {
     uint32_t dp_key = mc->datapath->tunnel_key;
-    if (!get_local_datapath(local_datapaths, dp_key)) {
+    struct local_datapath *ldp = get_local_datapath(local_datapaths, dp_key);
+    if (!ldp) {
         return;
     }
 
@@ -1444,7 +1445,10 @@  consider_mc_group(struct ovsdb_idl_index *sbrec_port_binding_by_name,
      *    - For logical patch ports, add actions to 'remote_ofpacts'
      *      instead.  (If we put them in 'ofpacts', then the output
      *      would happen on every hypervisor in the multicast group,
-     *      effectively duplicating the packet.)
+     *      effectively duplicating the packet.)  The only exception
+     *      is in case of transit switches (used by OVN-IC).  We need
+     *      patch ports to be processed on the remote side, after
+     *      crossing the AZ boundary.
      *
      *    - For chassisredirect ports, add actions to 'ofpacts' to
      *      set the output port to be the router patch port for which
@@ -1473,7 +1477,17 @@  consider_mc_group(struct ovsdb_idl_index *sbrec_port_binding_by_name,
         const char *lport_name = (port->parent_port && *port->parent_port) ?
                                   port->parent_port : port->logical_port;
 
-        if (!strcmp(port->type, "patch") || !strcmp(port->type, "localport")) {
+        if (!strcmp(port->type, "patch")) {
+            if (ldp->is_transit_switch) {
+                put_load(port->tunnel_key, MFF_LOG_OUTPORT, 0, 32,
+                         &ofpacts);
+                put_resubmit(OFTABLE_CHECK_LOOPBACK, &ofpacts);
+            } else {
+                put_load(port->tunnel_key, MFF_LOG_OUTPORT, 0, 32,
+                         &remote_ofpacts);
+                put_resubmit(OFTABLE_CHECK_LOOPBACK, &remote_ofpacts);
+            }
+        } else if (!strcmp(port->type, "localport")) {
             put_load(port->tunnel_key, MFF_LOG_OUTPORT, 0, 32,
                      &remote_ofpacts);
             put_resubmit(OFTABLE_CHECK_LOOPBACK, &remote_ofpacts);
diff --git a/lib/ovn-util.c b/lib/ovn-util.c
index c3da413aa..e6d0f7356 100644
--- a/lib/ovn-util.c
+++ b/lib/ovn-util.c
@@ -565,6 +565,12 @@  datapath_is_switch(const struct sbrec_datapath_binding *ldp)
     return smap_get(&ldp->external_ids, "logical-switch") != NULL;
 }
 
+bool
+datapath_is_transit_switch(const struct sbrec_datapath_binding *ldp)
+{
+    return smap_get(&ldp->external_ids, "interconn-ts") != NULL;
+}
+
 int
 datapath_snat_ct_zone(const struct sbrec_datapath_binding *dp)
 {
diff --git a/lib/ovn-util.h b/lib/ovn-util.h
index b212c64b7..743d28745 100644
--- a/lib/ovn-util.h
+++ b/lib/ovn-util.h
@@ -118,6 +118,7 @@  uint32_t ovn_logical_flow_hash(uint8_t table_id, enum ovn_pipeline pipeline,
 uint32_t ovn_logical_flow_hash_datapath(const struct uuid *logical_datapath,
                                         uint32_t hash);
 bool datapath_is_switch(const struct sbrec_datapath_binding *);
+bool datapath_is_transit_switch(const struct sbrec_datapath_binding *ldp);
 int datapath_snat_ct_zone(const struct sbrec_datapath_binding *ldp);
 void ovn_conn_show(struct unixctl_conn *conn, int argc OVS_UNUSED,
                    const char *argv[] OVS_UNUSED, void *idl_);
diff --git a/northd/northd.c b/northd/northd.c
index c714227b2..ecda98979 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -4289,6 +4289,12 @@  static void
 ovn_multicast_add(struct hmap *mcgroups, const struct multicast_group *group,
                   struct ovn_port *port)
 {
+    /* Store the chassis redirect port otherwise traffic will not be tunneled
+     * properly.
+     */
+    if (port->cr_port) {
+        port = port->cr_port;
+    }
     ovn_multicast_add_ports(mcgroups, port->od, group, &port, 1);
 }
 
diff --git a/tests/ovn.at b/tests/ovn.at
index ad08a7d63..957eb7850 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -22570,6 +22570,180 @@  OVN_CLEANUP_IC
 AT_CLEANUP
 ])
 
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([interconnection - static multicast])
+
+# Logical network:
+#
+#       AZ1                     |                     AZ2
+#   ---------------------------------------------------------------------
+#                               |
+#                               |     +-- LR2 --- LS2 --- LSP2 (sender)
+#                               |    /
+#     LSP1  --- LS1 --- LR1 --- TS ---
+#   (receiver)                  |    \
+#                               |     +-- LR3 --- LS3 --- LSP3 (receiver)
+#
+# LS1, LS2, LS3 configured to flood unregistered IP multicast.
+# LR1, LR2, LR3 configured to relay IP multicast.
+# LR1-LS1 configured to flood IP multicast traffic unconditionally.
+# LR3-LS3 configured to flood IP multicast traffic unconditionally.
+
+AT_CAPTURE_FILE([exp])
+AT_CAPTURE_FILE([rcv])
+check_packets() {
+    > exp
+    > rcv
+    if test "$1" = --uniq; then
+        sort="sort -u"; shift
+    else
+        sort=sort
+    fi
+    for tuple in "$@"; do
+        set $tuple; pcap=$1 type=$2
+        echo "--- $pcap" | tee -a exp >> rcv
+        $sort "$type" >> exp
+        $PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" $pcap | $sort >> rcv
+        echo | tee -a exp >> rcv
+    done
+
+    $at_diff exp rcv >/dev/null
+}
+
+ovn_init_ic_db
+ovn_start az1
+ovn_start az2
+
+net_add n1
+
+sim_add hv1
+as hv1
+check ovs-vsctl add-br br-phys
+ovn_az_attach az1 n1 br-phys 192.168.1.1 16
+check ovs-vsctl -- add-port br-int hv1-vif1 \
+    -- set interface hv1-vif1 external-ids:iface-id=lsp1 \
+       options:tx_pcap=hv1/vif1-tx.pcap \
+       options:rxq_pcap=hv1/vif1-rx.pcap
+check ovs-vsctl set open . external-ids:ovn-is-interconn=true
+
+sim_add hv2
+as hv2
+check ovs-vsctl add-br br-phys
+ovn_az_attach az2 n1 br-phys 192.168.2.1 16
+check ovs-vsctl -- add-port br-int hv2-vif1 \
+    -- set interface hv2-vif1 external-ids:iface-id=lsp2 \
+       options:tx_pcap=hv2/vif1-tx.pcap \
+       options:rxq_pcap=hv2/vif1-rx.pcap
+check ovs-vsctl -- add-port br-int hv2-vif2 \
+    -- set interface hv2-vif2 external-ids:iface-id=lsp3 \
+       options:tx_pcap=hv2/vif2-tx.pcap \
+       options:rxq_pcap=hv2/vif2-rx.pcap
+check ovs-vsctl set open . external-ids:ovn-is-interconn=true
+
+AT_CHECK([ovn-ic-nbctl create Transit_Switch name=ts], [0], [ignore])
+check ovn_as az1 ovn-nbctl wait-until logical_switch ts
+check ovn_as az2 ovn-nbctl wait-until logical_switch ts
+
+ovn_as az1
+check ovn-nbctl lr-add lr1 \
+    -- lrp-add lr1 lr1-ts 00:00:00:01:00:01 42.42.42.1/24 \
+    -- lrp-add lr1 lr1-ls1 00:00:00:01:01:00 43.43.43.1/24 \
+    -- lrp-set-gateway-chassis lr1-ts hv1
+check ovn-nbctl ls-add ls1 \
+    -- lsp-add ls1 ls1-lr1 \
+    -- lsp-set-addresses ls1-lr1 router \
+    -- lsp-set-type ls1-lr1 router \
+    -- lsp-set-options ls1-lr1 router-port=lr1-ls1 \
+    -- lsp-add ls1 lsp1
+check ovn-nbctl lsp-add ts ts-lr1 \
+    -- lsp-set-addresses ts-lr1 router \
+    -- lsp-set-type ts-lr1 router \
+    -- lsp-set-options ts-lr1 router-port=lr1-ts
+wait_for_ports_up
+
+ovn_as az2
+check ovn-nbctl lr-add lr2 \
+    -- lrp-add lr2 lr2-ts 00:00:00:02:00:01 42.42.42.2/24 \
+    -- lrp-add lr2 lr2-ls2 00:00:00:02:01:00 44.44.44.1/24 \
+    -- lrp-set-gateway-chassis lr2-ts hv2
+check ovn-nbctl ls-add ls2 \
+    -- lsp-add ls2 ls2-lr2 \
+    -- lsp-set-addresses ls2-lr2 router \
+    -- lsp-set-type ls2-lr2 router \
+    -- lsp-set-options ls2-lr2 router-port=lr2-ls2 \
+    -- lsp-add ls2 lsp2
+check ovn-nbctl lsp-add ts ts-lr2 \
+    -- lsp-set-addresses ts-lr2 router \
+    -- lsp-set-type ts-lr2 router \
+    -- lsp-set-options ts-lr2 router-port=lr2-ts
+
+check ovn-nbctl lr-add lr3 \
+    -- lrp-add lr3 lr3-ts 00:00:00:02:00:02 42.42.42.3/24 \
+    -- lrp-add lr3 lr3-ls3 00:00:00:02:02:00 44.44.45.1/24 \
+    -- lrp-set-gateway-chassis lr3-ts hv2
+check ovn-nbctl ls-add ls3 \
+    -- lsp-add ls3 ls3-lr3 \
+    -- lsp-set-addresses ls3-lr3 router \
+    -- lsp-set-type ls3-lr3 router \
+    -- lsp-set-options ls3-lr3 router-port=lr3-ls3 \
+    -- lsp-add ls3 lsp3
+check ovn-nbctl lsp-add ts ts-lr3 \
+    -- lsp-set-addresses ts-lr3 router \
+    -- lsp-set-type ts-lr3 router \
+    -- lsp-set-options ts-lr3 router-port=lr3-ts
+
+wait_for_ports_up
+
+dnl Enable unregistered IP multicast flooding and IP multicast relay.
+ovn_as az1
+check ovn-nbctl set logical_switch ls1 other_config:mcast_snoop="true" \
+    other_config:mcast_flood_unregistered="true"
+check ovn-nbctl set logical_router lr1 options:mcast_relay="true"
+check ovn-nbctl set logical_router_port lr1-ls1 options:mcast_flood="true"
+
+ovn_as az2
+check ovn-nbctl set logical_switch ls2 other_config:mcast_snoop="true" \
+    other_config:mcast_flood_unregistered="true"
+check ovn-nbctl set logical_router lr2 options:mcast_relay="true"
+check ovn-nbctl set logical_router_port lr2-ts options:mcast_flood="true"
+check ovn-nbctl set logical_switch ls3 other_config:mcast_snoop="true" \
+    other_config:mcast_flood_unregistered="true"
+check ovn-nbctl set logical_router lr3 options:mcast_relay="true"
+check ovn-nbctl set logical_router_port lr3-ls3 options:mcast_flood="true"
+
+check ovn_as az1 ovn-nbctl --wait=hv sync
+check ovn_as az2 ovn-nbctl --wait=hv sync
+
+# Pre-populate the hypervisors' ARP tables so that we don't lose any
+# packets for ARP resolution (native tunneling doesn't queue packets
+# for ARP resolution).
+OVN_POPULATE_ARP
+
+# Send an IP multicast packet from lsp2, it should be forwarded
+# statically to lsp1 and lsp3.
+> expected_az1
+> expected_az2
+send_ip_multicast_pkt hv2-vif1 hv2 \
+    000000000001 01005e000144 \
+    $(ip_to_hex 44 44 44 2) $(ip_to_hex 239 0 1 68) 1e 20 7c6b 11 \
+    e518e518000aed350000
+store_ip_multicast_pkt \
+    000000010100 01005e000144 \
+    $(ip_to_hex 44 44 44 2) $(ip_to_hex 239 0 1 68) 1e 1e 7e6b 11 \
+    e518e518000aed350000 expected_az1
+store_ip_multicast_pkt \
+    000000020200 01005e000144 \
+    $(ip_to_hex 44 44 44 2) $(ip_to_hex 239 0 1 68) 1e 1e 7e6b 11 \
+    e518e518000aed350000 expected_az2
+
+OVS_WAIT_UNTIL(
+  [check_packets 'hv1/vif1-tx.pcap expected_az1' \
+                 'hv2/vif2-tx.pcap expected_az2'],
+  [$at_diff -F'^---' exp rcv])
+
+AT_CLEANUP
+])
+
 OVN_FOR_EACH_NORTHD([
 AT_SETUP([ECMP static routes])
 ovn_start