We already store whether a datapath is a switch or not.  No need to do
an smap lookup through its external IDs every time we need this
information.

Also, move the functions that inspect Datapath_Binding.external_ids
out of ovn-util.c.  It makes more sense to just define them where
they're used.

Signed-off-by: Dumitru Ceara <[email protected]>
---
v2:
- Per Mark's comment: don't expose datapath_is_switch().
- Also moved other similar functions.
---
 controller/lflow.c          | 25 ++++++++++++++-----------
 controller/local_data.c     | 15 +++++++++++++++
 controller/ovn-controller.c | 10 ++++++++--
 controller/pinctrl.c        |  2 +-
 lib/ovn-util.c              | 18 ------------------
 lib/ovn-util.h              |  3 ---
 6 files changed, 38 insertions(+), 35 deletions(-)

diff --git a/controller/lflow.c b/controller/lflow.c
index b976c7d562..055c53a751 100644
--- a/controller/lflow.c
+++ b/controller/lflow.c
@@ -622,7 +622,7 @@ lflow_parse_ctrl_meter(const struct sbrec_logical_flow 
*lflow,
 
 static void
 add_matches_to_flow_table(const struct sbrec_logical_flow *lflow,
-                          const struct sbrec_datapath_binding *dp,
+                          const struct local_datapath *ldp,
                           struct hmap *matches, uint8_t ptable,
                           uint8_t output_ptable, struct ofpbuf *ovnacts,
                           bool ingress, struct lflow_ctx_in *l_ctx_in,
@@ -632,7 +632,7 @@ add_matches_to_flow_table(const struct sbrec_logical_flow 
*lflow,
         .sbrec_multicast_group_by_name_datapath
             = l_ctx_in->sbrec_multicast_group_by_name_datapath,
         .sbrec_port_binding_by_name = l_ctx_in->sbrec_port_binding_by_name,
-        .dp = dp,
+        .dp = ldp->datapath,
         .lflow = lflow,
         .lfrr = l_ctx_out->lfrr,
         .chassis_tunnels = l_ctx_in->chassis_tunnels,
@@ -652,7 +652,7 @@ add_matches_to_flow_table(const struct sbrec_logical_flow 
*lflow,
         .lookup_port = lookup_port_cb,
         .tunnel_ofport = tunnel_ofport_cb,
         .aux = &aux,
-        .is_switch = datapath_is_switch(dp),
+        .is_switch = ldp->is_switch,
         .group_table = l_ctx_out->group_table,
         .meter_table = l_ctx_out->meter_table,
         .lflow_uuid = lflow->header_.uuid,
@@ -674,13 +674,13 @@ add_matches_to_flow_table(const struct sbrec_logical_flow 
*lflow,
 
     struct expr_match *m;
     HMAP_FOR_EACH (m, hmap_node, matches) {
-        match_set_metadata(&m->match, htonll(dp->tunnel_key));
-        if (datapath_is_switch(dp)) {
+        match_set_metadata(&m->match, htonll(ldp->datapath->tunnel_key));
+        if (ldp->is_switch) {
             unsigned int reg_index
                 = (ingress ? MFF_LOG_INPORT : MFF_LOG_OUTPORT) - MFF_REG0;
             int64_t port_id = m->match.flow.regs[reg_index];
             if (port_id) {
-                int64_t dp_id = dp->tunnel_key;
+                int64_t dp_id = ldp->datapath->tunnel_key;
                 char buf[16];
                 get_unique_lport_key(dp_id, port_id, buf, sizeof(buf));
                 if (!sset_contains(l_ctx_in->related_lport_ids, buf)) {
@@ -731,7 +731,7 @@ add_matches_to_flow_table(const struct sbrec_logical_flow 
*lflow,
  */
 static struct expr *
 convert_match_to_expr(const struct sbrec_logical_flow *lflow,
-                      const struct sbrec_datapath_binding *dp,
+                      const struct local_datapath *ldp,
                       struct expr **prereqs,
                       const struct shash *addr_sets,
                       const struct shash *port_groups,
@@ -744,7 +744,8 @@ convert_match_to_expr(const struct sbrec_logical_flow 
*lflow,
 
     struct expr *e = expr_parse_string(lflow->match, &symtab, addr_sets,
                                        port_groups, &addr_sets_ref,
-                                       &port_groups_ref, dp->tunnel_key,
+                                       &port_groups_ref,
+                                       ldp->datapath->tunnel_key,
                                        &error);
     const char *addr_set_name;
     SSET_FOR_EACH (addr_set_name, &addr_sets_ref) {
@@ -791,7 +792,9 @@ consider_logical_flow__(const struct sbrec_logical_flow 
*lflow,
                         struct lflow_ctx_in *l_ctx_in,
                         struct lflow_ctx_out *l_ctx_out)
 {
-    if (!get_local_datapath(l_ctx_in->local_datapaths, dp->tunnel_key)) {
+    struct local_datapath *ldp = get_local_datapath(l_ctx_in->local_datapaths,
+                                                    dp->tunnel_key);
+    if (!ldp) {
         VLOG_DBG("lflow "UUID_FMT" is not for local datapath, skip",
                  UUID_ARGS(&lflow->header_.uuid));
         return;
@@ -904,7 +907,7 @@ consider_logical_flow__(const struct sbrec_logical_flow 
*lflow,
     /* Get match expr, either from cache or from lflow match. */
     switch (lcv_type) {
     case LCACHE_T_NONE:
-        expr = convert_match_to_expr(lflow, dp, &prereqs, l_ctx_in->addr_sets,
+        expr = convert_match_to_expr(lflow, ldp, &prereqs, l_ctx_in->addr_sets,
                                      l_ctx_in->port_groups, l_ctx_out->lfrr,
                                      &pg_addr_set_ref);
         if (!expr) {
@@ -968,7 +971,7 @@ consider_logical_flow__(const struct sbrec_logical_flow 
*lflow,
         break;
     }
 
-    add_matches_to_flow_table(lflow, dp, matches, ptable, output_ptable,
+    add_matches_to_flow_table(lflow, ldp, matches, ptable, output_ptable,
                               &ovnacts, ingress, l_ctx_in, l_ctx_out);
 
     /* Update cache if needed. */
diff --git a/controller/local_data.c b/controller/local_data.c
index 1857bfd1ba..98445902b7 100644
--- a/controller/local_data.c
+++ b/controller/local_data.c
@@ -50,6 +50,9 @@ static struct tracked_datapath *tracked_datapath_create(
     enum en_tracked_resource_type tracked_type,
     struct hmap *tracked_datapaths);
 
+static bool datapath_is_switch(const struct sbrec_datapath_binding *);
+static bool datapath_is_transit_switch(const struct sbrec_datapath_binding *);
+
 static uint64_t local_datapath_usage;
 
 struct local_datapath *
@@ -605,3 +608,15 @@ local_datapath_peer_port_add(struct local_datapath *ld,
     ld->peer_ports[ld->n_peer_ports - 1].local = local;
     ld->peer_ports[ld->n_peer_ports - 1].remote = remote;
 }
+
+static bool
+datapath_is_switch(const struct sbrec_datapath_binding *ldp)
+{
+    return smap_get(&ldp->external_ids, "logical-switch") != NULL;
+}
+
+static bool
+datapath_is_transit_switch(const struct sbrec_datapath_binding *ldp)
+{
+    return smap_get(&ldp->external_ids, "interconn-ts") != NULL;
+}
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 8631bccbc1..1a80ea87e2 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -634,6 +634,12 @@ alloc_id_to_ct_zone(const char *zone_name, struct simap 
*ct_zones,
     return true;
 }
 
+static int
+get_snat_ct_zone(const struct sbrec_datapath_binding *dp)
+{
+    return smap_get_int(&dp->external_ids, "snat-ct-zone", -1);
+}
+
 static void
 update_ct_zones(const struct shash *binding_lports,
                 const struct hmap *local_datapaths,
@@ -665,7 +671,7 @@ update_ct_zones(const struct shash *binding_lports,
         shash_add(&all_lds, dnat, ld);
         shash_add(&all_lds, snat, ld);
 
-        int req_snat_zone = datapath_snat_ct_zone(ld->datapath);
+        int req_snat_zone = get_snat_ct_zone(ld->datapath);
         if (req_snat_zone >= 0) {
             simap_put(&req_snat_zones, snat, req_snat_zone);
         }
@@ -1851,7 +1857,7 @@ ct_zones_datapath_binding_handler(struct engine_node 
*node, void *data)
             return false;
         }
 
-        int req_snat_zone = datapath_snat_ct_zone(dp);
+        int req_snat_zone = get_snat_ct_zone(dp);
         if (req_snat_zone == -1) {
             /* datapath snat ct zone is not set.  This condition will also hit
              * when CMS clears the snat-ct-zone for the logical router.
diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index 293aecea27..08eb90e6ec 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -4290,7 +4290,7 @@ run_buffered_binding(struct ovsdb_idl_index 
*sbrec_mac_binding_by_lport_ip,
          * a router datapath. Hence we can skip logical switch
          * datapaths.
          * */
-        if (datapath_is_switch(ld->datapath)) {
+        if (ld->is_switch) {
             continue;
         }
 
diff --git a/lib/ovn-util.c b/lib/ovn-util.c
index e6d0f73565..a22ae84fee 100644
--- a/lib/ovn-util.c
+++ b/lib/ovn-util.c
@@ -559,24 +559,6 @@ ovn_logical_flow_hash_datapath(const struct uuid 
*logical_datapath,
     return hash_add(hash, uuid_hash(logical_datapath));
 }
 
-bool
-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)
-{
-    return smap_get_int(&dp->external_ids, "snat-ct-zone", -1);
-}
-
 
 struct tnlid_node {
     struct hmap_node hmap_node;
diff --git a/lib/ovn-util.h b/lib/ovn-util.h
index 743d28745f..1fe91ba994 100644
--- a/lib/ovn-util.h
+++ b/lib/ovn-util.h
@@ -117,9 +117,6 @@ uint32_t ovn_logical_flow_hash(uint8_t table_id, enum 
ovn_pipeline pipeline,
                                const char *match, const char *actions);
 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_);
 
-- 
2.27.0

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to