Hey Donald when you get to bgpd-capability-cleanup.patch it touches many of the same bgp_notify_send() calls. The codes/subcodes that Paul has below look like a better fit than what is in our patch.
Daniel On Wed, Nov 25, 2015 at 2:35 PM, Daniel Walton <[email protected]> wrote: > Looks good > > Daniel > > On Wed, Nov 25, 2015 at 12:14 PM, Paul Jakma <[email protected]> wrote: > >> 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 >> [email protected] >> https://lists.quagga.net/mailman/listinfo/quagga-dev > > >
_______________________________________________ Quagga-dev mailing list [email protected] https://lists.quagga.net/mailman/listinfo/quagga-dev
