diff mbox series

[ovs-dev,v2,3/3] northd: Don't parse LSP addresses twice.

Message ID 20230302131110.1665580-4-i.maximets@ovn.org
State Accepted
Headers show
Series northd: Optimize parsing of LSP addresses. | 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

Ilya Maximets March 2, 2023, 1:11 p.m. UTC 
At the point of IPAM configuration all the addresses are already parsed.
No need to waste time parsing them again.

Reviewed-by: Simon Horman <simon.horman@corigine.com>
Acked-by: Mark Michelson <mmichels@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
---
 northd/northd.c | 54 ++++++++++++++++---------------------------------
 1 file changed, 17 insertions(+), 37 deletions(-)
diff mbox series

Patch

diff --git a/northd/northd.c b/northd/northd.c
index 6d37ea713..868a8c3b8 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -1497,7 +1497,10 @@  struct ovn_port {
     const struct nbrec_logical_switch_port *nbsp; /* May be NULL. */
 
     struct lport_addresses *lsp_addrs;  /* Logical switch port addresses. */
-    unsigned int n_lsp_addrs;
+    unsigned int n_lsp_addrs;  /* Total length of lsp_addrs. */
+    unsigned int n_lsp_non_router_addrs; /* Number of elements from the
+                                          * beginning of 'lsp_addrs' extracted
+                                          * directly from LSP 'addresses'. */
 
     struct lport_addresses *ps_addrs;   /* Port security addresses. */
     unsigned int n_ps_addrs;
@@ -1817,35 +1820,21 @@  ipam_insert_ip_for_datapath(struct ovn_datapath *od, uint32_t ip)
 }
 
 static void
-ipam_insert_lsp_addresses(struct ovn_datapath *od, struct ovn_port *op,
-                          char *address)
+ipam_insert_lsp_addresses(struct ovn_datapath *od,
+                          struct lport_addresses *laddrs)
 {
-    if (!od || !op || !address || !strcmp(address, "unknown")
-        || !strcmp(address, "router") || is_dynamic_lsp_address(address)) {
-        return;
-    }
-
-    struct lport_addresses laddrs;
-    if (!extract_lsp_addresses(address, &laddrs)) {
-        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
-        VLOG_WARN_RL(&rl, "Extract addresses failed.");
-        return;
-    }
-    ipam_insert_mac(&laddrs.ea, true);
+    ipam_insert_mac(&laddrs->ea, true);
 
     /* IP is only added to IPAM if the switch's subnet option
      * is set, whereas MAC is always added to MACAM. */
     if (!od->ipam_info.allocated_ipv4s) {
-        destroy_lport_addresses(&laddrs);
         return;
     }
 
-    for (size_t j = 0; j < laddrs.n_ipv4_addrs; j++) {
-        uint32_t ip = ntohl(laddrs.ipv4_addrs[j].addr);
+    for (size_t j = 0; j < laddrs->n_ipv4_addrs; j++) {
+        uint32_t ip = ntohl(laddrs->ipv4_addrs[j].addr);
         ipam_insert_ip_for_datapath(od, ip);
     }
-
-    destroy_lport_addresses(&laddrs);
 }
 
 static void
@@ -1855,29 +1844,21 @@  ipam_add_port_addresses(struct ovn_datapath *od, struct ovn_port *op)
         return;
     }
 
-    if (op->nbsp) {
+    if (op->n_lsp_non_router_addrs) {
         /* Add all the port's addresses to address data structures. */
-        for (size_t i = 0; i < op->nbsp->n_addresses; i++) {
-            ipam_insert_lsp_addresses(od, op, op->nbsp->addresses[i]);
-        }
-    } else if (op->nbrp) {
-        struct lport_addresses lrp_networks;
-        if (!extract_lrp_networks(op->nbrp, &lrp_networks)) {
-            static struct vlog_rate_limit rl
-                = VLOG_RATE_LIMIT_INIT(1, 1);
-            VLOG_WARN_RL(&rl, "Extract addresses failed.");
-            return;
+        for (size_t i = 0; i < op->n_lsp_non_router_addrs; i++) {
+            ipam_insert_lsp_addresses(od, &op->lsp_addrs[i]);
         }
-        ipam_insert_mac(&lrp_networks.ea, true);
+    } else if (op->lrp_networks.ea_s[0]) {
+        ipam_insert_mac(&op->lrp_networks.ea, true);
 
         if (!op->peer || !op->peer->nbsp || !op->peer->od || !op->peer->od->nbs
             || !smap_get(&op->peer->od->nbs->other_config, "subnet")) {
-            destroy_lport_addresses(&lrp_networks);
             return;
         }
 
-        for (size_t i = 0; i < lrp_networks.n_ipv4_addrs; i++) {
-            uint32_t ip = ntohl(lrp_networks.ipv4_addrs[i].addr);
+        for (size_t i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) {
+            uint32_t ip = ntohl(op->lrp_networks.ipv4_addrs[i].addr);
             /* If the router has the first IP address of the subnet, don't add
              * it to IPAM. We already added this when we initialized IPAM for
              * the datapath. This will just result in an erroneous message
@@ -1887,8 +1868,6 @@  ipam_add_port_addresses(struct ovn_datapath *od, struct ovn_port *op)
                 ipam_insert_ip_for_datapath(op->peer->od, ip);
             }
         }
-
-        destroy_lport_addresses(&lrp_networks);
     }
 }
 
@@ -2573,6 +2552,7 @@  join_logical_ports(struct northd_input *input_data,
                     }
                     op->n_lsp_addrs++;
                 }
+                op->n_lsp_non_router_addrs = op->n_lsp_addrs;
 
                 op->ps_addrs
                     = xmalloc(sizeof *op->ps_addrs * nbsp->n_port_security);