On 18 Dec 2024, at 20:12, Frode Nordahl wrote: > 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 > exported interfaces is added in this patch.
Should ‘interfaces’ be ‘routes’? A test program, run as part of the system tests to exercise the exported routes, is added in this patch. Some more comments inline. > 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 | 55 ++++++++++-- > tests/automake.mk | 1 + > tests/system-route.at | 158 +++++++++++++++++++++++++++++++++++ > tests/test-lib-route-table.c | 149 +++++++++++++++++++++++++++++++++ > 5 files changed, 358 insertions(+), 10 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 36d5c8620..544b7fa07 100644 > --- a/lib/route-table.c > +++ b/lib/route-table.c > @@ -222,8 +222,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 nlmsghdr *nlmsg, const struct rtmsg *rtm, > + const struct rtnexthop *rtnh, > + struct route_table_msg *change) Mega nit: Consider sticking to one argument per line for these longer ones. It makes it easier on the eyes (at least for me). Totally fine to ignore this—just a personal preference! > { > struct route_data_nexthop *rdnh = NULL; > bool parsed, ipv4 = false; > @@ -237,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[] = { > @@ -248,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)]; > @@ -271,9 +274,11 @@ route_table_parse__(struct ofpbuf *buf, size_t ofs, > > /* ovs_list_init / ovs_list_insert does not allocate any memory */ > 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; Add a new line here for clarity. > + if (!attrs[RTA_MULTIPATH]) { Should the earlier check be updated so that only one of the three attributes is present? > + rdnh->family = rtm->rtm_family; > + ovs_list_insert(&change->rd.nexthops, &rdnh->nexthop_node); > + } > > change->relevant = true; > > @@ -295,8 +300,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,6 +390,37 @@ route_table_parse__(struct ofpbuf *buf, size_t ofs, > goto error_out; > } > } > + if (attrs[RTA_MULTIPATH]) { We also need to update my earlier comment, i.e., if there is no RTA_MULTPATH, RTA_GATEWAY, or RTA_VIA attribute we should error out. > + 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); Add a new line here for clarity. > + 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); > + } > + } > + /* Add any additional RTA attribute processing before RTA_MULTIPATH. > */ > } else { > VLOG_DBG_RL(&rl, "received unparseable rtnetlink route message"); > goto error_out; > @@ -417,7 +454,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 > @@ -448,7 +485,7 @@ route_table_handle_msg(const struct route_table_msg > *change, > void *aux OVS_UNUSED) > { > if (change->relevant && change->nlmsg_type == RTM_NEWROUTE > - && ovs_list_is_singleton(&change->rd.nexthops)) { > + && !ovs_list_is_empty(&change->rd.nexthops)) { > const struct route_data *rd = &change->rd; > const struct route_data_nexthop *rdnh; > > 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 c9fed2fc6..0471ad684 100644 > --- a/tests/system-route.at > +++ b/tests/system-route.at > @@ -109,6 +109,9 @@ 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]) > + > dnl Add a route to a custom routing table and check that OVS doesn't cache > it. > AT_CHECK([ovs-appctl revalidator/wait]) > AT_CHECK([ovs-appctl coverage/read-counter route_table_dump > expout], [0]) > @@ -123,6 +126,9 @@ 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([ovs-appctl coverage/read-counter route_table_dump], [0], [expout]) > +AT_CHECK([ovstest test-lib-route-table-dump | awk '/rta_table_id: > 42/{print$1" "$15" "$16}'], [0], [dnl > +10.0.0.19/32 rta_table_id: 42 > +]) > > dnl Delete a route from the main table and check that OVS removes the route > dnl from the cache. > @@ -149,3 +155,155 @@ 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 The unit test looks good to me! However, we could try to make it a bit more within 80 characters wide. For example: diff --git a/tests/system-route.at b/tests/system-route.at index 0471ad684..ae1a95ba7 100644 --- a/tests/system-route.at +++ b/tests/system-route.at @@ -170,16 +170,19 @@ 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]) +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 +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 +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 ]) @@ -201,16 +204,20 @@ 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]) +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 +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 +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 ]) @@ -232,16 +239,20 @@ 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]) +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 +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 +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 ]) @@ -263,7 +274,10 @@ 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 +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 ]) @@ -281,28 +295,36 @@ 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 +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 +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 +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]) +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 +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 +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 ]) > diff --git a/tests/test-lib-route-table.c b/tests/test-lib-route-table.c > new file mode 100644 > index 000000000..a6c21b314 > --- /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; > + const char *cmd = argv[1]; > + static struct nln *nln OVS_UNUSED; Swap last to definitions. > + > + 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); _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
