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 <[email protected]> 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 */ > -- _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
