On Tue, Feb 03, 2015 at 10:39:35AM +0100, Martin Pieuchot wrote:
> Diff below changes rt_mpath_conflict() to no longer rely on a fully
> initialized rtentry. Right now it makes things prettier when adding
> a new route entry but later it will also help to dissociate "struct
> radix_node" from "struct rtentry".
>
> route(8) regression tests checking for conflicts are happy with this.
>
> Ok?
Reads fine. Not sure if the regress test is enough for this.
> Index: net/radix_mpath.c
> ===================================================================
> RCS file: /cvs/src/sys/net/radix_mpath.c,v
> retrieving revision 1.27
> diff -u -p -r1.27 radix_mpath.c
> --- net/radix_mpath.c 19 Dec 2014 17:14:40 -0000 1.27
> +++ net/radix_mpath.c 3 Feb 2015 09:20:38 -0000
> @@ -204,16 +204,15 @@ rt_mpath_matchgate(struct rtentry *rt, s
> * check if we have the same key/mask/gateway on the table already.
> */
> int
> -rt_mpath_conflict(struct radix_node_head *rnh, struct rtentry *rt,
> - struct sockaddr *netmask, int mpathok)
> +rt_mpath_conflict(struct radix_node_head *rnh, struct sockaddr *dst,
> + struct sockaddr *netmask, struct sockaddr *gate, u_int8_t prio, int
> mpathok)
> {
> - struct radix_node *rn, *rn1;
> + struct radix_node *rn1;
> struct rtentry *rt1;
> char *p, *q, *eq;
> int same, l, skip;
>
> - rn = (struct radix_node *)rt;
> - rn1 = rnh->rnh_lookup(rt_key(rt), netmask, rnh);
> + rn1 = rnh->rnh_lookup(dst, netmask, rnh);
> if (!rn1 || rn1->rn_flags & RNF_ROOT)
> return 0;
>
> @@ -224,8 +223,8 @@ rt_mpath_conflict(struct radix_node_head
> rt1 = (struct rtentry *)rn1;
>
> /* compare key. */
> - if (rt_key(rt1)->sa_len != rt_key(rt)->sa_len ||
> - bcmp(rt_key(rt1), rt_key(rt), rt_key(rt1)->sa_len))
> + if (rt_key(rt1)->sa_len != dst->sa_len ||
> + bcmp(rt_key(rt1), dst, rt_key(rt1)->sa_len))
> goto different;
>
> /* key was the same. compare netmask. hairy... */
> @@ -277,11 +276,11 @@ rt_mpath_conflict(struct radix_node_head
> }
>
> maskmatched:
> - if (!mpathok && rt1->rt_priority == rt->rt_priority)
> + if (!mpathok && rt1->rt_priority == prio)
> return EEXIST;
>
> /* key/mask were the same. compare gateway for all multipaths */
> - if (rt_mpath_matchgate(rt1, rt->rt_gateway, rt->rt_priority))
> + if (rt_mpath_matchgate(rt1, gate, prio))
> /* all key/mask/gateway are the same. conflicting entry. */
> return EEXIST;
>
> Index: net/radix_mpath.h
> ===================================================================
> RCS file: /cvs/src/sys/net/radix_mpath.h,v
> retrieving revision 1.14
> diff -u -p -r1.14 radix_mpath.h
> --- net/radix_mpath.h 25 Nov 2014 14:50:46 -0000 1.14
> +++ net/radix_mpath.h 3 Feb 2015 09:20:07 -0000
> @@ -54,8 +54,8 @@ void rn_mpath_adj_mpflag(struct radix_no
> int rn_mpath_active_count(struct radix_node *);
> struct rtentry *rt_mpath_matchgate(struct rtentry *, struct sockaddr *,
> u_int8_t);
> -int rt_mpath_conflict(struct radix_node_head *, struct rtentry *,
> - struct sockaddr *, int);
> +int rt_mpath_conflict(struct radix_node_head *, struct sockaddr *,
> + struct sockaddr *, struct sockaddr *, u_int8_t, int);
> struct rtentry *rtalloc_mpath(struct sockaddr *, u_int32_t *, u_int);
> int rn_mpath_inithead(void **, int);
> #endif /* _KERNEL */
> Index: net/route.c
> ===================================================================
> RCS file: /cvs/src/sys/net/route.c,v
> retrieving revision 1.203
> diff -u -p -r1.203 route.c
> --- net/route.c 28 Jan 2015 22:10:13 -0000 1.203
> +++ net/route.c 3 Feb 2015 09:23:26 -0000
> @@ -826,15 +826,25 @@ rtrequest1(int req, struct rt_addrinfo *
> if (info->rti_ifa == NULL && (error = rt_getifa(info, tableid)))
> return (error);
> ifa = info->rti_ifa;
> + if (prio == 0)
> + prio = ifa->ifa_ifp->if_priority + RTP_STATIC;
> +#ifndef SMALL_KERNEL
> + if (rn_mpath_capable(rnh)) {
> + /* do not permit exactly the same dst/mask/gw pair */
> + if (rt_mpath_conflict(rnh, info->rti_info[RTAX_DST],
> + info->rti_info[RTAX_NETMASK],
> + info->rti_info[RTAX_GATEWAY], prio,
> + info->rti_flags & RTF_MPATH)) {
> + return (EEXIST);
> + }
> + }
> +#endif
> rt = pool_get(&rtentry_pool, PR_NOWAIT | PR_ZERO);
> if (rt == NULL)
> return (ENOBUFS);
>
> rt->rt_flags = info->rti_flags;
> rt->rt_tableid = tableid;
> -
> - if (prio == 0)
> - prio = ifa->ifa_ifp->if_priority + RTP_STATIC;
> rt->rt_priority = prio; /* init routing priority */
> LIST_INIT(&rt->rt_timer);
> if ((error = rt_setgate(rt, info->rti_info[RTAX_DST],
> @@ -851,16 +861,6 @@ rtrequest1(int req, struct rt_addrinfo *
> info->rti_info[RTAX_DST]->sa_len);
> #ifndef SMALL_KERNEL
> if (rn_mpath_capable(rnh)) {
> - /* do not permit exactly the same dst/mask/gw pair */
> - if (rt_mpath_conflict(rnh, rt,
> - info->rti_info[RTAX_NETMASK],
> - info->rti_flags & RTF_MPATH)) {
> - if (rt->rt_gwroute)
> - rtfree(rt->rt_gwroute);
> - free(rt_key(rt), M_RTABLE, 0);
> - pool_put(&rtentry_pool, rt);
> - return (EEXIST);
> - }
> /* check the link state since the table supports it */
> if (LINK_STATE_IS_UP(ifa->ifa_ifp->if_link_state) &&
> ifa->ifa_ifp->if_flags & IFF_UP)
>
--
:wq Claudio
