On 19/01/15(Mon) 09:35, Todd C. Miller wrote:
> On Mon, 19 Jan 2015 11:49:53 +0100, Martin Pieuchot wrote:
>
> > Instead of rerolling rtrequest1(RTM_DELETE...) code in various places,
> > simply use rtdeletemsg() which also notify userland that the route entry
> > is going away.
>
> Since rtdeletemsg() may call rtfree() doesn't this mean that we can
> end up calling rtfree() twice? Or is rt->rt_refcnt never going to
> be zero in this case?
It is indeed confusing. I tried to check every cases but in the end I
think that it might be better to decouple the removal from the routing
table and the rtfree(). Updated diff below does that.
Another advantage I can see to this approach is to be more strict about
the reference counter on a per-rt basic.
Index: net/route.c
===================================================================
RCS file: /home/ncvs/src/sys/net/route.c,v
retrieving revision 1.200
diff -u -p -r1.200 route.c
--- net/route.c 18 Jan 2015 14:51:43 -0000 1.200
+++ net/route.c 21 Jan 2015 12:17:21 -0000
@@ -544,11 +544,6 @@ rtdeletemsg(struct rtentry *rt, u_int ta
rt_missmsg(RTM_DELETE, &info, info.rti_flags, ifp, error, tableid);
- /* Adjust the refcount */
- if (error == 0 && rt->rt_refcnt <= 0) {
- rt->rt_refcnt++;
- rtfree(rt);
- }
return (error);
}
@@ -556,11 +551,19 @@ int
rtflushclone1(struct radix_node *rn, void *arg, u_int id)
{
struct rtentry *rt, *parent;
+ int error;
rt = (struct rtentry *)rn;
parent = (struct rtentry *)arg;
- if ((rt->rt_flags & RTF_CLONED) != 0 && rt->rt_parent == parent)
- rtdeletemsg(rt, id);
+ if ((rt->rt_flags & RTF_CLONED) != 0 && rt->rt_parent == parent) {
+ error = rtdeletemsg(rt, id);
+
+ /* Adjust the refcount */
+ if (error == 0 && rt->rt_refcnt <= 0) {
+ rt->rt_refcnt++;
+ rtfree(rt);
+ }
+ }
return 0;
}
@@ -1632,8 +1635,17 @@ rt_if_remove_rtdelete(struct radix_node
if (rt->rt_ifp == ifp) {
int cloning = (rt->rt_flags & RTF_CLONING);
- if (rtdeletemsg(rt, id) == 0 && cloning)
- return (EAGAIN);
+ if (rtdeletemsg(rt, id) == 0) {
+
+ /* Adjust the refcount */
+ if (rt->rt_refcnt <= 0) {
+ rt->rt_refcnt++;
+ rtfree(rt);
+ }
+
+ if (cloning)
+ return (EAGAIN);
+ }
}
/*
Index: netinet/if_ether.c
===================================================================
RCS file: /home/ncvs/src/sys/netinet/if_ether.c,v
retrieving revision 1.141
diff -u -p -r1.141 if_ether.c
--- netinet/if_ether.c 13 Jan 2015 12:16:18 -0000 1.141
+++ netinet/if_ether.c 21 Jan 2015 12:17:21 -0000
@@ -789,6 +789,7 @@ arptfree(struct llinfo_arp *la)
struct rtentry *rt = la->la_rt;
struct sockaddr_dl *sdl;
u_int tid = 0;
+ int error;
if (rt == NULL)
panic("arptfree");
@@ -803,7 +804,13 @@ arptfree(struct llinfo_arp *la)
if (rt->rt_ifp)
tid = rt->rt_ifp->if_rdomain;
- rtdeletemsg(rt, tid);
+ error = rtdeletemsg(rt, tid);
+
+ /* Adjust the refcount */
+ if (error == 0 && rt->rt_refcnt <= 0) {
+ rt->rt_refcnt++;
+ rtfree(rt);
+ }
}
/*
Index: netinet/ip_icmp.c
===================================================================
RCS file: /home/ncvs/src/sys/netinet/ip_icmp.c,v
retrieving revision 1.129
diff -u -p -r1.129 ip_icmp.c
--- netinet/ip_icmp.c 22 Dec 2014 11:05:53 -0000 1.129
+++ netinet/ip_icmp.c 21 Jan 2015 12:17:21 -0000
@@ -1031,20 +1031,12 @@ icmp_mtudisc_timeout(struct rtentry *rt,
(RTF_DYNAMIC | RTF_HOST)) {
void *(*ctlfunc)(int, struct sockaddr *, u_int, void *);
struct sockaddr_in sa;
- struct rt_addrinfo info;
int s;
- memset(&info, 0, sizeof(info));
- info.rti_info[RTAX_DST] = rt_key(rt);
- info.rti_info[RTAX_NETMASK] = rt_mask(rt);
- info.rti_info[RTAX_GATEWAY] = rt->rt_gateway;
- info.rti_flags = rt->rt_flags;
-
sa = *(struct sockaddr_in *)rt_key(rt);
s = splsoftnet();
- rtrequest1(RTM_DELETE, &info, rt->rt_priority, NULL,
- r->rtt_tableid);
+ rtdeletemsg(rt, r->rtt_tableid);
/* Notify TCP layer of increased Path MTU estimate */
ctlfunc = inetsw[ip_protox[IPPROTO_TCP]].pr_ctlinput;
@@ -1083,18 +1075,10 @@ icmp_redirect_timeout(struct rtentry *rt
if ((rt->rt_flags & (RTF_DYNAMIC | RTF_HOST)) ==
(RTF_DYNAMIC | RTF_HOST)) {
- struct rt_addrinfo info;
int s;
- memset(&info, 0, sizeof(info));
- info.rti_info[RTAX_DST] = rt_key(rt);
- info.rti_info[RTAX_NETMASK] = rt_mask(rt);
- info.rti_info[RTAX_GATEWAY] = rt->rt_gateway;
- info.rti_flags = rt->rt_flags;
-
s = splsoftnet();
- rtrequest1(RTM_DELETE, &info, rt->rt_priority, NULL,
- r->rtt_tableid);
+ rtdeletemsg(rt, r->rtt_tableid);
splx(s);
}
}
Index: netinet6/icmp6.c
===================================================================
RCS file: /home/ncvs/src/sys/netinet6/icmp6.c,v
retrieving revision 1.153
diff -u -p -r1.153 icmp6.c
--- netinet6/icmp6.c 19 Jan 2015 13:53:55 -0000 1.153
+++ netinet6/icmp6.c 21 Jan 2015 12:17:21 -0000
@@ -1983,18 +1983,10 @@ icmp6_mtudisc_timeout(struct rtentry *rt
panic("icmp6_mtudisc_timeout: bad route to timeout");
if ((rt->rt_flags & (RTF_DYNAMIC | RTF_HOST)) ==
(RTF_DYNAMIC | RTF_HOST)) {
- struct rt_addrinfo info;
int s;
- 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_mask(rt);
-
s = splsoftnet();
- rtrequest1(RTM_DELETE, &info, rt->rt_priority, NULL,
- r->rtt_tableid);
+ rtdeletemsg(rt, r->rtt_tableid);
splx(s);
} else {
if (!(rt->rt_rmx.rmx_locks & RTV_MTU))
@@ -2009,18 +2001,10 @@ icmp6_redirect_timeout(struct rtentry *r
panic("icmp6_redirect_timeout: bad route to timeout");
if ((rt->rt_flags & (RTF_GATEWAY | RTF_DYNAMIC | RTF_HOST)) ==
(RTF_GATEWAY | RTF_DYNAMIC | RTF_HOST)) {
- struct rt_addrinfo info;
int s;
- 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_mask(rt);
-
s = splsoftnet();
- rtrequest1(RTM_DELETE, &info, rt->rt_priority, NULL,
- r->rtt_tableid);
+ rtdeletemsg(rt, r->rtt_tableid);
splx(s);
}
}
Index: netinet6/in6_ifattach.c
===================================================================
RCS file: /home/ncvs/src/sys/netinet6/in6_ifattach.c,v
retrieving revision 1.81
diff -u -p -r1.81 in6_ifattach.c
--- netinet6/in6_ifattach.c 10 Jan 2015 11:43:37 -0000 1.81
+++ netinet6/in6_ifattach.c 21 Jan 2015 12:17:21 -0000
@@ -666,15 +666,7 @@ in6_ifdetach(struct ifnet *ifp)
sin6.sin6_addr.s6_addr16[1] = htons(ifp->if_index);
rt = rtalloc(sin6tosa(&sin6), 0, ifp->if_rdomain);
if (rt && rt->rt_ifp == ifp) {
- struct rt_addrinfo info;
-
- 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_mask(rt);
- rtrequest1(RTM_DELETE, &info, rt->rt_priority, NULL,
- ifp->if_rdomain);
+ rtdeletemsg(rt, ifp->if_rdomain);
rtfree(rt);
}
@@ -686,15 +678,7 @@ in6_ifdetach(struct ifnet *ifp)
sin6.sin6_addr.s6_addr16[1] = htons(ifp->if_index);
rt = rtalloc(sin6tosa(&sin6), 0, ifp->if_rdomain);
if (rt && rt->rt_ifp == ifp) {
- struct rt_addrinfo info;
-
- 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_mask(rt);
- rtrequest1(RTM_DELETE, &info, rt->rt_priority, NULL,
- ifp->if_rdomain);
+ rtdeletemsg(rt, ifp->if_rdomain);
rtfree(rt);
}
