On Wed, Nov 4, 2020 at 12:33 PM Ben Pfaff <b...@ovn.org> wrote:
>
> From: Justin Pettit <jpet...@ovn.org>
>
> The upcoming ddlog implementation of northd needs these functions too,
> so they should be in a common library.
>
> Signed-off-by: Justin Pettit <jpet...@ovn.org>
Acked-by: Numan Siddique <num...@ovn.org>
One question: Does a submitter of a patch need to add a signed-off tag
when he/she is not an author of the patch ?
One small nit below.
Thanks
Numan
>
> ovn: Break out functions needed for ddlog.
> ---
> lib/ovn-util.c | 79 +++++++++++++++++++++++++++++++++++++++------
> lib/ovn-util.h | 11 +++++++
> northd/ovn-northd.c | 51 -----------------------------
> 3 files changed, 80 insertions(+), 61 deletions(-)
>
> diff --git a/lib/ovn-util.c b/lib/ovn-util.c
> index 321fc89e275a..abe6b04a7701 100644
> --- a/lib/ovn-util.c
> +++ b/lib/ovn-util.c
> @@ -13,17 +13,21 @@
> */
>
> #include <config.h>
> +
> +#include "ovn-util.h"
> +
> +#include <ctype.h>
> #include <unistd.h>
>
> #include "daemon.h"
> -#include "ovn-util.h"
> -#include "ovn-dirs.h"
> -#include "openvswitch/vlog.h"
> #include "openvswitch/ofp-parse.h"
> +#include "openvswitch/vlog.h"
> +#include "ovn-dirs.h"
> #include "ovn-nb-idl.h"
> #include "ovn-sb-idl.h"
> +#include "socket-util.h"
> +#include "svec.h"
> #include "unixctl.h"
> -#include <ctype.h>
>
> VLOG_DEFINE_THIS_MODULE(ovn_util);
>
> @@ -240,27 +244,37 @@ extract_ip_addresses(const char *address, struct
> lport_addresses *laddrs)
> bool
> extract_lrp_networks(const struct nbrec_logical_router_port *lrp,
> struct lport_addresses *laddrs)
> +{
> + return extract_lrp_networks__(lrp->mac, lrp->networks, lrp->n_networks,
> + laddrs);
> +}
> +
> +/* Separate out the body of 'extract_lrp_networks()' for use from DDlog,
> + * which does not know the 'nbrec_logical_router_port' type. */
> +bool
> +extract_lrp_networks__(char *mac, char **networks, size_t n_networks,
> + struct lport_addresses *laddrs)
> {
> memset(laddrs, 0, sizeof *laddrs);
>
> - if (!eth_addr_from_string(lrp->mac, &laddrs->ea)) {
> + if (!eth_addr_from_string(mac, &laddrs->ea)) {
> laddrs->ea = eth_addr_zero;
> return false;
> }
> snprintf(laddrs->ea_s, sizeof laddrs->ea_s, ETH_ADDR_FMT,
> ETH_ADDR_ARGS(laddrs->ea));
>
> - for (int i = 0; i < lrp->n_networks; i++) {
> + for (int i = 0; i < n_networks; i++) {
> ovs_be32 ip4;
> struct in6_addr ip6;
> unsigned int plen;
> char *error;
>
> - error = ip_parse_cidr(lrp->networks[i], &ip4, &plen);
> + error = ip_parse_cidr(networks[i], &ip4, &plen);
> if (!error) {
> if (!ip4) {
> static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5,
> 1);
> - VLOG_WARN_RL(&rl, "bad 'networks' %s", lrp->networks[i]);
> + VLOG_WARN_RL(&rl, "bad 'networks' %s", networks[i]);
> continue;
> }
>
> @@ -269,13 +283,13 @@ extract_lrp_networks(const struct
> nbrec_logical_router_port *lrp,
> }
> free(error);
>
> - error = ipv6_parse_cidr(lrp->networks[i], &ip6, &plen);
> + error = ipv6_parse_cidr(networks[i], &ip6, &plen);
> if (!error) {
> add_ipv6_netaddr(laddrs, ip6, plen);
> } else {
> static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
> VLOG_INFO_RL(&rl, "invalid syntax '%s' in networks",
> - lrp->networks[i]);
> + networks[i]);
> free(error);
> }
> }
> @@ -333,6 +347,23 @@ destroy_lport_addresses(struct lport_addresses *laddrs)
> free(laddrs->ipv6_addrs);
> }
>
> +/* Go through 'addresses' and add found IPv4 addresses to 'ipv4_addrs' and
> + * IPv6 addresses to 'ipv6_addrs'. */
> +void
> +split_addresses(const char *addresses, struct svec *ipv4_addrs,
> + struct svec *ipv6_addrs)
> +{
> + struct lport_addresses laddrs;
> + extract_lsp_addresses(addresses, &laddrs);
> + for (size_t k = 0; k < laddrs.n_ipv4_addrs; k++) {
> + svec_add(ipv4_addrs, laddrs.ipv4_addrs[k].addr_s);
> + }
> + for (size_t k = 0; k < laddrs.n_ipv6_addrs; k++) {
> + svec_add(ipv6_addrs, laddrs.ipv6_addrs[k].addr_s);
> + }
> + destroy_lport_addresses(&laddrs);
> +}
> +
> /* Allocates a key for NAT conntrack zone allocation for a provided
> * 'key' record and a 'type'.
> *
> @@ -663,3 +694,31 @@ ovn_smap_get_uint(const struct smap *smap, const char
> *key, unsigned int def)
>
> return u_value;
> }
> +
> +/* For a 'key' of the form "IP:port" or just "IP", sets 'port' and
> + * 'ip_address'. The caller must free() the memory allocated for
> + * 'ip_address'.
> + * Returns true if parsing of 'key' was successful, false otherwise.
> + */
> +bool
> +ip_address_and_port_from_lb_key(const char *key, char **ip_address,
> + uint16_t *port, int *addr_family)
> +{
> + struct sockaddr_storage ss;
> + if (!inet_parse_active(key, 0, &ss, false)) {
> + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
> + VLOG_WARN_RL(&rl, "bad ip address or port for load balancer key %s",
> + key);
> + *ip_address = NULL;
> + *port = 0;
> + *addr_family = 0;
> + return false;
> + }
> +
> + struct ds s = DS_EMPTY_INITIALIZER;
> + ss_format_address_nobracks(&ss, &s);
> + *ip_address = ds_steal_cstr(&s);
> + *port = ss_get_port(&ss);
> + *addr_family = ss.ss_family;
> + return true;
> +}
> diff --git a/lib/ovn-util.h b/lib/ovn-util.h
> index 6162f30225b3..a39cbef5a47e 100644
> --- a/lib/ovn-util.h
> +++ b/lib/ovn-util.h
> @@ -27,6 +27,7 @@
>
> struct nbrec_logical_router_port;
> struct sbrec_logical_flow;
> +struct svec;
> struct uuid;
> struct eth_addr;
> struct sbrec_port_binding;
> @@ -75,9 +76,16 @@ bool extract_lrp_networks(const struct
> nbrec_logical_router_port *,
> struct lport_addresses *);
> bool extract_sbrec_binding_first_mac(const struct sbrec_port_binding
> *binding,
> struct eth_addr *ea);
> +
> +bool extract_lrp_networks__(char *mac, char **networks, size_t n_networks,
> + struct lport_addresses *laddrs);
> +
> bool lport_addresses_is_empty(struct lport_addresses *);
> void destroy_lport_addresses(struct lport_addresses *);
>
> +void split_addresses(const char *addresses, struct svec *ipv4_addrs,
> + struct svec *ipv6_addrs);
> +
> char *alloc_nat_zone_key(const struct uuid *key, const char *type);
>
> const char *default_nb_db(void);
> @@ -219,4 +227,7 @@ char *str_tolower(const char *orig);
> case OVN_OPT_MONITOR: \
> case OVN_OPT_USER_GROUP:
>
> +bool ip_address_and_port_from_lb_key(const char *key, char **ip_address,
> + uint16_t *port, int *addr_family);
> +
> #endif
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 8800a3d3c10c..b55e154ef294 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -125,7 +125,6 @@ enum ovn_datapath_type {
> * functions can't be used in enums or switch cases.) */
> #define OVN_STAGE_BUILD(DP_TYPE, PIPELINE, TABLE) \
> (((DP_TYPE) << 9) | ((PIPELINE) << 8) | (TABLE))
> -
nit: seems like an unrelated change.
> /* A stage within an OVN logical switch or router.
> *
> * An "enum ovn_stage" indicates whether the stage is part of a logical
> switch
> @@ -2552,10 +2551,6 @@ join_logical_ports(struct northd_context *ctx,
> }
> }
>
> -static bool
> -ip_address_and_port_from_lb_key(const char *key, char **ip_address,
> - uint16_t *port, int *addr_family);
> -
> static void
> get_router_load_balancer_ips(const struct ovn_datapath *od,
> struct sset *all_ips_v4, struct sset
> *all_ips_v6)
> @@ -5079,34 +5074,6 @@ build_pre_acls(struct ovn_datapath *od, struct hmap
> *lflows)
> }
> }
>
> -/* For a 'key' of the form "IP:port" or just "IP", sets 'port' and
> - * 'ip_address'. The caller must free() the memory allocated for
> - * 'ip_address'.
> - * Returns true if parsing of 'key' was successful, false otherwise.
> - */
> -static bool
> -ip_address_and_port_from_lb_key(const char *key, char **ip_address,
> - uint16_t *port, int *addr_family)
> -{
> - struct sockaddr_storage ss;
> - if (!inet_parse_active(key, 0, &ss, false)) {
> - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
> - VLOG_WARN_RL(&rl, "bad ip address or port for load balancer key %s",
> - key);
> - *ip_address = NULL;
> - *port = 0;
> - *addr_family = 0;
> - return false;
> - }
> -
> - struct ds s = DS_EMPTY_INITIALIZER;
> - ss_format_address_nobracks(&ss, &s);
> - *ip_address = ds_steal_cstr(&s);
> - *port = ss_get_port(&ss);
> - *addr_family = ss.ss_family;
> - return true;
> -}
> -
> /*
> * Returns true if logical switch is configured with DNS records, false
> * otherwise.
> @@ -11513,24 +11480,6 @@ sync_address_set(struct northd_context *ctx, const
> char *name,
> addrs, n_addrs);
> }
>
> -/* Go through 'addresses' and add found IPv4 addresses to 'ipv4_addrs' and
> IPv6
> - * addresses to 'ipv6_addrs'.
> - */
> -static void
> -split_addresses(const char *addresses, struct svec *ipv4_addrs,
> - struct svec *ipv6_addrs)
> -{
> - struct lport_addresses laddrs;
> - extract_lsp_addresses(addresses, &laddrs);
> - for (size_t k = 0; k < laddrs.n_ipv4_addrs; k++) {
> - svec_add(ipv4_addrs, laddrs.ipv4_addrs[k].addr_s);
> - }
> - for (size_t k = 0; k < laddrs.n_ipv6_addrs; k++) {
> - svec_add(ipv6_addrs, laddrs.ipv6_addrs[k].addr_s);
> - }
> - destroy_lport_addresses(&laddrs);
> -}
> -
> /* OVN_Southbound Address_Set table contains same records as in north
> * bound, plus the records generated from Port_Group table in north bound.
> *
> --
> 2.26.2
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
