On 18 Dec 2024, at 20:12, Frode Nordahl 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]>
> ---
Recheck-request: github-robot
> 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)) {
> 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
