On Tue, Oct 18, 2022 at 7:23 AM Ales Musil <amu...@redhat.com> wrote:
>
> The current code was parsing the ip twice, once for
> the "inet_parse_active" and second time by calling
> either "ip_parse" or "ipv6_parse". Avoid that by
> getting the "in6_addr" directly from "sockaddr_storage".
>
> Signed-off-by: Ales Musil <amu...@redhat.com>
Thanks for the patch.
LGTM. Can you please also update the function comments in
lib/ovn-util.c now that the function
also sets 'struct in6_addr *ip'.
Acked-by: Numan Siddique <num...@ovn.org>
Numan
> ---
> lib/lb.c | 20 +++-----------------
> lib/ovn-util.c | 5 ++++-
> lib/ovn-util.h | 3 ++-
> utilities/ovn-trace.c | 20 ++++++--------------
> 4 files changed, 15 insertions(+), 33 deletions(-)
>
> diff --git a/lib/lb.c b/lib/lb.c
> index 77674ea28..ab5de38a8 100644
> --- a/lib/lb.c
> +++ b/lib/lb.c
> @@ -78,18 +78,11 @@ bool ovn_lb_vip_init(struct ovn_lb_vip *lb_vip, const
> char *lb_key,
> int addr_family;
>
> if (!ip_address_and_port_from_lb_key(lb_key, &lb_vip->vip_str,
> - &lb_vip->vip_port, &addr_family)) {
> + &lb_vip->vip, &lb_vip->vip_port,
> + &addr_family)) {
> return false;
> }
>
> - if (addr_family == AF_INET) {
> - ovs_be32 vip4;
> - ip_parse(lb_vip->vip_str, &vip4);
> - in6_addr_set_mapped_ipv4(&lb_vip->vip, vip4);
> - } else {
> - ipv6_parse(lb_vip->vip_str, &lb_vip->vip);
> - }
> -
> /* Format for backend ips: "IP1:port1,IP2:port2,...". */
> size_t n_backends = 0;
> size_t n_allocated_backends = 0;
> @@ -108,7 +101,7 @@ bool ovn_lb_vip_init(struct ovn_lb_vip *lb_vip, const
> char *lb_key,
> struct ovn_lb_backend *backend = &lb_vip->backends[n_backends];
> int backend_addr_family;
> if (!ip_address_and_port_from_lb_key(token, &backend->ip_str,
> - &backend->port,
> + &backend->ip, &backend->port,
> &backend_addr_family)) {
> continue;
> }
> @@ -118,13 +111,6 @@ bool ovn_lb_vip_init(struct ovn_lb_vip *lb_vip, const
> char *lb_key,
> continue;
> }
>
> - if (addr_family == AF_INET) {
> - ovs_be32 ip4;
> - ip_parse(backend->ip_str, &ip4);
> - in6_addr_set_mapped_ipv4(&backend->ip, ip4);
> - } else {
> - ipv6_parse(backend->ip_str, &backend->ip);
> - }
> n_backends++;
> }
> free(tokstr);
> diff --git a/lib/ovn-util.c b/lib/ovn-util.c
> index d80db179a..13659cd70 100644
> --- a/lib/ovn-util.c
> +++ b/lib/ovn-util.c
> @@ -788,7 +788,8 @@ ovn_smap_get_uint(const struct smap *smap, const char
> *key, unsigned int def)
> */
> bool
> ip_address_and_port_from_lb_key(const char *key, char **ip_address,
> - uint16_t *port, int *addr_family)
> + struct in6_addr *ip, uint16_t *port,
> + int *addr_family)
> {
> struct sockaddr_storage ss;
> if (!inet_parse_active(key, 0, &ss, false, NULL)) {
> @@ -798,12 +799,14 @@ ip_address_and_port_from_lb_key(const char *key, char
> **ip_address,
> *ip_address = NULL;
> *port = 0;
> *addr_family = 0;
> + memset(ip, 0, sizeof(*ip));
> return false;
> }
>
> struct ds s = DS_EMPTY_INITIALIZER;
> ss_format_address_nobracks(&ss, &s);
> *ip_address = ds_steal_cstr(&s);
> + *ip = ss_get_address(&ss);
> *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 145f974ed..a1f1cf0ad 100644
> --- a/lib/ovn-util.h
> +++ b/lib/ovn-util.h
> @@ -241,7 +241,8 @@ char *str_tolower(const char *orig);
> 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);
> + struct in6_addr *ip, uint16_t *port,
> + int *addr_family);
>
> /* Returns the internal OVN version. The caller must free the returned
> * value. */
> diff --git a/utilities/ovn-trace.c b/utilities/ovn-trace.c
> index d9e7129d9..6fa5137d9 100644
> --- a/utilities/ovn-trace.c
> +++ b/utilities/ovn-trace.c
> @@ -2601,23 +2601,15 @@ get_lb_vip_data(struct flow *uflow, struct in6_addr
> *vip,
> SBREC_LOAD_BALANCER_FOR_EACH (sbdb, ovnsb_idl) {
> struct smap_node *node;
> SMAP_FOR_EACH (node, &sbdb->vips) {
> - if (!ip_address_and_port_from_lb_key(node->key, vip_str,
> - port, &family)) {
> + if (!ip_address_and_port_from_lb_key(node->key, vip_str, vip,
> port,
> + &family)) {
> continue;
> }
>
> - if (family == AF_INET) {
> - ovs_be32 vip4;
> - ip_parse(*vip_str, &vip4);
> - in6_addr_set_mapped_ipv4(vip, vip4);
> - if (vip4 == uflow->ct_nw_dst) {
> - return true;
> - }
> - } else {
> - ipv6_parse(*vip_str, vip);
> - if (ipv6_addr_equals(vip, &uflow->ct_ipv6_dst)) {
> - return true;
> - }
> + if ((family == AF_INET
> + && in6_addr_get_mapped_ipv4(vip) == uflow->ct_nw_dst)
> + || ipv6_addr_equals(vip, &uflow->ct_ipv6_dst)) {
> + return true;
> }
> free(*vip_str);
> }
> --
> 2.37.3
>
> _______________________________________________
> 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
