This was working fine for IPv4 but partially by accident because IPv4
addresses don't contain ':'.  Fix it for the general case by trying to
parse explicit backends if parsing templates fails.

Also add unit and system tests for all supported cases.

Fixes: b45853fba816 ("lb: Support using templates.")
Signed-off-by: Dumitru Ceara <[email protected]>
Acked-by: Ales Musil <[email protected]>
(cherry picked from commit 7310a9859a6a5a9ad4faff1e5620e5cfa142eb7d)
---
NOTE: this is a custom backport because it picks up the tests added by
086744a645a7 ("northd: Use LB port_str in northd.").  It doesn't pick up
the rest of the changes from that commit because they're not applicable
on branch-22.12.
---
 lib/lb.c              |  51 ++++++++++++++---
 lib/lb.h              |   6 +-
 tests/ovn-nbctl.at    |  26 +++++++++
 tests/system-ovn.at   | 126 ++++++++++++++++++++++++++++++++++++------
 utilities/ovn-nbctl.c |   2 +-
 5 files changed, 182 insertions(+), 29 deletions(-)

diff --git a/lib/lb.c b/lib/lb.c
index c13d07b99c..b74e4d90c1 100644
--- a/lib/lb.c
+++ b/lib/lb.c
@@ -36,6 +36,7 @@ static const char *lb_neighbor_responder_mode_names[] = {
 static struct nbrec_load_balancer_health_check *
 ovn_lb_get_health_check(const struct nbrec_load_balancer *nbrec_lb,
                         const char *vip_port_str, bool template);
+static void ovn_lb_backends_clear(struct ovn_lb_vip *vip);
 
 struct ovn_lb_ip_set *
 ovn_lb_ip_set_create(void)
@@ -236,6 +237,8 @@ ovn_lb_backends_init_template(struct ovn_lb_vip *lb_vip, 
const char *value_)
             ds_put_format(&errors, "%s: should be a template of the form: "
                           "'^backendip_variable1[:^port_variable1|:port]', ",
                           atom);
+            free(backend_port);
+            free(backend_ip);
         }
         free(atom);
     }
@@ -283,8 +286,27 @@ ovn_lb_vip_init_template(struct ovn_lb_vip *lb_vip, const 
char *lb_key_,
                          lb_key_);
     }
 
+    /* Backends can either be templates or explicit IPs and ports. */
     lb_vip->address_family = address_family;
-    return ovn_lb_backends_init_template(lb_vip, lb_value);
+    lb_vip->template_backends = true;
+    char *template_error = ovn_lb_backends_init_template(lb_vip, lb_value);
+
+    if (template_error) {
+        lb_vip->template_backends = false;
+        ovn_lb_backends_clear(lb_vip);
+
+        char *explicit_error = ovn_lb_backends_init_explicit(lb_vip, lb_value);
+        if (explicit_error) {
+            char *error =
+                xasprintf("invalid backend: template (%s) OR explicit (%s)",
+                          template_error, explicit_error);
+            free(explicit_error);
+            free(template_error);
+            return error;
+        }
+        free(template_error);
+    }
+    return NULL;
 }
 
 /* Returns NULL on success, an error string on failure.  The caller is
@@ -302,15 +324,29 @@ ovn_lb_vip_init(struct ovn_lb_vip *lb_vip, const char 
*lb_key,
                                        address_family);
 }
 
-void
-ovn_lb_vip_destroy(struct ovn_lb_vip *vip)
+static void
+ovn_lb_backends_destroy(struct ovn_lb_vip *vip)
 {
-    free(vip->vip_str);
-    free(vip->port_str);
     for (size_t i = 0; i < vip->n_backends; i++) {
         free(vip->backends[i].ip_str);
         free(vip->backends[i].port_str);
     }
+}
+
+static void
+ovn_lb_backends_clear(struct ovn_lb_vip *vip)
+{
+    ovn_lb_backends_destroy(vip);
+    vip->backends = NULL;
+    vip->n_backends = 0;
+}
+
+void
+ovn_lb_vip_destroy(struct ovn_lb_vip *vip)
+{
+    free(vip->vip_str);
+    free(vip->port_str);
+    ovn_lb_backends_destroy(vip);
     free(vip->backends);
 }
 
@@ -355,11 +391,10 @@ ovn_lb_vip_format(const struct ovn_lb_vip *vip, struct ds 
*s, bool template)
 }
 
 void
-ovn_lb_vip_backends_format(const struct ovn_lb_vip *vip, struct ds *s,
-                           bool template)
+ovn_lb_vip_backends_format(const struct ovn_lb_vip *vip, struct ds *s)
 {
     bool needs_brackets = vip->address_family == AF_INET6 && vip->port_str
-                          && !template;
+                          && !vip->template_backends;
     for (size_t i = 0; i < vip->n_backends; i++) {
         struct ovn_lb_backend *backend = &vip->backends[i];
 
diff --git a/lib/lb.h b/lib/lb.h
index 55becc1bf7..e5a3875609 100644
--- a/lib/lb.h
+++ b/lib/lb.h
@@ -98,6 +98,9 @@ struct ovn_lb_vip {
                           */
     struct ovn_lb_backend *backends;
     size_t n_backends;
