On 03/02/15(Tue) 16:25, Claudio Jeker wrote:
> 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.
I believe it covers all the different conflict cases, but if you think
something is not covered we can always add new tests.
>
> > 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
>
