[ovs-dev] [PATCH v3 ovn 3/4] ovn-northd: Refactor parsing of *_force_snat_ip.

Dumitru Ceara dceara at redhat.com
Thu Sep 17 12:51:37 UTC 2020


Avoid reparsing the *_force_snat_ip addresses for every logical router port.
These addresses are defined once for the whole router.

Signed-off-by: Dumitru Ceara <dceara at redhat.com>
---
 lib/ovn-util.c      |    6 ++++
 lib/ovn-util.h      |    2 +
 northd/ovn-northd.c |   69 ++++++++++++++++++++++++---------------------------
 3 files changed, 40 insertions(+), 37 deletions(-)

diff --git a/lib/ovn-util.c b/lib/ovn-util.c
index cdb5e18..a8cf6c9 100644
--- a/lib/ovn-util.c
+++ b/lib/ovn-util.c
@@ -320,6 +320,12 @@ extract_sbrec_binding_first_mac(const struct sbrec_port_binding *binding,
     return ret;
 }
 
+bool
+empty_lport_addresses(struct lport_addresses *laddrs)
+{
+    return !laddrs->n_ipv4_addrs && !laddrs->n_ipv6_addrs;
+}
+
 void
 destroy_lport_addresses(struct lport_addresses *laddrs)
 {
diff --git a/lib/ovn-util.h b/lib/ovn-util.h
index d9aadcb..3343f28 100644
--- a/lib/ovn-util.h
+++ b/lib/ovn-util.h
@@ -74,7 +74,7 @@ bool extract_lrp_networks(const struct nbrec_logical_router_port *,
                           struct lport_addresses *);
 bool extract_sbrec_binding_first_mac(const struct sbrec_port_binding *binding,
                                      struct eth_addr *ea);
-
+bool empty_lport_addresses(struct lport_addresses *);
 void destroy_lport_addresses(struct lport_addresses *);
 
 char *alloc_nat_zone_key(const struct uuid *key, const char *type);
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index f79ed99..43bd7b5 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -625,6 +625,8 @@ struct ovn_datapath {
 
     /* SNAT IPs used by the router. */
     struct sset snat_ips;
+    struct lport_addresses dnat_force_snat_addrs;
+    struct lport_addresses lb_force_snat_addrs;
 
     struct ovn_port **localnet_ports;
     size_t n_localnet_ports;
@@ -670,32 +672,31 @@ nat_entry_is_v6(const struct ovn_nat *nat_entry)
 static void
 init_nat_entries(struct ovn_datapath *od)
 {
-    struct lport_addresses snat_addrs;
-
     if (!od->nbr) {
         return;
     }
 
     sset_init(&od->snat_ips);
-    if (get_force_snat_ip(od, "dnat", &snat_addrs)) {
-        if (snat_addrs.n_ipv4_addrs) {
-            sset_add(&od->snat_ips, snat_addrs.ipv4_addrs[0].addr_s);
+    if (get_force_snat_ip(od, "dnat", &od->dnat_force_snat_addrs)) {
+        if (&od->dnat_force_snat_addrs.n_ipv4_addrs) {
+            sset_add(&od->snat_ips,
+                     od->dnat_force_snat_addrs.ipv4_addrs[0].addr_s);
         }
-        if (snat_addrs.n_ipv6_addrs) {
-            sset_add(&od->snat_ips, snat_addrs.ipv6_addrs[0].addr_s);
+        if (&od->dnat_force_snat_addrs.n_ipv6_addrs) {
+            sset_add(&od->snat_ips,
+                     od->dnat_force_snat_addrs.ipv6_addrs[0].addr_s);
         }
-        destroy_lport_addresses(&snat_addrs);
     }
 
-    memset(&snat_addrs, 0, sizeof(snat_addrs));
-    if (get_force_snat_ip(od, "lb", &snat_addrs)) {
-        if (snat_addrs.n_ipv4_addrs) {
-            sset_add(&od->snat_ips, snat_addrs.ipv4_addrs[0].addr_s);
+    if (get_force_snat_ip(od, "lb", &od->lb_force_snat_addrs)) {
+        if (od->lb_force_snat_addrs.n_ipv4_addrs) {
+            sset_add(&od->snat_ips,
+                     od->lb_force_snat_addrs.ipv4_addrs[0].addr_s);
         }
-        if (snat_addrs.n_ipv6_addrs) {
-            sset_add(&od->snat_ips, snat_addrs.ipv6_addrs[0].addr_s);
+        if (od->lb_force_snat_addrs.n_ipv6_addrs) {
+            sset_add(&od->snat_ips,
+                     od->lb_force_snat_addrs.ipv6_addrs[0].addr_s);
         }
-        destroy_lport_addresses(&snat_addrs);
     }
 
     if (!od->nbr->n_nat) {
@@ -736,6 +737,9 @@ destroy_nat_entries(struct ovn_datapath *od)
     }
 
     sset_destroy(&od->snat_ips);
+    destroy_lport_addresses(&od->dnat_force_snat_addrs);
+    destroy_lport_addresses(&od->lb_force_snat_addrs);
+
     for (size_t i = 0; i < od->nbr->n_nat; i++) {
         destroy_lport_addresses(&od->nat_entries[i].ext_addrs);
     }
@@ -9477,12 +9481,10 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
 
         struct sset nat_entries = SSET_INITIALIZER(&nat_entries);
 
-        struct lport_addresses dnat_force_snat_addrs;
-        struct lport_addresses lb_force_snat_addrs;
-        bool dnat_force_snat_ip = get_force_snat_ip(od, "dnat",
-                                                    &dnat_force_snat_addrs);
-        bool lb_force_snat_ip = get_force_snat_ip(od, "lb",
-                                                  &lb_force_snat_addrs);
+        bool dnat_force_snat_ip =
+            !empty_lport_addresses(&od->dnat_force_snat_addrs);
+        bool lb_force_snat_ip =
+            !empty_lport_addresses(&od->lb_force_snat_addrs);
 
         for (int i = 0; i < od->nbr->n_nat; i++) {
             const struct nbrec_nat *nat;
@@ -9982,23 +9984,25 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
         /* Handle force SNAT options set in the gateway router. */
         if (!od->l3dgw_port) {
             if (dnat_force_snat_ip) {
-                if (dnat_force_snat_addrs.n_ipv4_addrs) {
+                if (od->dnat_force_snat_addrs.n_ipv4_addrs) {
                     build_lrouter_force_snat_flows(lflows, od, "4",
-                        dnat_force_snat_addrs.ipv4_addrs[0].addr_s, "dnat");
+                        od->dnat_force_snat_addrs.ipv4_addrs[0].addr_s,
+                        "dnat");
                 }
-                if (dnat_force_snat_addrs.n_ipv6_addrs) {
+                if (od->dnat_force_snat_addrs.n_ipv6_addrs) {
                     build_lrouter_force_snat_flows(lflows, od, "6",
-                        dnat_force_snat_addrs.ipv6_addrs[0].addr_s, "dnat");
+                        od->dnat_force_snat_addrs.ipv6_addrs[0].addr_s,
+                        "dnat");
                 }
             }
             if (lb_force_snat_ip) {
-                if (lb_force_snat_addrs.n_ipv4_addrs) {
+                if (od->lb_force_snat_addrs.n_ipv4_addrs) {
                     build_lrouter_force_snat_flows(lflows, od, "4",
-                        lb_force_snat_addrs.ipv4_addrs[0].addr_s, "lb");
+                        od->lb_force_snat_addrs.ipv4_addrs[0].addr_s, "lb");
                 }
-                if (lb_force_snat_addrs.n_ipv6_addrs) {
+                if (od->lb_force_snat_addrs.n_ipv6_addrs) {
                     build_lrouter_force_snat_flows(lflows, od, "6",
-                        lb_force_snat_addrs.ipv6_addrs[0].addr_s, "lb");
+                        od->lb_force_snat_addrs.ipv6_addrs[0].addr_s, "lb");
                 }
             }
 
@@ -10015,13 +10019,6 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
                           "ip", "flags.loopback = 1; ct_dnat;");
         }
 
-        if (dnat_force_snat_ip) {
-            destroy_lport_addresses(&dnat_force_snat_addrs);
-        }
-        if (lb_force_snat_ip) {
-            destroy_lport_addresses(&lb_force_snat_addrs);
-        }
-
         /* Load balancing and packet defrag are only valid on
          * Gateway routers or router with gateway port. */
         if (!smap_get(&od->nbr->options, "chassis") && !od->l3dgw_port) {



More information about the dev mailing list