On Tue, Mar 10, 2009 at 02:46:56PM +0100, Arnoud Vermeer wrote:
> Hi,
> 
> Elisa and I were looking at the production-pilot logs last night and 
> noticed the following:
> 

I finally found some time to look into this and your dumps. The problem is
actually with withdraws that are still totaly fucked up. So the following
diff should fix this issue -- beforehands we just never managed to
correctly withdraw IPv6 prefixes.

This diff fixes two bugs and makes bgpd do the same paranoic checking as
juniper and bails out if there is crap after an empty update.
With this I can correctly withdraw IPv6 prefixes.
-- 
:wq Claudio

Index: rde.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
retrieving revision 1.235
diff -u -p -r1.235 rde.c
--- rde.c       13 Jan 2009 21:35:16 -0000      1.235
+++ rde.c       11 Mar 2009 02:28:44 -0000
@@ -856,8 +856,16 @@ rde_update_dispatch(struct imsg *imsg)
                prefix_remove(peer, &prefix, prefixlen, F_ORIGINAL);
        }
 
-       if (attrpath_len == 0) /* 0 = no NLRI information in this message */
+       if (attrpath_len == 0) {
+               /* 0 = no NLRI information in this message */
+               if (nlri_len != 0) {
+                       /* crap at end of update which should not be there */
+                       rde_update_err(peer, ERR_UPDATE,
+                           ERR_UPD_ATTRLIST, NULL, 0);
+                       return (-1);
+               }
                return (0);
+       }
 
        /* withdraw MP_UNREACH_NLRI if available */
        if (mpa.unreach_len != 0) {
Index: rde_update.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde_update.c,v
retrieving revision 1.64
diff -u -p -r1.64 rde_update.c
--- rde_update.c        13 Jan 2009 21:35:16 -0000      1.64
+++ rde_update.c        11 Mar 2009 01:56:54 -0000
@@ -917,13 +917,7 @@ up_dump_mp_unreach(u_char *buf, u_int16_
                return (NULL);
 
        datalen += 3;   /* afi + safi */
-       if (datalen > 255) {
-               attrlen += 2 + datalen;
-               flags |= ATTR_EXTLEN;
-       } else {
-               attrlen += 1 + datalen;
-               buf++;
-       }
+
        /* prepend header, need to do it reverse */
        /* safi & afi */
        buf[--wpos] = SAFI_UNICAST;
@@ -933,11 +927,15 @@ up_dump_mp_unreach(u_char *buf, u_int16_
 
        /* attribute length */
        if (datalen > 255) {
+               attrlen += 2 + datalen;
+               flags |= ATTR_EXTLEN;
                wpos -= sizeof(u_int16_t);
                tmp = htons(datalen);
                memcpy(buf + wpos, &tmp, sizeof(u_int16_t));
-       } else
+       } else {
+               attrlen += 1 + datalen;
                buf[--wpos] = (u_char)datalen;
+       }
 
        /* mp attribute */
        buf[--wpos] = (u_char)ATTR_MP_UNREACH_NLRI;
@@ -958,7 +956,7 @@ up_dump_mp_unreach(u_char *buf, u_int16_
        /* total length includes the two 2-bytes length fields. */
        *len = attrlen + 2 * sizeof(u_int16_t);
 
-       return (buf);
+       return (buf + wpos);
 }
 
 u_char *

Reply via email to