On Fri, Nov 04, 2022 at 10:18:26AM +0300, Bars Bars wrote:
> Hi, Claudio!
> 
> It seems there were at least two issues:
> 1. VPN routes were never installed to fib (with errno 'Network is
> unreachable'
> returned when send_rtmsg tried to writev them)
> 2. kroute_remove brakes when prefix withdraw comes from rde (with 'Not
> handled AID')
> 
> I applied your patch and vpn routes now get installed to the fib!
> But kroute_remove cannot handle vpn prefixes withdrawal still.
> I manually triggered prefix withdraw on the other side of bgp session and
> hooked the prefix at kroute_remove just before it returned -1.
> "
> kroute_remove: rd 65001:100 10.42.200.9/32 NH ???
> kroute_remove: not handled AID
> "
> So I extend the patch abit and the issue 2 seems to go:
> (Not sure that I did it right. Also don't know if kf->nexthop = '???' is ok
> in kroute_remove during withdrawal, but fib reflects correctly.)
> 

It seems that was only part of the fix for withdraws. There is another bug
in the nlri parser where the code fails to properly jump over the implicit
label.

This diff seems to work for me. The util.c changes fix the problem when
parsing the MP_UNREACH_NLRI attribute.

I tested the v4 case but for v6 another can of worms showed up. It is not
posible to configure IPv6 addrs on mpe(4) which makes it impossible to
inject IPv6 VPN routes into the rdomain :(
-- 
:wq Claudio

Index: kroute.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/kroute.c,v
retrieving revision 1.301
diff -u -p -r1.301 kroute.c
--- kroute.c    18 Oct 2022 09:30:29 -0000      1.301
+++ kroute.c    4 Nov 2022 10:10:58 -0000
@@ -580,6 +580,9 @@ krVPN4_change(struct ktable *kt, struct 
            (kf->prefix.labelstack[2] << 8);
        mplslabel = htonl(mplslabel);
 
+       kf->flags |= F_MPLS;
+       kf->mplslabel = mplslabel;
+
        /* for blackhole and reject routes nexthop needs to be 127.0.0.1 */
        if (kf->flags & (F_BLACKHOLE|F_REJECT))
                kf->nexthop.v4.s_addr = htonl(INADDR_LOOPBACK);
@@ -590,6 +593,7 @@ krVPN4_change(struct ktable *kt, struct 
                        return (-1);
        } else {
                kr->mplslabel = mplslabel;
+               kr->flags |= F_MPLS;
                kr->ifindex = kf->ifindex;
                kr->nexthop.s_addr = kf->nexthop.v4.s_addr;
                rtlabel_unref(kr->labelid);
@@ -632,6 +636,9 @@ krVPN6_change(struct ktable *kt, struct 
            (kf->prefix.labelstack[2] << 8);
        mplslabel = htonl(mplslabel);
 
+       kf->flags |= F_MPLS;
+       kf->mplslabel = mplslabel;
+
        /* for blackhole and reject routes nexthop needs to be ::1 */
        if (kf->flags & (F_BLACKHOLE|F_REJECT))
                memcpy(&kf->nexthop.v6, &lo6, sizeof(kf->nexthop.v6));
@@ -642,6 +649,7 @@ krVPN6_change(struct ktable *kt, struct 
                        return (-1);
        } else {
                kr6->mplslabel = mplslabel;
+               kr6->flags |= F_MPLS;
                kr6->ifindex = kf->ifindex;
                memcpy(&kr6->nexthop, &kf->nexthop.v6, sizeof(struct in6_addr));
                kr6->nexthop_scope_id = kf->nexthop.scope_id;
@@ -1878,9 +1886,11 @@ kroute_remove(struct ktable *kt, struct 
 
        switch (kf->prefix.aid) {
        case AID_INET:
+       case AID_VPN_IPv4:
                multipath = kroute4_remove(kt, kf, any);
                break;
        case AID_INET6:
+       case AID_VPN_IPv6:
                multipath = kroute6_remove(kt, kf, any);
                break;
        default:
Index: util.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/util.c,v
retrieving revision 1.71
diff -u -p -r1.71 util.c
--- util.c      17 Aug 2022 15:15:26 -0000      1.71
+++ util.c      4 Nov 2022 10:08:24 -0000
@@ -131,7 +131,9 @@ log_rd(uint64_t rd)
                snprintf(buf, sizeof(buf), "rd %s:%hu", inet_ntoa(addr), u16);
                break;
        default:
-               return ("rd ?");
+               snprintf(buf, sizeof(buf), "rd #%016llx",
+                   (unsigned long long)rd);
+               break;
        }
        return (buf);
 }
@@ -596,6 +598,7 @@ nlri_get_vpn4(u_char *p, uint16_t len, s
                        return (-1);
                if (withdraw) {
                        /* on withdraw ignore the labelstack all together */
+                       p += 3;
                        plen += 3;
                        pfxlen -= 3 * 8;
                        break;
@@ -659,6 +662,7 @@ nlri_get_vpn6(u_char *p, uint16_t len, s
                        return (-1);
                if (withdraw) {
                        /* on withdraw ignore the labelstack all together */
+                       p += 3;
                        plen += 3;
                        pfxlen -= 3 * 8;
                        break;

Reply via email to