diff mbox series

[ovs-dev,v1,3/6] route-table: Expose data to ovn.

Message ID f7d03cf5e5ac10181a0562c2dfecf3ca7876e4a2.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 
previously the data generated in route-table was not exposed to ovn.
This is now changed and we expose a few additional fields we get from
the kernel.

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

Comments

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

> previously the data generated in route-table was not exposed to ovn.

Previously with a capital P, and should OVN be in capitals?

> This is now changed and we expose a few additional fields we get from
> the kernel.
>
> Signed-off-by: Felix Huettner <felix.huettner@stackit.cloud>

Some more comments below.

//Eelco


> ---
>  lib/route-table.c | 55 ++++++++++++++++++++++-------------------------
>  lib/route-table.h | 31 ++++++++++++++++++++++++++
>  2 files changed, 57 insertions(+), 29 deletions(-)
>
> diff --git a/lib/route-table.c b/lib/route-table.c
> index de573634d..918e86472 100644
> --- a/lib/route-table.c
> +++ b/lib/route-table.c
> @@ -47,27 +47,6 @@ VLOG_DEFINE_THIS_MODULE(route_table);
>
>  COVERAGE_DEFINE(route_table_dump);
>
> -struct route_data {
> -    /* Copied from struct rtmsg. */
> -    unsigned char rtm_dst_len;
> -    bool local;
> -
> -    /* 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;
> -};
> -
> -/* A digested version of a route message sent down by the kernel to indicate
> - * that a route has changed. */
> -struct route_table_msg {
> -    bool relevant;        /* Should this message be processed? */
> -    int nlmsg_type;       /* e.g. RTM_NEWROUTE, RTM_DELROUTE. */
> -    struct route_data rd; /* Data parsed from this message. */
> -};
> -
>  static struct ovs_mutex route_table_mutex = OVS_MUTEX_INITIALIZER;
>  static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
>
> @@ -84,7 +63,8 @@ static struct nln_notifier *name_notifier = NULL;
>  static bool route_table_valid = false;
>
>  static void route_table_reset(void);
> -static void route_table_handle_msg(const struct route_table_msg *);
> +static void route_table_handle_msg(const struct route_table_msg *,
> +                                   void *);
>  static int route_table_parse(struct ofpbuf *, void *change);
>  static void route_table_change(const struct route_table_msg *, void *);
>  static void route_map_clear(void);
> @@ -156,8 +136,12 @@ route_table_wait(void)
>      ovs_mutex_unlock(&route_table_mutex);
>  }
>
> -static bool
> -route_table_dump_one_table(const char *netns, unsigned char id)
> +bool
> +route_table_dump_one_table(
> +    char *netns,
> +    unsigned char id,
> +    void (*handle_msg)(const struct route_table_msg *, void *),

Maybe define a callback type for this?

> +    void *data)

Format looks odd, just fill the lines till the end.

