On Wed, Oct 19, 2022 at 1:51 AM Numan Siddique <num...@ovn.org> wrote:
> 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
>
Comment updated in v2.
Thanks,
Ales
>
> > ---
> > 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
> >
>
>
--
Ales Musil
Senior Software Engineer - OVN Core
Red Hat EMEA <https://www.redhat.com>
amu...@redhat.com IM: amusil
<https://red.ht/sig>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
