On Wed, Jul 22, 2026 at 8:57 PM Mairtin O'Loingsigh <[email protected]> wrote:
> On Tue, Jul 21, 2026 at 02:53:20PM +0200, Ales Musil via dev wrote: > > The IC routes were using mix of printf functions and inet_ntop to > > format the address. Use the normalize_v46 helpers instead, this also > > has the effect that the prefix is truly normalized so it doesn't > > contain host bits anymore. > > > > Also simplify the debug messages formatting. > > > > Signed-off-by: Ales Musil <[email protected]> > > --- > > ic/ovn-ic.c | 59 ++++++++++++----------------------------------------- > > 1 file changed, 13 insertions(+), 46 deletions(-) > > > > diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c > > index 26173a5f6..fd20c73be 100644 > > --- a/ic/ovn-ic.c > > +++ b/ic/ovn-ic.c > > @@ -2088,14 +2088,7 @@ add_static_to_routes_ad( > > > > ds_put_format(&msg, "Advertising static route: %s -> %s, ic > nexthop: ", > > nb_route->ip_prefix, nb_route->nexthop); > > - > > - if (IN6_IS_ADDR_V4MAPPED(&nexthop)) { > > - ds_put_format(&msg, IP_FMT, > > - IP_ARGS(in6_addr_get_mapped_ipv4(&nexthop))); > > - } else { > > - ipv6_format_addr(&nexthop, &msg); > > - } > > - > > + ipv6_format_mapped(&nexthop, &msg); > > ds_put_format(&msg, ", route_table: %s", > nb_route->route_table[0] > > ? nb_route->route_table > > : "<main>"); > > @@ -2159,12 +2152,7 @@ add_network_to_routes_ad(struct hmap *routes_ad, > const char *network, > > ds_put_format(&msg, " of lrp %s,", nb_lrp->name); > > } > > ds_put_format(&msg, " nexthop "); > > - if (IN6_IS_ADDR_V4MAPPED(&nexthop)) { > > - ds_put_format(&msg, IP_FMT, > > - IP_ARGS(in6_addr_get_mapped_ipv4(&nexthop))); > > - } else { > > - ipv6_format_addr(&nexthop, &msg); > > - } > > + ipv6_format_mapped(&nexthop, &msg); > > > > VLOG_DBG("%s", ds_cstr(&msg)); > > ds_destroy(&msg); > > @@ -2234,13 +2222,7 @@ add_lb_vip_to_routes_ad(struct hmap *routes_ad, > const char *vip_key, > > > > ds_put_format(&msg, "Adding lb vip route to <main> routing " > > "table: %s, nexthop ", vip_str); > > - > > - if (IN6_IS_ADDR_V4MAPPED(&nexthop)) { > > - ds_put_format(&msg, IP_FMT, > > - IP_ARGS(in6_addr_get_mapped_ipv4(&nexthop))); > > - } else { > > - ipv6_format_addr(&nexthop, &msg); > > - } > > + ipv6_format_mapped(&nexthop, &msg); > > > > VLOG_DBG("%s", ds_cstr(&msg)); > > ds_destroy(&msg); > > @@ -2769,36 +2751,20 @@ advertise_routes(struct ic_context *ctx, > > } > > icsbrec_route_index_destroy_row(isb_route_key); > > > > + struct ds prefix = DS_EMPTY_INITIALIZER; > > + > > /* Create the missing routes in IC-SB */ > > struct ic_route_info *route_adv; > > HMAP_FOR_EACH_SAFE (route_adv, node, routes_ad) { > > + ds_clear(&prefix); > > + ipv6_format_mapped(&route_adv->prefix, &prefix); > Nit: Should probably use normalize_v46_prefix here for the prefix and > drop the ds. > We can't, normalize_v46_prefix will mask the IP which is a change in behavior, also it will omit /32 and /128 from the string. > + ds_put_format(&prefix, "/%d", route_adv->plen); > > + char *nexthop_s = normalize_v46(&route_adv->nexthop); > > + > > isb_route = icsbrec_route_insert(ctx->ovnisb_unlocked_txn); > > icsbrec_route_set_transit_switch(isb_route, ts_name); > > icsbrec_route_set_availability_zone(isb_route, az); > > - > > - /* The prefix and the next hop are formatted independently: an > IPv4 > > - * prefix may be advertised with an IPv6 next hop ("IPv4 over > IPv6"). > > - */ > > - char *prefix_s, *nexthop_s; > > - if (IN6_IS_ADDR_V4MAPPED(&route_adv->prefix)) { > > - ovs_be32 ipv4 = > in6_addr_get_mapped_ipv4(&route_adv->prefix); > > - prefix_s = xasprintf(IP_FMT "/%d", IP_ARGS(ipv4), > route_adv->plen); > > - } else { > > - char network_s[INET6_ADDRSTRLEN]; > > - inet_ntop(AF_INET6, &route_adv->prefix, network_s, > > - INET6_ADDRSTRLEN); > > - prefix_s = xasprintf("%s/%d", network_s, route_adv->plen); > > - } > > - if (IN6_IS_ADDR_V4MAPPED(&route_adv->nexthop)) { > > - ovs_be32 nh = in6_addr_get_mapped_ipv4(&route_adv->nexthop); > > - nexthop_s = xasprintf(IP_FMT, IP_ARGS(nh)); > > - } else { > > - char network_s[INET6_ADDRSTRLEN]; > > - inet_ntop(AF_INET6, &route_adv->nexthop, network_s, > > - INET6_ADDRSTRLEN); > > - nexthop_s = xstrdup(network_s); > > - } > > - icsbrec_route_set_ip_prefix(isb_route, prefix_s); > > + icsbrec_route_set_ip_prefix(isb_route, ds_cstr(&prefix)); > > icsbrec_route_set_nexthop(isb_route, nexthop_s); > > icsbrec_route_set_origin(isb_route, route_adv->origin); > > icsbrec_route_set_route_table(isb_route, route_adv->route_table > > @@ -2808,7 +2774,6 @@ advertise_routes(struct ic_context *ctx, > > icsbrec_route_update_options_setkey(isb_route, > > ROUTE_OVERRIDE_CONNECTED, "true"); > > } > > - free(prefix_s); > > free(nexthop_s); > > > > ad_route_sync_external_ids(route_adv, isb_route); > > @@ -2816,6 +2781,8 @@ advertise_routes(struct ic_context *ctx, > > hmap_remove(routes_ad, &route_adv->node); > > free(route_adv); > > } > > + > > + ds_destroy(&prefix); > > } > > > > static void > > -- > > 2.55.0 > > > > _______________________________________________ > > dev mailing list > > [email protected] > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > > Hi Ales, > > Patch looks good, just one Nit around clearing the host bits for the > prefix. > > Acked-by: Mairtin O'Loingsigh <[email protected]> > > Thank you for the review Mairtin, applied to main. Regards, Ales _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