>  {
>      uint64_t reply_stub[NL_DUMP_BUFSIZE / 8];
>      struct ofpbuf request, reply, buf;
> @@ -171,7 +155,9 @@ route_table_dump_one_table(const char *netns, unsigned char id)
>
>      rq_msg = ofpbuf_put_zeros(&request, sizeof *rq_msg);
>      rq_msg->rtm_family = AF_UNSPEC;
> -    rq_msg->rtm_table = id;
> +    rq_msg->rtm_table = RT_TABLE_UNSPEC;
> +
> +    nl_msg_put_u32(&request, RTA_TABLE, id);
>
>      nl_dump_start(netns, &dump, NETLINK_ROUTE, &request);
>      ofpbuf_uninit(&request);
> @@ -187,7 +173,7 @@ route_table_dump_one_table(const char *netns, unsigned char id)
>              if (!(nlmsghdr->nlmsg_flags & NLM_F_DUMP_FILTERED)) {
>                  filtered = false;
>              }
> -            route_table_handle_msg(&msg);
> +            (*handle_msg)(&msg, data);
>          }
>      }
>      ofpbuf_uninit(&buf);
> @@ -213,7 +199,9 @@ route_table_reset(void)
>      COVERAGE_INC(route_table_dump);
>
>      for (size_t i = 0; i < ARRAY_SIZE(tables); i++) {
> -        if (!route_table_dump_one_table(NULL, tables[i])) {
> +        if (!route_table_dump_one_table(NULL, tables[i],
> +                                        route_table_handle_msg,
> +                                        NULL)) {
>              /* Got unfiltered reply, no need to dump further. */
>              break;
>          }
> @@ -235,6 +223,7 @@ route_table_parse(struct ofpbuf *buf, void *change_)
>          [RTA_MARK] = { .type = NL_A_U32, .optional = true },
>          [RTA_PREFSRC] = { .type = NL_A_U32, .optional = true },
>          [RTA_TABLE] = { .type = NL_A_U32, .optional = true },
> +        [RTA_PRIORITY] = { .type = NL_A_U32, .optional = true },
>      };
>
>      static const struct nl_policy policy6[] = {
> @@ -244,6 +233,7 @@ route_table_parse(struct ofpbuf *buf, void *change_)
>          [RTA_GATEWAY] = { .type = NL_A_IPV6, .optional = true },
>          [RTA_PREFSRC] = { .type = NL_A_IPV6, .optional = true },
>          [RTA_TABLE] = { .type = NL_A_U32, .optional = true },
> +        [RTA_PRIORITY] = { .type = NL_A_U32, .optional = true },
>      };
>
>      struct nlattr *attrs[ARRAY_SIZE(policy)];
> @@ -292,11 +282,14 @@ route_table_parse(struct ofpbuf *buf, void *change_)
>              && table_id != RT_TABLE_MAIN
>              && table_id != RT_TABLE_LOCAL) {
>              change->relevant = false;
> -        }
> +         }
> +        change->rd.rta_table_id = table_id;
>
>          change->nlmsg_type     = nlmsg->nlmsg_type;
>          change->rd.rtm_dst_len = rtm->rtm_dst_len + (ipv4 ? 96 : 0);
>          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]);
>
> @@ -346,6 +339,9 @@ route_table_parse(struct ofpbuf *buf, void *change_)
>          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]);
> +        }
>      } else {
>          VLOG_DBG_RL(&rl, "received unparseable rtnetlink route message");
>          return 0;
> @@ -365,7 +361,8 @@ route_table_change(const struct route_table_msg *change OVS_UNUSED,
>  }
>
>  static void
> -route_table_handle_msg(const struct route_table_msg *change)
> +route_table_handle_msg(const struct route_table_msg *change,
> +                       void *data OVS_UNUSED)
>  {
>      if (change->relevant && change->nlmsg_type == RTM_NEWROUTE) {
>          const struct route_data *rd = &change->rd;
> diff --git a/lib/route-table.h b/lib/route-table.h
> index 3a02d737a..4f44a8085 100644
> --- a/lib/route-table.h
> +++ b/lib/route-table.h
> @@ -26,6 +26,31 @@
>
>  #include "openvswitch/types.h"
>
> +struct route_data {
> +    /* Copied from struct rtmsg. */
> +    unsigned char rtm_dst_len;
> +    bool local;
> +
> +    /* 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;
> +};
> +
> +/* A digested version of a route message sent down by the kernel to indicate
> + * that a route has changed. */
> +struct route_table_msg {
> +    bool relevant;        /* Should this message be processed? */
> +    int nlmsg_type;       /* e.g. RTM_NEWROUTE, RTM_DELROUTE. */
> +    struct route_data rd; /* Data parsed from this message. */
> +};
> +
>  uint64_t route_table_get_change_seq(void);
>  void route_table_init(void);
>  void route_table_run(void);
> @@ -33,4 +58,10 @@ void route_table_wait(void);
>  bool route_table_fallback_lookup(const struct in6_addr *ip6_dst,
>                                   char name[],
>                                   struct in6_addr *gw6);
> +bool
> +route_table_dump_one_table(
> +    char *netns,
> +    unsigned char id,
> +    void (*handle_msg)(const struct route_table_msg *, void *),
> +    void *data);

Can you use the same format as the other function definitions?

>  #endif /* route-table.h */
> --
diff mbox series

Patch

diff --git a/lib/route-table.c b/lib/route-table.c
index de573634d..918e86472 100644
--- a/lib/route-table.c
+++ b/lib/route-table.c
@@ -47,27 +47,6 @@  VLOG_DEFINE_THIS_MODULE(route_table);
 
 COVERAGE_DEFINE(route_table_dump);
 
-struct route_data {
-    /* Copied from struct rtmsg. */
-    unsigned char rtm_dst_len;
-    bool local;
-
-    /* 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;
-};
-
-/* A digested version of a route message sent down by the kernel to indicate
- * that a route has changed. */
-struct route_table_msg {
-    bool relevant;        /* Should this message be processed? */
-    int nlmsg_type;       /* e.g. RTM_NEWROUTE, RTM_DELROUTE. */
-    struct route_data rd; /* Data parsed from this message. */
-};
-
 static struct ovs_mutex route_table_mutex = OVS_MUTEX_INITIALIZER;
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
 
@@ -84,7 +63,8 @@  static struct nln_notifier *name_notifier = NULL;
 static bool route_table_valid = false;
 
 static void route_table_reset(void);
-static void route_table_handle_msg(const struct route_table_msg *);
+static void route_table_handle_msg(const struct route_table_msg *,
+                                   void *);
 static int route_table_parse(struct ofpbuf *, void *change);
 static void route_table_change(const struct route_table_msg *, void *);
 static void route_map_clear(void);
@@ -156,8 +136,12 @@  route_table_wait(void)
     ovs_mutex_unlock(&route_table_mutex);
 }
 
-static bool
-route_table_dump_one_table(const char *netns, unsigned char id)
+bool
+route_table_dump_one_table(
+    char *netns,
+    unsigned char id,
+    void (*handle_msg)(const struct route_table_msg *, void *),
+    void *data)
 {
     uint64_t reply_stub[NL_DUMP_BUFSIZE / 8];
     struct ofpbuf request, reply, buf;
@@ -171,7 +155,9 @@  route_table_dump_one_table(const char *netns, unsigned char id)
 
     rq_msg = ofpbuf_put_zeros(&request, sizeof *rq_msg);
     rq_msg->rtm_family = AF_UNSPEC;
-    rq_msg->rtm_table = id;
+    rq_msg->rtm_table = RT_TABLE_UNSPEC;
+
+    nl_msg_put_u32(&request, RTA_TABLE, id);
 
     nl_dump_start(netns, &dump, NETLINK_ROUTE, &request);
     ofpbuf_uninit(&request);
@@ -187,7 +173,7 @@  route_table_dump_one_table(const char *netns, unsigned char id)
             if (!(nlmsghdr->nlmsg_flags & NLM_F_DUMP_FILTERED)) {
                 filtered = false;
             }
-            route_table_handle_msg(&msg);
+            (*handle_msg)(&msg, data);
         }
     }
     ofpbuf_uninit(&buf);
@@ -213,7 +199,9 @@  route_table_reset(void)
     COVERAGE_INC(route_table_dump);
 
     for (size_t i = 0; i < ARRAY_SIZE(tables); i++) {
-        if (!route_table_dump_one_table(NULL, tables[i])) {
+        if (!route_table_dump_one_table(NULL, tables[i],
+                                        route_table_handle_msg,
+                                        NULL)) {
             /* Got unfiltered reply, no need to dump further. */
             break;
         }
@@ -235,6 +223,7 @@  route_table_parse(struct ofpbuf *buf, void *change_)
         [RTA_MARK] = { .type = NL_A_U32, .optional = true },
         [RTA_PREFSRC] = { .type = NL_A_U32, .optional = true },
         [RTA_TABLE] = { .type = NL_A_U32, .optional = true },
+        [RTA_PRIORITY] = { .type = NL_A_U32, .optional = true },
     };
 
     static const struct nl_policy policy6[] = {
@@ -244,6 +233,7 @@  route_table_parse(struct ofpbuf *buf, void *change_)
         [RTA_GATEWAY] = { .type = NL_A_IPV6, .optional = true },
         [RTA_PREFSRC] = { .type = NL_A_IPV6, .optional = true },
         [RTA_TABLE] = { .type = NL_A_U32, .optional = true },
+        [RTA_PRIORITY] = { .type = NL_A_U32, .optional = true },
     };
 
     struct nlattr *attrs[ARRAY_SIZE(policy)];
@@ -292,11 +282,14 @@  route_table_parse(struct ofpbuf *buf, void *change_)
             && table_id != RT_TABLE_MAIN
             && table_id != RT_TABLE_LOCAL) {
             change->relevant = false;
-        }
+         }
+        change->rd.rta_table_id = table_id;
 
         change->nlmsg_type     = nlmsg->nlmsg_type;
         change->rd.rtm_dst_len = rtm->rtm_dst_len + (ipv4 ? 96 : 0);
         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]);
 
