[ovs-dev] [PATCH ovn v2] Add SCTP support to load balancers.
Mark Michelson
mmichels at redhat.com
Wed Mar 18 00:36:31 UTC 2020
This allows for load balancers to use SCTP as a supported protocol in
addition to the already-supported UDP and TCP.
With this patch, health checks are not supported for SCTP load
balancers. A test has been added to ensure that this is the case. Health
checks should be added for SCTP load balancers in the near future. When
that's done, the existing test can be updated to ensure that the SCTP
health check works properly.
Signed-off-by: Mark Michelson <mmichels at redhat.com>
---
v1 -> v2
* Fixed checkpatch complaints
* Fixed NULL pointer dereference in ovn-northd causing system tests to
fail
* Fixed broken undnat flow for sctp
---
lib/actions.c | 8 +--
northd/ovn-northd.c | 48 ++++++++++-------
ovn-nb.ovsschema | 6 +--
ovn-nb.xml | 10 ++--
tests/ovn.at | 121 +++++++++++++++++++++++++++++++++++++++++-
utilities/ovn-nbctl.c | 8 +--
6 files changed, 167 insertions(+), 34 deletions(-)
diff --git a/lib/actions.c b/lib/actions.c
index f22acddff..6351db765 100644
--- a/lib/actions.c
+++ b/lib/actions.c
@@ -1957,10 +1957,12 @@ validate_empty_lb_backends(struct action_context *ctx,
}
break;
case EMPTY_LB_PROTOCOL:
- if (strcmp(c->string, "tcp") && strcmp(c->string, "udp")) {
+ if (strcmp(c->string, "tcp") &&
+ strcmp(c->string, "udp") &&
+ strcmp(c->string, "sctp")) {
lexer_error(ctx->lexer,
- "Load balancer protocol '%s' is not 'tcp' or 'udp'",
- c->string);
+ "Load balancer protocol '%s' is not 'tcp', 'udp', "
+ "or 'sctp'", c->string);
return;
}
break;
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 4f94680b5..e557aa2a7 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -3174,10 +3174,21 @@ ovn_lb_create(struct northd_context *ctx, struct hmap *lbs,
lb->vips[n_vips].backend_ips = xstrdup(node->value);
struct nbrec_load_balancer_health_check *lb_health_check = NULL;
- for (size_t i = 0; i < nbrec_lb->n_health_check; i++) {
- if (!strcmp(nbrec_lb->health_check[i]->vip, node->key)) {
- lb_health_check = nbrec_lb->health_check[i];
- break;
+ if (nbrec_lb->protocol && !strcmp(nbrec_lb->protocol, "sctp")) {
+ if (nbrec_lb->n_health_check > 0) {
+ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
+ VLOG_WARN_RL(&rl,
+ "SCTP load balancers do not currently support "
+ "health checks. Not creating health checks for "
+ "load balancer " UUID_FMT,
+ UUID_ARGS(&nbrec_lb->header_.uuid));
+ }
+ } else {
+ for (size_t i = 0; i < nbrec_lb->n_health_check; i++) {
+ if (!strcmp(nbrec_lb->health_check[i]->vip, node->key)) {
+ lb_health_check = nbrec_lb->health_check[i];
+ break;
+ }
}
}
@@ -5559,10 +5570,13 @@ build_lb_rules(struct ovn_datapath *od, struct hmap *lflows, struct ovn_lb *lb)
const char *proto = NULL;
if (lb_vip->vip_port) {
- if (lb->nlb->protocol && !strcmp(lb->nlb->protocol, "udp")) {
- proto = "udp";
- } else {
- proto = "tcp";
+ proto = "tcp";
+ if (lb->nlb->protocol) {
+ if (!strcmp(lb->nlb->protocol, "udp")) {
+ proto = "udp";
+ } else if (!strcmp(lb->nlb->protocol, "sctp")) {
+ proto = "sctp";
+ }
}
}
@@ -7566,7 +7580,7 @@ static void
add_router_lb_flow(struct hmap *lflows, struct ovn_datapath *od,
struct ds *match, struct ds *actions, int priority,
const char *lb_force_snat_ip, struct lb_vip *lb_vip,
- bool is_udp, struct nbrec_load_balancer *lb,
+ const char *proto, struct nbrec_load_balancer *lb,
struct shash *meter_groups)
{
build_empty_lb_event_flow(od, lflows, lb_vip, lb, S_ROUTER_IN_DNAT,
@@ -7625,7 +7639,7 @@ add_router_lb_flow(struct hmap *lflows, struct ovn_datapath *od,
if (backend->port) {
ds_put_format(&undnat_match, " && %s.src == %d) || ",
- is_udp ? "udp" : "tcp", backend->port);
+ proto, backend->port);
} else {
ds_put_cstr(&undnat_match, ") || ");
}
@@ -9170,15 +9184,13 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
int prio = 110;
bool is_udp = nullable_string_is_equal(nb_lb->protocol, "udp");
+ bool is_sctp = nullable_string_is_equal(nb_lb->protocol,
+ "sctp");
+ const char *proto = is_udp ? "udp" : is_sctp ? "sctp" : "tcp";
if (lb_vip->vip_port) {
- if (is_udp) {
- ds_put_format(&match, " && udp && udp.dst == %d",
- lb_vip->vip_port);
- } else {
- ds_put_format(&match, " && tcp && tcp.dst == %d",
- lb_vip->vip_port);
- }
+ ds_put_format(&match, " && %s && %s.dst == %d", proto,
+ proto, lb_vip->vip_port);
prio = 120;
}
@@ -9187,7 +9199,7 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
od->l3redirect_port->json_key);
}
add_router_lb_flow(lflows, od, &match, &actions, prio,
- lb_force_snat_ip, lb_vip, is_udp,
+ lb_force_snat_ip, lb_vip, proto,
nb_lb, meter_groups);
}
}
diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema
index 843e979db..ea6f4e354 100644
--- a/ovn-nb.ovsschema
+++ b/ovn-nb.ovsschema
@@ -1,7 +1,7 @@
{
"name": "OVN_Northbound",
- "version": "5.20.0",
- "cksum": "2846067333 25243",
+ "version": "5.20.1",
+ "cksum": "721375950 25251",
"tables": {
"NB_Global": {
"columns": {
@@ -168,7 +168,7 @@
"min": 0, "max": "unlimited"}},
"protocol": {
"type": {"key": {"type": "string",
- "enum": ["set", ["tcp", "udp"]]},
+ "enum": ["set", ["tcp", "udp", "sctp"]]},
"min": 0, "max": 1}},
"health_check": {"type": {
"key": {"type": "uuid",
diff --git a/ovn-nb.xml b/ovn-nb.xml
index d06ff00f0..666a6cdea 100644
--- a/ovn-nb.xml
+++ b/ovn-nb.xml
@@ -1474,11 +1474,11 @@
<column name="protocol">
<p>
- Valid protocols are <code>tcp</code> or <code>udp</code>. This column
- is useful when a port number is provided as part of the
- <code>vips</code> column. If this column is empty and a port number
- is provided as part of <code>vips</code> column, OVN assumes the
- protocol to be <code>tcp</code>.
+ Valid protocols are <code>tcp</code>, <code>udp</code>, or
+ <code>sctp</code>. This column is useful when a port number is
+ provided as part of the <code>vips</code> column. If this column is
+ empty and a port number is provided as part of <code>vips</code>
+ column, OVN assumes the protocol to be <code>tcp</code>.
</p>
</column>
diff --git a/tests/ovn.at b/tests/ovn.at
index 8cdbad743..dfd135d81 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -1423,8 +1423,8 @@ trigger_event(event = "empty_lb_backends", meter="event-elb" vip = "10.0.0.1:80"
encodes as controller(userdata=00.00.00.0f.00.00.00.00.00.00.00.00.00.01.00.0b.31.30.2e.30.2e.30.2e.31.3a.38.30.00.02.00.03.74.63.70.00.03.00.24.31.32.33.34.35.36.37.38.2d.61.62.63.64.2d.39.38.37.36.2d.66.65.64.63.2d.31.31.31.31.39.66.38.65.37.64.36.63,meter_id=5)
# Testing invalid vip results in extra error messages from socket-util.c
-trigger_event(event = "empty_lb_backends", vip = "10.0.0.1:80", protocol = "sctp", load_balancer = "12345678-abcd-9876-fedc-11119f8e7d6c");
- Load balancer protocol 'sctp' is not 'tcp' or 'udp'
+trigger_event(event = "empty_lb_backends", vip = "10.0.0.1:80", protocol = "aarp", load_balancer = "12345678-abcd-9876-fedc-11119f8e7d6c");
+ Load balancer protocol 'aarp' is not 'tcp', 'udp', or 'sctp'
trigger_event(event = "empty_lb_backends", vip = "10.0.0.1:80", protocol = "tcp", load_balancer = "bacon");
Load balancer 'bacon' is not a UUID
@@ -17868,6 +17868,123 @@ AT_CHECK([cat lflows.txt], [0], [dnl
OVN_CLEANUP([hv1], [hv2])
AT_CLEANUP
+AT_SETUP([ovn -- SCTP Load balancer health checks])
+AT_KEYWORDS([lb sctp])
+
+# Currently this test just ensures that no service monitors get created when
+# An SCTP load balancer is configured to use health checks. Once SCTP load
+# balancers are modified to allow health checks, this test should be altered
+# to ensure the health check succeeds.
+
+ovn_start
+
+# Set up same network as previous health check test. As long as health checks
+# aren't allowed for SCTP load balancers, the network will not be used for
+# much. However, having the network in place will make it easy to alter when
+# health checks are allowed.
+
+net_add n1
+
+sim_add hv1
+as hv1
+ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.1
+ovs-vsctl -- add-port br-int hv1-vif1 -- \
+ set interface hv1-vif1 external-ids:iface-id=sw0-p1 \
+ options:tx_pcap=hv1/vif1-tx.pcap \
+ options:rxq_pcap=hv1/vif1-rx.pcap \
+ ofport-request=1
+ovs-vsctl -- add-port br-int hv1-vif2 -- \
+ set interface hv1-vif2 external-ids:iface-id=sw0-p2 \
+ options:tx_pcap=hv1/vif2-tx.pcap \
+ options:rxq_pcap=hv1/vif2-rx.pcap \
+ ofport-request=2
+
+sim_add hv2
+as hv2
+ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.2
+ovs-vsctl -- add-port br-int hv2-vif1 -- \
+ set interface hv2-vif1 external-ids:iface-id=sw1-p1 \
+ options:tx_pcap=hv2/vif1-tx.pcap \
+ options:rxq_pcap=hv2/vif1-rx.pcap \
+ ofport-request=1
+
+ovn-nbctl ls-add sw0
+
+ovn-nbctl lsp-add sw0 sw0-p1
+ovn-nbctl lsp-set-addresses sw0-p1 "50:54:00:00:00:03 10.0.0.3"
+ovn-nbctl lsp-set-port-security sw0-p1 "50:54:00:00:00:03 10.0.0.3"
+
+ovn-nbctl lsp-add sw0 sw0-p2
+ovn-nbctl lsp-set-addresses sw0-p2 "50:54:00:00:00:04 10.0.0.4"
+ovn-nbctl lsp-set-port-security sw0-p2 "50:54:00:00:00:04 10.0.0.4"
+
+# Create the second logical switch with one port
+ovn-nbctl ls-add sw1
+ovn-nbctl lsp-add sw1 sw1-p1
+ovn-nbctl lsp-set-addresses sw1-p1 "40:54:00:00:00:03 20.0.0.3"
+ovn-nbctl lsp-set-port-security sw1-p1 "40:54:00:00:00:03 20.0.0.3"
+
+# Create a logical router and attach both logical switches
+ovn-nbctl lr-add lr0
+ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.1/24
+ovn-nbctl lsp-add sw0 sw0-lr0
+ovn-nbctl lsp-set-type sw0-lr0 router
+ovn-nbctl lsp-set-addresses sw0-lr0 router
+ovn-nbctl lsp-set-options sw0-lr0 router-port=lr0-sw0
+
+ovn-nbctl lrp-add lr0 lr0-sw1 00:00:00:00:ff:02 20.0.0.1/24
+ovn-nbctl lsp-add sw1 sw1-lr0
+ovn-nbctl lsp-set-type sw1-lr0 router
+ovn-nbctl lsp-set-addresses sw1-lr0 router
+ovn-nbctl lsp-set-options sw1-lr0 router-port=lr0-sw1
+
+ovn-nbctl lb-add lb1 10.0.0.10:80 10.0.0.3:80,20.0.0.3:80 sctp
+
+ovn-nbctl --wait=sb set load_balancer . ip_port_mappings:10.0.0.3=sw0-p1:10.0.0.2
+ovn-nbctl --wait=sb set load_balancer . ip_port_mappings:20.0.0.3=sw1-p1:10.0.0.2
+
+ovn-nbctl --wait=sb -- --id=@hc create \
+Load_Balancer_Health_Check vip="10.0.0.10\:80" -- add Load_Balancer . \
+health_check @hc
+
+ovn-nbctl --wait=sb ls-lb-add sw0 lb1
+ovn-nbctl --wait=sb ls-lb-add sw1 lb1
+ovn-nbctl --wait=sb lr-lb-add lr0 lb1
+
+ovn-nbctl ls-add public
+ovn-nbctl lrp-add lr0 lr0-public 00:00:20:20:12:13 172.168.0.100/24
+ovn-nbctl lsp-add public public-lr0
+ovn-nbctl lsp-set-type public-lr0 router
+ovn-nbctl lsp-set-addresses public-lr0 router
+ovn-nbctl lsp-set-options public-lr0 router-port=lr0-public
+
+# localnet port
+ovn-nbctl lsp-add public ln-public
+ovn-nbctl lsp-set-type ln-public localnet
+ovn-nbctl lsp-set-addresses ln-public unknown
+ovn-nbctl lsp-set-options ln-public network_name=public
+
+# schedule the gw router port to a chassis. Change the name of the chassis
+ovn-nbctl --wait=hv lrp-set-gateway-chassis lr0-public hv1 20
+
+OVN_POPULATE_ARP
+ovn-nbctl --wait=hv sync
+
+# And now for the anticlimax. We need to ensure that there is no
+# service monitor in the southbound db.
+
+AT_CHECK([test 0 = `ovn-sbctl --bare --columns _uuid find \
+service_monitor | sed '/^$/d' | wc -l`])
+
+# Let's also be sure the warning message about SCTP load balancers is
+# is in the ovn-northd log
+
+AT_CHECK([test 1 = `grep -c "SCTP load balancers do not currently support health checks" northd/ovn-northd.log`])
+
+AT_CLEANUP
+
AT_SETUP([ovn -- ARP/ND request broadcast limiting])
ovn_start
diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
index e80058e61..59abe0051 100644
--- a/utilities/ovn-nbctl.c
+++ b/utilities/ovn-nbctl.c
@@ -2734,9 +2734,11 @@ nbctl_lb_add(struct ctl_context *ctx)
/* Validate protocol. */
lb_proto = ctx->argv[4];
is_update_proto = true;
- if (strcmp(lb_proto, "tcp") && strcmp(lb_proto, "udp")) {
- ctl_error(ctx, "%s: protocol must be one of \"tcp\", \"udp\".",
- lb_proto);
+ if (strcmp(lb_proto, "tcp") &&
+ strcmp(lb_proto, "udp") &&
+ strcmp(lb_proto, "sctp")) {
+ ctl_error(ctx, "%s: protocol must be one of \"tcp\", \"udp\", "
+ " or \"sctp\".", lb_proto);
return;
}
}
--
2.24.1
More information about the dev
mailing list