| Message ID | 20201104070246.2847579-10-blp@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | DDlog implementation of ovn-northd | expand |
On Wed, Nov 4, 2020 at 12:33 PM Ben Pfaff <blp@ovn.org> wrote: > > From: Justin Pettit <jpettit@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 <jpettit@ovn.org> Acked-by: Numan Siddique <numans@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 > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On Wed, Nov 04, 2020 at 11:19:45PM +0530, Numan Siddique wrote: > On Wed, Nov 4, 2020 at 12:33 PM Ben Pfaff <blp@ovn.org> wrote: > > > > From: Justin Pettit <jpettit@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 <jpettit@ovn.org> > > Acked-by: Numan Siddique <numans@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 ? If it's just a plain reposting of someone else's patch, I don't think it's needed, but that's not what I did here. I added the sign-off. > > 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. Oops. I fixed this. I applied this to OVN master.
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)) - /* 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. *