@@ -346,6 +339,9 @@  route_table_parse(struct ofpbuf *buf, void *change_)
         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]);
+        }
     } else {
         VLOG_DBG_RL(&rl, "received unparseable rtnetlink route message");
         return 0;
@@ -365,7 +361,8 @@  route_table_change(const struct route_table_msg *change OVS_UNUSED,
 }
 
 static void
-route_table_handle_msg(const struct route_table_msg *change)
+route_table_handle_msg(const struct route_table_msg *change,
+                       void *data OVS_UNUSED)
 {
     if (change->relevant && change->nlmsg_type == RTM_NEWROUTE) {
         const struct route_data *rd = &change->rd;
diff --git a/lib/route-table.h b/lib/route-table.h
index 3a02d737a..4f44a8085 100644
--- a/lib/route-table.h
+++ b/lib/route-table.h
@@ -26,6 +26,31 @@ 
 
 #include "openvswitch/types.h"
 
+struct route_data {
+    /* Copied from struct rtmsg. */
+    unsigned char rtm_dst_len;
+    bool local;
+
+    /* 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;
+};
+
+/* A digested version of a route message sent down by the kernel to indicate
+ * that a route has changed. */
+struct route_table_msg {
+    bool relevant;        /* Should this message be processed? */
+    int nlmsg_type;       /* e.g. RTM_NEWROUTE, RTM_DELROUTE. */
+    struct route_data rd; /* Data parsed from this message. */
+};
+
 uint64_t route_table_get_change_seq(void);
 void route_table_init(void);
 void route_table_run(void);
@@ -33,4 +58,10 @@  void route_table_wait(void);
 bool route_table_fallback_lookup(const struct in6_addr *ip6_dst,
                                  char name[],
                                  struct in6_addr *gw6);
+bool
+route_table_dump_one_table(
+    char *netns,
+    unsigned char id,
+    void (*handle_msg)(const struct route_table_msg *, void *),
+    void *data);
 #endif /* route-table.h */