diff mbox series

[ovs-dev,v1,2/3] northd: Add a flag 'only_dgp_peer_ports' to the SB Datapath.

Message ID 20250210153649.12676-1-numans@ovn.org
State Superseded
Headers show
Series controller: Optimize adding 'dps' to the local datapaths. | 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 fail github build: failed

Commit Message

Numan Siddique Feb. 10, 2025, 3:36 p.m. UTC
From: Numan Siddique <numans@ovn.org>

This flag is added to the external_ids column of SB Datapath_binding
table only if a logical switch meets the following
criteria:
  -  has one or more localnet ports.
  -  has one or more router ports and all its peers are distributed
     gateway ports.

ovn-controller in the following patch makes use of this flag.

Signed-off-by: Numan Siddique <numans@ovn.org>
---
 northd/northd.c     | 30 ++++++++++++++++++++---
 northd/northd.h     |  4 +++
 tests/ovn-northd.at | 59 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 89 insertions(+), 4 deletions(-)

Comments

Numan Siddique Feb. 10, 2025, 4:16 p.m. UTC | #1
On Mon, Feb 10, 2025 at 10:37 AM <numans@ovn.org> wrote:
>
> From: Numan Siddique <numans@ovn.org>
>
> This flag is added to the external_ids column of SB Datapath_binding
> table only if a logical switch meets the following
> criteria:
>   -  has one or more localnet ports.
>   -  has one or more router ports and all its peers are distributed
>      gateway ports.
>
> ovn-controller in the following patch makes use of this flag.
>
> Signed-off-by: Numan Siddique <numans@ovn.org>
> ---
>  northd/northd.c     | 30 ++++++++++++++++++++---
>  northd/northd.h     |  4 +++
>  tests/ovn-northd.at | 59 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 89 insertions(+), 4 deletions(-)
>
> diff --git a/northd/northd.c b/northd/northd.c
> index 7e2e6881ab..bf6485e02d 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -777,6 +777,14 @@ ovn_datapath_update_external_ids(struct ovn_datapath *od)
>              smap_add_format(&ids, "fdb_age_threshold",
>                              "%u", age_threshold);
>          }
> +
> +        /* A logical switch datapath is a true provider switch if

Please ignore this comment.  It's incorrect.  Forgot to clean this up.
I'll remove it in the next version.

Numan

> +         *   - It has (a) localnet port(s) and
> +         *   - All its logical router ports' peers are DGPs. */
> +        if (od->n_router_ports && od->n_router_ports == od->n_peer_dgw_ports
> +            && od->n_localnet_ports) {
> +            smap_add(&ids, "only_dgp_peer_ports", "true");
> +        }
>      }
>
>      /* Set snat-ct-zone */
> @@ -812,6 +820,20 @@ ovn_datapath_update_external_ids(struct ovn_datapath *od)
>      smap_destroy(&ids);
>  }
>
> +static void
> +update_datapaths_external_ids(struct ovn_datapaths *ls_datapaths,
> +                              struct ovn_datapaths *lr_datapaths)
> +{
> +    struct ovn_datapath *od;
> +    HMAP_FOR_EACH (od, key_node, &ls_datapaths->datapaths) {
> +        ovn_datapath_update_external_ids(od);
> +    }
> +
> +    HMAP_FOR_EACH (od, key_node, &lr_datapaths->datapaths) {
> +        ovn_datapath_update_external_ids(od);
> +    }
> +}
> +
>  static void
>  join_datapaths(const struct nbrec_logical_switch_table *nbrec_ls_table,
>                 const struct nbrec_logical_router_table *nbrec_lr_table,
> @@ -864,7 +886,6 @@ join_datapaths(const struct nbrec_logical_switch_table *nbrec_ls_table,
>              od->nbs = nbs;
>              ovs_list_remove(&od->list);
>              ovs_list_push_back(both, &od->list);
> -            ovn_datapath_update_external_ids(od);
>          } else {
>              od = ovn_datapath_create(datapaths, &nbs->header_.uuid,
>                                       nbs, NULL, NULL);
> @@ -892,7 +913,6 @@ join_datapaths(const struct nbrec_logical_switch_table *nbrec_ls_table,
>                  od->nbr = nbr;
>                  ovs_list_remove(&od->list);
>                  ovs_list_push_back(both, &od->list);
> -                ovn_datapath_update_external_ids(od);
>              } else {
>                  /* Can't happen! */
>                  static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
> @@ -1061,11 +1081,9 @@ build_datapaths(struct ovsdb_idl_txn *ovnsb_txn,
>          if (od->sb->tunnel_key != od->tunnel_key) {
>              sbrec_datapath_binding_set_tunnel_key(od->sb, od->tunnel_key);
>          }
> -        ovn_datapath_update_external_ids(od);
>      }
>      LIST_FOR_EACH (od, list, &nb_only) {
>          od->sb = sbrec_datapath_binding_insert(ovnsb_txn);
> -        ovn_datapath_update_external_ids(od);
>          sbrec_datapath_binding_set_tunnel_key(od->sb, od->tunnel_key);
>      }
>      ovn_destroy_tnlids(&dp_tnlids);
> @@ -2528,6 +2546,8 @@ join_logical_ports(const struct sbrec_port_binding_table *sbrec_pb_table,
>              /* Only used for the router type LSP whose peer is l3dgw_port */
>              op->peer->enable_router_port_acl = smap_get_bool(
>                      &op->peer->nbsp->options, "enable_router_port_acl", false);
> +
> +            op->peer->od->n_peer_dgw_ports++;
>          }
>      }
>
> @@ -18772,6 +18792,8 @@ ovnnb_db_run(struct northd_input *input_data,
>                  input_data->sbrec_ha_chassis_grp_by_name,
>                  &data->ls_datapaths.datapaths, &data->lr_datapaths.datapaths,
>                  &data->ls_ports, &data->lr_ports);
> +    update_datapaths_external_ids(&data->ls_datapaths,
> +                                  &data->lr_datapaths);
>      build_lb_port_related_data(ovnsb_txn,
>                                 input_data->sbrec_service_monitor_table,
>                                 input_data->svc_monitor_mac,
> diff --git a/northd/northd.h b/northd/northd.h
> index 5fca3526bd..a334415c66 100644
> --- a/northd/northd.h
> +++ b/northd/northd.h
> @@ -338,6 +338,10 @@ struct ovn_datapath {
>      size_t n_router_ports;
>      size_t n_allocated_router_ports;
>
> +    /* Indicates the number of router port's peers which are distributed
> +     * gateway ports (DGPs). */
> +    size_t n_peer_dgw_ports;
> +
>      struct hmap port_tnlids;
>      uint32_t port_key_hint;
>
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index 352339296c..01f0fe4f18 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -14605,6 +14605,65 @@ check rbr_match_custom
>
>  AT_CLEANUP
>
> +OVN_FOR_EACH_NORTHD_NO_HV([
> +AT_SETUP([ovn-northd -- only_dgp_peer_ports flag in SB datapath_binding])
> +ovn_start
> +
> +check ovn-nbctl ls-add sw0
> +check ovn-nbctl --wait=sb sync
> +
> +# Check that there is no option "only_dgp_peer_ports" in the
> +# SB.datapath_binding's external_ids.
> +AT_CHECK([ovn-sbctl get datapath_binding sw0 external_ids:only_dgp_peer_ports], [1], [ignore], [ignore])
> +
> +# only_dgp_peer_ports flag will be set in the SB:datapath_binding's external_ids
> +# only if the logical switch has
> +#  -  one or more localnet ports.
> +#  -  All its router ports' peeers are DGPs if one or more router
> +#     ports are present.
> +
> +check ovn-nbctl --wait=sb lsp-add sw0 ln-sw0 -- lsp-set-type ln-sw0 localnet
> +
> +AT_CHECK([ovn-sbctl get datapath_binding sw0 external_ids:only_dgp_peer_ports], [1], [ignore], [ignore])
> +
> +check ovn-nbctl lsp-add sw0 sw0-lr0 -- lsp-set-type sw0-lr0 router
> +check ovn-nbctl --wait=sb lsp-set-options sw0-lr0 router-port=lr0-sw0
> +
> +AT_CHECK([ovn-sbctl get datapath_binding sw0 external_ids:only_dgp_peer_ports], [1], [ignore], [ignore])
> +
> +check ovn-nbctl lr-add lr0
> +check ovn-nbctl --wait=sb lrp-add lr0 lr0-sw0 00:00:02:01:02:03 172.16.1.1/24
> +
> +AT_CHECK([ovn-sbctl get datapath_binding sw0 external_ids:only_dgp_peer_ports], [1], [ignore], [ignore])
> +
> +check ovn-nbctl --wait=sb lrp-set-gateway-chassis lr0-sw0 ovn-gw-1
> +AT_CHECK([ovn-sbctl get datapath_binding sw0 external_ids:only_dgp_peer_ports], [0], [dnl
> +"true"
> +])
> +
> +check ovn-nbctl lsp-add sw0 sw0-lr1 -- lsp-set-type sw0-lr1 router
> +check ovn-nbctl --wait=sb lsp-set-options sw0-lr1 router-port=lr1-sw0
> +check ovn-nbctl lr-add lr1
> +check ovn-nbctl --wait=sb lrp-add lr1 lr1-sw0 00:00:02:01:02:03 172.16.1.1/24
> +
> +AT_CHECK([ovn-sbctl get datapath_binding sw0 external_ids:only_dgp_peer_ports], [1], [ignore], [ignore])
> +
> +check ovn-nbctl --wait=sb lrp-set-gateway-chassis lr1-sw0 ovn-gw-1
> +AT_CHECK([ovn-sbctl get datapath_binding sw0 external_ids:only_dgp_peer_ports], [0], [dnl
> +"true"
> +])
> +
> +# Just add a normal port.
> +check ovn-nbctl --wait=sb lsp-add sw0 sw0p1
> +
> +check ovn-nbctl --wait=sb lrp-set-gateway-chassis lr1-sw0 ovn-gw-1
> +AT_CHECK([ovn-sbctl get datapath_binding sw0 external_ids:only_dgp_peer_ports], [0], [dnl
> +"true"
> +])
> +
> +AT_CLEANUP
> +])
> +
>  OVN_FOR_EACH_NORTHD_NO_HV([
>  AT_SETUP([ACL mismatched tiers])
>  ovn_start
> --
> 2.48.1
>
diff mbox series

Patch

diff --git a/northd/northd.c b/northd/northd.c
index 7e2e6881ab..bf6485e02d 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -777,6 +777,14 @@  ovn_datapath_update_external_ids(struct ovn_datapath *od)
             smap_add_format(&ids, "fdb_age_threshold",
                             "%u", age_threshold);
         }
+
+        /* A logical switch datapath is a true provider switch if
+         *   - It has (a) localnet port(s) and
+         *   - All its logical router ports' peers are DGPs. */
+        if (od->n_router_ports && od->n_router_ports == od->n_peer_dgw_ports
+            && od->n_localnet_ports) {
+            smap_add(&ids, "only_dgp_peer_ports", "true");
+        }
     }
 
     /* Set snat-ct-zone */
@@ -812,6 +820,20 @@  ovn_datapath_update_external_ids(struct ovn_datapath *od)
     smap_destroy(&ids);
 }
 
+static void
+update_datapaths_external_ids(struct ovn_datapaths *ls_datapaths,
+                              struct ovn_datapaths *lr_datapaths)
+{
+    struct ovn_datapath *od;
+    HMAP_FOR_EACH (od, key_node, &ls_datapaths->datapaths) {
+        ovn_datapath_update_external_ids(od);
+    }
+
+    HMAP_FOR_EACH (od, key_node, &lr_datapaths->datapaths) {
+        ovn_datapath_update_external_ids(od);
+    }
+}
+
 static void
 join_datapaths(const struct nbrec_logical_switch_table *nbrec_ls_table,
                const struct nbrec_logical_router_table *nbrec_lr_table,
@@ -864,7 +886,6 @@  join_datapaths(const struct nbrec_logical_switch_table *nbrec_ls_table,
             od->nbs = nbs;
             ovs_list_remove(&od->list);
             ovs_list_push_back(both, &od->list);
-            ovn_datapath_update_external_ids(od);
         } else {
             od = ovn_datapath_create(datapaths, &nbs->header_.uuid,
                                      nbs, NULL, NULL);
@@ -892,7 +913,6 @@  join_datapaths(const struct nbrec_logical_switch_table *nbrec_ls_table,
                 od->nbr = nbr;
                 ovs_list_remove(&od->list);
                 ovs_list_push_back(both, &od->list);
-                ovn_datapath_update_external_ids(od);
             } else {
                 /* Can't happen! */
                 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
@@ -1061,11 +1081,9 @@  build_datapaths(struct ovsdb_idl_txn *ovnsb_txn,
         if (od->sb->tunnel_key != od->tunnel_key) {
             sbrec_datapath_binding_set_tunnel_key(od->sb, od->tunnel_key);
         }
-        ovn_datapath_update_external_ids(od);
     }
     LIST_FOR_EACH (od, list, &nb_only) {
         od->sb = sbrec_datapath_binding_insert(ovnsb_txn);
-        ovn_datapath_update_external_ids(od);
         sbrec_datapath_binding_set_tunnel_key(od->sb, od->tunnel_key);
     }
     ovn_destroy_tnlids(&dp_tnlids);
@@ -2528,6 +2546,8 @@  join_logical_ports(const struct sbrec_port_binding_table *sbrec_pb_table,
             /* Only used for the router type LSP whose peer is l3dgw_port */
             op->peer->enable_router_port_acl = smap_get_bool(
                     &op->peer->nbsp->options, "enable_router_port_acl", false);
+
+            op->peer->od->n_peer_dgw_ports++;
         }
     }
 
@@ -18772,6 +18792,8 @@  ovnnb_db_run(struct northd_input *input_data,
                 input_data->sbrec_ha_chassis_grp_by_name,
                 &data->ls_datapaths.datapaths, &data->lr_datapaths.datapaths,
                 &data->ls_ports, &data->lr_ports);
+    update_datapaths_external_ids(&data->ls_datapaths,
+                                  &data->lr_datapaths);
     build_lb_port_related_data(ovnsb_txn,
                                input_data->sbrec_service_monitor_table,
                                input_data->svc_monitor_mac,
diff --git a/northd/northd.h b/northd/northd.h
index 5fca3526bd..a334415c66 100644
--- a/northd/northd.h
+++ b/northd/northd.h
@@ -338,6 +338,10 @@  struct ovn_datapath {
     size_t n_router_ports;
     size_t n_allocated_router_ports;
 
+    /* Indicates the number of router port's peers which are distributed
+     * gateway ports (DGPs). */
+    size_t n_peer_dgw_ports;
+
     struct hmap port_tnlids;
     uint32_t port_key_hint;
 
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 352339296c..01f0fe4f18 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -14605,6 +14605,65 @@  check rbr_match_custom
 
 AT_CLEANUP
 
+OVN_FOR_EACH_NORTHD_NO_HV([
+AT_SETUP([ovn-northd -- only_dgp_peer_ports flag in SB datapath_binding])
+ovn_start
+
+check ovn-nbctl ls-add sw0
+check ovn-nbctl --wait=sb sync
+
+# Check that there is no option "only_dgp_peer_ports" in the
+# SB.datapath_binding's external_ids.
+AT_CHECK([ovn-sbctl get datapath_binding sw0 external_ids:only_dgp_peer_ports], [1], [ignore], [ignore])
+
+# only_dgp_peer_ports flag will be set in the SB:datapath_binding's external_ids
+# only if the logical switch has
+#  -  one or more localnet ports.
+#  -  All its router ports' peeers are DGPs if one or more router
+#     ports are present.
+
+check ovn-nbctl --wait=sb lsp-add sw0 ln-sw0 -- lsp-set-type ln-sw0 localnet
+
+AT_CHECK([ovn-sbctl get datapath_binding sw0 external_ids:only_dgp_peer_ports], [1], [ignore], [ignore])
+
+check ovn-nbctl lsp-add sw0 sw0-lr0 -- lsp-set-type sw0-lr0 router
+check ovn-nbctl --wait=sb lsp-set-options sw0-lr0 router-port=lr0-sw0
+
+AT_CHECK([ovn-sbctl get datapath_binding sw0 external_ids:only_dgp_peer_ports], [1], [ignore], [ignore])
+
+check ovn-nbctl lr-add lr0
+check ovn-nbctl --wait=sb lrp-add lr0 lr0-sw0 00:00:02:01:02:03 172.16.1.1/24
+
+AT_CHECK([ovn-sbctl get datapath_binding sw0 external_ids:only_dgp_peer_ports], [1], [ignore], [ignore])
+
+check ovn-nbctl --wait=sb lrp-set-gateway-chassis lr0-sw0 ovn-gw-1
+AT_CHECK([ovn-sbctl get datapath_binding sw0 external_ids:only_dgp_peer_ports], [0], [dnl
+"true"
+])
+
+check ovn-nbctl lsp-add sw0 sw0-lr1 -- lsp-set-type sw0-lr1 router
+check ovn-nbctl --wait=sb lsp-set-options sw0-lr1 router-port=lr1-sw0
+check ovn-nbctl lr-add lr1
+check ovn-nbctl --wait=sb lrp-add lr1 lr1-sw0 00:00:02:01:02:03 172.16.1.1/24
+
+AT_CHECK([ovn-sbctl get datapath_binding sw0 external_ids:only_dgp_peer_ports], [1], [ignore], [ignore])
+
+check ovn-nbctl --wait=sb lrp-set-gateway-chassis lr1-sw0 ovn-gw-1
+AT_CHECK([ovn-sbctl get datapath_binding sw0 external_ids:only_dgp_peer_ports], [0], [dnl
+"true"
+])
+
+# Just add a normal port.
+check ovn-nbctl --wait=sb lsp-add sw0 sw0p1
+
+check ovn-nbctl --wait=sb lrp-set-gateway-chassis lr1-sw0 ovn-gw-1
+AT_CHECK([ovn-sbctl get datapath_binding sw0 external_ids:only_dgp_peer_ports], [0], [dnl
+"true"
+])
+
+AT_CLEANUP
+])
+
 OVN_FOR_EACH_NORTHD_NO_HV([
 AT_SETUP([ACL mismatched tiers])
 ovn_start