The LB validation allowed to mix template backends and explicit
backends for IPv4, but not for IPv6. Make sure we allow mixing
for both IP versions. This also fixes memory leak that could happen
within the mixed IPv6 backends.
Fixes: 7310a9859a6a ("lb: Allow IPv6 template LBs to use explicit backends.")
Signed-off-by: Ales Musil <[email protected]>
---
lib/lb.c | 213 ++++++++++++++++++++++++---------------------
lib/lb.h | 4 +-
tests/ovn-nbctl.at | 4 +
3 files changed, 118 insertions(+), 103 deletions(-)
diff --git a/lib/lb.c b/lib/lb.c
index e67a5fcfd..2b2904d1e 100644
--- a/lib/lb.c
+++ b/lib/lb.c
@@ -24,7 +24,53 @@
VLOG_DEFINE_THIS_MODULE(lb);
-static void ovn_lb_backends_clear(struct ovn_lb_vip *vip);
+static bool
+ovn_lb_backend_init_explicit(const struct ovn_lb_vip *lb_vip,
+ struct ovn_lb_backend *backend,
+ const char *token, struct ds *errors)
+{
+ int backend_addr_family;
+ if (!ip_address_and_port_from_lb_key(token, &backend->ip_str,
+ &backend->ip, &backend->port,
+ &backend_addr_family)) {
+ if (lb_vip->port_str) {
+ ds_put_format(errors, "%s: should be an IP address and a "
+ "port number with : as a separator, ",
+ token);
+ } else {
+ ds_put_format(errors, "%s: should be an IP address, ", token);
+ }
+ return false;
+ }
+
+ if (lb_vip->address_family != backend_addr_family) {
+ free(backend->ip_str);
+ ds_put_format(errors, "%s: IP address family is different from "
+ "VIP %s, ",
+ token, lb_vip->vip_str);
+ return false;
+ }
+
+ if (lb_vip->port_str) {
+ if (!backend->port) {
+ free(backend->ip_str);
+ ds_put_format(errors, "%s: should be an IP address and "
+ "a port number with : as a separator, ",
+ token);
+ return false;
+ }
+ } else {
+ if (backend->port) {
+ free(backend->ip_str);
+ ds_put_format(errors, "%s: should be an IP address, ", token);
+ return false;
+ }
+ }
+
+ backend->port_str =
+ backend->port ? xasprintf("%"PRIu16, backend->port) : NULL;
+ return true;
+}
/* Format for backend ips: "IP1:port1,IP2:port2,...". */
static char *
@@ -47,46 +93,10 @@ ovn_lb_backends_init_explicit(struct ovn_lb_vip *lb_vip,
const char *value)
}
struct ovn_lb_backend *backend = &lb_vip->backends[lb_vip->n_backends];
- int backend_addr_family;
- if (!ip_address_and_port_from_lb_key(token, &backend->ip_str,
- &backend->ip, &backend->port,
- &backend_addr_family)) {
- if (lb_vip->port_str) {
- ds_put_format(&errors, "%s: should be an IP address and a "
- "port number with : as a separator, ",
- token);
- } else {
- ds_put_format(&errors, "%s: should be an IP address, ", token);
- }
+ if (!ovn_lb_backend_init_explicit(lb_vip, backend, token, &errors)) {
continue;
}
- if (lb_vip->address_family != backend_addr_family) {
- free(backend->ip_str);
- ds_put_format(&errors, "%s: IP address family is different from "
- "VIP %s, ",
- token, lb_vip->vip_str);
- continue;
- }
-
- if (lb_vip->port_str) {
- if (!backend->port) {
- free(backend->ip_str);
- ds_put_format(&errors, "%s: should be an IP address and "
- "a port number with : as a separator, ",
- token);
- continue;
- }
- } else {
- if (backend->port) {
- free(backend->ip_str);
- ds_put_format(&errors, "%s: should be an IP address, ", token);
- continue;
- }
- }
-
- backend->port_str =
- backend->port ? xasprintf("%"PRIu16, backend->port) : NULL;
lb_vip->n_backends++;
}
free(tokstr);
@@ -118,6 +128,47 @@ ovn_lb_vip_init_explicit(struct ovn_lb_vip *lb_vip, const
char *lb_key,
return ovn_lb_backends_init_explicit(lb_vip, lb_value);
}
+static bool
+ovn_lb_backend_init_template(struct ovn_lb_backend *backend, const char *token,
+ struct ds *errors)
+{
+ char *atom = xstrdup(token);
+ char *save_ptr = NULL;
+ bool success = false;
+ char *backend_ip = NULL;
+ char *backend_port = NULL;
+
+ for (char *subatom = strtok_r(atom, ":", &save_ptr); subatom;
+ subatom = strtok_r(NULL, ":", &save_ptr)) {
+ if (backend_ip && backend_port) {
+ success = false;
+ break;
+ }
+ success = true;
+ if (!backend_ip) {
+ backend_ip = xstrdup(subatom);
+ } else {
+ backend_port = xstrdup(subatom);
+ }
+ }
+
+ if (success) {
+ backend->ip_str = backend_ip;
+ backend->port_str = backend_port;
+ backend->port = 0;
+ memset(&backend->ip, 0, sizeof backend->ip);
+ } else {
+ 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);
+
+ return success;
+}
+
/* Parses backends of a templated LB VIP.
* For now only the following template forms are supported:
* A.
@@ -130,6 +181,8 @@ ovn_lb_vip_init_explicit(struct ovn_lb_vip *lb_vip, const
char *lb_key,
* IP1_2:PORT1_2 on chassis-2
* and 'backends_variable2' may expand to IP2_1:PORT2_1 on chassis-1
* IP2_2:PORT2_2 on chassis-2
+ * C.
+ * backendip1[:port],backendip2[:port]
*/
static char *
ovn_lb_backends_init_template(struct ovn_lb_vip *lb_vip, const char *value_)
@@ -140,50 +193,28 @@ ovn_lb_backends_init_template(struct ovn_lb_vip *lb_vip,
const char *value_)
size_t n_allocated_backends = 0;
lb_vip->n_backends = 0;
- for (char *backend = strtok_r(value, ",", &save_ptr); backend;
- backend = strtok_r(NULL, ",", &save_ptr)) {
-
- char *atom = xstrdup(backend);
- char *save_ptr2 = NULL;
- bool success = false;
- char *backend_ip = NULL;
- char *backend_port = NULL;
+ for (char *token = strtok_r(value, ",", &save_ptr); token;
+ token = strtok_r(NULL, ",", &save_ptr)) {
- for (char *subatom = strtok_r(atom, ":", &save_ptr2); subatom;
- subatom = strtok_r(NULL, ":", &save_ptr2)) {
- if (backend_ip && backend_port) {
- success = false;
- break;
- }
- success = true;
- if (!backend_ip) {
- backend_ip = xstrdup(subatom);
- } else {
- backend_port = xstrdup(subatom);
- }
+ if (lb_vip->n_backends == n_allocated_backends) {
+ lb_vip->backends = x2nrealloc(lb_vip->backends,
+ &n_allocated_backends,
+ sizeof *lb_vip->backends);
}
- if (success) {
- if (lb_vip->n_backends == n_allocated_backends) {
- lb_vip->backends = x2nrealloc(lb_vip->backends,
- &n_allocated_backends,
- sizeof *lb_vip->backends);
+ struct ovn_lb_backend *backend = &lb_vip->backends[lb_vip->n_backends];
+ if (token[0] && token[0] == '^') {
+ if (!ovn_lb_backend_init_template(backend, token, &errors)) {
+ continue;
}
-
- struct ovn_lb_backend *lb_backend =
- &lb_vip->backends[lb_vip->n_backends];
- lb_backend->ip_str = backend_ip;
- lb_backend->port_str = backend_port;
- lb_backend->port = 0;
- lb_vip->n_backends++;
} else {
- ds_put_format(&errors, "%s: should be a template of the form: "
- "'^backendip_variable1[:^port_variable1|:port]', ",
- atom);
- free(backend_port);
- free(backend_ip);
+ if (!ovn_lb_backend_init_explicit(lb_vip, backend,
+ token, &errors)) {
+ continue;
+ }
}
- free(atom);
+
+ lb_vip->n_backends++;
}
free(value);
@@ -231,23 +262,13 @@ ovn_lb_vip_init_template(struct ovn_lb_vip *lb_vip, const
char *lb_key_,
/* Backends can either be templates or explicit IPs and ports. */
lb_vip->address_family = address_family;
- 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);
+ char *error = xasprintf("invalid backend: template (%s)",
+ template_error);
free(template_error);
return error;
- }
- free(template_error);
}
return NULL;
}
@@ -274,14 +295,7 @@ ovn_lb_backends_destroy(struct ovn_lb_vip *vip)
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;
+ free(vip->backends);
}
void
@@ -290,7 +304,6 @@ 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);
}
static void
@@ -320,11 +333,11 @@ 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 needs_brackets = vip->address_family == AF_INET6 && vip->port_str
- && !vip->template_backends;
for (size_t i = 0; i < vip->n_backends; i++) {
struct ovn_lb_backend *backend = &vip->backends[i];
-
+ bool needs_brackets = vip->address_family == AF_INET6 &&
+ vip->port_str &&
+ !ipv6_addr_equals(&backend->ip, &in6addr_any);
if (needs_brackets) {
ds_put_char(s, '[');
}
diff --git a/lib/lb.h b/lib/lb.h
index bcc677f82..e758765bd 100644
--- a/lib/lb.h
+++ b/lib/lb.h
@@ -38,9 +38,7 @@ 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;
};
diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
index 81ab4a47f..fc74e5c62 100644
--- a/tests/ovn-nbctl.at
+++ b/tests/ovn-nbctl.at
@@ -1594,6 +1594,8 @@ 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
+check ovn-nbctl --template lb-add lb8 ^vip:^vport "^backend,^port,1.1.1.1:111"
udp ipv4
+check ovn-nbctl --template lb-add lb9 ^vip:^vport
"^backend,^port,[[1::1]]:111" udp ipv6
AT_CHECK([ovn-nbctl lb-list | uuidfilt], [0], [dnl
UUID LB PROTO VIP
IPs
@@ -1605,6 +1607,8 @@ UUID LB
PROTO VIP
<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
+<8> lb8 udp ^vip:^vport ^backend,^port,1.1.1.1:111
+<9> lb9 udp ^vip:^vport
^backend,^port,[[1::1]]:111
])
])
--
2.48.1
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
