Timo - I agree that your use of finer grained RT_SCOPE_LINK and RT_SCOPE_UNIVERSE is a good thing. My only real problem with the patch was the removal of this line:
- resolved_hop->flags |= NEXTHOP_FLAG_ONLINK; Put it back and I believe we would be ok. donald On Tue, Nov 3, 2015 at 6:31 AM, Timo Teras <[email protected]> wrote: > On Tue, 3 Nov 2015 06:08:01 -0800 > Donald Sharp <[email protected]> wrote: > > > For our version of ospf unnumbered Cumulus is using this: > > RT_SCOPE_LINK + RTNH_F_ONLINK -> Unnumbered interface > > RT_SCOPE_LINK - IFINDEX based link. > > RT_SCOPE_UNIVERSE - Everything else. > > > > This patch would remove that ability to use the scope + RTNH_F_ONLINK > > to indicate to the kernel that a link is UNNUMBERED and should just > > be trusted. > > Yes and no. > > Looking at the quagga-re onlink commit, it seems it modifies the zapi > protocol, and other lib functions to implement additional thinks. > > You are apparently doing something more complicated than just stamping > the scope based on route type. While my patch only attempts to set the > scope properly for the current functionality. This is probably step to > right direction, but not enough for ospf unnumbered functionality. > > I'm also ok to have the unnumbered base patches instead of this. If the > they result in above described RT_SCOPE_* usage. > > Thanks, > Timo > > > > On Mon, Nov 2, 2015 at 6:50 AM, Timo Teräs <[email protected]> wrote: > > > > > In linux, 'scope' is a hint of distance of the IP. And this is > > > evident from the fact that only lower scope can be used as recursive > > > via lookup result. This changes all interface routes scope to link > > > so kernel will allow regular routes to use it as via. Then we do > > > not need to use the 'onlink' attribute. > > > > > > Signed-off-by: Timo Teräs <[email protected]> > > > --- > > > zebra/rt_netlink.c | 6 +++++- > > > zebra/zebra_rib.c | 21 ++++++--------------- > > > 2 files changed, 11 insertions(+), 16 deletions(-) > > > > > > diff --git a/zebra/rt_netlink.c b/zebra/rt_netlink.c > > > index 02a2d72..dd75e79 100644 > > > --- a/zebra/rt_netlink.c > > > +++ b/zebra/rt_netlink.c > > > @@ -1639,7 +1639,7 @@ netlink_route_multipath (int cmd, struct > > > prefix *p, struct rib *rib, > > > req.r.rtm_table = rib->table; > > > req.r.rtm_dst_len = p->prefixlen; > > > req.r.rtm_protocol = RTPROT_ZEBRA; > > > - req.r.rtm_scope = RT_SCOPE_UNIVERSE; > > > + req.r.rtm_scope = RT_SCOPE_LINK; > > > > > > if ((rib->flags & ZEBRA_FLAG_BLACKHOLE) || (rib->flags & > > > ZEBRA_FLAG_REJECT)) > > > discard = 1; > > > @@ -1707,6 +1707,10 @@ netlink_route_multipath (int cmd, struct > > > prefix *p, struct rib *rib, > > > if (cmd == RTM_DELROUTE && !CHECK_FLAG (nexthop->flags, > > > NEXTHOP_FLAG_FIB)) > > > continue; > > > > > > + if (nexthop->type != NEXTHOP_TYPE_IFINDEX && > > > + nexthop->type != NEXTHOP_TYPE_IFNAME) > > > + req.r.rtm_scope = RT_SCOPE_UNIVERSE; > > > + > > > nexthop_num++; > > > } > > > > > > diff --git a/zebra/zebra_rib.c b/zebra/zebra_rib.c > > > index 385e2d6..4e276b0 100644 > > > --- a/zebra/zebra_rib.c > > > +++ b/zebra/zebra_rib.c > > > @@ -441,26 +441,17 @@ nexthop_active_ipv4 (struct rib *rib, struct > > > nexthop *nexthop, int set, > > > { > > > resolved_hop->type = newhop->type; > > > resolved_hop->gate.ipv4 = > > > newhop->gate.ipv4; - > > > - if (newhop->ifindex) > > > - { > > > - resolved_hop->type = > > > NEXTHOP_TYPE_IPV4_IFINDEX; > > > - resolved_hop->ifindex = > > > newhop->ifindex; > > > - } > > > + resolved_hop->ifindex = newhop->ifindex; > > > } > > > > > > - /* If the resolving route is an interface > > > route, > > > - * it means the gateway we are looking up is > > > connected > > > - * to that interface. (The actual network > > > is _not_ onlink). > > > - * Therefore, the resolved route should > > > have the original > > > - * gateway as nexthop as it is directly > > > connected. > > > - * > > > - * On Linux, we have to set the onlink > > > netlink flag because > > > - * otherwise, the kernel won't accept the > > > route. */ > > > + /* If the resolving route is an interface > > > route, it > > > + * means the gateway we are looking up is > > > connected > > > + * to that interface. Therefore, the > > > resolved route > > > + * should have the original gateway as > > > nexthop as it > > > + * is directly connected. */ > > > if (newhop->type == NEXTHOP_TYPE_IFINDEX > > > || newhop->type == NEXTHOP_TYPE_IFNAME) > > > { > > > - resolved_hop->flags |= > > > NEXTHOP_FLAG_ONLINK; resolved_hop->type = NEXTHOP_TYPE_IPV4_IFINDEX; > > > resolved_hop->gate.ipv4 = > > > nexthop->gate.ipv4; resolved_hop->ifindex = newhop->ifindex; > > > -- > > > 2.6.1 > > > > > > > > > _______________________________________________ > > > Quagga-dev mailing list > > > [email protected] > > > https://lists.quagga.net/mailman/listinfo/quagga-dev > >
_______________________________________________ Quagga-dev mailing list [email protected] https://lists.quagga.net/mailman/listinfo/quagga-dev
