On Fri, Jan 3, 2025 at 6:16 PM Eelco Chaudron <[email protected]> wrote: > > 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’?
The sentence was intended to refer to the "exported API" rather than interface names and routes, in retrospect it is not clear so I'll reword it. > 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! Done, > > { > > 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. Done. > > + if (!attrs[RTA_MULTIPATH]) { > > Should the earlier check be updated so that only one of the three attributes > is present? Done. > > + 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. Done. > > + 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. Done. > > + 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: I incorporated the below diff as is, thanks! > 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 > ]) > -- Frode Nordahl > > 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. Done. > > + > > + 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
