CEASE NOTIFICATION for OPEN parsing errors seems, to my reading of RFC4271
ยง6.2 to be incorrect.

* bgp_packet.c: (bgp_open_receive) OPEN/UNACEP_HOLDTIME is not an
  appropriate error subcode if bgp_open_option_parse returns an error.  Set
  it to "Unspecific". Where a more specific subcode is appropriate, then lower
  level should send that.
* bgp_open.c: (bgp_open_option_parse) Malformed OPENs should result in
  NOTIFICATION with OPEN error, and OPEN/UNSPECIFIC sub-code - not CEASE.
  (bgp_capability_{parse,orf_entry}) ditto.
* bgpd.h: Add BGP_NOTIFY_OPEN_UNSPECIFIC for 0.  Note that IANA lists 0 as
  reserved in the OPEN error sub-code registry, but RFC4271 page 32 says 0
  is the "Unspecific" OPEN error subcode.

  Have emailed IANA, they says it's a known errate to 4271 under review.

  Some inspiration from Cumulus' bgpd-capability-cleanup.patch, though
  v different result.
---
 bgpd/bgp_open.c   | 15 +++++++++------
 bgpd/bgp_packet.c |  2 +-
 bgpd/bgpd.h       |  1 +
 3 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/bgpd/bgp_open.c b/bgpd/bgp_open.c
index b9d6e93..4709871 100644
--- a/bgpd/bgp_open.c
+++ b/bgpd/bgp_open.c
@@ -235,7 +235,7 @@ bgp_capability_orf_entry (struct peer *peer, struct 
capability_header *hdr)
       zlog_info ("%s ORF Capability entry length error,"
                  " Cap length %u, num %u",
                  peer->host, hdr->length, entry.num);
-      bgp_notify_send (peer, BGP_NOTIFY_CEASE, 0);
+      bgp_notify_send (peer, BGP_NOTIFY_OPEN_ERR, BGP_NOTIFY_OPEN_UNSPECIFIC);
       return -1;
     }
 
@@ -469,7 +469,7 @@ bgp_capability_parse (struct peer *peer, size_t length, int 
*mp_capability,
       if (stream_get_getp(s) + 2 > end)
        {
          zlog_info ("%s Capability length error (< header)", peer->host);
-         bgp_notify_send (peer, BGP_NOTIFY_CEASE, 0);
+         bgp_notify_send (peer, BGP_NOTIFY_OPEN_ERR, 
BGP_NOTIFY_OPEN_UNSPECIFIC);
          return -1;
        }
       
@@ -481,7 +481,7 @@ bgp_capability_parse (struct peer *peer, size_t length, int 
*mp_capability,
       if (start + caphdr.length > end)
        {
          zlog_info ("%s Capability length error (< length)", peer->host);
-         bgp_notify_send (peer, BGP_NOTIFY_CEASE, 0);
+         bgp_notify_send (peer, BGP_NOTIFY_OPEN_ERR, 
BGP_NOTIFY_OPEN_UNSPECIFIC);
          return -1;
        }
       
@@ -511,7 +511,8 @@ bgp_capability_parse (struct peer *peer, size_t length, int 
*mp_capability,
                              LOOKUP (capcode_str, caphdr.code),
                              caphdr.length, 
                             (unsigned) cap_minsizes[caphdr.code]);
-                  bgp_notify_send (peer, BGP_NOTIFY_CEASE, 0);
+                  bgp_notify_send (peer, BGP_NOTIFY_OPEN_ERR,
+                                  BGP_NOTIFY_OPEN_UNSPECIFIC);
                   return -1;
                 }
           /* we deliberately ignore unknown codes, see below */
@@ -727,7 +728,8 @@ bgp_open_option_parse (struct peer *peer, u_char length, 
int *mp_capability)
       if (STREAM_READABLE(s) < 2)
        {
          zlog_info ("%s Option length error", peer->host);
-         bgp_notify_send (peer, BGP_NOTIFY_CEASE, 0);
+         bgp_notify_send (peer, BGP_NOTIFY_OPEN_ERR, 
+                                BGP_NOTIFY_OPEN_UNSPECIFIC);
          return -1;
        }
 
@@ -739,7 +741,8 @@ bgp_open_option_parse (struct peer *peer, u_char length, 
int *mp_capability)
       if (STREAM_READABLE (s) < opt_length)
        {
          zlog_info ("%s Option length error", peer->host);
-         bgp_notify_send (peer, BGP_NOTIFY_CEASE, 0);
+         bgp_notify_send (peer, BGP_NOTIFY_OPEN_ERR, 
+                                BGP_NOTIFY_OPEN_UNSPECIFIC);
          return -1;
        }
 
diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c
index e1ae494..032808f 100644
--- a/bgpd/bgp_packet.c
+++ b/bgpd/bgp_packet.c
@@ -1540,7 +1540,7 @@ bgp_open_receive (struct peer *peer, bgp_size_t size)
         {
           bgp_notify_send (peer,
                  BGP_NOTIFY_OPEN_ERR,
-                 BGP_NOTIFY_OPEN_UNACEP_HOLDTIME);
+                 BGP_NOTIFY_OPEN_UNSPECIFIC);
          return ret;
         }
     }
diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h
index 95c16de..51501f1 100644
--- a/bgpd/bgpd.h
+++ b/bgpd/bgpd.h
@@ -674,6 +674,7 @@ struct bgp_nlri
 #define BGP_NOTIFY_HEADER_MAX                    4
 
 /* BGP_NOTIFY_OPEN_ERR sub codes.  */
+#define BGP_NOTIFY_OPEN_UNSPECIFIC               0
 #define BGP_NOTIFY_OPEN_UNSUP_VERSION            1
 #define BGP_NOTIFY_OPEN_BAD_PEER_AS              2
 #define BGP_NOTIFY_OPEN_BAD_BGP_IDENT            3
-- 
2.5.0


_______________________________________________
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev

Reply via email to