[ovs-dev] [PATCH 2/3] ovn-northd: Make use of svec for storing lists of addresses.

Han Zhou zhouhan at gmail.com
Sun Jul 29 06:14:29 UTC 2018


On Thu, Jul 26, 2018 at 3:51 AM, Jakub Sitnicki <jkbs at redhat.com> wrote:
>
> Get rid of what is, esentially, an open-coded version of svec.
>
> Signed-off-by: Jakub Sitnicki <jkbs at redhat.com>
> ---
>  ovn/northd/ovn-northd.c | 47
+++++++++++++++-------------------------------
>  tests/ovn.at            | 50
+++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 65 insertions(+), 32 deletions(-)
>
> diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
> index 572022897..52f47c5ed 100644
> --- a/ovn/northd/ovn-northd.c
> +++ b/ovn/northd/ovn-northd.c
> @@ -39,6 +39,7 @@
>  #include "openvswitch/poll-loop.h"
>  #include "smap.h"
>  #include "sset.h"
> +#include "svec.h"
>  #include "stream.h"
>  #include "stream-ssl.h"
>  #include "unixctl.h"
> @@ -6675,54 +6676,36 @@ sync_address_sets(struct northd_context *ctx)
>      /* sync port group generated address sets first */
>      const struct nbrec_port_group *nb_port_group;
>      NBREC_PORT_GROUP_FOR_EACH (nb_port_group, ctx->ovnnb_idl) {
> -        char **ipv4_addrs = xcalloc(1, sizeof *ipv4_addrs);
> -        size_t n_ipv4_addrs = 0;
> -        size_t n_ipv4_addrs_buf = 1;
> -        char **ipv6_addrs = xcalloc(1, sizeof *ipv6_addrs);
> -        size_t n_ipv6_addrs = 0;
> -        size_t n_ipv6_addrs_buf = 1;
> +        struct svec ipv4_addrs = SVEC_EMPTY_INITIALIZER;
> +        struct svec ipv6_addrs = SVEC_EMPTY_INITIALIZER;
>          for (size_t i = 0; i < nb_port_group->n_ports; i++) {
>              for (size_t j = 0; j < nb_port_group->ports[i]->n_addresses;
j++) {
>                  struct lport_addresses laddrs;
>
 extract_lsp_addresses(nb_port_group->ports[i]->addresses[j],
>                                       &laddrs);
> -                while (n_ipv4_addrs_buf < n_ipv4_addrs +
laddrs.n_ipv4_addrs) {
> -                    n_ipv4_addrs_buf *= 2;
> -                    ipv4_addrs = xrealloc(ipv4_addrs,
> -                            n_ipv4_addrs_buf * sizeof *ipv4_addrs);
> -                }
>                  for (size_t k = 0; k < laddrs.n_ipv4_addrs; k++) {
> -                    ipv4_addrs[n_ipv4_addrs++] =
> -                        xstrdup(laddrs.ipv4_addrs[k].addr_s);
> -                }
> -                while (n_ipv6_addrs_buf < n_ipv6_addrs +
laddrs.n_ipv6_addrs) {
> -                    n_ipv6_addrs_buf *= 2;
> -                    ipv6_addrs = xrealloc(ipv6_addrs,
> -                            n_ipv6_addrs_buf * sizeof *ipv6_addrs);
> +                    svec_add(&ipv4_addrs, laddrs.ipv4_addrs[k].addr_s);
>                  }
>                  for (size_t k = 0; k < laddrs.n_ipv6_addrs; k++) {
> -                    ipv6_addrs[n_ipv6_addrs++] =
> -                        xstrdup(laddrs.ipv6_addrs[k].addr_s);
> +                    svec_add(&ipv6_addrs, laddrs.ipv6_addrs[k].addr_s);
>                  }
>                  destroy_lport_addresses(&laddrs);
>              }
>          }
>          char *ipv4_addrs_name = xasprintf("%s_ip4", nb_port_group->name);
>          char *ipv6_addrs_name = xasprintf("%s_ip6", nb_port_group->name);
> -        sync_address_set(ctx, ipv4_addrs_name, (const char **)ipv4_addrs,
> -                         n_ipv4_addrs, &sb_address_sets);
> -        sync_address_set(ctx, ipv6_addrs_name, (const char **)ipv6_addrs,
> -                         n_ipv6_addrs, &sb_address_sets);
> +        sync_address_set(ctx, ipv4_addrs_name,
> +                         /* "char **" is not compatible with "const char
**" */
> +                         (const char **)ipv4_addrs.names,
> +                         ipv4_addrs.n, &sb_address_sets);
> +        sync_address_set(ctx, ipv6_addrs_name,
> +                         /* "char **" is not compatible with "const char
**" */
> +                         (const char **)ipv6_addrs.names,
> +                         ipv6_addrs.n, &sb_address_sets);
>          free(ipv4_addrs_name);
>          free(ipv6_addrs_name);
> -        for (size_t i = 0; i < n_ipv4_addrs; i++) {
> -            free(ipv4_addrs[i]);
> -        }
> -        free(ipv4_addrs);
> -        for (size_t i = 0; i < n_ipv6_addrs; i++) {
> -            free(ipv6_addrs[i]);
> -        }
> -        free(ipv6_addrs);
> +        svec_destroy(&ipv4_addrs);
> +        svec_destroy(&ipv6_addrs);
>      }
>
>      /* sync user defined address sets, which may overwrite port group
> diff --git a/tests/ovn.at b/tests/ovn.at
> index e80c965a3..e7bdfe250 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -10217,6 +10217,56 @@ done
>  OVN_CLEANUP([hv1], [hv2], [hv3])
>  AT_CLEANUP
>
> +AT_SETUP([ovn -- Address Set generation from Port Groups (static
addressing)])
> +ovn_start
> +
> +ovn-nbctl ls-add ls1
> +
> +ovn-nbctl lsp-add ls1 lp1
> +ovn-nbctl lsp-add ls1 lp2
> +ovn-nbctl lsp-add ls1 lp3
> +
> +ovn-nbctl lsp-set-addresses lp1 "02:00:00:00:00:01 10.0.0.1 2001:db8::1"
> +ovn-nbctl lsp-set-addresses lp2 "02:00:00:00:00:02 10.0.0.2 2001:db8::2"
> +ovn-nbctl lsp-set-addresses lp3 "02:00:00:00:00:03 10.0.0.3 2001:db8::3"
> +
> +ovn-nbctl create Port_Group name=pg1
> +ovn-nbctl create Port_Group name=pg2
> +
> +ovn-nbctl --id=@p get Logical_Switch_Port lp1 -- add Port_Group pg1
ports @p
> +ovn-nbctl --id=@p get Logical_Switch_Port lp2 -- add Port_Group pg1
ports @p
> +ovn-nbctl --id=@p get Logical_Switch_Port lp2 -- add Port_Group pg2
ports @p
> +ovn-nbctl --id=@p get Logical_Switch_Port lp3 -- add Port_Group pg2
ports @p
> +
> +ovn-nbctl --wait=sb sync
> +
> +dnl Check if port group address sets were populated with ports' addresses
> +AT_CHECK([ovn-sbctl get Address_Set pg1_ip4 addresses],
> +         [0], [[["10.0.0.1", "10.0.0.2"]]
> +])
> +AT_CHECK([ovn-sbctl get Address_Set pg2_ip4 addresses],
> +         [0], [[["10.0.0.2", "10.0.0.3"]]
> +])
> +AT_CHECK([ovn-sbctl get Address_Set pg1_ip6 addresses],
> +         [0], [[["2001:db8::1", "2001:db8::2"]]
> +])
> +AT_CHECK([ovn-sbctl get Address_Set pg2_ip6 addresses],
> +         [0], [[["2001:db8::2", "2001:db8::3"]]
> +])
> +
> +ovn-nbctl --wait=sb lsp-set-addresses lp1 \
> +    "02:00:00:00:00:01 10.0.0.11 2001:db8::11"
> +
> +dnl Check if updated address got propagated to the port group address
sets
> +AT_CHECK([ovn-sbctl get Address_Set pg1_ip4 addresses],
> +         [0], [[["10.0.0.11", "10.0.0.2"]]
> +])
> +AT_CHECK([ovn-sbctl get Address_Set pg1_ip6 addresses],
> +         [0], [[["2001:db8::11", "2001:db8::2"]]
> +])
> +
> +AT_CLEANUP
> +
>  AT_SETUP([ovn -- ACL conjunction])
>  ovn_start
>
> --
> 2.14.4
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Thanks for simplify the code and adding test.

Acked-by: Han Zhou <hzhou8 at ebay.com>


More information about the dev mailing list