+    bool template_backends; /* True if the backends are templates. False if
+                             * they're explicitly specified.
+                             */
     bool empty_backend_rej;
     int address_family;
 };
@@ -203,7 +206,6 @@ char *ovn_lb_vip_init(struct ovn_lb_vip *lb_vip, const char 
*lb_key,
 void ovn_lb_vip_destroy(struct ovn_lb_vip *vip);
 void ovn_lb_vip_format(const struct ovn_lb_vip *vip, struct ds *s,
                        bool template);
-void ovn_lb_vip_backends_format(const struct ovn_lb_vip *vip, struct ds *s,
-                                bool template);
+void ovn_lb_vip_backends_format(const struct ovn_lb_vip *vip, struct ds *s);
 
 #endif /* OVN_LIB_LB_H 1 */
diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
index 8885ac9fcc..8233617536 100644
--- a/tests/ovn-nbctl.at
+++ b/tests/ovn-nbctl.at
@@ -1473,6 +1473,32 @@ UUID                                    LB               
   PROTO      VIP
 
 dnl ---------------------------------------------------------------------
 
+OVN_NBCTL_TEST([ovn_nbctl_template_lbs], [Template LBs], [
+check ovn-nbctl --template lb-add lb0 ^vip ^backend
+check ovn-nbctl --template lb-add lb1 ^vip:^vport ^backend udp
+check ovn-nbctl --template lb-add lb2 ^vip:^vport ^backend udp ipv4
+check ovn-nbctl --template lb-add lb3 ^vip:^vport ^backend udp ipv6
+check ovn-nbctl --template lb-add lb4 ^vip:^vport ^backend:^bport udp ipv4
+check ovn-nbctl --template lb-add lb5 ^vip:^vport ^backend:^bport udp ipv6
+check ovn-nbctl --template lb-add lb6 ^vip:^vport 1.1.1.1:111 udp ipv4
+check ovn-nbctl --template lb-add lb7 ^vip:^vport [[1::1]]:111 udp ipv6
+
+AT_CHECK([ovn-nbctl lb-list | uuidfilt], [0], [dnl
+UUID                                    LB                  PROTO      VIP     
       IPs
+<0>    lb0                 tcp        ^vip           ^backend
+<1>    lb1                 udp        ^vip:^vport    ^backend
+<2>    lb2                 udp        ^vip:^vport    ^backend
+<3>    lb3                 udp        ^vip:^vport    ^backend
+<4>    lb4                 udp        ^vip:^vport    ^backend:^bport
+<5>    lb5                 udp        ^vip:^vport    ^backend:^bport
+<6>    lb6                 udp        ^vip:^vport    1.1.1.1:111
+<7>    lb7                 udp        ^vip:^vport    [[1::1]]:111
+])
+
+])
+
+dnl ---------------------------------------------------------------------
+
 OVN_NBCTL_TEST([ovn_nbctl_basic_lr], [basic logical router commands], [
 AT_CHECK([ovn-nbctl lr-add lr0])
 AT_CHECK([ovn-nbctl lr-list | uuidfilt], [0], [dnl
diff --git a/tests/system-ovn.at b/tests/system-ovn.at
index 1e67678465..1c7fdcb8d0 100644
--- a/tests/system-ovn.at
+++ b/tests/system-ovn.at
@@ -9410,6 +9410,7 @@ AT_CLEANUP
 
 OVN_FOR_EACH_NORTHD([
 AT_SETUP([load-balancer template IPv4])
+AT_SKIP_IF([test $HAVE_TCPDUMP = no])
 AT_SKIP_IF([test $HAVE_NC = no])
 AT_KEYWORDS([ovnlb templates])
 
@@ -9436,8 +9437,8 @@ start_daemon ovn-controller
 #         |
 # VM2 ----+
 #
-# A templated load balancer applied on LS1 and GW-Router with
-# VM1 as backend.  The VIP should be accessible from both VM2 and VM3.
+# Four templated load balancer applied on LS1 and GW-Router with
+# VM1 as backend.  The VIPs should be accessible from both VM2 and VM3.
 
 check ovn-nbctl                                                   \
     -- lr-add rtr                                                 \
@@ -9458,15 +9459,34 @@ check ovn-nbctl                                         
          \
     -- lsp-set-options ls2-rtr router-port=rtr-ls2                \
     -- lsp-add ls2 vm3 -- lsp-set-addresses vm3 00:00:00:00:00:03
 
-# Add a template LB that eventually expands to:
+# Add a TCP template LB that eventually expands to:
 # VIP=66.66.66.66:666 backends=42.42.42.2:4242 proto=tcp
+# And a UDP template LB that eventually expands to:
+# VIP=66.66.66.66:777 backends=42.42.42.2:4343 proto=udp
 
-AT_CHECK([ovn-nbctl -- create chassis_template_var chassis="hv1" 
variables="{vip=66.66.66.66,vport=666,backends=\"42.42.42.2:4242\"}"],
+AT_CHECK([ovn-nbctl -- create chassis_template_var chassis="hv1" \
+    
variables="{vip=66.66.66.66,vport1=666,backends1=\"42.42.42.2:4242\",vport2=777,backends2=\"42.42.42.2:4343\",vport3=888,vport4=999}"],
          [0], [ignore])
 
-check ovn-nbctl --template lb-add lb-test "^vip:^vport" "^backends" tcp \
-    -- ls-lb-add ls1 lb-test                                            \
-    -- lr-lb-add rtr lb-test
+check ovn-nbctl --template lb-add lb-test-tcp "^vip:^vport1" "^backends1" tcp \
+    -- ls-lb-add ls1 lb-test-tcp                                              \
+    -- lr-lb-add rtr lb-test-tcp
+
+check ovn-nbctl --template lb-add lb-test-udp "^vip:^vport2" "^backends2" udp \
+    -- ls-lb-add ls1 lb-test-udp                                              \
+    -- lr-lb-add rtr lb-test-udp
+
+# Add a TCP template LB with explicit backends that eventually expands to:
+# VIP=66.66.66.66:888 backends=42.42.42.2:4242 proto=tcp
+# And a UDP template LB that eventually expands to:
+# VIP=66.66.66.66:999 backends=42.42.42.2:4343 proto=udp
+check ovn-nbctl --template lb-add lb-test-tcp2 "^vip:^vport3" 
"42.42.42.2:4242" tcp ipv4 \
+    -- ls-lb-add ls1 lb-test-tcp2                                              
          \
+    -- lr-lb-add rtr lb-test-tcp2
+
+check ovn-nbctl --template lb-add lb-test-udp2 "^vip:^vport4" 
"42.42.42.2:4343" udp ipv4 \
+    -- ls-lb-add ls1 lb-test-udp2                                              
          \
+    -- lr-lb-add rtr lb-test-udp2
 
 ADD_NAMESPACES(vm1)
 ADD_VETH(vm1, vm1, br-int, "42.42.42.2/24", "00:00:00:00:00:01", "42.42.42.1")
@@ -9483,24 +9503,50 @@ check ovn-nbctl --wait=hv sync
 
 AT_CHECK([ovn-appctl -t ovn-controller debug/dump-local-template-vars | sort], 
[0], [dnl
 Local template vars:
-name: 'backends' value: '42.42.42.2:4242'
+name: 'backends1' value: '42.42.42.2:4242'
+name: 'backends2' value: '42.42.42.2:4343'
 name: 'vip' value: '66.66.66.66'
-name: 'vport' value: '666'
+name: 'vport1' value: '666'
+name: 'vport2' value: '777'
+name: 'vport3' value: '888'
+name: 'vport4' value: '999'
 ])
 
 # Start IPv4 TCP server on vm1.
 NETNS_DAEMONIZE([vm1], [nc -k -l 42.42.42.2 4242], [nc-vm1.pid])
 
+NETNS_DAEMONIZE([vm1],
+    [tcpdump -n -i vm1 -nnleX -c6 udp and dst 42.42.42.2 and dst port 4343 > 
vm1.pcap 2>/dev/null],
+    [tcpdump1.pid])
+
 # Make sure connecting to the VIP works (hairpin, via ls and via lr).
 NS_CHECK_EXEC([vm1], [nc 66.66.66.66 666 -z], [0], [ignore], [ignore])
 NS_CHECK_EXEC([vm2], [nc 66.66.66.66 666 -z], [0], [ignore], [ignore])
 NS_CHECK_EXEC([vm3], [nc 66.66.66.66 666 -z], [0], [ignore], [ignore])
 
+NS_CHECK_EXEC([vm1], [echo a | nc -u 66.66.66.66 777 &], [0])
+NS_CHECK_EXEC([vm2], [echo a | nc -u 66.66.66.66 777 &], [0])
+NS_CHECK_EXEC([vm3], [echo a | nc -u 66.66.66.66 777 &], [0])
+
+NS_CHECK_EXEC([vm1], [nc 66.66.66.66 888 -z], [0], [ignore], [ignore])
+NS_CHECK_EXEC([vm2], [nc 66.66.66.66 888 -z], [0], [ignore], [ignore])
+NS_CHECK_EXEC([vm3], [nc 66.66.66.66 888 -z], [0], [ignore], [ignore])
+
+NS_CHECK_EXEC([vm1], [echo a | nc -u 66.66.66.66 999 &], [0])
+NS_CHECK_EXEC([vm2], [echo a | nc -u 66.66.66.66 999 &], [0])
+NS_CHECK_EXEC([vm3], [echo a | nc -u 66.66.66.66 999 &], [0])
+
+OVS_WAIT_UNTIL([
+    requests=`grep "UDP" -c vm1.pcap`
+    test "${requests}" -ge "6"
+])
+
 AT_CLEANUP
 ])
 
 OVN_FOR_EACH_NORTHD([
 AT_SETUP([load-balancer template IPv6])
+AT_SKIP_IF([test $HAVE_TCPDUMP = no])
 AT_SKIP_IF([test $HAVE_NC = no])
 AT_KEYWORDS([ovnlb templates])
 
@@ -9527,8 +9573,8 @@ start_daemon ovn-controller
 #         |
 # VM2 ----+
 #
-# A templated load balancer applied on LS1 and GW-Router with
-# VM1 as backend.  The VIP should be accessible from both VM2 and VM3.
+# Four templated load balancer applied on LS1 and GW-Router with
+# VM1 as backend.  The VIPs should be accessible from both VM2 and VM3.
 
 check ovn-nbctl                                                   \
     -- lr-add rtr                                                 \
@@ -9550,14 +9596,33 @@ check ovn-nbctl                                         
          \
     -- lsp-add ls2 vm3 -- lsp-set-addresses vm3 00:00:00:00:00:03
 
 # Add a template LB that eventually expands to:
-# VIP=6666::1 backends=[4242::2]:4242 proto=tcp
+# VIP=[6666::1]:666 backends=[4242::2]:4242 proto=tcp
+# Add a template LB that eventually expands to:
+# VIP=[6666::1]:777 backends=[4242::2]:4343 proto=udp
 
-AT_CHECK([ovn-nbctl -- create chassis_template_var chassis="hv1" 
variables="{vip=\"6666::1\",vport=666,backends=\"[[4242::2]]:4242\"}"],
+AT_CHECK([ovn-nbctl -- create chassis_template_var chassis="hv1" \
+    
variables="{vip=\"6666::1\",vport1=666,backends1=\"[[4242::2]]:4242\",vport2=777,backends2=\"[[4242::2]]:4343\",vport3=888,vport4=999}"],
          [0], [ignore])
 
-check ovn-nbctl --template lb-add lb-test "^vip:^vport" "^backends" tcp ipv6 \
-    -- ls-lb-add ls1 lb-test                                                 \
-    -- lr-lb-add rtr lb-test
+check ovn-nbctl --template lb-add lb-test-tcp "^vip:^vport1" "^backends1" tcp 
ipv6 \
+    -- ls-lb-add ls1 lb-test-tcp                                               
    \
+    -- lr-lb-add rtr lb-test-tcp
+
+check ovn-nbctl --template lb-add lb-test-udp "^vip:^vport2" "^backends2" udp 
ipv6 \
+    -- ls-lb-add ls1 lb-test-udp                                               
    \
+    -- lr-lb-add rtr lb-test-udp
+
+# Add a TCP template LB with explicit backends that eventually expands to:
+# VIP=[6666::1]:888 backends=[4242::2]:4242 proto=tcp
+# And a UDP template LB that eventually expands to:
+# VIP=[6666::1]:999 backends=[4242::2]:4343 proto=udp
+check ovn-nbctl --template lb-add lb-test-tcp2 "^vip:^vport3" 
"[[4242::2]]:4242" tcp ipv6 \
+    -- ls-lb-add ls1 lb-test-tcp2                                              
           \
+    -- lr-lb-add rtr lb-test-tcp2
+
+check ovn-nbctl --template lb-add lb-test-udp2 "^vip:^vport4" 
"[[4242::2]]:4343" udp ipv6 \
+    -- ls-lb-add ls1 lb-test-udp2                                              
           \
+    -- lr-lb-add rtr lb-test-udp2
 
 ADD_NAMESPACES(vm1)
 ADD_VETH(vm1, vm1, br-int, "4242::2/64", "00:00:00:00:00:01", "4242::1")
@@ -9577,19 +9642,44 @@ check ovn-nbctl --wait=hv sync
 
 AT_CHECK([ovn-appctl -t ovn-controller debug/dump-local-template-vars | sort], 
[0], [dnl
 Local template vars:
-name: 'backends' value: '[[4242::2]]:4242'
+name: 'backends1' value: '[[4242::2]]:4242'
+name: 'backends2' value: '[[4242::2]]:4343'
 name: 'vip' value: '6666::1'
-name: 'vport' value: '666'
+name: 'vport1' value: '666'
+name: 'vport2' value: '777'
+name: 'vport3' value: '888'
+name: 'vport4' value: '999'
 ])
 
 # Start IPv6 TCP server on vm1.
 NETNS_DAEMONIZE([vm1], [nc -k -l 4242::2 4242], [nc-vm1.pid])
 
+NETNS_DAEMONIZE([vm1],
+    [tcpdump -n -i vm1 -nnleX -c6 udp and dst 4242::2 and dst port 4343 > 
vm1.pcap 2>/dev/null],
+    [tcpdump1.pid])
+
 # Make sure connecting to the VIP works (hairpin, via ls and via lr).
 NS_CHECK_EXEC([vm1], [nc 6666::1 666 -z], [0], [ignore], [ignore])
 NS_CHECK_EXEC([vm2], [nc 6666::1 666 -z], [0], [ignore], [ignore])
 NS_CHECK_EXEC([vm3], [nc 6666::1 666 -z], [0], [ignore], [ignore])
 
+NS_CHECK_EXEC([vm1], [echo a | nc -u 6666::1 777 &], [0])
+NS_CHECK_EXEC([vm2], [echo a | nc -u 6666::1 777 &], [0])
+NS_CHECK_EXEC([vm3], [echo a | nc -u 6666::1 777 &], [0])
+
+NS_CHECK_EXEC([vm1], [nc 6666::1 888 -z], [0], [ignore], [ignore])
+NS_CHECK_EXEC([vm2], [nc 6666::1 888 -z], [0], [ignore], [ignore])
+NS_CHECK_EXEC([vm3], [nc 6666::1 888 -z], [0], [ignore], [ignore])
+
+NS_CHECK_EXEC([vm1], [echo a | nc -u 6666::1 999 &], [0])
+NS_CHECK_EXEC([vm2], [echo a | nc -u 6666::1 999 &], [0])
+NS_CHECK_EXEC([vm3], [echo a | nc -u 6666::1 999 &], [0])
+
+OVS_WAIT_UNTIL([
+    requests=`grep "UDP" -c vm1.pcap`
+    test "${requests}" -ge "6"
+])
+
 AT_CLEANUP
 ])
 
diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
index 9d4fb8c757..699fa4ff42 100644
--- a/utilities/ovn-nbctl.c
+++ b/utilities/ovn-nbctl.c
@@ -3026,7 +3026,7 @@ nbctl_lb_add(struct ctl_context *ctx)
     }
 
     ovn_lb_vip_format(&lb_vip_parsed, &lb_vip_normalized, template);
-    ovn_lb_vip_backends_format(&lb_vip_parsed, &lb_ips_new, template);
+    ovn_lb_vip_backends_format(&lb_vip_parsed, &lb_ips_new);
     ovn_lb_vip_destroy(&lb_vip_parsed);
 
     const struct nbrec_load_balancer *lb = NULL;
-- 
2.31.1

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

Reply via email to