On 09/06/19(Sun) 15:41, Martin Pieuchot wrote:
> [...]
> Another way to prevent stack exhaustion would be to return a reference
> to any `rt' that needs to be deleted instead of deleting it in place.
Diff below does that by adding a new `prt' argument to rtable_walk().
I'm willing to create a manual for rtable_walk(9) if you agree with the
direction below.
The idea is to break the iteration as soon as we encounter a route entry
that we want to delete. For this purpose I changed the 3 walkers to make
them return EEXIST. This is just a convention, but coherency helps ;)
So with this approach the calls to rtdeletemsg() and rtrequest_delete()
are now in the same function as rtable_walk(). I decided to refcount
`rt'. This is not strictly necessary as long as the NET_LOCK() is not
released between rtable_walk() and rtdeletemsg(). But we'll need it as
soon as we make the rtentry layer NET_LOCK() free :)
Obviously since now that we're breaking the iteration we have to roll
the loop ourself. In all the error handling, EAFNOSUPPORT is the only
value which is returned by the rtable_*() layer.
I haven't converted mroute/mroute6 walkers yet. I'm waiting for your
comments.
rt_ifa_purge() also contains an improvement, use the AF of the `ifa'
instead of iterating over all existing ones :)
While here I changed some function signatures to return the possible
error. I'm not checking them for the moment.
Index: net/if_spppsubr.c
===================================================================
RCS file: /cvs/src/sys/net/if_spppsubr.c,v
retrieving revision 1.174
diff -u -p -r1.174 if_spppsubr.c
--- net/if_spppsubr.c 19 Feb 2018 08:59:52 -0000 1.174
+++ net/if_spppsubr.c 11 Jun 2019 15:30:10 -0000
@@ -4167,7 +4167,7 @@ sppp_update_gw(struct ifnet *ifp)
/* update routing table */
for (tid = 0; tid <= RT_TABLEID_MAX; tid++) {
- rtable_walk(tid, AF_INET, sppp_update_gw_walker, ifp);
+ rtable_walk(tid, AF_INET, NULL, sppp_update_gw_walker, ifp);
}
}
Index: net/route.c
===================================================================
RCS file: /cvs/src/sys/net/route.c,v
retrieving revision 1.384
diff -u -p -r1.384 route.c
--- net/route.c 11 May 2019 16:47:02 -0000 1.384
+++ net/route.c 11 Jun 2019 16:24:39 -0000
@@ -156,7 +156,7 @@ void rt_timer_init(void);
int rt_setgwroute(struct rtentry *, u_int);
void rt_putgwroute(struct rtentry *);
int rtflushclone1(struct rtentry *, void *, u_int);
-void rtflushclone(unsigned int, struct rtentry *);
+int rtflushclone(struct ifnet *ifp, struct rtentry *, unsigned int);
int rt_ifa_purge_walker(struct rtentry *, void *, unsigned int);
struct rtentry *rt_match(struct sockaddr *, uint32_t *, int, unsigned int);
int rt_clone(struct rtentry **, struct sockaddr *, unsigned int);
@@ -701,7 +701,6 @@ rtflushclone1(struct rtentry *rt, void *
{
struct rtentry *cloningrt = arg;
struct ifnet *ifp;
- int error;
if (!ISSET(rt->rt_flags, RTF_CLONED))
return 0;
@@ -721,23 +720,35 @@ rtflushclone1(struct rtentry *rt, void *
if (ifp == NULL)
return 0;
- error = rtdeletemsg(rt, ifp, id);
- if (error == 0)
- error = EAGAIN;
-
if_put(ifp);
- return error;
+ return EEXIST;
}
-void
-rtflushclone(unsigned int rtableid, struct rtentry *parent)
+int
+rtflushclone(struct ifnet *ifp, struct rtentry *parent, unsigned int rtableid)
{
+ struct rtentry *rt = NULL;
+ int error;
#ifdef DIAGNOSTIC
if (!parent || (parent->rt_flags & RTF_CLONING) == 0)
panic("rtflushclone: called with a non-cloning route");
#endif
- rtable_walk(rtableid, rt_key(parent)->sa_family, rtflushclone1, parent);
+
+ do {
+ error = rtable_walk(rtableid, rt_key(parent)->sa_family, &rt,
+ rtflushclone1, parent);
+ if (rt != NULL && error == EEXIST) {
+ error = rtdeletemsg(rt, ifp, rtableid);
+ if (error == 0)
+ error = EAGAIN;
+ }
+ rtfree(rt);
+ rt = NULL;
+ } while (error == EAGAIN);
+
+ return error;
+
}
int
@@ -779,7 +790,7 @@ rtrequest_delete(struct rt_addrinfo *inf
/* Clean up any cloned children. */
if (ISSET(rt->rt_flags, RTF_CLONING))
- rtflushclone(tableid, rt);
+ rtflushclone(ifp, rt, tableid);
rtfree(rt->rt_parent);
rt->rt_parent = NULL;
@@ -1241,12 +1252,13 @@ rt_ifa_dellocal(struct ifaddr *ifa)
/*
* Remove all addresses attached to ``ifa''.
*/
-void
+int
rt_ifa_purge(struct ifaddr *ifa)
{
struct ifnet *ifp = ifa->ifa_ifp;
+ struct rtentry *rt = NULL;
unsigned int rtableid;
- int i;
+ int error, af = ifa->ifa_addr->sa_family;
KASSERT(ifp != NULL);
@@ -1254,27 +1266,38 @@ rt_ifa_purge(struct ifaddr *ifa)
/* skip rtables that are not in the rdomain of the ifp */
if (rtable_l2(rtableid) != ifp->if_rdomain)
continue;
- for (i = 1; i <= AF_MAX; i++) {
- rtable_walk(rtableid, i, rt_ifa_purge_walker, ifa);
- }
+
+ do {
+ error = rtable_walk(rtableid, af, &rt,
+ rt_ifa_purge_walker, ifa);
+ if (rt != NULL && error == EEXIST) {
+ error = rtdeletemsg(rt, ifp, rtableid);
+ if (error == 0)
+ error = EAGAIN;
+ }
+ rtfree(rt);
+ rt = NULL;
+ } while (error == EAGAIN);
+
+ if (error == EAFNOSUPPORT)
+ error = 0;
+
+ if (error)
+ break;
}
+
+ return error;
}
int
rt_ifa_purge_walker(struct rtentry *rt, void *vifa, unsigned int rtableid)
{
struct ifaddr *ifa = vifa;
- struct ifnet *ifp = ifa->ifa_ifp;
- int error;
- if (rt->rt_ifa != ifa)
- return (0);
-
- if ((error = rtdeletemsg(rt, ifp, rtableid))) {
- return (error);
- }
+ if (rt->rt_ifa == ifa)
+ return EEXIST;
- return (EAGAIN);
+ return 0;
}
/*
@@ -1596,23 +1619,42 @@ rtlabel_unref(u_int16_t id)
}
}
-void
+int
rt_if_track(struct ifnet *ifp)
{
- int i;
- u_int tid;
+ unsigned int rtableid;
+ struct rtentry *rt = NULL;
+ int i, error;
- for (tid = 0; tid < rtmap_limit; tid++) {
+ for (rtableid = 0; rtableid < rtmap_limit; rtableid++) {
/* skip rtables that are not in the rdomain of the ifp */
- if (rtable_l2(tid) != ifp->if_rdomain)
+ if (rtable_l2(rtableid) != ifp->if_rdomain)
continue;
for (i = 1; i <= AF_MAX; i++) {
- if (!rtable_mpath_capable(tid, i))
+ if (!rtable_mpath_capable(rtableid, i))
continue;
- rtable_walk(tid, i, rt_if_linkstate_change, ifp);
+ do {
+ error = rtable_walk(rtableid, i, &rt,
+ rt_if_linkstate_change, ifp);
+ if (rt != NULL && error == EEXIST) {
+ error = rtdeletemsg(rt, ifp, rtableid);
+ if (error == 0)
+ error = EAGAIN;
+ }
+ rtfree(rt);
+ rt = NULL;
+ } while (error == EAGAIN);
+
+ if (error == EAFNOSUPPORT)
+ error = 0;
+
+ if (error)
+ break;
}
}
+
+ return (error);
}
int
@@ -1647,9 +1689,7 @@ rt_if_linkstate_change(struct rtentry *r
*/
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);
+ return (EEXIST);
}
if (!ISSET(rt->rt_flags, RTF_UP))
@@ -1780,7 +1820,7 @@ int
db_show_arptab(void)
{
db_printf("Route tree for AF_INET\n");
- rtable_walk(0, AF_INET, db_show_rtentry, NULL);
+ rtable_walk(0, AF_INET, NULL, db_show_rtentry, NULL);
return (0);
}
#endif /* DDB */
Index: net/route.h
===================================================================
RCS file: /cvs/src/sys/net/route.h,v
retrieving revision 1.175
diff -u -p -r1.175 route.h
--- net/route.h 28 Apr 2019 17:59:51 -0000 1.175
+++ net/route.h 11 Jun 2019 15:30:10 -0000
@@ -451,7 +451,7 @@ void rtfree(struct rtentry *);
int rt_ifa_add(struct ifaddr *, int, struct sockaddr *, unsigned int);
int rt_ifa_del(struct ifaddr *, int, struct sockaddr *, unsigned int);
-void rt_ifa_purge(struct ifaddr *);
+int rt_ifa_purge(struct ifaddr *);
int rt_ifa_addlocal(struct ifaddr *);
int rt_ifa_dellocal(struct ifaddr *);
void rtredirect(struct sockaddr *, struct sockaddr *, struct sockaddr *,
struct rtentry **, unsigned int);
@@ -459,7 +459,7 @@ int rtrequest(int, struct rt_addrinfo *
u_int);
int rtrequest_delete(struct rt_addrinfo *, u_int8_t, struct ifnet *,
struct rtentry **, u_int);
-void rt_if_track(struct ifnet *);
+int rt_if_track(struct ifnet *);
int rt_if_linkstate_change(struct rtentry *, void *, u_int);
int rtdeletemsg(struct rtentry *, struct ifnet *, u_int);
#endif /* _KERNEL */
Index: net/rtable.c
===================================================================
RCS file: /cvs/src/sys/net/rtable.c,v
retrieving revision 1.68
diff -u -p -r1.68 rtable.c
--- net/rtable.c 5 Mar 2019 19:07:56 -0000 1.68
+++ net/rtable.c 11 Jun 2019 16:01:39 -0000
@@ -664,6 +664,7 @@ leave:
struct rtable_walk_cookie {
int (*rwc_func)(struct rtentry *, void *, unsigned int);
void *rwc_arg;
+ struct rtentry **rwc_prt;
unsigned int rwc_rid;
};
@@ -679,16 +680,21 @@ rtable_walk_helper(struct art_node *an,
int error = 0;
SRPL_FOREACH(rt, &sr, &an->an_rtlist, rt_next) {
- if ((error = (*rwc->rwc_func)(rt, rwc->rwc_arg, rwc->rwc_rid)))
+ error = (*rwc->rwc_func)(rt, rwc->rwc_arg, rwc->rwc_rid);
+ if (error != 0)
break;
}
+ if (rwc->rwc_prt != NULL && rt != NULL) {
+ rtref(rt);
+ *rwc->rwc_prt = rt;
+ }
SRPL_LEAVE(&sr);
return (error);
}
int
-rtable_walk(unsigned int rtableid, sa_family_t af,
+rtable_walk(unsigned int rtableid, sa_family_t af, struct rtentry **prt,
int (*func)(struct rtentry *, void *, unsigned int), void *arg)
{
struct art_root *ar;
@@ -701,10 +707,10 @@ rtable_walk(unsigned int rtableid, sa_fa
rwc.rwc_func = func;
rwc.rwc_arg = arg;
+ rwc.rwc_prt = prt;
rwc.rwc_rid = rtableid;
- while ((error = art_walk(ar, rtable_walk_helper, &rwc)) == EAGAIN)
- continue;
+ error = art_walk(ar, rtable_walk_helper, &rwc);
return (error);
}
Index: net/rtable.h
===================================================================
RCS file: /cvs/src/sys/net/rtable.h,v
retrieving revision 1.23
diff -u -p -r1.23 rtable.h
--- net/rtable.h 28 Apr 2019 17:59:51 -0000 1.23
+++ net/rtable.h 11 Jun 2019 15:30:10 -0000
@@ -48,7 +48,7 @@ int rtable_insert(unsigned int, struct
struct rtentry *);
int rtable_delete(unsigned int, struct sockaddr *,
struct sockaddr *, struct rtentry *);
-int rtable_walk(unsigned int, sa_family_t,
+int rtable_walk(unsigned int, sa_family_t, struct rtentry **,
int (*)(struct rtentry *, void *, unsigned int), void *);
int rtable_mpath_capable(unsigned int, sa_family_t);
Index: net/rtsock.c
===================================================================
RCS file: /cvs/src/sys/net/rtsock.c,v
retrieving revision 1.287
diff -u -p -r1.287 rtsock.c
--- net/rtsock.c 5 Jun 2019 12:53:43 -0000 1.287
+++ net/rtsock.c 11 Jun 2019 15:30:10 -0000
@@ -931,7 +931,7 @@ rtm_output(struct rt_msghdr *rtm, struct
NET_LOCK();
ifp->if_rtrequest(ifp, RTM_INVALIDATE, rt);
/* Reset the MTU of the gateway route. */
- rtable_walk(tableid, rt_key(rt)->sa_family,
+ rtable_walk(tableid, rt_key(rt)->sa_family, NULL,
route_cleargateway, rt);
NET_UNLOCK();
if_put(ifp);
@@ -1911,7 +1911,8 @@ sysctl_rtable(int *name, u_int namelen,
if (af != 0 && af != i)
continue;
- error = rtable_walk(tableid, i, sysctl_dumpentry, &w);
+ error = rtable_walk(tableid, i, NULL, sysctl_dumpentry,
+ &w);
if (error == EAFNOSUPPORT)
error = 0;
if (error)
Index: netinet/ip_mroute.c
===================================================================
RCS file: /cvs/src/sys/netinet/ip_mroute.c,v
retrieving revision 1.126
diff -u -p -r1.126 ip_mroute.c
--- netinet/ip_mroute.c 4 Jun 2019 16:11:13 -0000 1.126
+++ netinet/ip_mroute.c 11 Jun 2019 15:30:10 -0000
@@ -476,8 +476,10 @@ mrt_sysctl_mfc(void *oldp, size_t *oldle
msa.msa_len = *oldlenp;
msa.msa_needed = 0;
- for (rtableid = 0; rtableid <= RT_TABLEID_MAX; rtableid++)
- rtable_walk(rtableid, AF_INET, mrt_rtwalk_mfcsysctl, &msa);
+ for (rtableid = 0; rtableid <= RT_TABLEID_MAX; rtableid++) {
+ rtable_walk(rtableid, AF_INET, NULL, mrt_rtwalk_mfcsysctl,
+ &msa);
+ }
if (msa.msa_minfos != NULL && msa.msa_needed > 0 &&
(error = copyout(msa.msa_minfos, oldp, msa.msa_needed)) != 0) {
@@ -549,7 +551,7 @@ ip_mrouter_done(struct socket *so)
NET_ASSERT_LOCKED();
/* Delete all remaining installed multicast routes. */
- rtable_walk(rtableid, AF_INET, mrouter_rtwalk_delete, NULL);
+ rtable_walk(rtableid, AF_INET, NULL, mrouter_rtwalk_delete, NULL);
TAILQ_FOREACH(ifp, &ifnet, if_list) {
if (ifp->if_rdomain != rtableid)
Index: netinet6/ip6_mroute.c
===================================================================
RCS file: /cvs/src/sys/netinet6/ip6_mroute.c,v
retrieving revision 1.119
diff -u -p -r1.119 ip6_mroute.c
--- netinet6/ip6_mroute.c 4 Jun 2019 16:11:13 -0000 1.119
+++ netinet6/ip6_mroute.c 11 Jun 2019 15:30:10 -0000
@@ -454,8 +454,10 @@ mrt6_sysctl_mfc(void *oldp, size_t *oldl
msa.ms6a_len = *oldlenp;
msa.ms6a_needed = 0;
- for (rtableid = 0; rtableid <= RT_TABLEID_MAX; rtableid++)
- rtable_walk(rtableid, AF_INET6, mrt6_rtwalk_mf6csysctl, &msa);
+ for (rtableid = 0; rtableid <= RT_TABLEID_MAX; rtableid++) {
+ rtable_walk(rtableid, AF_INET6, NULL, mrt6_rtwalk_mf6csysctl,
+ &msa);
+ }
if (msa.ms6a_minfos != NULL && msa.ms6a_needed > 0 &&
(error = copyout(msa.ms6a_minfos, oldp, msa.ms6a_needed)) != 0) {
@@ -523,7 +525,7 @@ ip6_mrouter_done(struct socket *so)
NET_ASSERT_LOCKED();
/* Delete all remaining installed multicast routes. */
- rtable_walk(rtableid, AF_INET6, mrouter6_rtwalk_delete, NULL);
+ rtable_walk(rtableid, AF_INET6, NULL, mrouter6_rtwalk_delete, NULL);
/* Unregister all interfaces in the domain. */
TAILQ_FOREACH(ifp, &ifnet, if_list) {
Index: netinet6/nd6.h
===================================================================
RCS file: /cvs/src/sys/netinet6/nd6.h,v
retrieving revision 1.74
diff -u -p -r1.74 nd6.h
--- netinet6/nd6.h 27 Nov 2017 15:41:30 -0000 1.74
+++ netinet6/nd6.h 11 Jun 2019 15:30:10 -0000
@@ -193,7 +193,7 @@ void nd6_dad_stop(struct ifaddr *);
void nd6_rtr_cache(struct mbuf *, int, int, int);
int in6_ifdel(struct ifnet *, struct in6_addr *);
-void rt6_flush(struct in6_addr *, struct ifnet *);
+int rt6_flush(struct in6_addr *, struct ifnet *);
void nd6_expire_timer_update(struct in6_ifaddr *);
#endif /* _KERNEL */
Index: netinet6/nd6_rtr.c
===================================================================
RCS file: /cvs/src/sys/netinet6/nd6_rtr.c,v
retrieving revision 1.166
diff -u -p -r1.166 nd6_rtr.c
--- netinet6/nd6_rtr.c 23 Jan 2018 22:06:42 -0000 1.166
+++ netinet6/nd6_rtr.c 11 Jun 2019 16:26:43 -0000
@@ -173,28 +173,48 @@ nd6_rtr_cache(struct mbuf *m, int off, i
* it shouldn't be called when acting as a router.
* The gateway must already contain KAME's hack for link-local scope.
*/
-void
+int
rt6_flush(struct in6_addr *gateway, struct ifnet *ifp)
{
+ struct rt_addrinfo info;
+ struct sockaddr_in6 sa_mask;
+ struct rtentry *rt = NULL;
+ int error;
+
NET_ASSERT_LOCKED();
/* We'll care only link-local addresses */
if (!IN6_IS_ADDR_LINKLOCAL(gateway))
- return;
+ return (0);
KASSERT(gateway->s6_addr16[1] != 0);
- rtable_walk(ifp->if_rdomain, AF_INET6, rt6_deleteroute, gateway);
+ do {
+ error = rtable_walk(ifp->if_rdomain, AF_INET6, &rt,
+ rt6_deleteroute, gateway);
+ if (rt != NULL && error == EEXIST) {
+ memset(&info, 0, sizeof(info));
+ info.rti_flags = rt->rt_flags;
+ info.rti_info[RTAX_DST] = rt_key(rt);
+ info.rti_info[RTAX_GATEWAY] = rt->rt_gateway;
+ info.rti_info[RTAX_NETMASK] = rt_plen2mask(rt,
+ &sa_mask);
+ error = rtrequest_delete(&info, RTP_ANY, ifp, NULL,
+ ifp->if_rdomain);
+ if (error == 0)
+ error = EAGAIN;
+ }
+ rtfree(rt);
+ rt = NULL;
+ } while (error == EAGAIN);
+
+ return (error);
}
int
rt6_deleteroute(struct rtentry *rt, void *arg, unsigned int id)
{
- struct ifnet *ifp;
- struct rt_addrinfo info;
struct in6_addr *gate = (struct in6_addr *)arg;
- struct sockaddr_in6 sa_mask;
- int error = 0;
if (rt->rt_gateway == NULL || rt->rt_gateway->sa_family != AF_INET6)
return (0);
@@ -217,16 +237,5 @@ rt6_deleteroute(struct rtentry *rt, void
if ((rt->rt_flags & RTF_HOST) == 0)
return (0);
- ifp = if_get(rt->rt_ifidx);
- if (ifp != NULL) {
- bzero(&info, sizeof(info));
- info.rti_flags = rt->rt_flags;
- info.rti_info[RTAX_DST] = rt_key(rt);
- info.rti_info[RTAX_GATEWAY] = rt->rt_gateway;
- info.rti_info[RTAX_NETMASK] = rt_plen2mask(rt, &sa_mask);
- error = rtrequest_delete(&info, RTP_ANY, ifp, NULL, id);
- }
- if_put(ifp);
-
- return (error != 0 ? error : EAGAIN);
+ return (EEXIST);
}
