On Wed, Dec 18, 2024 at 8:12 PM Frode Nordahl <[email protected]> wrote: > > In preparation for adding support for parsing routes with multiple > next hops per destination, we need dynamic storage for them. > > Storage for the primary next hop is set aside in struct route_data > to avoid unnecessary memory allocations for the common case of one > next hop. > > The ovs-router module does not support storing multiple next hop > information, and the existing parser does not understand them, so > we retain this behavior by only passing single next hop routes for > storage in ovs-router. > > This patch makes this change in isolation so that it is easier to > review subsequent patches. > > Signed-off-by: Frode Nordahl <[email protected]> > --- > lib/route-table.c | 94 ++++++++++++++++++++++++++++++++++++++++------- > 1 file changed, 81 insertions(+), 13 deletions(-) > > diff --git a/lib/route-table.c b/lib/route-table.c > index 4e8f35199..b6f0223c0 100644 > --- a/lib/route-table.c > +++ b/lib/route-table.c > @@ -32,6 +32,7 @@ > #include "netlink.h" > #include "netlink-notifier.h" > #include "netlink-socket.h" > +#include "openvswitch/list.h" > #include "openvswitch/ofpbuf.h" > #include "ovs-router.h" > #include "packets.h" > @@ -47,7 +48,34 @@ VLOG_DEFINE_THIS_MODULE(route_table); > > COVERAGE_DEFINE(route_table_dump); > > +struct route_data_nexthop { > + struct ovs_list nexthop_node; > + > + struct in6_addr addr; > + char ifname[IFNAMSIZ]; /* Interface name. */ > +}; > + > struct route_data { > + /* Routes can have multiple next hops per destination. > + * > + * Each next hop has its own set of attributes such as address family, > + * interface and IP address. > + * > + * When retrieving information about a route from the kernel, in the case > + * of multiple next hops, information is provided as nested attributes. > + * > + * A linked list with struct route_data_nexthop entries is used to store > + * this information as we parse each attribute. > + * > + * For the common case of one next hop, the nexthops list will contain a > + * single entry pointing to the struct route_data _primary_next_hop > + * element. > + * > + * Any dynamically allocated list elements can be freed with a call to > the > + * route_data_destroy function. */ > + struct ovs_list nexthops; > + struct route_data_nexthop _primary_next_hop; > + > /* Copied from struct rtmsg. */ > unsigned char rtm_dst_len; > unsigned char rtm_protocol; > @@ -56,8 +84,6 @@ struct route_data { > /* Extracted from Netlink attributes. */ > struct in6_addr rta_dst; /* 0 if missing. */ > struct in6_addr rta_prefsrc; /* 0 if missing. */ > - struct in6_addr rta_gw; > - char ifname[IFNAMSIZ]; /* Interface name. */ > uint32_t mark; > uint32_t rta_table_id; /* 0 if missing. */ > uint32_t rta_priority; /* 0 if missing. */ > @@ -89,12 +115,24 @@ static bool route_table_valid = false; > static void route_table_reset(void); > static void route_table_handle_msg(const struct route_table_msg *); > static int route_table_parse(struct ofpbuf *, void *change); > -static void route_table_change(const struct route_table_msg *, void *); > +static void route_table_change(struct route_table_msg *, void *aux); > static void route_map_clear(void); > > static void name_table_init(void); > static void name_table_change(const struct rtnetlink_change *, void *); > > +static void > +route_data_destroy(struct route_data *rd) > +{ > + struct route_data_nexthop *rdnh; > + > + LIST_FOR_EACH_POP (rdnh, nexthop_node, &rd->nexthops) { > + if (rdnh && rdnh != &rd->_primary_next_hop) { > + free(rdnh); > + } > + } > +} > + > uint64_t > route_table_get_change_seq(void) > { > @@ -190,6 +228,7 @@ route_table_dump_one_table(unsigned char id) > filtered = false; > } > route_table_handle_msg(&msg); > + route_data_destroy(&msg.rd); > } > } > ofpbuf_uninit(&buf); > @@ -222,13 +261,12 @@ route_table_reset(void) > } > } > > -/* Return RTNLGRP_IPV4_ROUTE or RTNLGRP_IPV6_ROUTE on success, 0 on parse > - * error. */ > static int > route_table_parse__(struct ofpbuf *buf, size_t ofs, > const struct nlmsghdr *nlmsg, > const struct rtmsg *rtm, struct route_table_msg *change) > { > + struct route_data_nexthop *rdnh = NULL; > bool parsed, ipv4 = false; > > static const struct nl_policy policy[] = { > @@ -269,6 +307,12 @@ route_table_parse__(struct ofpbuf *buf, size_t ofs, > int rta_oif; /* Output interface index. */ > > memset(change, 0, sizeof *change); > + > + /* ovs_list_init / ovs_list_insert does not allocate any memory */ > + ovs_list_init(&change->rd.nexthops); > + rdnh = &change->rd._primary_next_hop; > + ovs_list_insert(&change->rd.nexthops, &rdnh->nexthop_node); > + > change->relevant = true; > > if (rtm->rtm_scope == RT_SCOPE_NOWHERE) { > @@ -292,7 +336,7 @@ route_table_parse__(struct ofpbuf *buf, size_t ofs, > if (attrs[RTA_OIF]) { > rta_oif = nl_attr_get_u32(attrs[RTA_OIF]); > > - if (!if_indextoname(rta_oif, change->rd.ifname)) { > + if (!if_indextoname(rta_oif, rdnh->ifname)) { > int error = errno; > > VLOG_DBG_RL(&rl, "Could not find interface name[%u]: %s", > @@ -300,7 +344,7 @@ route_table_parse__(struct ofpbuf *buf, size_t ofs, > if (error == ENXIO) { > change->relevant = false; > } else { > - return 0; > + goto error_out; > } > } > } > @@ -330,9 +374,9 @@ route_table_parse__(struct ofpbuf *buf, size_t ofs, > if (ipv4) { > ovs_be32 gw; > gw = nl_attr_get_be32(attrs[RTA_GATEWAY]); > - in6_addr_set_mapped_ipv4(&change->rd.rta_gw, gw); > + in6_addr_set_mapped_ipv4(&rdnh->addr, gw); > } else { > - change->rd.rta_gw = nl_attr_get_in6_addr(attrs[RTA_GATEWAY]); > + rdnh->addr = nl_attr_get_in6_addr(attrs[RTA_GATEWAY]); > } > } > if (attrs[RTA_MARK]) { > @@ -343,13 +387,27 @@ route_table_parse__(struct ofpbuf *buf, size_t ofs, > } > } else { > VLOG_DBG_RL(&rl, "received unparseable rtnetlink route message"); > - return 0; > + goto error_out; > } > > /* Success. */ > return ipv4 ? RTNLGRP_IPV4_ROUTE : RTNLGRP_IPV6_ROUTE; > + > +error_out: > + route_data_destroy(&change->rd); > + return 0; > } > > +/* Parse Netlink message in buf, which is expected to contain UAPI rtmsg > + * header and associated route attributes. > + * > + * Return RTNLGRP_IPV4_ROUTE or RTNLGRP_IPV6_ROUTE on success, 0 on parse > + * error. > + * > + * On success, memory may have be allocated, and it is the callers > + * responsibility to free it with a call to route_data_destroy. > + * > + * In case of error, any allocated memory will be freed before return. */ > static int > route_table_parse(struct ofpbuf *buf, void *change) > { > @@ -373,7 +431,7 @@ route_table_standard_table(uint32_t table_id) > } > > static void > -route_table_change(const struct route_table_msg *change OVS_UNUSED, > +route_table_change(struct route_table_msg *change, > void *aux OVS_UNUSED) > { > if (!change > @@ -381,16 +439,26 @@ route_table_change(const struct route_table_msg *change > OVS_UNUSED, > && route_table_standard_table(change->rd.rta_table_id))) { > route_table_valid = false; > } > + if (change) { > + route_data_destroy(&change->rd); > + } > } > > static void > route_table_handle_msg(const struct route_table_msg *change) > { > - if (change->relevant && change->nlmsg_type == RTM_NEWROUTE) { > + if (change->relevant && change->nlmsg_type == RTM_NEWROUTE > + && ovs_list_is_singleton(&change->rd.nexthops)) {
Note that this is changed to !ovs_list_is_empty(&change->rd.nexthops) in patch 15. I missed that I touched this in both patches, hope that's OK. -- Frode Nordahl > const struct route_data *rd = &change->rd; > + const struct route_data_nexthop *rdnh; > + > + /* The ovs-router module does currently not implement lookup nor > + * storage for routes with multiple next hops. */ > + rdnh = CONTAINER_OF(ovs_list_front(&change->rd.nexthops), > + const struct route_data_nexthop, nexthop_node); > > ovs_router_insert(rd->mark, &rd->rta_dst, rd->rtm_dst_len, > - rd->local, rd->ifname, &rd->rta_gw, > + rd->local, rdnh->ifname, &rdnh->addr, > &rd->rta_prefsrc); > } > } > -- > 2.45.2 > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
