Blob Blame History Raw
From 74e1bf4dd0f6c62602ab708eabb5534a274a4d75 Mon Sep 17 00:00:00 2001
From: Dumitru Ceara <dceara@redhat.com>
Date: Tue, 28 Apr 2020 12:39:26 +0200
Subject: [PATCH] ovn-northd: Optimize flows for LB Hairpin traffic.

In order to detect that traffic was hairpinned due to logical switch load
balancers we need to match on source and destination IPs of packets (and
protocol ports potentially) in table ls_in_pre_hairpin.

For this, until now, we created 2 logical flows for each backend of a load
balancer VIP. However, in scenarios where large load balancers (i.e.,
with large numbers of backends) are applied to multiple logical
switches, this might generate logical flow count explosion.

One optimization is to generate a single logical flow per VIP that
combines all conditions generated for each backend. This reduces load on
the SB DB because of lower number of logical flows and also reduces
overall DB size because of less overhead due to other fields on the
logical_flow records.

Comparison of various performance aspects when running OVN with the NB
database attached to the bug report on a deployment with all VIFs bound
to a single node (62 load balancer VIPs with 513 load balancer
backends, applied on 106 logical switches):

Without this patch:
- SB database size: 60MB
- # of pre-hairpin logical flows: 109074
- # of logical flows: 159414
- ovn-controller max loop iteration time when processing SB DB: 8803ms
- ovn-northd max loop iteration time: 3988ms

With this patch:
- SB database size: 29MB (~50% decrease)
- # of pre-hairpin logical flows: 13250 (~88% decrease)
- # of logical flows: 63590 (~60% decrease)
- ovn-controller max loop iteration time when processing SB DB: 5585ms
- ovn-northd max loop iteration time: 1594ms

Reported-by: Aniket Bhat <anbhat@redhat.com>
Reported-at: https://bugzilla.redhat.com/1827403
Fixes: 1be8ac65bc60 ("ovn-northd: Support hairpinning for logical switch load balancing.")
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Numan Siddique <numans@ovn.org>
(cherry picked from upstream commit 97e82ae5f135a088c9e95b49122d8217718d23f4)

Change-Id: Id713209f8bd159e8ad924e91681bab784606faff
---
 northd/ovn-northd.8.xml |  4 +--
 northd/ovn-northd.c     | 79 ++++++++++++++++++++++++++++++++-----------------
 2 files changed, 54 insertions(+), 29 deletions(-)

diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index d39e259..1f81742 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -559,14 +559,14 @@
     <h3>Ingress Table 11: Pre-Hairpin</h3>
     <ul>
       <li>
-        For all configured load balancer backends a priority-2 flow that
+        For all configured load balancer VIPs a priority-2 flow that
         matches on traffic that needs to be hairpinned, i.e., after load
         balancing the destination IP matches the source IP, which sets
         <code>reg0[6] = 1 </code> and executes <code>ct_snat(VIP)</code>
         to force replies to these packets to come back through OVN.
       </li>
       <li>
