On Mon, Mar 21, 2016 at 03:54:38PM +0100, Peter Hessler wrote:
> We ran into a situation where we accidentally blackholed traffic going to
> a new Internet Exchange.  When we added the new vlans and new peers, the
> nexthop address on that vlan was *not* our neighbor's address, but
> instead used our own IP address on that new interface.  "bgpctl show rib",
> showed the correct IP, but the wrong one was used when installing into
> the fib.
> 
> Restarting bgpd installed the correct nexthop.
> 
> Additionally, a user had reported that "network inet connected" didn't
> work any more.  This fix also convinces bgpd to redistribute the network
> as soon as it is created.  The underlying cause was a combination of
> missing the F_CONNECTED flag, as well as having an ifindex of 0, which
> is lo0's index.
> 
> OK?

I noticed this breakage as well some time ago but never managed to debug
it (too many production systems and not enough test systems).
This was caused by the change of cloning routes to show the IP address
instead of the link local one.
Now about the diff I'm not 100% sure. It for sure changes beheviour a bit.
ifindex is now always set and not only for connected routes. Not sure if
this is bad or not...
mpath should probably be set to 0 in the RTF_CONNECTED case to keep same
behaviour. On the other hand it is now possible to have multiple routes to
the same network even for connected routes (not sure if it is possible on
the same prio or not).

IMO this is a more conservative version...

                case AF_INET:
                case AF_INET6:
                        if (rtm->rtm_flags & RTF_CONNECTED) {
                                flags |= F_CONNECTED;
                                ifindex = rtm->rtm_index;
                                sa = NULL;
                                mpath = 0;
                        }
                        break;
 
I think this is less dangerous or what you think?

> Index: kroute.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/kroute.c,v
> retrieving revision 1.207
> diff -u -p -u -p -r1.207 kroute.c
> --- kroute.c  5 Dec 2015 18:28:04 -0000       1.207
> +++ kroute.c  21 Mar 2016 14:44:52 -0000
> @@ -3178,6 +3178,12 @@ dispatch_rtmsg_addr(struct rt_msghdr *rt
>                       sa = NULL;
>                       mpath = 0;      /* link local stuff can't be mpath */
>                       break;
> +             case AF_INET:
> +             case AF_INET6:
> +                     if (rtm->rtm_flags & RTF_CONNECTED)
> +                             flags |= F_CONNECTED;
> +                     ifindex = rtm->rtm_index;
> +                     break;
>               }
>  
>       if (rtm->rtm_type == RTM_DELETE) {
> 
> 
> 
> 
> -- 
> If you are a fatalist, what can you do about it?
>               -- Ann Edwards-Duff

-- 
:wq Claudio

Reply via email to