diff mbox series

[ovs-dev,v1,5/6] route-table: Support parsing multipath routes.

Message ID 702e5e3f34bf9a07257f07c92808c67b2af26146.1729784574.git.felix.huettner@stackit.cloud
State Superseded
Delegated to: Eelco Chaudron
Headers show
Series OVS Patches for OVN Fabric Integration | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed

Commit Message

Felix Huettner Oct. 24, 2024, 3:46 p.m. UTC 
multipath routes have separate structures for each of the nexthops.
Previously if a multipath route was received it was treated as a route
without a nexthop.
Now these nexthops are parsed (up to a limit) and the ovs router uses
the first route in the list.
OVN will use the other routes.

Signed-off-by: Felix Huettner <felix.huettner@stackit.cloud>
---
 lib/route-table.c | 148 +++++++++++++++++++++++++++++++++++++---------
 lib/route-table.h |  12 +++-
 2 files changed, 131 insertions(+), 29 deletions(-)

Comments

Eelco Chaudron Nov. 14, 2024, 12:32 p.m. UTC | #1 
On 24 Oct 2024, at 17:46, Felix Huettner via dev wrote:

> multipath routes have separate structures for each of the nexthops.

Capital M for multipath.

> Previously if a multipath route was received it was treated as a route
> without a nexthop.
> Now these nexthops are parsed (up to a limit) and the ovs router uses
> the first route in the list.
> OVN will use the other routes.
>
> Signed-off-by: Felix Huettner <felix.huettner@stackit.cloud>
> ---

Hi Felix, see some comments below.

//Eelco

