Any reason you are not adding the free(9) sizes in rtrequest(), too?
OK florian@
On Wed, Jun 08, 2016 at 04:23:33PM +0200, Martin Pieuchot wrote:
> Being able to remove the requirement of an configured address for every
> route entry would have multiple benefit:
>
> . We could add route before the interface gets an address (useful in
> some p2p configurations)
> . The kernel wouldn't have to manage stale ifas
> . The network data structures would be less coupled, making it easier
> to write SMP safe code (yes I found new bugs!)
>
> In order to achieve such goal we should preserve the "source routing"
> feature. Which mean that route entry MUST still carry an address.
>
> So the diff below adds a `rt_addr' field that will for the moment be a
> copy of `rt_ifa->ifa_addr'. Once all the `rt_ifa' occurrences are
> converted to `rt_addr' we can relax the logic for adding a route.
>
> This is the first of 14 steps.
>
> Index: net/route.c
> ===================================================================
> RCS file: /cvs/src/sys/net/route.c,v
> retrieving revision 1.307
> diff -u -p -r1.307 route.c
> --- net/route.c 8 Jun 2016 13:26:06 -0000 1.307
> +++ net/route.c 8 Jun 2016 14:19:48 -0000
> @@ -139,6 +139,8 @@
> #include <net/if_enc.h>
> #endif
>
> +#define ROUNDUP(a) (a>0 ? (1 + (((a) - 1) | (sizeof(long) - 1))) :
> sizeof(long))
> +
> /* Give some jitter to hash, to avoid synchronization between routers. */
> static uint32_t rt_hashjitter;
>
> @@ -151,6 +153,7 @@ struct pool rtentry_pool; /* pool for r
> struct pool rttimer_pool; /* pool for rttimer structures */
>
> void rt_timer_init(void);
> +int rt_setaddr(struct rtentry *, struct sockaddr *);
> int rtflushclone1(struct rtentry *, void *, u_int);
> void rtflushclone(unsigned int, struct rtentry *);
> int rt_if_remove_rtdelete(struct rtentry *, void *, u_int);
> @@ -434,7 +437,6 @@ rtref(struct rtentry *rt)
> void
> rtfree(struct rtentry *rt)
> {
> - struct ifaddr *ifa;
> int refcnt;
>
> if (rt == NULL)
> @@ -452,16 +454,14 @@ rtfree(struct rtentry *rt)
>
> KERNEL_LOCK();
> rt_timer_remove_all(rt);
> - ifa = rt->rt_ifa;
> - if (ifa)
> - ifafree(ifa);
> + ifafree(rt->rt_ifa);
> rtlabel_unref(rt->rt_labelid);
> #ifdef MPLS
> if (rt->rt_flags & RTF_MPLS)
> free(rt->rt_llinfo, M_TEMP, sizeof(struct rt_mpls));
> #endif
> - if (rt->rt_gateway)
> - free(rt->rt_gateway, M_RTABLE, 0);
> + free(rt->rt_addr, M_RTABLE, ROUNDUP(rt->rt_addr->sa_len));
> + free(rt->rt_gateway, M_RTABLE, ROUNDUP(rt->rt_gateway->sa_len));
> free(rt_key(rt), M_RTABLE, rt_key(rt)->sa_len);
> KERNEL_UNLOCK();
>
> @@ -486,7 +486,7 @@ rt_sendmsg(struct rtentry *rt, int cmd,
> ifp = if_get(rt->rt_ifidx);
> if (ifp != NULL) {
> info.rti_info[RTAX_IFP] = sdltosa(ifp->if_sadl);
> - info.rti_info[RTAX_IFA] = rt->rt_ifa->ifa_addr;
> + info.rti_info[RTAX_IFA] = rt->rt_addr;
> }
>
> rt_missmsg(cmd, &info, rt->rt_flags, rt->rt_priority, rt->rt_ifidx, 0,
> @@ -767,8 +767,6 @@ ifa_ifwithroute(int flags, struct sockad
> return (ifa);
> }
>
> -#define ROUNDUP(a) (a>0 ? (1 + (((a) - 1) | (sizeof(long) - 1))) :
> sizeof(long))
> -
> int
> rt_getifa(struct rt_addrinfo *info, u_int rtid)
> {
> @@ -958,7 +956,7 @@ rtrequest(int req, struct rt_addrinfo *i
> * will get the new address and interface later.
> */
> info->rti_ifa = NULL;
> - info->rti_info[RTAX_IFA] = rt->rt_ifa->ifa_addr;
> + info->rti_info[RTAX_IFA] = rt->rt_addr;
> }
>
> info->rti_flags = rt->rt_flags | (RTF_CLONED|RTF_HOST);
> @@ -1090,10 +1088,12 @@ rtrequest(int req, struct rt_addrinfo *i
> * the routing table because the radix MPATH code use
> * it to (re)order routes.
> */
> - if ((error = rt_setgate(rt, info->rti_info[RTAX_GATEWAY]))) {
> + if ((error = rt_setaddr(rt, ifa->ifa_addr)) ||
> + (error = rt_setgate(rt, info->rti_info[RTAX_GATEWAY]))) {
> ifafree(ifa);
> rtfree(rt->rt_parent);
> rtfree(rt->rt_gwroute);
> + free(rt->rt_addr, M_RTABLE, 0);
> free(rt->rt_gateway, M_RTABLE, 0);
> free(ndst, M_RTABLE, dlen);
> pool_put(&rtentry_pool, rt);
> @@ -1124,6 +1124,7 @@ rtrequest(int req, struct rt_addrinfo *i
> ifafree(ifa);
> rtfree(rt->rt_parent);
> rtfree(rt->rt_gwroute);
> + free(rt->rt_addr, M_RTABLE, 0);
> free(rt->rt_gateway, M_RTABLE, 0);
> free(ndst, M_RTABLE, dlen);
> pool_put(&rtentry_pool, rt);
> @@ -1145,6 +1146,24 @@ rtrequest(int req, struct rt_addrinfo *i
> rtfree(rt);
> break;
> }
> +
> + return (0);
> +}
> +
> +int
> +rt_setaddr(struct rtentry *rt, struct sockaddr *addr)
> +{
> + int alen = ROUNDUP(addr->sa_len);
> + struct sockaddr *sa;
> +
> + KASSERT(rt->rt_addr == NULL);
> +
> + sa = malloc(alen, M_RTABLE, M_NOWAIT);
> + if (sa == NULL)
> + return (ENOBUFS);
> +
> + memmove(sa, addr, alen);
> + rt->rt_addr = sa;
>
> return (0);
> }
> Index: net/route.h
> ===================================================================
> RCS file: /cvs/src/sys/net/route.h,v
> retrieving revision 1.136
> diff -u -p -r1.136 route.h
> --- net/route.h 3 Jun 2016 02:56:59 -0000 1.136
> +++ net/route.h 8 Jun 2016 14:19:48 -0000
> @@ -99,6 +99,7 @@ struct rtentry {
> struct sockaddr *rt_dest; /* destination */
> SRPL_ENTRY(rtentry) rt_next; /* Next multipath entry to our dst. */
> #endif
> + struct sockaddr *rt_addr; /* the answer: address to use */
> struct sockaddr *rt_gateway; /* value */
> struct ifaddr *rt_ifa; /* the answer: interface addr to use */
> caddr_t rt_llinfo; /* pointer to link level info cache or
> Index: net/rtsock.c
> ===================================================================
> RCS file: /cvs/src/sys/net/rtsock.c,v
> retrieving revision 1.190
> diff -u -p -r1.190 rtsock.c
> --- net/rtsock.c 3 Jun 2016 02:56:59 -0000 1.190
> +++ net/rtsock.c 8 Jun 2016 14:19:49 -0000
> @@ -695,7 +695,7 @@ report:
> ifp = if_get(rt->rt_ifidx);
> if (ifp != NULL && rtm->rtm_addrs & (RTA_IFP|RTA_IFA)) {
> info.rti_info[RTAX_IFP] = sdltosa(ifp->if_sadl);
> - info.rti_info[RTAX_IFA] = rt->rt_ifa->ifa_addr;
> + info.rti_info[RTAX_IFA] = rt->rt_addr;
> if (ifp->if_flags & IFF_POINTOPOINT)
> info.rti_info[RTAX_BRD] =
> rt->rt_ifa->ifa_dstaddr;
> @@ -1279,7 +1279,7 @@ sysctl_dumpentry(struct rtentry *rt, voi
> ifp = if_get(rt->rt_ifidx);
> if (ifp != NULL) {
> info.rti_info[RTAX_IFP] = sdltosa(ifp->if_sadl);
> - info.rti_info[RTAX_IFA] = rt->rt_ifa->ifa_addr;
> + info.rti_info[RTAX_IFA] = rt->rt_addr;
> if (ifp->if_flags & IFF_POINTOPOINT)
> info.rti_info[RTAX_BRD] = rt->rt_ifa->ifa_dstaddr;
> }
> Index: netinet/if_ether.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/if_ether.c,v
> retrieving revision 1.213
> diff -u -p -r1.213 if_ether.c
> --- netinet/if_ether.c 6 Jun 2016 07:07:11 -0000 1.213
> +++ netinet/if_ether.c 8 Jun 2016 14:19:49 -0000
> @@ -388,7 +388,7 @@ arpresolve(struct ifnet *ifp, struct rte
> rt->rt_expire = time_uptime;
> if (la->la_asked++ < arp_maxtries)
> arprequest(ifp,
> -
> &satosin(rt->rt_ifa->ifa_addr)->sin_addr.s_addr,
> + &satosin(rt->rt_addr)->sin_addr.s_addr,
> &satosin(dst)->sin_addr.s_addr,
> ac->ac_enaddr);
> else {
> Index: netmpls/mpls_input.c
> ===================================================================
> RCS file: /cvs/src/sys/netmpls/mpls_input.c,v
> retrieving revision 1.54
> diff -u -p -r1.54 mpls_input.c
> --- netmpls/mpls_input.c 4 Dec 2015 11:13:21 -0000 1.54
> +++ netmpls/mpls_input.c 8 Jun 2016 14:19:49 -0000
> @@ -374,7 +374,7 @@ mpls_do_error(struct mbuf *m, int type,
> m_freem(m);
> return (NULL);
> }
> - if (rt->rt_ifa->ifa_addr->sa_family == AF_INET)
> + if (rt->rt_addr->sa_family == AF_INET)
> ia = ifatoia(rt->rt_ifa);
> else {
> /* XXX this needs fixing, if the MPLS is on an IP
>
--
I'm not entirely sure you are real.
