| Message ID | 20230328143138.1942211-1-dceara@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,branch-22.12] lb: Allow IPv6 template LBs to use explicit backends. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | fail | github build: failed |
On Tue, Mar 28, 2023 at 4:31 PM Dumitru Ceara <dceara@redhat.com> wrote: > 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 <dceara@redhat.com> > Acked-by: Ales Musil <amusil@redhat.com> > (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 > > Looks good to me, thanks. Acked-by: Ales Musil <amusil@redhat.com>
On 3/29/23 11:17, Ales Musil wrote: > On Tue, Mar 28, 2023 at 4:31 PM Dumitru Ceara <dceara@redhat.com> wrote: > >> 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 <dceara@redhat.com> >> Acked-by: Ales Musil <amusil@redhat.com> >> (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 >> >> > Looks good to me, thanks. > > Acked-by: Ales Musil <amusil@redhat.com> > Thanks, applied to 22.12.
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;
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 <dceara@redhat.com> Acked-by: Ales Musil <amusil@redhat.com> (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(-)