>  lib/route-table.c | 148 +++++++++++++++++++++++++++++++++++++---------
>  lib/route-table.h |  12 +++-
>  2 files changed, 131 insertions(+), 29 deletions(-)
>
> diff --git a/lib/route-table.c b/lib/route-table.c
> index 9b5c9f29d..80325f6f9 100644
> --- a/lib/route-table.c
> +++ b/lib/route-table.c
> @@ -207,6 +207,107 @@ route_table_reset(void)
>      }
>  }
>
> +static bool
> +route_table_add_nexthop(struct route_table_msg *change,
> +                        bool ipv4,
> +                        struct nlattr *nl_gw,
> +                        uint32_t ifindex)
> +{
> +    ovs_assert(change->rd.n_nexthops < MAX_ROUTE_DATA_NEXTHOP);

We should not assert in this case, just return false and add a debug log.

> +
> +    struct route_data_nexthop *nh = &change->rd.nexthops[
> +            change->rd.n_nexthops];
> +
> +    if (nl_gw) {
> +        if (ipv4) {
> +            ovs_be32 gw;

cr/lf between definitions and code.

> +            gw = nl_attr_get_be32(nl_gw);
> +            in6_addr_set_mapped_ipv4(&nh->rta_gw, gw);
> +        } else {
> +            nh->rta_gw = nl_attr_get_in6_addr(nl_gw);
> +        }
> +    }
> +
> +    if (ifindex && !if_indextoname(ifindex, nh->ifname)) {
> +        int error = errno;
> +
> +        VLOG_DBG_RL(&rl, "Could not find interface name[%u]: %s",
> +                    ifindex, ovs_strerror(error));
> +        if (error == ENXIO) {
> +            change->relevant = false;
> +        } else {
> +            return 0;

return false;

> +        }
> +    }
> +    change->rd.n_nexthops++;
> +    return 1;

return true here.

> +}
> +
> +
> +/* The following function uses the RTNH defines of rtnetlink.h
> + * As these rely on casts from char * to struct rtnexthop * clang will issue
> + * a warning about alignment even though the defines will ensure that the data
> + * is alligned. */
> +#ifdef __clang__
> +#pragma clang diagnostic push
> +#pragma clang diagnostic ignored "-Wcast-align"
> +#endif
> +
> +static bool
> +route_table_parse_multipath(struct route_table_msg *change,
> +                            struct nlattr *multipath,
> +                            bool ipv4)
> +{
> +    static const struct nl_policy policy[] = {
> +        [RTA_GATEWAY] = { .type = NL_A_U32, .optional = true },
> +    };
> +
> +    static const struct nl_policy policy6[] = {
> +        [RTA_GATEWAY] = { .type = NL_A_IPV6, .optional = true },
> +    };
> +
> +    struct nlattr *attrs[ARRAY_SIZE(policy)];
> +
> +    size_t len = RTA_PAYLOAD((struct rtattr *) multipath);
> +    struct rtnexthop *rtnh = RTA_DATA(multipath);
> +
> +    while (len >= sizeof(*rtnh) && len >= rtnh->rtnh_len &&
> +           change->rd.n_nexthops < MAX_ROUTE_DATA_NEXTHOP) {
> +
> +        bool parsed;
> +
> +        struct ofpbuf buf;

Put definitions together and use reverse c-tree:

        struct ofpbuf buf;
        bool parsed;

        ofpbuf_use_const(&buf, RTNH_DATA(rtnh),


> +        ofpbuf_use_const(&buf, RTNH_DATA(rtnh),
> +                         rtnh->rtnh_len - sizeof(*rtnh));
> +        if (ipv4) {
> +            parsed = nl_policy_parse(&buf, 0, policy, attrs,
> +                                     ARRAY_SIZE(policy));
> +        } else {
> +            parsed = nl_policy_parse(&buf, 0, policy6, attrs,
> +                                     ARRAY_SIZE(policy6));
> +        }
> +
> +        if (!parsed) {
> +            VLOG_DBG_RL(&rl, "received unparseable rtnetlink route message");
> +            return 0;

Return true/false in this function.

> +        }
> +
> +        if (!route_table_add_nexthop(change, ipv4, attrs[RTA_GATEWAY],
> +                                     rtnh->rtnh_ifindex)) {
> +            return 0;
> +        }
> +
> +        len -= RTNH_ALIGN(rtnh->rtnh_len);
> +        rtnh = RTNH_NEXT(rtnh);
> +    }
> +
> +    return 1;
> +}
> +
> +#ifdef __clang__
> +#pragma clang diagnostic pop
> +#endif
> +
>  /* Return RTNLGRP_IPV4_ROUTE or RTNLGRP_IPV6_ROUTE on success, 0 on parse
>   * error. */
>  int
> @@ -223,6 +324,7 @@ route_table_parse(struct ofpbuf *buf, void *change_)
>          [RTA_PREFSRC] = { .type = NL_A_U32, .optional = true },
>          [RTA_TABLE] = { .type = NL_A_U32, .optional = true },
>          [RTA_PRIORITY] = { .type = NL_A_U32, .optional = true },
> +        [RTA_MULTIPATH] = { .type = NL_A_NESTED, .optional = true },
>      };
>
>      static const struct nl_policy policy6[] = {
> @@ -233,6 +335,7 @@ route_table_parse(struct ofpbuf *buf, void *change_)
>          [RTA_PREFSRC] = { .type = NL_A_IPV6, .optional = true },
>          [RTA_TABLE] = { .type = NL_A_U32, .optional = true },
>          [RTA_PRIORITY] = { .type = NL_A_U32, .optional = true },
> +        [RTA_MULTIPATH] = { .type = NL_A_NESTED, .optional = true },
>      };
>
>      struct nlattr *attrs[ARRAY_SIZE(policy)];
> @@ -255,7 +358,6 @@ route_table_parse(struct ofpbuf *buf, void *change_)
>      if (parsed) {
>          const struct nlmsghdr *nlmsg;
>          uint32_t table_id;
> -        int rta_oif;      /* Output interface index. */
>
>          nlmsg = buf->data;
>
> @@ -289,21 +391,6 @@ route_table_parse(struct ofpbuf *buf, void *change_)
>          change->rd.local = rtm->rtm_type == RTN_LOCAL;
>          change->rd.plen = rtm->rtm_dst_len;
>          change->rd.rtm_protocol = rtm->rtm_protocol;
> -        if (attrs[RTA_OIF]) {
> -            rta_oif = nl_attr_get_u32(attrs[RTA_OIF]);
> -
> -            if (!if_indextoname(rta_oif, change->rd.ifname)) {
> -                int error = errno;
> -
> -                VLOG_DBG_RL(&rl, "Could not find interface name[%u]: %s",
> -                            rta_oif, ovs_strerror(error));
> -                if (error == ENXIO) {
> -                    change->relevant = false;
> -                } else {
> -                    return 0;
> -                }
> -            }
> -        }
>
>          if (attrs[RTA_DST]) {
>              if (ipv4) {
> @@ -326,21 +413,28 @@ route_table_parse(struct ofpbuf *buf, void *change_)
>                      nl_attr_get_in6_addr(attrs[RTA_PREFSRC]);
>              }
>          }
> -        if (attrs[RTA_GATEWAY]) {
> -            if (ipv4) {
> -                ovs_be32 gw;
> -                gw = nl_attr_get_be32(attrs[RTA_GATEWAY]);
> -                in6_addr_set_mapped_ipv4(&change->rd.rta_gw, gw);
> -            } else {
> -                change->rd.rta_gw = nl_attr_get_in6_addr(attrs[RTA_GATEWAY]);
> -            }
> -        }
>          if (attrs[RTA_MARK]) {
>              change->rd.mark = nl_attr_get_u32(attrs[RTA_MARK]);
>          }
>          if (attrs[RTA_PRIORITY]) {
>              change->rd.rta_priority = nl_attr_get_u32(attrs[RTA_PRIORITY]);
>          }
> +
> +        if (attrs[RTA_GATEWAY] || attrs[RTA_OIF]) {
> +            uint32_t ifindex = 0;
> +            if (attrs[RTA_OIF]) {
> +                ifindex = nl_attr_get_u32(attrs[RTA_OIF]);
> +            }
> +            if (!route_table_add_nexthop(change, ipv4, attrs[RTA_GATEWAY],
> +                                         ifindex)) {
> +                return 0;
> +            }
> +        } else if (attrs[RTA_MULTIPATH]) {
> +            if (!route_table_parse_multipath(change,
> +                                             attrs[RTA_MULTIPATH], ipv4)) {
> +                return 0;
> +            }
> +        }
>      } else {
>          VLOG_DBG_RL(&rl, "received unparseable rtnetlink route message");
>          return 0;
> @@ -367,8 +461,8 @@ route_table_handle_msg(const struct route_table_msg *change,
>          const struct route_data *rd = &change->rd;
>
>          ovs_router_insert(rd->mark, &rd->rta_dst, rd->rtm_dst_len,
> -                          rd->local, rd->ifname, &rd->rta_gw,
> -                          &rd->rta_prefsrc);
> +                          rd->local, rd->nexthops[0].ifname,
> +                          &rd->nexthops[0].rta_gw, &rd->rta_prefsrc);
>      }
>  }
>
> diff --git a/lib/route-table.h b/lib/route-table.h
> index 968652d66..f96dbebb3 100644
> --- a/lib/route-table.h
> +++ b/lib/route-table.h
> @@ -26,6 +26,13 @@
>
>  #include "openvswitch/ofpbuf.h"
>
> +#define MAX_ROUTE_DATA_NEXTHOP 8
> +
> +struct route_data_nexthop {
> +    struct in6_addr rta_gw;
> +    char ifname[IFNAMSIZ]; /* Interface name. */
> +};
> +
>  struct route_data {
>      /* Copied from struct rtmsg. */
>      unsigned char rtm_dst_len;
> @@ -34,13 +41,14 @@ 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. */
>      unsigned char plen;
>      unsigned char rtm_protocol;
>      uint32_t rta_priority;
> +
> +    size_t n_nexthops;
> +    struct route_data_nexthop nexthops[MAX_ROUTE_DATA_NEXTHOP];
>  };
>
>  /* A digested version of a route message sent down by the kernel to indicate
> -- 
> 2.47.0
diff mbox series

Patch

diff --git a/lib/route-table.c b/lib/route-table.c
index 9b5c9f29d..80325f6f9 100644
--- a/lib/route-table.c
+++ b/lib/route-table.c
@@ -207,6 +207,107 @@  route_table_reset(void)
     }
 }
 
+static bool
+route_table_add_nexthop(struct route_table_msg *change,
+                        bool ipv4,
+                        struct nlattr *nl_gw,
+                        uint32_t ifindex)
+{
+    ovs_assert(change->rd.n_nexthops < MAX_ROUTE_DATA_NEXTHOP);
+
+    struct route_data_nexthop *nh = &change->rd.nexthops[
+            change->rd.n_nexthops];
+
+    if (nl_gw) {
+        if (ipv4) {
+            ovs_be32 gw;
+            gw = nl_attr_get_be32(nl_gw);
+            in6_addr_set_mapped_ipv4(&nh->rta_gw, gw);
+        } else {
+            nh->rta_gw = nl_attr_get_in6_addr(nl_gw);
+        }
+    }
+
+    if (ifindex && !if_indextoname(ifindex, nh->ifname)) {
+        int error = errno;
+
+        VLOG_DBG_RL(&rl, "Could not find interface name[%u]: %s",
+                    ifindex, ovs_strerror(error));
+        if (error == ENXIO) {
+            change->relevant = false;
+        } else {
+            return 0;
+        }
+    }
+    change->rd.n_nexthops++;
+    return 1;
+}
+
+
+/* The following function uses the RTNH defines of rtnetlink.h
+ * As these rely on casts from char * to struct rtnexthop * clang will issue
+ * a warning about alignment even though the defines will ensure that the data
+ * is alligned. */
+#ifdef __clang__
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wcast-align"
+#endif
+
+static bool
+route_table_parse_multipath(struct route_table_msg *change,
+                            struct nlattr *multipath,
+                            bool ipv4)
+{
+    static const struct nl_policy policy[] = {
+        [RTA_GATEWAY] = { .type = NL_A_U32, .optional = true },
+    };
+
+    static const struct nl_policy policy6[] = {
+        [RTA_GATEWAY] = { .type = NL_A_IPV6, .optional = true },
+    };
+
+    struct nlattr *attrs[ARRAY_SIZE(policy)];
+
+    size_t len = RTA_PAYLOAD((struct rtattr *) multipath);
+    struct rtnexthop *rtnh = RTA_DATA(multipath);
+
+    while (len >= sizeof(*rtnh) && len >= rtnh->rtnh_len &&
+           change->rd.n_nexthops < MAX_ROUTE_DATA_NEXTHOP) {
+
+        bool parsed;
+
+        struct ofpbuf buf;
+        ofpbuf_use_const(&buf, RTNH_DATA(rtnh),
+                         rtnh->rtnh_len - sizeof(*rtnh));
+        if (ipv4) {
+            parsed = nl_policy_parse(&buf, 0, policy, attrs,
+                                     ARRAY_SIZE(policy));
+        } else {
+            parsed = nl_policy_parse(&buf, 0, policy6, attrs,
+                                     ARRAY_SIZE(policy6));
+        }
+
+        if (!parsed) {
+            VLOG_DBG_RL(&rl, "received unparseable rtnetlink route message");
+            return 0;
+        }
+
+        if (!route_table_add_nexthop(change, ipv4, attrs[RTA_GATEWAY],
+                                     rtnh->rtnh_ifindex)) {
+            return 0;
+        }
+
+        len -= RTNH_ALIGN(rtnh->rtnh_len);
+        rtnh = RTNH_NEXT(rtnh);
+    }
+
+    return 1;
+}
+
+#ifdef __clang__
+#pragma clang diagnostic pop
+#endif
+
 /* Return RTNLGRP_IPV4_ROUTE or RTNLGRP_IPV6_ROUTE on success, 0 on parse
  * error. */
 int