-        For all configured load balancer backends a priority-1 flow that
+        For all configured load balancer VIPs a priority-1 flow that
         matches on replies to hairpinned traffic, i.e., destination IP is VIP,
         source IP is the backend IP and source L4 port is backend port, which
         sets <code>reg0[6] = 1 </code> and executes <code>ct_snat;</code>.
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 0082e2e..5ada3ae 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -5542,52 +5542,77 @@ build_lb_hairpin_rules(struct ovn_datapath *od, struct hmap *lflows,
                        struct ovn_lb *lb, struct lb_vip *lb_vip,
                        const char *ip_match, const char *proto)
 {
+    if (lb_vip->n_backends == 0) {
+        return;
+    }
+
+    struct ds action = DS_EMPTY_INITIALIZER;
+    struct ds match_initiator = DS_EMPTY_INITIALIZER;
+    struct ds match_reply = DS_EMPTY_INITIALIZER;
+    struct ds proto_match = DS_EMPTY_INITIALIZER;
+
     /* Ingress Pre-Hairpin table.
-     * - Priority 2: SNAT load balanced traffic that needs to be hairpinned.
+     * - Priority 2: SNAT load balanced traffic that needs to be hairpinned:
+     *   - Both SRC and DST IP match backend->ip and destination port
+     *     matches backend->port.
      * - Priority 1: unSNAT replies to hairpinned load balanced traffic.
+     *   - SRC IP matches backend->ip, DST IP matches LB VIP and source port
+     *     matches backend->port.
      */
+    ds_put_char(&match_reply, '(');
     for (size_t i = 0; i < lb_vip->n_backends; i++) {
         struct lb_vip_backend *backend = &lb_vip->backends[i];
-        struct ds action = DS_EMPTY_INITIALIZER;
-        struct ds match = DS_EMPTY_INITIALIZER;
-        struct ds proto_match = DS_EMPTY_INITIALIZER;
 
         /* Packets that after load balancing have equal source and
-         * destination IPs should be hairpinned. SNAT them so that the reply
-         * traffic is directed also through OVN.
+         * destination IPs should be hairpinned.
          */
         if (lb_vip->vip_port) {
-            ds_put_format(&proto_match, " && %s && %s.dst == %"PRIu16,
-                          proto, proto, backend->port);
+            ds_put_format(&proto_match, " && %s.dst == %"PRIu16,
+                          proto, backend->port);
         }
-        ds_put_format(&match, "%s.src == %s && %s.dst == %s%s",
+        ds_put_format(&match_initiator, "(%s.src == %s && %s.dst == %s%s)",
                       ip_match, backend->ip, ip_match, backend->ip,
                       ds_cstr(&proto_match));
-        ds_put_format(&action, REGBIT_HAIRPIN " = 1; ct_snat(%s);",
-                      lb_vip->vip);
-        ovn_lflow_add_with_hint(lflows, od, S_SWITCH_IN_PRE_HAIRPIN, 2,
-                                ds_cstr(&match), ds_cstr(&action),
-                                &lb->nlb->header_);
 
-        /* If the packets are replies for hairpinned traffic, UNSNAT them. */
+        /* Replies to hairpinned traffic are originated by backend->ip:port. */
         ds_clear(&proto_match);
-        ds_clear(&match);
         if (lb_vip->vip_port) {
-            ds_put_format(&proto_match, " && %s && %s.src == %"PRIu16,
-                          proto, proto, backend->port);
+            ds_put_format(&proto_match, " && %s.src == %"PRIu16, proto,
+                          backend->port);
         }
-        ds_put_format(&match, "%s.src == %s && %s.dst == %s%s",
-                      ip_match, backend->ip, ip_match, lb_vip->vip,
+        ds_put_format(&match_reply, "(%s.src == %s%s)", ip_match, backend->ip,
                       ds_cstr(&proto_match));
-        ovn_lflow_add_with_hint(lflows, od, S_SWITCH_IN_PRE_HAIRPIN, 1,
-                                ds_cstr(&match),
-                                REGBIT_HAIRPIN " = 1; ct_snat;",
-                                &lb->nlb->header_);
+        ds_clear(&proto_match);
 
-        ds_destroy(&action);
-        ds_destroy(&match);
-        ds_destroy(&proto_match);
+        if (i < lb_vip->n_backends - 1) {
+            ds_put_cstr(&match_initiator, " || ");
+            ds_put_cstr(&match_reply, " || ");
+        }
     }
+    ds_put_char(&match_reply, ')');
+
+    /* SNAT hairpinned initiator traffic so that the reply traffic is
+     * also directed through OVN.
+     */
+    ds_put_format(&action, REGBIT_HAIRPIN " = 1; ct_snat(%s);",
+                  lb_vip->vip);
+    ovn_lflow_add_with_hint(lflows, od, S_SWITCH_IN_PRE_HAIRPIN, 2,
+                            ds_cstr(&match_initiator), ds_cstr(&action),
+                            &lb->nlb->header_);
+
+    /* Replies to hairpinned traffic are destined to the LB VIP. */
+    ds_put_format(&match_reply, " && %s.dst == %s", ip_match, lb_vip->vip);
+
+    /* UNSNAT replies for hairpinned traffic. */
+    ovn_lflow_add_with_hint(lflows, od, S_SWITCH_IN_PRE_HAIRPIN, 1,
+                            ds_cstr(&match_reply),
+                            REGBIT_HAIRPIN " = 1; ct_snat;",
+                            &lb->nlb->header_);
+
+    ds_destroy(&action);
+    ds_destroy(&match_initiator);
+    ds_destroy(&match_reply);
+    ds_destroy(&proto_match);
 }
 
 static void
-- 
1.8.3.1