On Thu, Apr 24, 2014 at 02:52:27PM +0200, Martin Pieuchot wrote:
> On 24/04/14(Thu) 13:43, Henning Brauer wrote:
> > * Martin Pieuchot <[email protected]> [2014-04-24 13:24]:
> > > This ifp pointer is only needed by rt_getifa() to find an address, so
> > > make it a local variable.
> > >
> > > The rtrequest1(9) change might introduce a negligible slowdown since
> > > I remove the already known ifp pointer. But this only happens in the
> > > case described in the comment just before and I would bet because of
> > > carp_setroute(), still nobody to fix this? It's not better than
> > > OpenSSL...
> > >
> > > In the rtsock chunk, the two pointers are equivalent.
> > >
> > > Ok?
> >
> > yup.
>
> And now with proper ifp initialization, pointed by bluhm@.
OK bluhm@
>
> Index: net/route.c
> ===================================================================
> RCS file: /home/ncvs/src/sys/net/route.c,v
> retrieving revision 1.163
> diff -u -p -r1.163 route.c
> --- net/route.c 23 Apr 2014 09:30:57 -0000 1.163
> +++ net/route.c 24 Apr 2014 12:45:04 -0000
> @@ -691,16 +691,17 @@ int
> rt_getifa(struct rt_addrinfo *info, u_int rtid)
> {
> struct ifaddr *ifa;
> + struct ifnet *ifp = NULL;
>
> /*
> * ifp may be specified by sockaddr_dl when protocol address
> * is ambiguous
> */
> - if (info->rti_ifp == NULL && info->rti_info[RTAX_IFP] != NULL) {
> + if (info->rti_info[RTAX_IFP] != NULL) {
> struct sockaddr_dl *sdl;
>
> sdl = (struct sockaddr_dl *)info->rti_info[RTAX_IFP];
> - info->rti_ifp = if_get(sdl->sdl_index);
> + ifp = if_get(sdl->sdl_index);
> }
>
> if (info->rti_ifa == NULL && info->rti_info[RTAX_IFA] != NULL)
> @@ -713,8 +714,8 @@ rt_getifa(struct rt_addrinfo *info, u_in
> if ((sa = info->rti_info[RTAX_GATEWAY]) == NULL)
> sa = info->rti_info[RTAX_DST];
>
> - if (sa != NULL && info->rti_ifp != NULL)
> - info->rti_ifa = ifaof_ifpforaddr(sa, info->rti_ifp);
> + if (sa != NULL && ifp != NULL)
> + info->rti_ifa = ifaof_ifpforaddr(sa, ifp);
> else if (info->rti_info[RTAX_DST] != NULL &&
> info->rti_info[RTAX_GATEWAY] != NULL)
> info->rti_ifa = ifa_ifwithroute(info->rti_flags,
> @@ -729,9 +730,6 @@ rt_getifa(struct rt_addrinfo *info, u_in
> if ((ifa = info->rti_ifa) == NULL)
> return (ENETUNREACH);
>
> - if (info->rti_ifp == NULL)
> - info->rti_ifp = ifa->ifa_ifp;
> -
> return (0);
> }
>
> @@ -828,8 +826,10 @@ rtrequest1(int req, struct rt_addrinfo *
> info->rti_ifa = rt->rt_ifa;
> } else {
> /*
> - * The interface address at the cloning route
> - * is not longer referenced by an interface.
> + * The address of the cloning route is not longer
> + * configured on an interface, but its descriptor
> + * is still there because of reference counting.
> + *
> * Try to find a similar active address and use
> * it for the cloned route. The cloning route
> * will get the new address and interface later.
> @@ -837,7 +837,6 @@ rtrequest1(int req, struct rt_addrinfo *
> info->rti_ifa = NULL;
> info->rti_info[RTAX_IFA] = rt->rt_ifa->ifa_addr;
> }
> - info->rti_ifp = rt->rt_ifp;
> info->rti_flags = rt->rt_flags & ~(RTF_CLONING | RTF_STATIC);
> info->rti_flags |= RTF_CLONED;
> info->rti_info[RTAX_GATEWAY] = rt->rt_gateway;
> Index: net/route.h
> ===================================================================
> RCS file: /home/ncvs/src/sys/net/route.h,v
> retrieving revision 1.91
> diff -u -p -r1.91 route.h
> --- net/route.h 10 Apr 2014 13:47:21 -0000 1.91
> +++ net/route.h 24 Apr 2014 12:45:04 -0000
> @@ -299,7 +299,6 @@ struct rt_addrinfo {
> struct sockaddr *rti_info[RTAX_MAX];
> int rti_flags;
> struct ifaddr *rti_ifa;
> - struct ifnet *rti_ifp;
> struct rt_msghdr *rti_rtm;
> u_char rti_mpls;
> };
> Index: net/rtsock.c
> ===================================================================
> RCS file: /home/ncvs/src/sys/net/rtsock.c,v
> retrieving revision 1.142
> diff -u -p -r1.142 rtsock.c
> --- net/rtsock.c 18 Mar 2014 10:47:34 -0000 1.142
> +++ net/rtsock.c 24 Apr 2014 12:45:04 -0000
> @@ -768,7 +768,7 @@ report:
> ifafree(rt->rt_ifa);
> rt->rt_ifa = ifa;
> ifa->ifa_refcnt++;
> - rt->rt_ifp = info.rti_ifp;
> + rt->rt_ifp = ifa->ifa_ifp;
> #ifndef SMALL_KERNEL
> /* recheck link state after ifp change*/
> rt_if_linkstate_change(
