ic: Add support for spine-leaf topology for transit switches.

Signed-off-by: Ilya Maximets <[email protected]>
igsilya · Jan 23, 2025 · ec74a9a · ec74a9a
1 parent 5b893e4
commit ec74a9a
Show file tree

Hide file tree

Showing 3 changed files with 237 additions and 22 deletions.
diff --git a/controller/binding.c b/controller/binding.c
@@ -2112,6 +2112,10 @@ build_local_bindings(struct binding_ctx_in *b_ctx_in,
     }
 }
 
+static bool consider_patch_port_for_local_datapaths(
+        const struct sbrec_port_binding *,
+        struct binding_ctx_in *, struct binding_ctx_out *);
+
 void
 binding_run(struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out)
 {
@@ -2155,7 +2159,9 @@ binding_run(struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out)
         switch (lport_type) {
         case LP_PATCH:
             update_related_lport(pb, b_ctx_out);
+            consider_patch_port_for_local_datapaths(pb, b_ctx_in, b_ctx_out);
             break;
+
         case LP_VTEP:
             update_related_lport(pb, b_ctx_out);
             struct lport *vtep_lport = xmalloc(sizeof *vtep_lport);
@@ -2925,7 +2931,7 @@ handle_updated_vif_lport(const struct sbrec_port_binding *pb,
     return true;
 }
 
-static void
+static bool
 consider_patch_port_for_local_datapaths(const struct sbrec_port_binding *pb,
                                         struct binding_ctx_in *b_ctx_in,
                                         struct binding_ctx_out *b_ctx_out)
@@ -2976,15 +2982,30 @@ consider_patch_port_for_local_datapaths(const struct sbrec_port_binding *pb,
         }
     }
 
-    if (sbrec_port_binding_is_updated(pb, SBREC_PORT_BINDING_COL_TYPE) &&
-       (pb->chassis == b_ctx_in->chassis_rec ||
-            is_additional_chassis(pb, b_ctx_in->chassis_rec))) {
+    /* If this chassis is requested - try to claim. */
+    if (pb->requested_chassis == b_ctx_in->chassis_rec) {
+        return claim_lport(pb, NULL, b_ctx_in->chassis_rec, NULL,
+                           !b_ctx_in->ovnsb_idl_txn, false,
+                           b_ctx_out->tracked_dp_bindings,
+                           b_ctx_out->if_mgr,
+                           b_ctx_out->postponed_ports);
+    }
+    /* If this chassis is claimed, but not requested to be or requested for
+     * some other chassis, but claimed by us - release. */
+    if ((!pb->requested_chassis && !pb->n_additional_chassis && pb->chassis)
+        || pb->chassis == b_ctx_in->chassis_rec
+        || is_additional_chassis(pb, b_ctx_in->chassis_rec)
+        || if_status_is_port_claimed(b_ctx_out->if_mgr, pb->logical_port)) {
+
         remove_local_lports(pb->logical_port, b_ctx_out);
-        release_lport(pb, b_ctx_in->chassis_rec,
-                             !b_ctx_in->ovnsb_idl_txn,
-                             b_ctx_out->tracked_dp_bindings,
-                             b_ctx_out->if_mgr);
+        if (!release_lport(pb, b_ctx_in->chassis_rec,
+                           !b_ctx_in->ovnsb_idl_txn,
+                           b_ctx_out->tracked_dp_bindings,
+                           b_ctx_out->if_mgr)) {
+            return false;
+        }
     }
+    return true;
 }
 
 static bool
@@ -3042,7 +3063,8 @@ handle_updated_port(struct binding_ctx_in *b_ctx_in,
 
     case LP_PATCH:
         update_related_lport(pb, b_ctx_out);
-        consider_patch_port_for_local_datapaths(pb, b_ctx_in, b_ctx_out);
+        handled = consider_patch_port_for_local_datapaths(pb, b_ctx_in,
+                                                              b_ctx_out);
         break;
 
     case LP_VTEP:
@@ -3084,8 +3106,9 @@ handle_updated_port(struct binding_ctx_in *b_ctx_in,
                          distributed_port, pb->logical_port);
             break;
         }
-        consider_patch_port_for_local_datapaths(distributed_pb, b_ctx_in,
-                                                b_ctx_out);
+        handled = consider_patch_port_for_local_datapaths(distributed_pb,
+                                                          b_ctx_in,
+                                                          b_ctx_out);
         break;
 
     case LP_EXTERNAL:

diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
@@ -503,10 +503,31 @@ find_crp_for_sb_pb(struct ic_context *ctx,
     return find_crp_from_lrp(ctx, peer);
 }
 
+static const struct nbrec_logical_switch_port *
+get_lsp_by_ts_port_name(struct ic_context *ctx, const char *ts_port_name)
+{
+    const struct nbrec_logical_switch_port *nb_lsp;
+    const struct nbrec_logical_switch_port *nb_lsp_key =
+        nbrec_logical_switch_port_index_init_row(ctx->nbrec_port_by_name);
+    nbrec_logical_switch_port_index_set_name(nb_lsp_key, ts_port_name);
+    nb_lsp = nbrec_logical_switch_port_index_find(ctx->nbrec_port_by_name,
+                                                  nb_lsp_key);
+    nbrec_logical_switch_port_index_destroy_row(nb_lsp_key);
+
+    return nb_lsp;
+}
+
 static const char *
-get_lrp_address_for_sb_pb(struct ic_context *ctx,
-                          const struct sbrec_port_binding *sb_pb)
+get_lp_address_for_sb_pb(struct ic_context *ctx,
+                         const struct sbrec_port_binding *sb_pb)
 {
+    const struct nbrec_logical_switch_port *nb_lsp;
+
+    nb_lsp = get_lsp_by_ts_port_name(ctx, sb_pb->logical_port);
+    if (!strcmp(nb_lsp->type, "switch")) {
+        return "unknown";
+    }
+
     const struct sbrec_port_binding *peer = find_peer_port(ctx, sb_pb);
     if (!peer) {
         return NULL;
@@ -597,7 +618,7 @@ sync_local_port(struct ic_context *ctx,
                 const struct nbrec_logical_switch_port *lsp)
 {
     /* Sync address from NB to ISB */
-    const char *address = get_lrp_address_for_sb_pb(ctx, sb_pb);
+    const char *address = get_lp_address_for_sb_pb(ctx, sb_pb);
     if (!address) {
         VLOG_DBG("Can't get logical router port address for logical"
                  " switch port %s", sb_pb->logical_port);
@@ -616,6 +637,10 @@ sync_local_port(struct ic_context *ctx,
         if (strcmp(crp->chassis->name, isb_pb->gateway)) {
             icsbrec_port_binding_set_gateway(isb_pb, crp->chassis->name);
         }
+    } else if (!strcmp(lsp->type, "switch") && sb_pb->chassis) {
+        if (strcmp(sb_pb->chassis->name, isb_pb->gateway)) {
+            icsbrec_port_binding_set_gateway(isb_pb, sb_pb->chassis->name);
+        }
     } else {
         if (isb_pb->gateway[0]) {
             icsbrec_port_binding_set_gateway(isb_pb, "");
@@ -715,7 +740,7 @@ create_isb_pb(struct ic_context *ctx,
     icsbrec_port_binding_set_logical_port(isb_pb, sb_pb->logical_port);
     icsbrec_port_binding_set_tunnel_key(isb_pb, pb_tnl_key);
 
-    const char *address = get_lrp_address_for_sb_pb(ctx, sb_pb);
+    const char *address = get_lp_address_for_sb_pb(ctx, sb_pb);
     if (address) {
         icsbrec_port_binding_set_address(isb_pb, address);
     }
@@ -804,7 +829,8 @@ port_binding_run(struct ic_context *ctx,
         for (int i = 0; i < ls->n_ports; i++) {
             lsp = ls->ports[i];
 
-            if (!strcmp(lsp->type, "router")) {
+            if (!strcmp(lsp->type, "router")
+                || !strcmp(lsp->type, "switch")) {
                 /* The port is local. */
                 sb_pb = find_lsp_in_sb(ctx, lsp);
                 if (!sb_pb) {
@@ -1306,13 +1332,8 @@ static const char *
 get_lrp_name_by_ts_port_name(struct ic_context *ctx, const char *ts_port_name)
 {
     const struct nbrec_logical_switch_port *nb_lsp;
-    const struct nbrec_logical_switch_port *nb_lsp_key =
-        nbrec_logical_switch_port_index_init_row(ctx->nbrec_port_by_name);
-    nbrec_logical_switch_port_index_set_name(nb_lsp_key, ts_port_name);
-    nb_lsp = nbrec_logical_switch_port_index_find(ctx->nbrec_port_by_name,
-                                                  nb_lsp_key);
-    nbrec_logical_switch_port_index_destroy_row(nb_lsp_key);
 
+    nb_lsp = get_lsp_by_ts_port_name(ctx, ts_port_name);
     if (!nb_lsp) {
         return NULL;
     }
@@ -1773,6 +1794,16 @@ route_run(struct ic_context *ctx,
     ICSBREC_PORT_BINDING_FOR_EACH_EQUAL (isb_pb, isb_pb_key,
                                          ctx->icsbrec_port_binding_by_az)
     {
+        const struct nbrec_logical_switch_port *nb_lsp;
+
+        nb_lsp = get_lsp_by_ts_port_name(ctx, isb_pb->logical_port);
+        if (!strcmp(nb_lsp->type, "switch")) {
+            VLOG_DBG("IC-SB Port_Binding '%s' on ts '%s' corresponds to a "
+                     "switch port, not considering for route collection.",
+                     isb_pb->logical_port, isb_pb->transit_switch);
+            continue;
+        }
+
         const char *ts_lrp_name =
             get_lrp_name_by_ts_port_name(ctx, isb_pb->logical_port);
         if (!ts_lrp_name) {

diff --git a/tests/ovn-ic.at b/tests/ovn-ic.at
@@ -2557,3 +2557,164 @@ OVN_CLEANUP_IC([az1], [az2])
 
 AT_CLEANUP
 ])
+
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([spine-leaf: 2 AZs, 2 HVs, 2 LSs, connected via transit spine switch])
+AT_KEYWORDS([spine leaf])
+AT_SKIP_IF([test $HAVE_SCAPY = no])
+
+ovn_init_ic_db
+
+ovn_start az1
+ovn_start az2
+
+# Logical network:
+# Single network 172.16.1.0/24.  Two switches with VIF ports on HVs in two
+# separate AZs, connected to a spine transit switch via 'switch' ports.
+
+ovn-ic-nbctl ts-add spine
+ovn_as az1 check ovn-nbctl ls-add ls1
+ovn_as az2 check ovn-nbctl ls-add ls2
+
+# Connect ls1 to spine.
+ovn_as az1
+check ovn-nbctl lsp-add spine spine-to-ls1
+check ovn-nbctl lsp-add ls1 ls1-to-spine
+check ovn-nbctl lsp-set-type spine-to-ls1 switch peer=ls1-to-spine
+check ovn-nbctl lsp-set-type ls1-to-spine switch peer=spine-to-ls1
+
+# Connect ls2 to spine.
+ovn_as az2
+check ovn-nbctl lsp-add spine spine-to-ls2
+check ovn-nbctl lsp-add ls2 ls2-to-spine
+check ovn-nbctl lsp-set-type spine-to-ls2 switch peer=ls2-to-spine
+check ovn-nbctl lsp-set-type ls2-to-spine switch peer=spine-to-ls2
+
+# Create logical port ls1-lp1 in ls1
+ovn_as az1 check ovn-nbctl lsp-add ls1 ls1-lp1 \
+-- lsp-set-addresses ls1-lp1 "f0:00:00:01:02:01 172.16.1.1"
+# Create logical port ls1-lp2 in ls1
+ovn_as az1 check ovn-nbctl lsp-add ls1 ls1-lp2 \
+-- lsp-set-addresses ls1-lp2 "f0:00:00:01:02:02 172.16.1.2"
+
+# Create logical port ls2-lp1 in ls2
+ovn_as az2 check ovn-nbctl lsp-add ls2 ls2-lp1 \
+-- lsp-set-addresses ls2-lp1 "f0:00:00:01:02:03 172.16.1.3"
+# Create logical port ls2-lp2 in ls2
+ovn_as az2 check ovn-nbctl lsp-add ls2 ls2-lp2 \
+-- lsp-set-addresses ls2-lp2 "f0:00:00:01:02:04 172.16.1.4"
+
+# Create hypervisors and OVS ports corresponding to logical ports.
+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 set open . external-ids:ovn-is-interconn=true
+ovs-vsctl -- add-port br-int vif1 -- \
+    set interface vif1 external-ids:iface-id=ls1-lp1 \
+    options:tx_pcap=hv1/vif1-tx.pcap \
+    options:rxq_pcap=hv1/vif1-rx.pcap \
+    ofport-request=1
+ovs-vsctl -- add-port br-int vif2 -- \
+    set interface vif2 external-ids:iface-id=ls1-lp2 \
+    options:tx_pcap=hv1/vif2-tx.pcap \
+    options:rxq_pcap=hv1/vif2-rx.pcap \
+    ofport-request=2
+
+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 set open . external-ids:ovn-is-interconn=true
+ovs-vsctl -- add-port br-int vif3 -- \
+    set interface vif3 external-ids:iface-id=ls2-lp1 \
+    options:tx_pcap=hv2/vif3-tx.pcap \
+    options:rxq_pcap=hv2/vif3-rx.pcap \
+    ofport-request=3
+ovs-vsctl -- add-port br-int vif4 -- \
+    set interface vif4 external-ids:iface-id=ls2-lp2 \
+    options:tx_pcap=hv2/vif4-tx.pcap \
+    options:rxq_pcap=hv2/vif4-rx.pcap \
+    ofport-request=4
+
+# Bind transit switch ports to their chassis.
+check ovn_as az1 ovn-nbctl lsp-set-options spine-to-ls1 requested-chassis=hv1
+check ovn_as az2 ovn-nbctl lsp-set-options spine-to-ls2 requested-chassis=hv2
+
+# 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
+
+ovn_as az1
+check ovn-nbctl --wait=hv sync
+ovn-sbctl dump-flows > az1/sbflows
+
+#wait_for_ports_up
+ovn_as az2
+check ovn-nbctl --wait=hv sync
+ovn-sbctl dump-flows > az2/sbflows
+
+check ovn-ic-nbctl --wait=sb sync
+
+ovn-ic-nbctl show > ic-nbctl.dump
+AT_CAPTURE_FILE([ic-nbctl.dump])
+
+(echo "---------ISB dump-----"
+ ovn-ic-sbctl show
+ echo "---------------------"
+ ovn-ic-sbctl list gateway
+ echo "---------------------"
+ ovn-ic-sbctl list datapath_binding
+ echo "---------------------"
+ ovn-ic-sbctl list port_binding
+ echo "---------------------"
+ ovn-ic-sbctl list route
+ echo "---------------------") > ic-sbctl.dump
+AT_CAPTURE_FILE([ic-sbctl.dump])
+
+# Send ip packets between the two ports.
+src_mac="f0:00:00:01:02:01"
+dst_mac="f0:00:00:01:02:03"
+src_ip=172.16.1.1
+dst_ip=172.16.1.3
+packet=$(fmt_pkt "Ether(dst='${dst_mac}', src='${src_mac}')/ \
+                  IP(src='${src_ip}', dst='${dst_ip}')/ \
+                  UDP(sport=1538, dport=4369)")
+check as hv1 ovs-appctl netdev-dummy/receive vif1 $packet
+
+# Check that datapath is not doing any extra work and sends the packet out
+# through the tunnel.
+AT_CHECK([ovn_as az1 as hv1 ovs-appctl ofproto/trace --names \
+                br-int in_port=vif1 $packet > ofproto-trace-1])
+AT_CAPTURE_FILE([ofproto-trace-1])
+AT_CHECK([grep 'Megaflow:' ofproto-trace-1], [0], [dnl
+Megaflow: recirc_id=0,eth,ip,in_port=vif1,dl_src=f0:00:00:01:02:01,dl_dst=f0:00:00:01:02:03,nw_ecn=0,nw_frag=no
+])
+AT_CHECK([grep -q \
+  'Datapath actions: tnl_push(tnl_port(genev_sys_6081).*out_port(br-phys))' \
+  ofproto-trace-1])
+
+# It's a little problematic to trace the other side, but we can check
+# datapath actions.
+AT_CHECK([as hv2 ovs-appctl dpctl/dump-flows --names \
+            | grep actions | sed 's/.*\(actions:.*\)/\1/' | sort], [0], [dnl
+actions:tnl_pop(genev_sys_6081)
+actions:vif3
+])
+
+# No modifications expected.
+AT_CHECK([echo $packet > expected])
+
+AT_CHECK([touch empty])
+
+# Check that it is delivered where needed and not delivered where not.
+OVN_CHECK_PACKETS([hv1/vif2-tx.pcap], [empty])
+OVN_CHECK_PACKETS([hv2/vif3-tx.pcap], [expected])
+OVN_CHECK_PACKETS([hv2/vif4-tx.pcap], [empty])
+
+OVN_CLEANUP_IC([az1], [az2])
+AT_CLEANUP
+])