On Sat, Mar 14, 2020 at 12:52 AM Mark Michelson <[email protected]> wrote:
>
> 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 <[email protected]>
Hi Mark,
There are few checkpatch warnings with this patch. And below system
tests are failing
******
7: ovn -- load-balancing FAILED (ovs-macros.at:220)
8: ovn -- load-balancing - IPv6 FAILED (ovs-macros.at:220)
9: ovn -- load-balancing - same subnet. FAILED (ovs-macros.at:220)
10: ovn -- load-balancing - same subnet. - IPv6 FAILED (ovs-macros.at:220)
11: ovn -- load balancing in gateway router FAILED (ovs-macros.at:220)
12: ovn -- load balancing in gateway router - IPv6 FAILED (ovs-macros.at:220)
13: ovn -- multiple gateway routers, load-balancing FAILED (ovs-macros.at:220)
14: ovn -- multiple gateway routers, load-balancing - IPv6 FAILED
(ovs-macros.at:220)
15: ovn -- load balancing in router with gateway router port FAILED
(ovs-macros.at:220)
16: ovn -- load balancing in router with gateway router port - IPv6
FAILED (ovs-macros.at:220)
*******
Thanks
Numan
> ---
> lib/actions.c | 6 ++-
> northd/ovn-northd.c | 43 +++++++++------
> ovn-nb.ovsschema | 6 +--
> ovn-nb.xml | 10 ++--
> tests/ovn.at | 121 +++++++++++++++++++++++++++++++++++++++++-
> utilities/ovn-nbctl.c | 8 +--
> 6 files changed, 164 insertions(+), 30 deletions(-)
>
> diff --git a/lib/actions.c b/lib/actions.c
> index f22acddff..4101cc426 100644
> --- a/lib/actions.c
> +++ b/lib/actions.c
> @@ -1957,9 +1957,11 @@ 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'",
> + "Load balancer protocol '%s' is not 'tcp', 'udp', or
> 'sctp'",
> c->string);
> return;
> }
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 4f94680b5..c1fdf56e8 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 (!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";
> + }
> }
> }
>
> @@ -9170,15 +9184,14 @@ 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");
>
> 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);
> - }
> + const char *proto = is_udp ? "udp" : is_sctp
> + ? "sctp"
> + : "tcp";
> + ds_put_format(&match, " && %s && %s.dst == %d", proto,
> + proto, lb_vip->vip_port);
> prio = 120;
> }
>
> 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..d280b25ec 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
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
