Note that the internal handling of routes in the ovs-route module does currently not support multipath routes, so when presented with one the first occurrence will be stored. This is not a regression as these routes were previously not considered at all.
Storing the information in the route-table module data structure will allow external to OVS projects make use of this data. A test program run as part of the system tests that exercise the route table API is added in this patch. Co-Authored-by: Felix Huettner <[email protected]> Signed-off-by: Felix Huettner <[email protected]> Signed-off-by: Frode Nordahl <[email protected]> --- Makefile.am | 5 +- lib/route-table.c | 61 ++++++++++-- tests/automake.mk | 1 + tests/system-route.at | 184 +++++++++++++++++++++++++++++++++++ tests/test-lib-route-table.c | 149 ++++++++++++++++++++++++++++ 5 files changed, 389 insertions(+), 11 deletions(-) create mode 100644 tests/test-lib-route-table.c diff --git a/Makefile.am b/Makefile.am index dc5c34a6a..a61a1cadf 100644 --- a/Makefile.am +++ b/Makefile.am @@ -339,6 +339,8 @@ check-tabs: fi .PHONY: check-tabs +# NOTE: test-lib-route-table.c excluded due to use of system() to execute +# ip route commands provided as arguments by test suite. ALL_LOCAL += thread-safety-check thread-safety-check: @cd $(srcdir); \ @@ -346,7 +348,8 @@ thread-safety-check: grep -n -f build-aux/thread-safety-forbidden \ `git ls-files | grep '\.[ch]$$' \ | $(EGREP) -v '^datapath-windows|^lib/sflow|^third-party'` /dev/null \ - | $(EGREP) -v ':[ ]*/?\*'; \ + | $(EGREP) -v ':[ ]*/?\*' \ + | $(EGREP) -v '^tests/test-lib-route-table.c'; \ then \ echo "See above for list of calls to functions that are"; \ echo "forbidden due to thread safety issues"; \ diff --git a/lib/route-table.c b/lib/route-table.c index 56f7dd167..c9f1c241f 100644 --- a/lib/route-table.c +++ b/lib/route-table.c @@ -223,7 +223,9 @@ route_table_reset(void) static int route_table_parse__(struct ofpbuf *buf, size_t ofs, const struct nlmsghdr *nlmsg, - const struct rtmsg *rtm, struct route_table_msg *change) + const struct rtmsg *rtm, + const struct rtnexthop *rtnh, + struct route_table_msg *change) { bool parsed, ipv4 = false; @@ -236,6 +238,7 @@ route_table_parse__(struct ofpbuf *buf, size_t ofs, [RTA_TABLE] = { .type = NL_A_U32, .optional = true }, [RTA_PRIORITY] = { .type = NL_A_U32, .optional = true }, [RTA_VIA] = { .type = NL_A_RTA_VIA, .optional = true }, + [RTA_MULTIPATH] = { .type = NL_A_NESTED, .optional = true }, }; static const struct nl_policy policy6[] = { @@ -247,6 +250,7 @@ route_table_parse__(struct ofpbuf *buf, size_t ofs, [RTA_TABLE] = { .type = NL_A_U32, .optional = true }, [RTA_PRIORITY] = { .type = NL_A_U32, .optional = true }, [RTA_VIA] = { .type = NL_A_RTA_VIA, .optional = true }, + [RTA_MULTIPATH] = { .type = NL_A_NESTED, .optional = true }, }; struct nlattr *attrs[ARRAY_SIZE(policy)]; @@ -270,9 +274,12 @@ route_table_parse__(struct ofpbuf *buf, size_t ofs, memset(change, 0, sizeof *change); ovs_list_init(&change->rd.nexthops); - rdnh = &change->rd._primary_next_hop; - rdnh->family = rtm->rtm_family; - ovs_list_insert(&change->rd.nexthops, &rdnh->nexthop_node); + rdnh = rtnh ? xzalloc(sizeof *rdnh) : &change->rd._primary_next_hop; + + if (!attrs[RTA_MULTIPATH]) { + rdnh->family = rtm->rtm_family; + ovs_list_insert(&change->rd.nexthops, &rdnh->nexthop_node); + } change->relevant = true; @@ -294,8 +301,9 @@ route_table_parse__(struct ofpbuf *buf, size_t ofs, change->rd.rtm_dst_len = rtm->rtm_dst_len; change->rd.rtm_protocol = rtm->rtm_protocol; change->rd.rtn_local = rtm->rtm_type == RTN_LOCAL; - if (attrs[RTA_OIF]) { - rta_oif = nl_attr_get_u32(attrs[RTA_OIF]); + if (attrs[RTA_OIF] || rtnh) { + rta_oif = rtnh + ? rtnh->rtnh_ifindex : nl_attr_get_u32(attrs[RTA_OIF]); if (!if_indextoname(rta_oif, rdnh->ifname)) { int error = errno; @@ -384,11 +392,44 @@ route_table_parse__(struct ofpbuf *buf, size_t ofs, goto error_out; } } - if (!attrs[RTA_OIF] && !attrs[RTA_GATEWAY] && !attrs[RTA_VIA]) { - VLOG_DBG_RL(&rl, "route message needs an RTA_OIF, RTA_GATEWAY or " - "RTA_VIA attribute"); + if (attrs[RTA_MULTIPATH]) { + const struct nlattr *nla; + size_t left; + + if (rtnh) { + VLOG_DBG_RL(&rl, "unexpected nested RTA_MULTIPATH attribute"); + goto error_out; + } + + NL_NESTED_FOR_EACH (nla, left, attrs[RTA_MULTIPATH]) { + struct route_table_msg mp_change; + struct rtnexthop *mp_rtnh; + struct ofpbuf mp_buf; + + ofpbuf_use_data(&mp_buf, nla, nla->nla_len); + mp_rtnh = ofpbuf_try_pull(&mp_buf, sizeof *mp_rtnh); + + if (!mp_rtnh) { + VLOG_DBG_RL(&rl, "got short message while parsing " + "multipath attribute"); + goto error_out; + } + + if (!route_table_parse__(&mp_buf, 0, nlmsg, rtm, mp_rtnh, + &mp_change)) { + goto error_out; + } + ovs_list_push_back_all(&change->rd.nexthops, + &mp_change.rd.nexthops); + } + } + if (!attrs[RTA_OIF] && !attrs[RTA_GATEWAY] + && !attrs[RTA_VIA] && !attrs[RTA_MULTIPATH]) { + VLOG_DBG_RL(&rl, "route message needs an RTA_OIF, RTA_GATEWAY, " + "RTA_VIA or RTA_MULTIPATH attribute"); goto error_out; } + /* Add any additional RTA attribute processing before RTA_MULTIPATH. */ } else { VLOG_DBG_RL(&rl, "received unparseable rtnetlink route message"); goto error_out; @@ -422,7 +463,7 @@ route_table_parse(struct ofpbuf *buf, void *change) rtm = ofpbuf_at(buf, NLMSG_HDRLEN, sizeof *rtm); return route_table_parse__(buf, NLMSG_HDRLEN + sizeof *rtm, - nlmsg, rtm, change); + nlmsg, rtm, NULL, change); } static bool diff --git a/tests/automake.mk b/tests/automake.mk index edfc2cb33..59f538761 100644 --- a/tests/automake.mk +++ b/tests/automake.mk @@ -498,6 +498,7 @@ endif if LINUX tests_ovstest_SOURCES += \ + tests/test-lib-route-table.c \ tests/test-netlink-conntrack.c \ tests/test-netlink-policy.c \ tests/test-psample.c diff --git a/tests/system-route.at b/tests/system-route.at index 010a3412a..66bfd0e8e 100644 --- a/tests/system-route.at +++ b/tests/system-route.at @@ -111,8 +111,13 @@ Cached: 10.0.0.0/24 dev p1-route SRC 10.0.0.17 Cached: 10.0.0.17/32 dev p1-route SRC 10.0.0.17 local Cached: 10.0.0.18/32 dev p1-route SRC 10.0.0.17]) +dnl Negative check for custom routing table using route-table library. +AT_CHECK([ovstest test-lib-route-table-dump | grep rta_table_id:\ 42], [1]) +AT_CHECK([ovstest test-lib-route-table-dump | grep rta_table_id:\ 1042], [1]) + dnl Add a route to a custom routing table and check that OVS doesn't cache it. AT_CHECK([ip route add 10.0.0.19/32 dev p1-route table 42]) +AT_CHECK([ip route add 10.0.0.20/32 dev p1-route table 1042]) AT_CHECK([ip route show table 42 | grep 'p1-route' | grep -q '10.0.0.19']) dnl Give the main thread a chance to act. AT_CHECK([ovs-appctl revalidator/wait]) @@ -122,6 +127,11 @@ Cached: 10.0.0.0/24 dev p1-route SRC 10.0.0.17 Cached: 10.0.0.17/32 dev p1-route SRC 10.0.0.17 local Cached: 10.0.0.18/32 dev p1-route SRC 10.0.0.17 ]) +AT_CHECK([ovstest test-lib-route-table-dump | \ + awk '/rta_table_id:.*42/{print$1" "$15" "$16}' | sort], [0], [dnl +10.0.0.19/32 rta_table_id: 42 +10.0.0.20/32 rta_table_id: 1042 +]) dnl Delete a route from the main table and check that OVS removes the route dnl from the cache. @@ -148,3 +158,177 @@ OVS_WAIT_UNTIL([test $(ovs-appctl ovs/route/show | grep -c 'p1-route') -eq 0 ]) OVS_TRAFFIC_VSWITCHD_STOP AT_CLEANUP + +AT_SETUP([ovs-route - add system route with multiple nexthop - ipv4]) +AT_KEYWORDS([route]) +OVS_TRAFFIC_VSWITCHD_START() + +dnl Create tap ports. +AT_CHECK([ip tuntap add name p1-route mode tap]) +AT_CHECK([ip link set p1-route up]) +on_exit 'ip link del p1-route' +AT_CHECK([ip tuntap add name p2-route mode tap]) +AT_CHECK([ip link set p2-route up]) +on_exit 'ip link del p2-route' + +AT_CHECK([ip addr add 192.168.42.10/24 dev p1-route], [0], [stdout]) +AT_CHECK([ip addr add 192.168.51.10/24 dev p2-route], [0], [stdout]) +AT_CHECK([ip route add 172.16.42.0/24 nexthop via 192.168.42.1 \ + dev p1-route nexthop via 192.168.51.1 dev p2-route], [0], [stdout]) + +dnl NOTE: At the time of this writing, it is expected that only the first route +dnl will be stored in ovs-router. +OVS_WAIT_UNTIL_EQUAL([ovs-appctl ovs/route/show | grep -E '172.16.42.0/24' | \ + sort], [dnl +Cached: 172.16.42.0/24 dev p1-route GW 192.168.42.1 SRC 192.168.42.10]) + +dnl Confirm that both nexthops are available when using the route-table library +dnl directly. +AT_CHECK([ovstest test-lib-route-table-dump | grep 172.16.42.0.*nexthop | sort], + [0], [dnl + 172.16.42.0/24 nexthop family: AF_INET addr: 192.168.42.1 ifname: p1-route + 172.16.42.0/24 nexthop family: AF_INET addr: 192.168.51.1 ifname: p2-route +]) + +OVS_TRAFFIC_VSWITCHD_STOP +AT_CLEANUP + +AT_SETUP([ovs-route - add system route - ipv4 via multiple ipv6 nexthop]) +AT_KEYWORDS([route]) +OVS_TRAFFIC_VSWITCHD_START() + +dnl Create tap ports. +AT_CHECK([ip tuntap add name p1-route mode tap]) +AT_CHECK([ip link set p1-route up]) +on_exit 'ip link del p1-route' +AT_CHECK([ip tuntap add name p2-route mode tap]) +AT_CHECK([ip link set p2-route up]) +on_exit 'ip link del p2-route' + +AT_CHECK([ip -6 addr add fc00:db8:dead::10/64 dev p1-route], [0], [stdout]) +AT_CHECK([ip -6 addr add fc00:db8:beef::10/64 dev p2-route], [0], [stdout]) +AT_CHECK([ip route add 172.16.42.0/24 nexthop via inet6 fc00:db8:dead::1 \ + dev p1-route nexthop via inet6 fc00:db8:beef::1 dev p2-route], + [0], [stdout]) + +dnl NOTE: At the time of this writing, it is expected that only the first route +dnl will be stored in ovs-router. +OVS_WAIT_UNTIL_EQUAL([ovs-appctl ovs/route/show | grep -E '172.16.42.0/24' | \ + sort], [dnl +Cached: 172.16.42.0/24 dev p1-route GW fc00:db8:dead::1 SRC fc00:db8:dead::10]) + +dnl Confirm that both nexthops are available when using the route-table library +dnl directly. +AT_CHECK([ovstest test-lib-route-table-dump | grep 172.16.42.0.*nexthop | sort], + [0], [dnl + 172.16.42.0/24 nexthop family: AF_INET6 addr: fc00:db8:beef::1 ifname: p2-route + 172.16.42.0/24 nexthop family: AF_INET6 addr: fc00:db8:dead::1 ifname: p1-route +]) + +OVS_TRAFFIC_VSWITCHD_STOP +AT_CLEANUP + +AT_SETUP([ovs-route - add system route with multiple nexthop - ipv6]) +AT_KEYWORDS([route]) +OVS_TRAFFIC_VSWITCHD_START() + +dnl Create tap ports. +AT_CHECK([ip tuntap add name p1-route mode tap]) +AT_CHECK([ip link set p1-route up]) +on_exit 'ip link del p1-route' +AT_CHECK([ip tuntap add name p2-route mode tap]) +AT_CHECK([ip link set p2-route up]) +on_exit 'ip link del p2-route' + +AT_CHECK([ip -6 addr add fc00:db8:dead::10/64 dev p1-route], [0], [stdout]) +AT_CHECK([ip -6 addr add fc00:db8:beef::10/64 dev p2-route], [0], [stdout]) +AT_CHECK([ip -6 route add fc00:db8:cafe::/64 nexthop via fc00:db8:dead::1 \ + dev p1-route nexthop via fc00:db8:beef::1 dev p2-route], + [0], [stdout]) + +dnl NOTE: At the time of this writing, it is expected that only the first route +dnl will be stored in ovs-router. +OVS_WAIT_UNTIL_EQUAL([ovs-appctl ovs/route/show | \ + grep -E 'fc00:db8:cafe::/64' | sort], [dnl +Cached: fc00:db8:cafe::/64 dev p1-route GW fc00:db8:dead::1 SRC fc00:db8:dead::10]) + +dnl Confirm that both nexthops are available when using the route-table library +dnl directly. +AT_CHECK([ovstest test-lib-route-table-dump | grep fc00:db8:cafe::.*nexthop | \ + sort], [0], [dnl + fc00:db8:cafe::/64 nexthop family: AF_INET6 addr: fc00:db8:beef::1 ifname: p2-route + fc00:db8:cafe::/64 nexthop family: AF_INET6 addr: fc00:db8:dead::1 ifname: p1-route +]) + +OVS_TRAFFIC_VSWITCHD_STOP +AT_CLEANUP + +AT_SETUP([route-table - exported functions work for netlink-notifier]) +AT_KEYWORDS([route]) + +dnl Create tap ports. +AT_CHECK([ip tuntap add name p1-route mode tap]) +AT_CHECK([ip link set p1-route up]) +on_exit 'ip link del p1-route' +AT_CHECK([ip tuntap add name p2-route mode tap]) +AT_CHECK([ip link set p2-route up]) +on_exit 'ip link del p2-route' + +AT_CHECK([ip -6 addr add fc00:db8:dead::10/64 dev p1-route], [0], [stdout]) +AT_CHECK([ip -6 addr add fc00:db8:beef::10/64 dev p2-route], [0], [stdout]) + +AT_CHECK([ovstest test-lib-route-table-monitor 'ip route add 172.16.42.0/24 \ + nexthop via inet6 fc00:db8:dead::1 dev p1-route \ + nexthop via inet6 fc00:db8:beef::1 dev p2-route' | \ + grep 172.16.42.0.*nexthop | sort], [0], [dnl + 172.16.42.0/24 nexthop family: AF_INET6 addr: fc00:db8:beef::1 ifname: p2-route + 172.16.42.0/24 nexthop family: AF_INET6 addr: fc00:db8:dead::1 ifname: p1-route +]) + +AT_CLEANUP + +AT_SETUP([route-table - route attributes]) +AT_KEYWORDS([route]) + +dnl Create tap ports. +AT_CHECK([ip tuntap add name p1-route mode tap]) +AT_CHECK([ip link set p1-route up]) +on_exit 'ip link del p1-route' + + +dnl Add ip address. +AT_CHECK([ip addr add 10.0.0.17/24 dev p1-route], [0], [stdout]) +AT_CHECK([ovstest test-lib-route-table-dump | \ + awk '/^10.0.0.17/{print$1" "$6" "$7}'], [0], [dnl +10.0.0.17/32 rtm_protocol: RTPROT_KERNEL +]) + +dnl Add route. +AT_CHECK([ip route add 192.168.10.12/32 dev p1-route via 10.0.0.18], [0], + [stdout]) +AT_CHECK([ovstest test-lib-route-table-dump | \ + awk '/^192.168.10.12/{print$1" "$17" "$18}'], [0], [dnl +192.168.10.12/32 rta_priority: 0 +]) +AT_CHECK([ovstest test-lib-route-table-dump | \ + awk '/^192.168.10.12/{print$1" "$6" "$7}'], [0], [dnl +192.168.10.12/32 rtm_protocol: RTPROT_BOOT +]) + +dnl Delete route. +AT_CHECK([ip route del 192.168.10.12/32 dev p1-route via 10.0.0.18], [0], + [stdout]) + +dnl Add route with priority. +AT_CHECK([ip route add 192.168.10.12/32 dev p1-route via 10.0.0.18 metric 42], + [0], [stdout]) +AT_CHECK([ovstest test-lib-route-table-dump | \ + awk '/^192.168.10.12/{print$1" "$17" "$18}'], [0], [dnl +192.168.10.12/32 rta_priority: 42 +]) +AT_CHECK([ovstest test-lib-route-table-dump | \ + awk '/^192.168.10.12/{print$1" "$6" "$7}'], [0], [dnl +192.168.10.12/32 rtm_protocol: RTPROT_BOOT +]) + +AT_CLEANUP diff --git a/tests/test-lib-route-table.c b/tests/test-lib-route-table.c new file mode 100644 index 000000000..61d97e06f --- /dev/null +++ b/tests/test-lib-route-table.c @@ -0,0 +1,149 @@ +/* + * Copyright (c) 2024 Canonical Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include <config.h> + +#undef NDEBUG + +#include <linux/rtnetlink.h> +#include <stdio.h> +#include <stdlib.h> + +#include "netlink-notifier.h" +#include "ovstest.h" +#include "packets.h" +#include "route-table.h" + +static char * +rt_prot_name(unsigned char p) +{ + /* We concentrate on the most used protocols, as they are the ones most + * likely to be defined in the build environment. */ + return p == RTPROT_UNSPEC ? "RTPROT_UNSPEC" : + p == RTPROT_REDIRECT ? "RTPROT_REDIRECT" : + p == RTPROT_KERNEL ? "RTPROT_KERNEL" : + p == RTPROT_BOOT ? "RTPROT_BOOT" : + p == RTPROT_STATIC ? "RTPROT_STATIC" : + p == RTPROT_RA ? "RTPROT_RA" : + p == RTPROT_DHCP ? "RTPROT_DHCP" : + p == RTPROT_BGP ? "RTPROT_BGP" : + "UNKNOWN"; +} + +static char * +rt_table_name(uint32_t id) +{ + static char tid[11] = ""; + + snprintf(tid, sizeof tid, "%"PRIu32, id); + + return id == RT_TABLE_UNSPEC ? "RT_TABLE_UNSPEC" : + id == RT_TABLE_COMPAT ? "RT_TABLE_COMPAT" : + id == RT_TABLE_DEFAULT ? "RT_TABLE_DEFAULT" : + id == RT_TABLE_MAIN ? "RT_TABLE_MAIN" : + id == RT_TABLE_LOCAL ? "RT_TABLE_LOCAL" : + tid; +} + +static void +test_lib_route_table_handle_msg(const struct route_table_msg *change, + void *data OVS_UNUSED) +{ + struct ds nexthop_addr = DS_EMPTY_INITIALIZER; + struct ds rta_prefsrc = DS_EMPTY_INITIALIZER; + const struct route_data *rd = &change->rd; + struct ds rta_dst = DS_EMPTY_INITIALIZER; + const struct route_data_nexthop *rdnh; + + ipv6_format_mapped(&change->rd.rta_prefsrc, &rta_prefsrc); + ipv6_format_mapped(&change->rd.rta_dst, &rta_dst); + + printf("%s/%u relevant: %d nlmsg_type: %d rtm_protocol: %s (%u) " + "rtn_local: %d rta_prefsrc: %s rta_mark: %"PRIu32" " + "rta_table_id: %s rta_priority: %"PRIu32"\n", + ds_cstr(&rta_dst), rd->rtm_dst_len, change->relevant, + change->nlmsg_type, rt_prot_name(rd->rtm_protocol), + rd->rtm_protocol, rd->rtn_local, ds_cstr(&rta_prefsrc), + rd->rta_mark, rt_table_name(rd->rta_table_id), rd->rta_priority); + + LIST_FOR_EACH (rdnh, nexthop_node, &rd->nexthops) { + ds_clear(&nexthop_addr); + ipv6_format_mapped(&rdnh->addr, &nexthop_addr); + printf(" %s/%u nexthop family: %s addr: %s ifname: %s\n", + ds_cstr(&rta_dst), rd->rtm_dst_len, + rdnh->family == AF_INET ? "AF_INET" : + rdnh->family == AF_INET6 ? "AF_INET6" : + "UNKNOWN", + ds_cstr(&nexthop_addr), + rdnh->ifname); + } + + ds_destroy(&nexthop_addr); + ds_destroy(&rta_prefsrc); + ds_destroy(&rta_dst); +} + +static void +test_lib_route_table_dump(int argc OVS_UNUSED, char *argv[] OVS_UNUSED) +{ + route_table_dump_one_table(RT_TABLE_UNSPEC, + test_lib_route_table_handle_msg, + NULL); +} + +static void +test_lib_route_table_change(struct route_table_msg *change, + void *aux OVS_UNUSED) +{ + test_lib_route_table_handle_msg(change, NULL); + route_data_destroy(&change->rd); +} + +static void +test_lib_route_table_monitor(int argc, char *argv[]) +{ + static struct nln_notifier *route6_notifier OVS_UNUSED; + static struct nln_notifier *route_notifier OVS_UNUSED; + static struct route_table_msg rtmsg; + static struct nln *nln OVS_UNUSED; + const char *cmd = argv[1]; + + if (argc != 2) { + printf("usage: ovstest %s 'ip route add ...'\n", argv[0]); + exit(EXIT_FAILURE); + } + + nln = nln_create(NETLINK_ROUTE, route_table_parse, &rtmsg); + + route_notifier = + nln_notifier_create(nln, RTNLGRP_IPV4_ROUTE, + (nln_notify_func *) test_lib_route_table_change, + NULL); + route6_notifier = + nln_notifier_create(nln, RTNLGRP_IPV6_ROUTE, + (nln_notify_func *) test_lib_route_table_change, + NULL); + nln_run(nln); + nln_wait(nln); + int rc = system(cmd); + if (rc) { + exit(rc); + } + nln_run(nln); +} + +OVSTEST_REGISTER("test-lib-route-table-monitor", test_lib_route_table_monitor); +OVSTEST_REGISTER("test-lib-route-table-dump", test_lib_route_table_dump); -- 2.47.1 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
