On Wed, Feb 15, 2023 at 1:38 PM Dumitru Ceara <[email protected]> wrote:
> These need to eventually expand to either:
> a. [<valid-IPv6-addr>]:port
> or
> b. <valid-IPv6-addr>
>
> In order to achieve that, do some pre-processing in northd. The LB
> template test cases are also updated to cover this case. The only
> affected flows were those related to LB hairpin traffic.
>
> Fixes: b45853fba816 ("lb: Support using templates.")
> Signed-off-by: Dumitru Ceara <[email protected]>
> ---
> include/ovn/lex.h | 2 ++
> lib/lb.c | 48 +++++++++++++++++++++++++++++++++++++++++----
> lib/lb.h | 5 +++++
> lib/lex.c | 4 ++--
> northd/northd.c | 2 +-
> tests/system-ovn.at | 6 ++++--
> 6 files changed, 58 insertions(+), 9 deletions(-)
>
> diff --git a/include/ovn/lex.h b/include/ovn/lex.h
> index 9159b7a263..64d33361f4 100644
> --- a/include/ovn/lex.h
> +++ b/include/ovn/lex.h
> @@ -29,6 +29,8 @@
>
> struct ds;
>
> +#define LEX_TEMPLATE_PREFIX '^'
> +
> /* Token type. */
> enum lex_type {
> LEX_T_END, /* end of input */
> diff --git a/lib/lb.c b/lib/lb.c
> index e0e97572f7..d20bb990db 100644
> --- a/lib/lb.c
> +++ b/lib/lb.c
> @@ -316,11 +316,10 @@ ovn_lb_vip_destroy(struct ovn_lb_vip *vip)
> free(vip->backends);
> }
>
> -void
> -ovn_lb_vip_format(const struct ovn_lb_vip *vip, struct ds *s, bool
> template)
> +static void
> +ovn_lb_vip_format__(const struct ovn_lb_vip *vip, struct ds *s,
> + bool needs_brackets)
> {
> - bool needs_brackets = vip->address_family == AF_INET6 && vip->port_str
> - && !template;
> if (needs_brackets) {
> ds_put_char(s, '[');
> }
> @@ -333,6 +332,30 @@ ovn_lb_vip_format(const struct ovn_lb_vip *vip,
> struct ds *s, bool template)
> }
> }
>
> +/* Formats the VIP in the way the ovn-controller expects it, that is,
> + * template IPv6 variables need to be between brackets too.
> + */
> +static char *
> +ovn_lb_vip6_template_format_internal(const struct ovn_lb_vip *vip)
> +{
> + struct ds s = DS_EMPTY_INITIALIZER;
> +
> + if (vip->vip_str && *vip->vip_str == LEX_TEMPLATE_PREFIX) {
> + ovn_lb_vip_format__(vip, &s, true);
> + } else {
> + ovn_lb_vip_format(vip, &s, !!vip->port_str);
> + }
> + return ds_steal_cstr(&s);
> +}
> +
> +void
> +ovn_lb_vip_format(const struct ovn_lb_vip *vip, struct ds *s, bool
> template)
> +{
> + bool needs_brackets = vip->address_family == AF_INET6 && vip->port_str
> + && !template;
> + ovn_lb_vip_format__(vip, s, needs_brackets);
> +}
> +
> void
> ovn_lb_vip_backends_format(const struct ovn_lb_vip *vip, struct ds *s,
> bool template)
> @@ -515,6 +538,7 @@ ovn_northd_lb_create(const struct nbrec_load_balancer
> *nbrec_lb,
> lb->n_vips = smap_count(&nbrec_lb->vips);
> lb->vips = xcalloc(lb->n_vips, sizeof *lb->vips);
> lb->vips_nb = xcalloc(lb->n_vips, sizeof *lb->vips_nb);
> + smap_init(&lb->template_vips);
> lb->controller_event = smap_get_bool(&nbrec_lb->options, "event",
> false);
>
> bool routable = smap_get_bool(&nbrec_lb->options, "add_route", false);
> @@ -566,6 +590,12 @@ ovn_northd_lb_create(const struct nbrec_load_balancer
> *nbrec_lb,
> } else {
> sset_add(&lb->ips_v6, lb_vip->vip_str);
> }
> +
> + if (lb->template && address_family == AF_INET6) {
> + smap_add_nocopy(&lb->template_vips,
> + ovn_lb_vip6_template_format_internal(lb_vip),
> + xstrdup(node->value));
> + }
> n_vips++;
> }
>
> @@ -610,6 +640,15 @@ ovn_northd_lb_find(const struct hmap *lbs, const
> struct uuid *uuid)
> return NULL;
> }
>
> +const struct smap *
> +ovn_northd_lb_get_vips(const struct ovn_northd_lb *lb)
> +{
> + if (!smap_is_empty(&lb->template_vips)) {
> + return &lb->template_vips;
> + }
> + return &lb->nlb->vips;
> +}
> +
> void
> ovn_northd_lb_add_lr(struct ovn_northd_lb *lb, size_t n,
> struct ovn_datapath **ods)
> @@ -637,6 +676,7 @@ ovn_northd_lb_destroy(struct ovn_northd_lb *lb)
> }
> free(lb->vips);
> free(lb->vips_nb);
> + smap_destroy(&lb->template_vips);
> sset_destroy(&lb->ips_v4);
> sset_destroy(&lb->ips_v6);
> free(lb->selection_fields);
> diff --git a/lib/lb.h b/lib/lb.h
> index 7594553d53..657a1c8da9 100644
> --- a/lib/lb.h
> +++ b/lib/lb.h
> @@ -19,6 +19,7 @@
>
> #include <sys/types.h>
> #include <netinet/in.h>
> +#include "lib/smap.h"
> #include "openvswitch/hmap.h"
> #include "ovn-util.h"
> #include "sset.h"
> @@ -62,6 +63,9 @@ struct ovn_northd_lb {
> char *selection_fields;
> struct ovn_lb_vip *vips;
> struct ovn_northd_lb_vip *vips_nb;
> + struct smap template_vips; /* Slightly changed template VIPs,
> populated
> + * if needed. Until now it's only required
> + * for IPv6 template load balancers. */
> size_t n_vips;
>
> enum lb_neighbor_responder_mode neigh_mode;
> @@ -129,6 +133,7 @@ struct ovn_northd_lb *ovn_northd_lb_create(const
> struct nbrec_load_balancer *,
> size_t n_datapaths);
> struct ovn_northd_lb *ovn_northd_lb_find(const struct hmap *,
> const struct uuid *);
> +const struct smap *ovn_northd_lb_get_vips(const struct ovn_northd_lb *);
> void ovn_northd_lb_destroy(struct ovn_northd_lb *);
> void ovn_northd_lb_add_lr(struct ovn_northd_lb *lb, size_t n,
> struct ovn_datapath **ods);
> diff --git a/lib/lex.c b/lib/lex.c
> index 5251868b5a..a8b9812bbb 100644
> --- a/lib/lex.c
> +++ b/lib/lex.c
> @@ -782,7 +782,7 @@ next:
> p = lex_parse_port_group(p, token);
> break;
>
> - case '^':
> + case LEX_TEMPLATE_PREFIX:
> p = lex_parse_template(p, token);
> break;
>
> @@ -1061,7 +1061,7 @@ lexer_parse_template_string(const char *s, const
> struct smap *template_vars,
> struct sset *template_vars_ref)
> {
> /* No '^' means no templates. */
> - if (!strchr(s, '^')) {
> + if (!strchr(s, LEX_TEMPLATE_PREFIX)) {
> return lex_str_use(s);
> }
>
> diff --git a/northd/northd.c b/northd/northd.c
> index 79f65e6e8d..345d80d373 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -4448,7 +4448,7 @@ sync_lbs(struct northd_input *input_data, struct
> ovsdb_idl_txn *ovnsb_txn,
>
> /* Update columns. */
> sbrec_load_balancer_set_name(lb->slb, lb->nlb->name);
> - sbrec_load_balancer_set_vips(lb->slb, &lb->nlb->vips);
> + sbrec_load_balancer_set_vips(lb->slb, ovn_northd_lb_get_vips(lb));
> sbrec_load_balancer_set_protocol(lb->slb, lb->nlb->protocol);
> sbrec_load_balancer_set_datapath_group(lb->slb, dpg->dp_group);
> sbrec_load_balancer_set_options(lb->slb, &options);
> diff --git a/tests/system-ovn.at b/tests/system-ovn.at
> index 2ece0f571b..cee1fd9218 100644
> --- a/tests/system-ovn.at
> +++ b/tests/system-ovn.at
> @@ -9510,7 +9510,8 @@ name: 'vport' value: '666'
> # Start IPv4 TCP server on vm1.
> NETNS_DAEMONIZE([vm1], [nc -k -l 42.42.42.2 4242], [nc-vm1.pid])
>
> -# Make sure connecting to the VIP works.
> +# 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])
>
> @@ -9603,7 +9604,8 @@ name: 'vport' value: '666'
> # Start IPv6 TCP server on vm1.
> NETNS_DAEMONIZE([vm1], [nc -k -l 4242::2 4242], [nc-vm1.pid])
>
> -# Make sure connecting to the VIP works.
> +# 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])
>
> --
> 2.31.1
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
Looks good to me, thanks.
Acked-by: Ales Musil <[email protected]>
--
Ales Musil
Senior Software Engineer - OVN Core
Red Hat EMEA <https://www.redhat.com>
[email protected] IM: amusil
<https://red.ht/sig>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
