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
route table API is added in this patch.

Acked-by: Eelco Chaudron <[email protected]>
Co-Authored-by: Felix Huettner <[email protected]>
Signed-off-by: Felix Huettner <[email protected]>
Signed-off-by: Frode Nordahl <[email protected]>
Signed-off-by: Ilya Maximets <[email protected]>
  • Loading branch information
@fnordahl @felixhuettner @igsilya
2 people authored and igsilya committed Jan 16, 2025
1 parent 50f7d20 commit 91fc511
Show file tree
Hide file tree
Showing 5 changed files with 394 additions and 11 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
66 changes: 56 additions & 10 deletions lib/route-table.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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[] = {
Expand All @@ -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)];
Expand All @@ -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;

Expand All @@ -294,8 +301,14 @@ 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) {
VLOG_DBG_RL(&rl, "unexpected RTA_OIF attribute while parsing "
"nested RTA_MULTIPATH attributes");
goto error_out;
}
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 @@ -386,11 +399,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;
Expand Down Expand Up @@ -424,7 +470,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
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
184 changes: 184 additions & 0 deletions tests/system-route.at
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand All @@ -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.
Expand All @@ -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
Loading

0 comments on commit 91fc511

Please sign in to comment.