On 04/09/17(Mon) 13:10, Gerhard Roth wrote:
> Hi,
>
> I noticed a problem with the routing table that is easy to reproduce: put
> multiple IPs on the same carp interface:
Great bug analysis, however returning EAGAIN for every route update is a
no go if you have a big routing table like BGP full feeds.
We should return EAGAIN only if the multipath list contains multiple
elements.
But since we're now returning EAGAIN much often we want to skip route
entries that have already been borough UP/taken down.
Diff below does that, does it work for you? Do you mind adding a
regression test?
Index: net/route.c
===================================================================
RCS file: /cvs/src/sys/net/route.c,v
retrieving revision 1.366
diff -u -p -r1.366 route.c
--- net/route.c 11 Aug 2017 21:24:19 -0000 1.366
+++ net/route.c 4 Sep 2017 12:08:16 -0000
@@ -1550,6 +1550,7 @@ rt_if_linkstate_change(struct rtentry *r
{
struct ifnet *ifp = arg;
struct sockaddr_in6 sa_mask;
+ int error;
if (rt->rt_ifidx != ifp->if_index)
return (0);
@@ -1561,38 +1562,37 @@ rt_if_linkstate_change(struct rtentry *r
}
if (LINK_STATE_IS_UP(ifp->if_link_state) && ifp->if_flags & IFF_UP) {
- if (!(rt->rt_flags & RTF_UP)) {
- /* bring route up */
- rt->rt_flags |= RTF_UP;
- rtable_mpath_reprio(id, rt_key(rt),
- rt_plen2mask(rt, &sa_mask),
- rt->rt_priority & RTP_MASK, rt);
- }
+ if (ISSET(rt->rt_flags, RTF_UP))
+ return (0);
+
+ /* bring route up */
+ rt->rt_flags |= RTF_UP;
+ error = rtable_mpath_reprio(id, rt_key(rt),
+ rt_plen2mask(rt, &sa_mask), rt->rt_priority & RTP_MASK, rt);
} else {
- if (rt->rt_flags & RTF_UP) {
- /*
- * Remove redirected and cloned routes (mainly ARP)
- * from down interfaces so we have a chance to get
- * new routes from a better source.
- */
- if (ISSET(rt->rt_flags, RTF_CLONED|RTF_DYNAMIC) &&
- !ISSET(rt->rt_flags, RTF_CACHED|RTF_BFD)) {
- int error;
-
- if ((error = rtdeletemsg(rt, ifp, id)))
- return (error);
- return (EAGAIN);
- }
- /* take route down */
- rt->rt_flags &= ~RTF_UP;
- rtable_mpath_reprio(id, rt_key(rt),
- rt_plen2mask(rt, &sa_mask),
- rt->rt_priority | RTP_DOWN, rt);
+ /*
+ * Remove redirected and cloned routes (mainly ARP)
+ * from down interfaces so we have a chance to get
+ * new routes from a better source.
+ */
+ if (ISSET(rt->rt_flags, RTF_CLONED|RTF_DYNAMIC) &&
+ !ISSET(rt->rt_flags, RTF_CACHED|RTF_BFD)) {
+ if ((error = rtdeletemsg(rt, ifp, id)))
+ return (error);
+ return (EAGAIN);
}
+
+ if (!ISSET(rt->rt_flags, RTF_UP))
+ return (0);
+
+ /* take route down */
+ rt->rt_flags &= ~RTF_UP;
+ error = rtable_mpath_reprio(id, rt_key(rt),
+ rt_plen2mask(rt, &sa_mask), rt->rt_priority | RTP_DOWN, rt);
}
if_group_routechange(rt_key(rt), rt_plen2mask(rt, &sa_mask));
- return (0);
+ return (error);
}
struct sockaddr *
Index: net/rtable.c
===================================================================
RCS file: /cvs/src/sys/net/rtable.c,v
retrieving revision 1.61
diff -u -p -r1.61 rtable.c
--- net/rtable.c 30 Jul 2017 18:18:08 -0000 1.61
+++ net/rtable.c 4 Sep 2017 12:02:00 -0000
@@ -751,6 +751,7 @@ rtable_mpath_reprio(unsigned int rtablei
rt->rt_priority = prio;
rtable_mpath_insert(an, rt);
rtfree(rt);
+ error = EAGAIN;
}
rw_exit_write(&ar->ar_lock);
