Skip to content

Commit

Permalink
route-table: Support parsing multipath routes.
Browse files Browse the repository at this point in the history 
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.

Co-Authored-by: Felix Huettner <[email protected]>
Signed-off-by: Felix Huettner <[email protected]>
Signed-off-by: Frode Nordahl <[email protected]>
Signed-off-by: 0-day Robot <[email protected]>
  • Loading branch information
@fnordahl @felixhuettner @ovsrobot
2 people authored and ovsrobot committed Dec 18, 2024
1 parent 650789c commit 3370de4
Show file tree
Hide file tree
Showing 5 changed files with 358 additions and 10 deletions.
5 changes: 4 additions & 1 deletion Makefile.am
View file
Original file line number Diff line number Diff line change
Expand Up @@ -339,14 +339,17 @@ 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); \
if test -e .git && (git --version) >/dev/null 2>&1 && \
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"; \
Expand Down
55 changes: 46 additions & 9 deletions lib/route-table.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
struct route_data_nexthop *rdnh = NULL;
bool parsed, ipv4 = false;
Expand All @@ -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[] = {
Expand All @@ -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)];
Expand All @@ -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;
if (!attrs[RTA_MULTIPATH]) {
rdnh->family = rtm->rtm_family;
ovs_list_insert(&change->rd.nexthops, &rdnh->nexthop_node);
}

change->relevant = true;

Expand All @@ -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;
Expand Down Expand Up @@ -384,6 +390,37 @@ route_table_parse__(struct ofpbuf *buf, size_t ofs,
goto error_out;
}
}
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);
}
}
/* Add any additional RTA attribute processing before RTA_MULTIPATH. */
} else {
VLOG_DBG_RL(&rl, "received unparseable rtnetlink route message");
goto error_out;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;

Expand Down
1 change: 1 addition & 0 deletions tests/automake.mk
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
158 changes: 158 additions & 0 deletions tests/system-route.at
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand All @@ -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.
Expand All @@ -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
Loading

0 comments on commit 3370de4

Please sign in to comment.