@@ -223,6 +324,7 @@  route_table_parse(struct ofpbuf *buf, void *change_)
         [RTA_PREFSRC] = { .type = NL_A_U32, .optional = true },
         [RTA_TABLE] = { .type = NL_A_U32, .optional = true },
         [RTA_PRIORITY] = { .type = NL_A_U32, .optional = true },
+        [RTA_MULTIPATH] = { .type = NL_A_NESTED, .optional = true },
     };
 
     static const struct nl_policy policy6[] = {
@@ -233,6 +335,7 @@  route_table_parse(struct ofpbuf *buf, void *change_)
         [RTA_PREFSRC] = { .type = NL_A_IPV6, .optional = true },
         [RTA_TABLE] = { .type = NL_A_U32, .optional = true },
         [RTA_PRIORITY] = { .type = NL_A_U32, .optional = true },
+        [RTA_MULTIPATH] = { .type = NL_A_NESTED, .optional = true },
     };
 
     struct nlattr *attrs[ARRAY_SIZE(policy)];
@@ -255,7 +358,6 @@  route_table_parse(struct ofpbuf *buf, void *change_)
     if (parsed) {
         const struct nlmsghdr *nlmsg;
         uint32_t table_id;
-        int rta_oif;      /* Output interface index. */
 
         nlmsg = buf->data;
 
@@ -289,21 +391,6 @@  route_table_parse(struct ofpbuf *buf, void *change_)
         change->rd.local = rtm->rtm_type == RTN_LOCAL;
         change->rd.plen = rtm->rtm_dst_len;
         change->rd.rtm_protocol = rtm->rtm_protocol;
-        if (attrs[RTA_OIF]) {
-            rta_oif = nl_attr_get_u32(attrs[RTA_OIF]);
-
-            if (!if_indextoname(rta_oif, change->rd.ifname)) {
-                int error = errno;
-
-                VLOG_DBG_RL(&rl, "Could not find interface name[%u]: %s",
-                            rta_oif, ovs_strerror(error));
-                if (error == ENXIO) {
-                    change->relevant = false;
-                } else {
-                    return 0;
-                }
-            }
-        }
 
         if (attrs[RTA_DST]) {
             if (ipv4) {
@@ -326,21 +413,28 @@  route_table_parse(struct ofpbuf *buf, void *change_)
                     nl_attr_get_in6_addr(attrs[RTA_PREFSRC]);
             }
         }
-        if (attrs[RTA_GATEWAY]) {
-            if (ipv4) {
-                ovs_be32 gw;
-                gw = nl_attr_get_be32(attrs[RTA_GATEWAY]);
-                in6_addr_set_mapped_ipv4(&change->rd.rta_gw, gw);
-            } else {
-                change->rd.rta_gw = nl_attr_get_in6_addr(attrs[RTA_GATEWAY]);
-            }
-        }
         if (attrs[RTA_MARK]) {
             change->rd.mark = nl_attr_get_u32(attrs[RTA_MARK]);
         }
         if (attrs[RTA_PRIORITY]) {
             change->rd.rta_priority = nl_attr_get_u32(attrs[RTA_PRIORITY]);
         }
+
+        if (attrs[RTA_GATEWAY] || attrs[RTA_OIF]) {
+            uint32_t ifindex = 0;
+            if (attrs[RTA_OIF]) {
+                ifindex = nl_attr_get_u32(attrs[RTA_OIF]);
+            }
+            if (!route_table_add_nexthop(change, ipv4, attrs[RTA_GATEWAY],
+                                         ifindex)) {
+                return 0;
+            }
+        } else if (attrs[RTA_MULTIPATH]) {
+            if (!route_table_parse_multipath(change,
+                                             attrs[RTA_MULTIPATH], ipv4)) {
+                return 0;
+            }
+        }
     } else {
         VLOG_DBG_RL(&rl, "received unparseable rtnetlink route message");
         return 0;
@@ -367,8 +461,8 @@  route_table_handle_msg(const struct route_table_msg *change,
         const struct route_data *rd = &change->rd;
 
         ovs_router_insert(rd->mark, &rd->rta_dst, rd->rtm_dst_len,
-                          rd->local, rd->ifname, &rd->rta_gw,
-                          &rd->rta_prefsrc);
+                          rd->local, rd->nexthops[0].ifname,
+                          &rd->nexthops[0].rta_gw, &rd->rta_prefsrc);
     }
 }
 
diff --git a/lib/route-table.h b/lib/route-table.h
index 968652d66..f96dbebb3 100644
--- a/lib/route-table.h
+++ b/lib/route-table.h
@@ -26,6 +26,13 @@ 
 
 #include "openvswitch/ofpbuf.h"
 
+#define MAX_ROUTE_DATA_NEXTHOP 8
+
+struct route_data_nexthop {
+    struct in6_addr rta_gw;
+    char ifname[IFNAMSIZ]; /* Interface name. */
+};
+
 struct route_data {
     /* Copied from struct rtmsg. */
     unsigned char rtm_dst_len;
@@ -34,13 +41,14 @@  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. */
     unsigned char plen;
     unsigned char rtm_protocol;
     uint32_t rta_priority;
+
+    size_t n_nexthops;
+    struct route_data_nexthop nexthops[MAX_ROUTE_DATA_NEXTHOP];
 };
 
 /* A digested version of a route message sent down by the kernel to indicate