diff mbox series

[ovs-dev,v2,1/4] ic: remove orphan ovn interconnection routes

Message ID 20221206102001.3058710-2-odivlad@gmail.com
State Changes Requested
Headers show
Series OVN IC multiple same routes fixes | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/github-robot-_ovn-kubernetes success github build: passed

Commit Message

Vladislav Odintsov Dec. 6, 2022, 10:19 a.m. UTC 
Before this patch if one deletes transit switch through which there were
routes in ICSB:Route table, such routes were left forever in the DB.

Now we validate that each ICSB:Route has an appropriate transit switch.

Signed-off-by: Vladislav Odintsov <odivlad@gmail.com>
---
 ic/ovn-ic.c     | 40 +++++++++++++++++++++++++++
 tests/ovn-ic.at | 73 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 113 insertions(+)

Comments

Dumitru Ceara Dec. 9, 2022, 2:43 p.m. UTC | #1 
On 12/6/22 11:19, Vladislav Odintsov wrote:
> Before this patch if one deletes transit switch through which there were
> routes in ICSB:Route table, such routes were left forever in the DB.
> 
> Now we validate that each ICSB:Route has an appropriate transit switch.
> 
> Signed-off-by: Vladislav Odintsov <odivlad@gmail.com>
> ---
>  ic/ovn-ic.c     | 40 +++++++++++++++++++++++++++
>  tests/ovn-ic.at | 73 +++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 113 insertions(+)
> 
> diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
> index e5c193d9d..d3bc08761 100644
> --- a/ic/ovn-ic.c
> +++ b/ic/ovn-ic.c
> @@ -71,6 +71,7 @@ struct ic_context {
>      struct ovsdb_idl_index *icsbrec_port_binding_by_az;
>      struct ovsdb_idl_index *icsbrec_port_binding_by_ts;
>      struct ovsdb_idl_index *icsbrec_port_binding_by_ts_az;
> +    struct ovsdb_idl_index *icsbrec_route_by_az;
>      struct ovsdb_idl_index *icsbrec_route_by_ts;
>      struct ovsdb_idl_index *icsbrec_route_by_ts_az;
>  };
> @@ -1612,6 +1613,38 @@ advertise_lr_routes(struct ic_context *ctx,
>      hmap_destroy(&routes_ad);
>  }
>  
> +static void
> +delete_orphan_ic_routes(struct ic_context *ctx,
> +                         const struct icsbrec_availability_zone *az)
> +{
> +    const struct icsbrec_route *isb_route, *isb_route_key =
> +        icsbrec_route_index_init_row(ctx->icsbrec_route_by_az);
> +    icsbrec_route_index_set_availability_zone(isb_route_key, az);
> +
> +    const struct icnbrec_transit_switch *t_sw, *t_sw_key;
> +
> +    ICSBREC_ROUTE_FOR_EACH_EQUAL (isb_route, isb_route_key,
> +                                  ctx->icsbrec_route_by_az)
> +    {
> +        t_sw_key = icnbrec_transit_switch_index_init_row(
> +            ctx->icnbrec_transit_switch_by_name);
> +        icnbrec_transit_switch_index_set_name(t_sw_key,
> +            isb_route->transit_switch);
> +        t_sw = icnbrec_transit_switch_index_find(
> +            ctx->icnbrec_transit_switch_by_name, t_sw_key);
> +        icnbrec_transit_switch_index_destroy_row(t_sw_key);
> +
> +        if (!t_sw) {
> +            VLOG_WARN("Deleting orphan ICDB:Route: %s->%s (%s, rtb:%s, "
> +                      "transit switch: %s)", isb_route->ip_prefix,
> +                      isb_route->nexthop, isb_route->origin,
> +                      isb_route->route_table, isb_route->transit_switch);

I guess this slipped through.  We agreed to change this to VLOG_INFO_RL
in v1.  With that addressed please feel free to add my ack to v3:

Acked-by: Dumitru Ceara <dceara@redhat.com>

Thanks!
diff mbox series

Patch

diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
index e5c193d9d..d3bc08761 100644
--- a/ic/ovn-ic.c
+++ b/ic/ovn-ic.c
@@ -71,6 +71,7 @@  struct ic_context {
     struct ovsdb_idl_index *icsbrec_port_binding_by_az;
     struct ovsdb_idl_index *icsbrec_port_binding_by_ts;
     struct ovsdb_idl_index *icsbrec_port_binding_by_ts_az;
+    struct ovsdb_idl_index *icsbrec_route_by_az;
     struct ovsdb_idl_index *icsbrec_route_by_ts;
     struct ovsdb_idl_index *icsbrec_route_by_ts_az;
 };
@@ -1612,6 +1613,38 @@  advertise_lr_routes(struct ic_context *ctx,
     hmap_destroy(&routes_ad);
 }
 
+static void
+delete_orphan_ic_routes(struct ic_context *ctx,
+                         const struct icsbrec_availability_zone *az)
+{
+    const struct icsbrec_route *isb_route, *isb_route_key =
+        icsbrec_route_index_init_row(ctx->icsbrec_route_by_az);
+    icsbrec_route_index_set_availability_zone(isb_route_key, az);
+
+    const struct icnbrec_transit_switch *t_sw, *t_sw_key;
+
+    ICSBREC_ROUTE_FOR_EACH_EQUAL (isb_route, isb_route_key,
+                                  ctx->icsbrec_route_by_az)
+    {
+        t_sw_key = icnbrec_transit_switch_index_init_row(
+            ctx->icnbrec_transit_switch_by_name);
+        icnbrec_transit_switch_index_set_name(t_sw_key,
+            isb_route->transit_switch);
+        t_sw = icnbrec_transit_switch_index_find(
+            ctx->icnbrec_transit_switch_by_name, t_sw_key);
+        icnbrec_transit_switch_index_destroy_row(t_sw_key);
+
+        if (!t_sw) {
+            VLOG_WARN("Deleting orphan ICDB:Route: %s->%s (%s, rtb:%s, "
+                      "transit switch: %s)", isb_route->ip_prefix,
+                      isb_route->nexthop, isb_route->origin,
+                      isb_route->route_table, isb_route->transit_switch);
+            icsbrec_route_delete(isb_route);
+        }
+    }
+    icsbrec_route_index_destroy_row(isb_route_key);
+}
+
 static void
 route_run(struct ic_context *ctx,
           const struct icsbrec_availability_zone *az)
@@ -1620,6 +1653,8 @@  route_run(struct ic_context *ctx,
         return;
     }
 
+    delete_orphan_ic_routes(ctx, az);
+
     struct hmap ic_lrs = HMAP_INITIALIZER(&ic_lrs);
     const struct icsbrec_port_binding *isb_pb;
     const struct icsbrec_port_binding *isb_pb_key =
@@ -1908,6 +1943,10 @@  main(int argc, char *argv[])
                                   &icsbrec_port_binding_col_transit_switch,
                                   &icsbrec_port_binding_col_availability_zone);
 
+    struct ovsdb_idl_index *icsbrec_route_by_az
+        = ovsdb_idl_index_create1(ovnisb_idl_loop.idl,
+                                  &icsbrec_route_col_availability_zone);
+
     struct ovsdb_idl_index *icsbrec_route_by_ts
         = ovsdb_idl_index_create1(ovnisb_idl_loop.idl,
                                   &icsbrec_route_col_transit_switch);
@@ -1962,6 +2001,7 @@  main(int argc, char *argv[])
                 .icsbrec_port_binding_by_az = icsbrec_port_binding_by_az,
                 .icsbrec_port_binding_by_ts = icsbrec_port_binding_by_ts,
                 .icsbrec_port_binding_by_ts_az = icsbrec_port_binding_by_ts_az,
+                .icsbrec_route_by_az = icsbrec_route_by_az,
                 .icsbrec_route_by_ts = icsbrec_route_by_ts,
                 .icsbrec_route_by_ts_az = icsbrec_route_by_ts_az,
             };
diff --git a/tests/ovn-ic.at b/tests/ovn-ic.at
index 0bdfc55e6..e234b7fb9 100644
--- a/tests/ovn-ic.at
+++ b/tests/ovn-ic.at
@@ -121,6 +121,79 @@  OVN_CLEANUP_IC
 AT_CLEANUP
 ])
 
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([ovn-ic -- route deletion upon TS deletion])
+
+ovn_init_ic_db
+net_add n1
+
+# 1 GW per AZ
+for i in 1 2; do
+    az=az$i
+    ovn_start $az
+    sim_add gw-$az
+    as gw-$az
+    check ovs-vsctl add-br br-phys
+    ovn_az_attach $az n1 br-phys 192.168.1.$i
+    check ovs-vsctl set open . external-ids:ovn-is-interconn=true
+    check ovn-nbctl set nb-global . \
+        options:ic-route-adv=true \
+        options:ic-route-adv-default=true \
+        options:ic-route-learn=true \
+        options:ic-route-learn-default=true
+done
+
+create_ic_infra() {
+    az_id=$1
+    ts_id=$2
+    az=az$i
+
+    lsp=lsp${az_id}-${ts_id}
+    lrp=lrp${az_id}-${ts_id}
+    ts=ts${az_id}-${ts_id}
+    lr=lr${az_id}-${ts_id}
+
+    ovn_as $az
+
+    check ovn-ic-nbctl ts-add $ts
+    check ovn-nbctl lr-add $lr
+    check ovn-nbctl lrp-add $lr $lrp 00:00:00:00:00:0$az_id 10.0.$az_id.1/24
+    check ovn-nbctl lrp-set-gateway-chassis $lrp gw-$az
+
+    check ovn-nbctl lsp-add $ts $lsp -- \
+        lsp-set-addresses $lsp router -- \
+        lsp-set-type $lsp router -- \
+        lsp-set-options $lsp router-port=$lrp
+
+    check ovn-nbctl lr-route-add $lr 192.168.0.0/16 10.0.$az_id.10
+}
+
+create_ic_infra 1 1
+create_ic_infra 1 2
+create_ic_infra 2 1
+
+ovn_as az1
+
+wait_row_count ic-sb:Route 3 ip_prefix=192.168.0.0/16
+
+# remove transit switch 1 (from az1) and check if its route is deleted
+# same route from another AZ and ts should remain, as
+check ovn-ic-nbctl ts-del ts1-1
+sleep 2
+ovn-ic-sbctl list route
+ovn-ic-nbctl list transit_switch
+wait_row_count ic-sb:route 2 ip_prefix=192.168.0.0/16
+ovn-ic-sbctl list route
+
+for i in 1 2; do
+    az=az$i
+    OVN_CLEANUP_SBOX(gw-$az)
+    OVN_CLEANUP_AZ([$az])
+done
+OVN_CLEANUP_IC
+AT_CLEANUP
+])
+
 OVN_FOR_EACH_NORTHD([
 AT_SETUP([ovn-ic -- gateway sync])