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]>
---
V2:
- Addressed Ales' comments:
- Improved error handling when parsing backends.
---
lib/lb.c | 51 ++++++++++++++++++++++++++++++------
lib/lb.h | 6 +++--
tests/ovn-nbctl.at | 26 +++++++++++++++++++
tests/system-ovn.at | 60 +++++++++++++++++++++++++++++++++++++------
utilities/ovn-nbctl.c | 2 +-
5 files changed, 126 insertions(+), 19 deletions(-)
diff --git a/lib/lb.c b/lib/lb.c
index 66c8152750..f88c1855b7 100644
--- a/lib/lb.c
+++ b/lib/lb.c
@@ -38,6 +38,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)
@@ -238,6 +239,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);
}
@@ -285,8 +288,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
@@ -304,15 +326,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);
}
@@ -357,11 +393,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 ddd41703da..e24f519dbb 100644
--- a/lib/lb.h
+++ b/lib/lb.h
@@ -96,6 +96,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;
};
@@ -211,8 +214,7 @@ 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);
struct ovn_lb_5tuple {
struct hmap_node hmap_node;
diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
index 2fffe18500..aa5baade40 100644
--- a/tests/ovn-nbctl.at
+++ b/tests/ovn-nbctl.at
@@ -1482,6 +1482,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 8afb4db564..53daf27f23 100644
--- a/tests/system-ovn.at
+++ b/tests/system-ovn.at
@@ -9045,7 +9045,7 @@ start_daemon ovn-controller
# |
# VM2 ----+
#
-# Two templated load balancer applied on LS1 and GW-Router with
+# 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 \
@@ -9073,7 +9073,7 @@ check ovn-nbctl
\
# 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,vport1=666,backends1=\"42.42.42.2:4242\",vport2=777,backends2=\"42.42.42.2:4343\"}"],
+
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-tcp "^vip:^vport1" "^backends1" tcp \
@@ -9084,6 +9084,18 @@ 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")
@@ -9104,13 +9116,15 @@ name: 'backends2' value: '42.42.42.2:4343'
name: 'vip' value: '66.66.66.66'
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 -c3 udp and dst 42.42.42.2 and dst port 4343 >
vm1.pcap 2>/dev/null],
+ [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).
@@ -9122,9 +9136,17 @@ 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 "3"
+ test "${requests}" -ge "6"
])
AT_CLEANUP
@@ -9159,7 +9181,7 @@ start_daemon ovn-controller
# |
# VM2 ----+
#
-# Two templated load balancer applied on LS1 and GW-Router with
+# 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 \
@@ -9187,7 +9209,7 @@ check ovn-nbctl
\
# VIP=[6666::1]:777 backends=[4242::2]:4343 proto=udp
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\"}"],
+
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-tcp "^vip:^vport1" "^backends1" tcp
ipv6 \
@@ -9198,6 +9220,18 @@ check ovn-nbctl --template lb-add lb-test-udp
"^vip:^vport2" "^backends2" udp ip
-- 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")
OVS_WAIT_UNTIL([test "$(ip netns exec vm1 ip a | grep 4242::2 | grep
tentative)" = ""])
@@ -9221,13 +9255,15 @@ name: 'backends2' value: '[[4242::2]]:4343'
name: 'vip' value: '6666::1'
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 -c3 udp and dst 4242::2 and dst port 4343 >
vm1.pcap 2>/dev/null],
+ [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).
@@ -9239,9 +9275,17 @@ 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 "3"
+ test "${requests}" -ge "6"
])
AT_CLEANUP
diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
index 45572fd304..22313ecd55 100644
--- a/utilities/ovn-nbctl.c
+++ b/utilities/ovn-nbctl.c
@@ -3033,7 +3033,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
