On 01/09/15(Tue) 00:23, Alexander Bluhm wrote:
> On Fri, Aug 28, 2015 at 12:47:51PM +0200, Martin Pieuchot wrote:
> > The rtvalid() function checks if the route entry rt is still valid and
> > can be used. Cached entries that are no longer valid should be
> > released
> > by calling rtfree().
>
> I like it. As it does some checks and returns a boolian value, I
> wonder wether rtisvalid() would be a better name.
>
> > + if (rtvalid(rt) == 0) {
> ...
> As the function returns a boolean value I perfer the logical check with !.
> if (rtvalid(rt)) { route is valid }
> if (!rtvalid(rt)) { route is not valid }
Thanks for your comments. I integrated all your suggestions locally and
committed the function now called rtisvalid(9).
Now I still believe that such conversions should not be committed all at
once because they might have undesirable side effects. So here's a
first diff that I need for my rtalloc(9) rewrite. Are you ok with it?
Index: netinet/ip_output.c
===================================================================
RCS file: /cvs/src/sys/netinet/ip_output.c,v
retrieving revision 1.287
diff -u -p -r1.287 ip_output.c
--- netinet/ip_output.c 31 Aug 2015 07:17:12 -0000 1.287
+++ netinet/ip_output.c 1 Sep 2015 12:57:06 -0000
@@ -168,12 +168,13 @@ ip_output(struct mbuf *m0, struct mbuf *
dst = satosin(&ro->ro_dst);
/*
- * If there is a cached route, check that it is to the same
- * destination and is still up. If not, free it and try again.
+ * If there is a cached route, check that it is to the
+ * same destination and is still valid. If not, free
+ * it and try again.
*/
- if (ro->ro_rt && ((ro->ro_rt->rt_flags & RTF_UP) == 0 ||
+ if (!rtisvalid(ro->ro_rt) ||
dst->sin_addr.s_addr != ip->ip_dst.s_addr ||
- ro->ro_tableid != m->m_pkthdr.ph_rtableid)) {
+ ro->ro_tableid != m->m_pkthdr.ph_rtableid) {
rtfree(ro->ro_rt);
ro->ro_rt = NULL;
}
@@ -195,7 +196,9 @@ ip_output(struct mbuf *m0, struct mbuf *
ro->ro_rt = rtalloc_mpath(&ro->ro_dst,
NULL, ro->ro_tableid);
- if (ro->ro_rt == NULL) {
+ if (!rtisvalid(ro->ro_rt)) {
+ rtfree(ro->ro_rt);
+ ro->ro_rt = NULL;
ipstat.ips_noroute++;
error = EHOSTUNREACH;
goto bad;
@@ -296,12 +299,13 @@ reroute:
dst = satosin(&ro->ro_dst);
/*
- * If there is a cached route, check that it is to the same
- * destination and is still up. If not, free it and try again.
+ * If there is a cached route, check that it is to the
+ * same destination and is still valid. If not, free
+ * it and try again.
*/
- if (ro->ro_rt && ((ro->ro_rt->rt_flags & RTF_UP) == 0 ||
+ if (!rtisvalid(ro->ro_rt) ||
dst->sin_addr.s_addr != ip->ip_dst.s_addr ||
- ro->ro_tableid != m->m_pkthdr.ph_rtableid)) {
+ ro->ro_tableid != m->m_pkthdr.ph_rtableid) {
rtfree(ro->ro_rt);
ro->ro_rt = NULL;
}
@@ -323,7 +327,9 @@ reroute:
ro->ro_rt = rtalloc_mpath(&ro->ro_dst,
&ip->ip_src.s_addr, ro->ro_tableid);
- if (ro->ro_rt == NULL) {
+ if (!rtisvalid(ro->ro_rt)) {
+ rtfree(ro->ro_rt);
+ ro->ro_rt = NULL;
ipstat.ips_noroute++;
error = EHOSTUNREACH;
goto bad;
Index: netinet6/in6_src.c
===================================================================
RCS file: /cvs/src/sys/netinet6/in6_src.c,v
retrieving revision 1.51
diff -u -p -r1.51 in6_src.c
--- netinet6/in6_src.c 8 Jun 2015 22:19:28 -0000 1.51
+++ netinet6/in6_src.c 1 Sep 2015 12:55:26 -0000
@@ -247,13 +247,12 @@ in6_selectsrc(struct in6_addr **in6src,
* our src addr is taken from the i/f, else punt.
*/
if (ro) {
- if (ro->ro_rt && ((ro->ro_rt->rt_flags & RTF_UP) == 0 ||
- !IN6_ARE_ADDR_EQUAL(&ro->ro_dst.sin6_addr, dst))) {
+ if (!rtisvalid(ro->ro_rt) ||
+ !IN6_ARE_ADDR_EQUAL(&ro->ro_dst.sin6_addr, dst)) {
rtfree(ro->ro_rt);
ro->ro_rt = NULL;
}
- if (ro->ro_rt == (struct rtentry *)0 ||
- ro->ro_rt->rt_ifp == (struct ifnet *)0) {
+ if (ro->ro_rt == NULL) {
struct sockaddr_in6 *sa6;
/* No route yet, so try to acquire one */
@@ -419,10 +418,9 @@ selectroute(struct sockaddr_in6 *dstsock
* cached destination, in case of sharing the cache with IPv4.
*/
if (ro) {
- if (ro->ro_rt &&
- (!(ro->ro_rt->rt_flags & RTF_UP) ||
+ if (!rtisvalid(ro->ro_rt) ||
sin6tosa(&ro->ro_dst)->sa_family != AF_INET6 ||
- !IN6_ARE_ADDR_EQUAL(&ro->ro_dst.sin6_addr, dst))) {
+ !IN6_ARE_ADDR_EQUAL(&ro->ro_dst.sin6_addr, dst)) {
rtfree(ro->ro_rt);
ro->ro_rt = NULL;
}
