tech-net archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: [PATCH] Deletion of route scrubs ifa flag IFA_ROUTE even if it's not the automatic route



On Tue, 2009-03-03 at 22:47 +0000, Roy Marples wrote:
> To make ammends, attached is a patch to fix this. It is intended for
> pullup to netbsd-5.

And here is a different patch.
We add a new function, rt_ifa_connected which returns 1 if the route is
the connected route (prefix, subnet, etc) for the ifa.
This makes the code in rtsock.c and route.c a lot simpler. I've also
added some debug messages via the existing RT_DPRINTF define.

For reference, I've also added a patch which applies this before my
origonal patch was comitted to show the intent of what we're trying to
do here.

I think this pretty much covers everything now. Anyone have any issues
with this before I commit?

Thanks

Roy
Index: sys/net/route.h
===================================================================
RCS file: /cvsroot/src/sys/net/route.h,v
retrieving revision 1.72
diff -u -p -r1.72 route.h
--- sys/net/route.h     11 Jan 2009 02:45:54 -0000      1.72
+++ sys/net/route.h     9 Mar 2009 09:55:15 -0000
@@ -342,6 +342,7 @@ void         rt_ieee80211msg(struct ifnet *, in
 void    rt_ifmsg(struct ifnet *);
 void    rt_maskedcopy(const struct sockaddr *,
            struct sockaddr *, const struct sockaddr *);
+int     rt_ifa_connected(const struct rtentry *, const struct ifaddr *);
 void    rt_missmsg(int, struct rt_addrinfo *, int, int);
 struct mbuf *rt_msg1(int, struct rt_addrinfo *, void *, int);
 void    rt_newaddrmsg(int, struct ifaddr *, int, struct rtentry *);
Index: sys/net/route.c
===================================================================
RCS file: /cvsroot/src/sys/net/route.c,v
retrieving revision 1.115
diff -u -p -r1.115 route.c
--- sys/net/route.c     20 Feb 2009 10:57:19 -0000      1.115
+++ sys/net/route.c     9 Mar 2009 09:55:15 -0000
@@ -672,8 +672,17 @@ rtrequest1(int req, struct rt_addrinfo *
                        rt->rt_parent = NULL;
                }
                rt->rt_flags &= ~RTF_UP;
-               if ((ifa = rt->rt_ifa) && ifa->ifa_rtrequest)
-                       ifa->ifa_rtrequest(RTM_DELETE, rt, info);
+               if ((ifa = rt->rt_ifa)) {
+                       if (ifa->ifa_flags & IFA_ROUTE &&
+                           rt_ifa_connected(rt, ifa)) {
+                               RT_DPRINTF("rt->_rt_key = %p, "
+                                   "deleted IFA_ROUTE\n",
+                                   (void *)rt->_rt_key);
+                               ifa->ifa_flags &= ~IFA_ROUTE;
+                       }
+                       if (ifa->ifa_rtrequest)
+                               ifa->ifa_rtrequest(RTM_DELETE, rt, info);
+               }
                rttrash++;
                if (ret_nrt)
                        *ret_nrt = rt;
@@ -690,9 +699,8 @@ rtrequest1(int req, struct rt_addrinfo *
                        senderr(EINVAL);
                ifa = rt->rt_ifa;
                flags = rt->rt_flags & ~(RTF_CLONING | RTF_STATIC);
-               flags |= RTF_CLONED;
+               flags |= RTF_CLONED | RTF_HOST;
                gateway = rt->rt_gateway;
-               flags |= RTF_HOST;
                goto makeroute;
 
        case RTM_ADD:
@@ -859,6 +867,30 @@ rt_maskedcopy(const struct sockaddr *src
 }
 
 /*
+ * Is this route the connected route for the ifa?
+ */
+int
+rt_ifa_connected(const struct rtentry *rt, const struct ifaddr *ifa)
+{
+       const struct sockaddr *key, *dst, *odst;
+       struct sockaddr_storage maskeddst;
+
+       key = rt_getkey(rt);
+       dst = rt->rt_flags & RTF_HOST ? ifa->ifa_dstaddr : ifa->ifa_addr;
+       if (dst == NULL ||
+           dst->sa_family != key->sa_family ||
+           dst->sa_len != key->sa_len)
+               return 0;
+       if ((rt->rt_flags & RTF_HOST) == 0 && ifa->ifa_netmask) {
+               odst = dst;
+               dst = (struct sockaddr *)&maskeddst;
+               rt_maskedcopy(odst, (struct sockaddr *)&maskeddst,
+                   ifa->ifa_netmask);
+       }
+       return (memcmp(dst, key, dst->sa_len) == 0);
+}
+
+/*
  * Set up or tear down a routing table entry, normally
  * for an interface.
  */
Index: sys/net/rtsock.c
===================================================================
RCS file: /cvsroot/src/sys/net/rtsock.c,v
retrieving revision 1.123
diff -u -p -r1.123 rtsock.c
--- sys/net/rtsock.c    20 Feb 2009 10:54:57 -0000      1.123
+++ sys/net/rtsock.c    9 Mar 2009 09:55:15 -0000
@@ -222,7 +222,7 @@ route_output(struct mbuf *m, ...)
        struct rtentry *rt = NULL;
        struct rtentry *saved_nrt = NULL;
        struct rt_addrinfo info;
-       int len, error = 0, ifa_route = 0;
+       int len, error = 0;
        struct ifnet *ifp = NULL;
        struct ifaddr *ifa = NULL, *oifa;
        struct socket *so;
@@ -301,12 +301,6 @@ route_output(struct mbuf *m, ...)
                error = rtrequest1(rtm->rtm_type, &info, &saved_nrt);
                if (error == 0) {
                        (rt = saved_nrt)->rt_refcnt++;
-                       ifa = rt_get_ifa(rt);
-                       /*
-                        * If deleting an automatic route, scrub the flag.
-                        */
-                       if (ifa->ifa_flags & IFA_ROUTE)
-                               ifa->ifa_flags &= ~IFA_ROUTE;
                        goto report;
                }
                break;
@@ -360,12 +354,12 @@ route_output(struct mbuf *m, ...)
                                if (info.rti_info[RTAX_IFA]->sa_family ==
                                    AF_INET) {
                                        printf("%s: copying out RTAX_IFA %s ",
-                                           __func__, inet_ntoa(
+                                           __func__, inet_ntoa((
                                            (const struct sockaddr_in *)
-                                           info.rti_info[RTAX_IFA])->sin_addr);
+                                           
info.rti_info[RTAX_IFA])->sin_addr));
                                        printf("for info.rti_info[RTAX_DST] %s "
                                            "ifa_getifa %p ifa_seqno %p\n",
-                                           inet_ntoa(
+                                           inet_ntoa((
                                            (const struct sockaddr_in *)
                                            info.rti_info[RTAX_DST])->sin_addr),
                                            (void *)rtifa->ifa_getifa,
@@ -425,30 +419,26 @@ route_output(struct mbuf *m, ...)
                                ifp = ifa->ifa_ifp;
                        }
                        oifa = rt->rt_ifa;
-                       if (oifa && oifa->ifa_flags & IFA_ROUTE) {
-                               /*
-                                * If changing an automatically added route,
-                                * remove the flag and store the fact.
-                                */
-                               oifa->ifa_flags &= ~IFA_ROUTE;
-                               ifa_route = 1;
-                       }
-                       if (ifa) {
-                               if (oifa != ifa) {
-                                       if (oifa && oifa->ifa_rtrequest) {
-                                               oifa->ifa_rtrequest(RTM_DELETE,
-                                                   rt, &info);
-                                       }
-                                       /*
-                                        * If changing an automatically added
-                                        * route, store this if not static.
-                                        */
-                                       if (ifa_route &&
-                                           !(rt->rt_flags & RTF_STATIC))
+                       if (ifa && oifa != ifa) {
+                               if (oifa && oifa->ifa_rtrequest)
+                                       oifa->ifa_rtrequest(RTM_DELETE,
+                                           rt, &info);
+                               if (oifa && oifa->ifa_flags & IFA_ROUTE &&
+                                   rt_ifa_connected(rt, oifa))
+                               {
+                                       RT_DPRINTF("rt->_rt_key = %p, "
+                                           "change deleted IFA_ROUTE\n",
+                                           (void *)rt->_rt_key);
+                                       oifa->ifa_flags &= ~IFA_ROUTE;
+                                       if (rt_ifa_connected(rt, ifa)) {
+                                               RT_DPRINTF("rt->_rt_key = %p, "
+                                                   "change added IFA_ROUTE\n",
+                                                   (void *)rt->_rt_key);
                                                ifa->ifa_flags |= IFA_ROUTE;
-                                       rt_replace_ifa(rt, ifa);
-                                       rt->rt_ifp = ifp;
+                                       }
                                }
+                               rt_replace_ifa(rt, ifa);
+                               rt->rt_ifp = ifp;
                        }
                        rt_setmetrics(rtm->rtm_inits, &rtm->rtm_rmx,
                            &rt->rt_rmx);
Index: sys/netinet/in.c
===================================================================
RCS file: /cvsroot/src/sys/netinet/in.c,v
retrieving revision 1.131
diff -u -p -r1.131 in.c
--- sys/netinet/in.c    12 Feb 2009 19:05:36 -0000      1.131
+++ sys/netinet/in.c    9 Mar 2009 09:55:15 -0000
@@ -993,16 +993,13 @@ in_addprefix(struct in_ifaddr *target, i
 
        /*
         * noone seem to have prefix route.  insert it.
+        * if it already exists, don't report back an error.
         */
        error = rtinit(&target->ia_ifa, RTM_ADD, flags);
        if (error == 0)
                target->ia_flags |= IFA_ROUTE;
-       else if (error == EEXIST) {
-               /* 
-                * the fact the route already exists is not an error.
-                */ 
-               error = 0;
-       }
+       else if (error == EEXIST)
+               return 0;
        return error;
 }
 
Index: sys/net/route.h
===================================================================
RCS file: /cvsroot/src/sys/net/route.h,v
retrieving revision 1.72
diff -u -p -r1.72 route.h
--- sys/net/route.h     11 Jan 2009 02:45:54 -0000      1.72
+++ sys/net/route.h     9 Mar 2009 10:02:23 -0000
@@ -342,6 +342,7 @@ void         rt_ieee80211msg(struct ifnet *, in
 void    rt_ifmsg(struct ifnet *);
 void    rt_maskedcopy(const struct sockaddr *,
            struct sockaddr *, const struct sockaddr *);
+int     rt_ifa_connected(const struct rtentry *, const struct ifaddr *);
 void    rt_missmsg(int, struct rt_addrinfo *, int, int);
 struct mbuf *rt_msg1(int, struct rt_addrinfo *, void *, int);
 void    rt_newaddrmsg(int, struct ifaddr *, int, struct rtentry *);
Index: sys/net/route.c
===================================================================
RCS file: /cvsroot/src/sys/net/route.c,v
retrieving revision 1.115
diff -u -p -r1.115 route.c
--- sys/net/route.c     20 Feb 2009 10:57:19 -0000      1.115
+++ sys/net/route.c     9 Mar 2009 10:02:42 -0000
@@ -672,8 +672,17 @@ rtrequest1(int req, struct rt_addrinfo *
                        rt->rt_parent = NULL;
                }
                rt->rt_flags &= ~RTF_UP;
-               if ((ifa = rt->rt_ifa) && ifa->ifa_rtrequest)
-                       ifa->ifa_rtrequest(RTM_DELETE, rt, info);
+               if ((ifa = rt->rt_ifa)) {
+                       if (ifa->ifa_flags & IFA_ROUTE &&
+                           rt_ifa_connected(rt, ifa)) {
+                               RT_DPRINTF("rt->_rt_key = %p, "
+                                   "deleted IFA_ROUTE\n",
+                                   (void *)rt->_rt_key);
+                               ifa->ifa_flags &= ~IFA_ROUTE;
+                       }
+                       if (ifa->ifa_rtrequest)
+                               ifa->ifa_rtrequest(RTM_DELETE, rt, info);
+               }
                rttrash++;
                if (ret_nrt)
                        *ret_nrt = rt;
@@ -690,9 +699,8 @@ rtrequest1(int req, struct rt_addrinfo *
                        senderr(EINVAL);
                ifa = rt->rt_ifa;
                flags = rt->rt_flags & ~(RTF_CLONING | RTF_STATIC);
-               flags |= RTF_CLONED;
+               flags |= RTF_CLONED | RTF_HOST;
                gateway = rt->rt_gateway;
-               flags |= RTF_HOST;
                goto makeroute;
 
        case RTM_ADD:
@@ -859,6 +867,30 @@ rt_maskedcopy(const struct sockaddr *src
 }
 
 /*
+ * Is this route the connected route for the ifa?
+ */
+int
+rt_ifa_connected(const struct rtentry *rt, const struct ifaddr *ifa)
+{
+       const struct sockaddr *key, *dst, *odst;
+       struct sockaddr_storage maskeddst;
+
+       key = rt_getkey(rt);
+       dst = rt->rt_flags & RTF_HOST ? ifa->ifa_dstaddr : ifa->ifa_addr;
+       if (dst == NULL ||
+           dst->sa_family != key->sa_family ||
+           dst->sa_len != key->sa_len)
+               return 0;
+       if ((rt->rt_flags & RTF_HOST) == 0 && ifa->ifa_netmask) {
+               odst = dst;
+               dst = (struct sockaddr *)&maskeddst;
+               rt_maskedcopy(odst, (struct sockaddr *)&maskeddst,
+                   ifa->ifa_netmask);
+       }
+       return (memcmp(dst, key, dst->sa_len) == 0);
+}
+
+/*
  * Set up or tear down a routing table entry, normally
  * for an interface.
  */
Index: sys/net/rtsock.c
===================================================================
RCS file: /cvsroot/src/sys/net/rtsock.c,v
retrieving revision 1.118
diff -u -p -r1.118 rtsock.c
--- sys/net/rtsock.c    17 Dec 2008 20:51:37 -0000      1.118
+++ sys/net/rtsock.c    9 Mar 2009 10:03:54 -0000
@@ -215,7 +224,7 @@ route_output(struct mbuf *m, ...)
        struct rt_addrinfo info;
        int len, error = 0;
        struct ifnet *ifp = NULL;
-       struct ifaddr *ifa = NULL;
+       struct ifaddr *ifa = NULL, *oifa;
        struct socket *so;
        va_list ap;
        sa_family_t family;
@@ -345,12 +354,12 @@ route_output(struct mbuf *m, ...)
                                if (info.rti_info[RTAX_IFA]->sa_family ==
                                    AF_INET) {
                                        printf("%s: copying out RTAX_IFA %s ",
-                                           __func__, inet_ntoa(
+                                           __func__, inet_ntoa((
                                            (const struct sockaddr_in *)
-                                           info.rti_info[RTAX_IFA])->sin_addr);
+                                           
info.rti_info[RTAX_IFA])->sin_addr));
                                        printf("for info.rti_info[RTAX_DST] %s "
                                            "ifa_getifa %p ifa_seqno %p\n",
-                                           inet_ntoa(
+                                           inet_ntoa((
                                            (const struct sockaddr_in *)
                                            info.rti_info[RTAX_DST])->sin_addr),
                                            (void *)rtifa->ifa_getifa,
@@ -409,16 +418,27 @@ route_output(struct mbuf *m, ...)
                            rt_getkey(rt), info.rti_info[RTAX_GATEWAY])))) {
                                ifp = ifa->ifa_ifp;
                        }
-                       if (ifa) {
-                               struct ifaddr *oifa = rt->rt_ifa;
-                               if (oifa != ifa) {
-                                       if (oifa && oifa->ifa_rtrequest) {
-                                               oifa->ifa_rtrequest(RTM_DELETE,
-                                                   rt, &info);
+                       oifa = rt->rt_ifa;
+                       if (ifa && oifa != ifa) {
+                               if (oifa && oifa->ifa_rtrequest)
+                                       oifa->ifa_rtrequest(RTM_DELETE,
+                                           rt, &info);
+                               if (oifa && oifa->ifa_flags & IFA_ROUTE &&
+                                   rt_ifa_connected(rt, oifa))
+                               {
+                                       RT_DPRINTF("rt->_rt_key = %p, "
+                                           "change deleted IFA_ROUTE\n",
+                                           (void *)rt->_rt_key);
+                                       oifa->ifa_flags &= ~IFA_ROUTE;
+                                       if (rt_ifa_connected(rt, ifa)) {
+                                               RT_DPRINTF("rt->_rt_key = %p, "
+                                                   "change added IFA_ROUTE\n",
+                                                   (void *)rt->_rt_key);
+                                               ifa->ifa_flags |= IFA_ROUTE;
                                        }
-                                       rt_replace_ifa(rt, ifa);
-                                       rt->rt_ifp = ifp;
                                }
+                               rt_replace_ifa(rt, ifa);
+                               rt->rt_ifp = ifp;
                        }
                        rt_setmetrics(rtm->rtm_inits, &rtm->rtm_rmx,
                            &rt->rt_rmx);
Index: sys/netinet/in.c
===================================================================
RCS file: /cvsroot/src/sys/netinet/in.c,v
retrieving revision 1.129
diff -u -p -r1.129 in.c
--- sys/netinet/in.c    17 Dec 2008 20:51:37 -0000      1.129
+++ sys/netinet/in.c    9 Mar 2009 10:06:17 -0000
@@ -1000,10 +993,13 @@ in_addprefix(struct in_ifaddr *target, i
 
        /*
         * noone seem to have prefix route.  insert it.
+        * if it already exists, don't report back an error.
         */
        error = rtinit(&target->ia_ifa, RTM_ADD, flags);
        if (error == 0)
                target->ia_flags |= IFA_ROUTE;
+       else if (error == EEXIST)
+               return 0;
        return error;
 }
 

Attachment: signature.asc
Description: This is a digitally signed message part



Home | Main Index | Thread Index | Old Index