On Wed, Jan 09, 2019 at 12:43:15PM -0500, Laine Stump wrote:
When checking the setting of accept_ra, we have assumed that all
routes have a single nexthop, so the interface of the route would be
in the RTA_OIF attribute of the netlink RTM_NEWROUTE message. But
multipath routes don't have an RTA_OIF; instead, they have an
RTA_MULTIPATH attribute, which is an array of rtnexthop, with each
rtnexthop having an interface. This patch adds a loop to look at the
setting of accept_ra of the interface for every rtnexthop in the
array.
Signed-off-by: Laine Stump <laine(a)laine.org>
---
src/util/virnetdevip.c | 37 +++++++++++++++++++++++++++++++++++++
1 file changed, 37 insertions(+)
diff --git a/src/util/virnetdevip.c b/src/util/virnetdevip.c
index c032ecacfc..e0965bcc1d 100644
--- a/src/util/virnetdevip.c
+++ b/src/util/virnetdevip.c
@@ -566,6 +566,43 @@ virNetDevIPCheckIPv6ForwardingCallback(struct nlmsghdr *resp,
return 0;
}
+ /* if no RTA_OIF was found, see if this is a multipath route (one
+ * which has an array of nexthops, each with its own interface)
+ */
+
+ rta_attr = (struct rtattr *)nlmsg_find_attr(resp, sizeof(struct rtmsg),
RTA_MULTIPATH);
+ if (rta_attr) {
+ /* The data of the attribute is an array of rtnexthop */
+ struct rtnexthop *nh = RTA_DATA(rta_attr);
+ size_t len = RTA_PAYLOAD(rta_attr);
+
+ /* validate the attribute array length */
+ len = MIN(len, ((char *)resp + NLMSG_PAYLOAD(resp, 0) - (char *)rta_attr));
I was fine until ... ^here,
you lost me there, can you elaborate on that witchcraft?
+
+ while (len >= sizeof(*nh) && len >= nh->rtnh_len) {
+ /* check accept_ra for the interface of each nexthop */
+
+ ifname = virNetDevGetName(nh->rtnh_ifindex);
+
+ if (ifname)
+ accept_ra = virNetDevIPGetAcceptRA(ifname);
+
+ VIR_DEBUG("Checking multipath route nexthop device %s (%d), accept_ra:
%d",
+ ifname, nh->rtnh_ifindex, accept_ra);
+
+ if (!ifname ||
+ (accept_ra != 2 && virNetDevIPCheckIPv6ForwardingAddIF(data,
&ifname) < 0)) {
+ return -1;
+ }
+
+ VIR_FREE(ifname); /* in case it wasn't added to the array */
I'd drop ^this commentary, otherwise it just
leaves the reader wondering what happens with free() in the other case, IOW
VIR_APPEND_ELEMENT happens :)
Erik
+ data->hasRARoutes = true;
+
+ len -= NLMSG_ALIGN(nh->rtnh_len);
+ nh = RTNH_NEXT(nh);
+ }
+ }
+