From efcece2da3054d9a0e5b5d2233549b3323428023 Mon Sep 17 00:00:00 2001 From: Martin Mares Date: Tue, 25 Apr 2000 21:13:25 +0000 Subject: [PATCH] Better reporting of both local and remote errors. --- proto/bgp/attrs.c | 12 ++-- proto/bgp/bgp.c | 12 ++-- proto/bgp/bgp.h | 6 +- proto/bgp/packets.c | 131 +++++++++++++++++++++++++++++++------------- 4 files changed, 109 insertions(+), 52 deletions(-) diff --git a/proto/bgp/attrs.c b/proto/bgp/attrs.c index 1a75884e..8bf730bc 100644 --- a/proto/bgp/attrs.c +++ b/proto/bgp/attrs.c @@ -22,7 +22,7 @@ #include "bgp.h" -static int bgp_mandatory_attrs[] = { BA_ORIGIN, BA_AS_PATH, BA_NEXT_HOP }; +static byte bgp_mandatory_attrs[] = { BA_ORIGIN, BA_AS_PATH, BA_NEXT_HOP }; struct attr_desc { char *name; /* FIXME: Use the same names as in filters */ @@ -805,12 +805,12 @@ bgp_decode_attrs(struct bgp_conn *conn, byte *attr, unsigned int len, struct lin /* Check if all mandatory attributes are present */ if (mandatory) { - for(i=0; i < sizeof(bgp_mandatory_attrs)/sizeof(bgp_mandatory_attrs[0]); i++) + for(i=0; i < ARRAY_SIZE(bgp_mandatory_attrs); i++) { code = bgp_mandatory_attrs[i]; if (!(seen[code/8] & (1 << (code%8)))) { - bgp_error(conn, 3, 3, code, 1); + bgp_error(conn, 3, 3, &bgp_mandatory_attrs[i], 1); return NULL; } } @@ -854,11 +854,11 @@ bgp_decode_attrs(struct bgp_conn *conn, byte *attr, unsigned int len, struct lin return a; malformed: - bgp_error(conn, 3, 1, len, 0); + bgp_error(conn, 3, 1, NULL, 0); return NULL; err: - bgp_error(conn, 3, errcode, code, 0); /* FIXME: Return attribute data! */ + bgp_error(conn, 3, errcode, z-2, l+2); return NULL; } @@ -868,7 +868,7 @@ bgp_get_attr(eattr *a, byte *buf) unsigned int i = EA_ID(a->id); struct attr_desc *d; - if (i && i < sizeof(bgp_attr_table)/sizeof(bgp_attr_table[0])) + if (i && i < ARRAY_SIZE(bgp_attr_table)) { d = &bgp_attr_table[i]; buf += bsprintf(buf, "%s", d->name); diff --git a/proto/bgp/bgp.c b/proto/bgp/bgp.c index 27ade8ed..403d8583 100644 --- a/proto/bgp/bgp.c +++ b/proto/bgp/bgp.c @@ -94,7 +94,7 @@ bgp_graceful_close_conn(struct bgp_conn *c) case BS_OPENSENT: case BS_OPENCONFIRM: case BS_ESTABLISHED: - bgp_error(c, 6, 0, 0, 0); + bgp_error(c, 6, 0, NULL, 0); return 1; default: bug("bgp_graceful_close_conn: Unknown state %d", c->state); @@ -160,7 +160,7 @@ bgp_hold_timeout(timer *t) struct bgp_conn *conn = t->data; DBG("BGP: Hold timeout, closing connection\n"); - bgp_error(conn, 4, 0, 0, 0); + bgp_error(conn, 4, 0, NULL, 0); } static void @@ -408,16 +408,16 @@ bgp_init(struct proto_config *C) } void -bgp_error(struct bgp_conn *c, unsigned code, unsigned subcode, unsigned data, unsigned len) +bgp_error(struct bgp_conn *c, unsigned code, unsigned subcode, byte *data, int len) { - DBG("BGP: Error %d,%d,%d,%d\n", code, subcode, data, len); /* FIXME: Better messages */ if (c->error_flag) return; + bgp_log_error(c->bgp, "Error", code, subcode, data, (len > 0) ? len : -len); c->error_flag = 1; c->notify_code = code; c->notify_subcode = subcode; - c->notify_arg = data; - c->notify_arg_size = len; + c->notify_data = data; + c->notify_size = (len > 0) ? len : 0; if (c->primary) proto_notify_state(&c->bgp->p, PS_STOP); bgp_schedule_packet(c, PKT_NOTIFICATION); diff --git a/proto/bgp/bgp.h b/proto/bgp/bgp.h index 2e22111d..85399c1b 100644 --- a/proto/bgp/bgp.h +++ b/proto/bgp/bgp.h @@ -38,7 +38,8 @@ struct bgp_conn { struct timer *hold_timer; struct timer *keepalive_timer; int packets_to_send; /* Bitmap of packet types to be sent */ - int notify_code, notify_subcode, notify_arg, notify_arg_size; + int notify_code, notify_subcode, notify_size; + byte *notify_data; int error_flag; /* Error state, ignore all input */ int primary; /* This connection is primary */ unsigned hold_time, keepalive_time; /* Times calculated from my and neighbor's requirements */ @@ -90,7 +91,7 @@ extern struct linpool *bgp_linpool; void bgp_start_timer(struct timer *t, int value); void bgp_check(struct bgp_config *c); -void bgp_error(struct bgp_conn *c, unsigned code, unsigned subcode, unsigned data, unsigned len); +void bgp_error(struct bgp_conn *c, unsigned code, unsigned subcode, byte *data, int len); void bgp_close_conn(struct bgp_conn *c); /* attrs.c */ @@ -109,6 +110,7 @@ void bgp_free_bucket(struct bgp_proto *p, struct bgp_bucket *buck); void bgp_schedule_packet(struct bgp_conn *conn, int type); void bgp_tx(struct birdsock *sk); int bgp_rx(struct birdsock *sk, int size); +void bgp_log_error(struct bgp_proto *p, char *msg, unsigned code, unsigned subcode, byte *data, unsigned len); /* Packet types */ diff --git a/proto/bgp/packets.c b/proto/bgp/packets.c index 504af93b..2f25a300 100644 --- a/proto/bgp/packets.c +++ b/proto/bgp/packets.c @@ -21,17 +21,11 @@ static byte * bgp_create_notification(struct bgp_conn *conn, byte *buf) { - DBG("BGP: Sending notification: code=%d, sub=%d, arg=%d:%d\n", conn->notify_code, conn->notify_subcode, conn->notify_arg, conn->notify_arg_size); + DBG("BGP: Sending notification: code=%d, sub=%d\n", conn->notify_code, conn->notify_subcode); buf[0] = conn->notify_code; buf[1] = conn->notify_subcode; - switch (conn->notify_arg_size) - { - case 0: return buf + 1; - case 1: buf[2] = conn->notify_arg; return buf+3; - case 2: put_u16(buf+2, conn->notify_arg); return buf+4; - case 4: put_u32(buf+2, conn->notify_arg); return buf+6; - default: bug("bgp_create_notification: unknown error code size"); - } + memcpy(buf+2, conn->notify_data, conn->notify_size); + return buf + 2 + conn->notify_size; } static byte * @@ -205,7 +199,7 @@ bgp_parse_options(struct bgp_conn *conn, byte *opt, int len) while (len > 0) { if (len < 2 || len < 2 + opt[1]) - { bgp_error(conn, 2, 0, 0, 0); return 0; } + { bgp_error(conn, 2, 0, NULL, 0); return 0; } #ifdef LOCAL_DEBUG { int i; @@ -228,7 +222,7 @@ bgp_parse_options(struct bgp_conn *conn, byte *opt, int len) * to do so. Also, capability negotiation with * Cisco routers doesn't work without that. */ - bgp_error(conn, 2, 4, opt[0], 1); + bgp_error(conn, 2, 4, opt, opt[1]); return 0; } len -= 2 + opt[1]; @@ -248,26 +242,26 @@ bgp_rx_open(struct bgp_conn *conn, byte *pkt, int len) /* Check state */ if (conn->state != BS_OPENSENT) - { bgp_error(conn, 5, 0, conn->state, 0); } + { bgp_error(conn, 5, 0, NULL, 0); } /* Check message contents */ if (len < 29 || len != 29 + pkt[28]) - { bgp_error(conn, 1, 2, len, 2); return; } + { bgp_error(conn, 1, 2, pkt+16, 2); return; } if (pkt[19] != BGP_VERSION) - { bgp_error(conn, 2, 1, pkt[19], 1); return; } /* RFC 1771 says 16 bits, draft-09 tells to use 8 */ + { bgp_error(conn, 2, 1, pkt+19, 1); return; } /* RFC 1771 says 16 bits, draft-09 tells to use 8 */ as = get_u16(pkt+20); hold = get_u16(pkt+22); id = get_u32(pkt+24); DBG("BGP: OPEN as=%d hold=%d id=%08x\n", as, hold, id); if (cf->remote_as && as != p->remote_as) - { bgp_error(conn, 2, 2, as, 0); return; } + { bgp_error(conn, 2, 2, pkt+20, -2); return; } if (hold > 0 && hold < 3) - { bgp_error(conn, 2, 6, hold, 0); return; } + { bgp_error(conn, 2, 6, pkt+22, 2); return; } p->remote_id = id; if (bgp_parse_options(conn, pkt+29, pkt[28])) return; if (!id || id == 0xffffffff || id == p->local_id) - { bgp_error(conn, 2, 3, id, 0); return; } + { bgp_error(conn, 2, 3, pkt+24, -4); return; } /* Check the other connection */ other = (conn == &p->outgoing_conn) ? &p->incoming_conn : &p->outgoing_conn; @@ -286,14 +280,14 @@ bgp_rx_open(struct bgp_conn *conn, byte *pkt, int len) { /* Should close the other connection */ DBG("BGP: Collision, closing the other connection\n"); - bgp_error(other, 6, 0, 0, 0); + bgp_error(other, 6, 0, NULL, 0); break; } /* Fall thru */ case BS_ESTABLISHED: /* Should close this connection */ DBG("BGP: Collision, closing this connection\n"); - bgp_error(conn, 6, 0, 0, 0); + bgp_error(conn, 6, 0, NULL, 0); return; default: bug("bgp_rx_open: Unknown state"); @@ -322,7 +316,7 @@ bgp_rx_open(struct bgp_conn *conn, byte *pkt, int len) int b = *pp++; \ int q; \ ll--; \ - if (b > BITS_PER_IP_ADDRESS) { bgp_error(conn, 3, 10, b, 0); return; } \ + if (b > BITS_PER_IP_ADDRESS) { bgp_error(conn, 3, 10, NULL, 0); return; } \ q = (b+7) / 8; \ if (ll < q) goto malformed; \ memcpy(&prefix, pp, q); \ @@ -348,13 +342,13 @@ bgp_rx_update(struct bgp_conn *conn, byte *pkt, int len) DBG("BGP: UPDATE\n"); if (conn->state != BS_ESTABLISHED) - { bgp_error(conn, 5, 0, conn->state, 0); return; } + { bgp_error(conn, 5, 0, NULL, 0); return; } bgp_start_timer(conn->hold_timer, conn->hold_time); /* Find parts of the packet and check sizes */ if (len < 23) { - bgp_error(conn, 1, 2, len, 2); + bgp_error(conn, 1, 2, pkt+16, 2); return; } withdrawn = pkt + 21; @@ -403,7 +397,69 @@ bgp_rx_update(struct bgp_conn *conn, byte *pkt, int len) malformed: if (a) rta_free(a); - bgp_error(conn, 3, 1, len, 0); + bgp_error(conn, 3, 1, NULL, 0); +} + +static struct { + byte major, minor; + byte *msg; +} bgp_msg_table[] = { + { 1, 0, "Invalid message header" }, + { 1, 1, "Connection not synchronized" }, + { 1, 2, "Bad message length" }, + { 1, 3, "Bad message type" }, + { 2, 0, "Invalid OPEN message" }, + { 2, 1, "Unsupported version number" }, + { 2, 2, "Bad peer AS" }, + { 2, 3, "Bad BGP identifier" }, + { 2, 4, "Unsupported optional parameter" }, + { 2, 5, "Authentication failure" }, + { 2, 6, "Unacceptable hold time" }, + { 2, 7, "Required capability missing" }, /* capability negotiation draft */ + { 3, 0, "Invalid UPDATE message" }, + { 3, 1, "Malformed attribute list" }, + { 3, 2, "Unrecognized well-known attribute" }, + { 3, 3, "Missing mandatory attribute" }, + { 3, 4, "Invalid attribute flags" }, + { 3, 5, "Invalid attribute length" }, + { 3, 6, "Invalid ORIGIN attribute" }, + { 3, 7, "AS routing loop" }, /* Deprecated */ + { 3, 8, "Invalid NEXT_HOP attribute" }, + { 3, 9, "Optional attribute error" }, + { 3, 10, "Invalid network field" }, + { 3, 11, "Malformed AS_PATH" }, + { 4, 0, "Hold timer expired" }, + { 5, 0, "Finite state machine error" }, + { 6, 0, "Cease" } +}; + +void +bgp_log_error(struct bgp_proto *p, char *msg, unsigned code, unsigned subcode, byte *data, unsigned len) +{ + byte *name, namebuf[16]; + byte *t, argbuf[36]; + unsigned i; + + bsprintf(namebuf, "%d.%d", code, subcode); + name = namebuf; + for (i=0; i < ARRAY_SIZE(bgp_msg_table); i++) + if (bgp_msg_table[i].major == code && bgp_msg_table[i].minor == subcode) + { + name = bgp_msg_table[i].msg; + break; + } + t = argbuf; + if (len) + { + *t++ = ':'; + *t++ = ' '; + if (len > 16) + len = 16; + for (i=0; ip.name, msg, name, argbuf); } static void @@ -413,18 +469,10 @@ bgp_rx_notification(struct bgp_conn *conn, byte *pkt, int len) if (len < 21) { - bgp_error(conn, 1, 2, len, 2); + bgp_error(conn, 1, 2, pkt+16, 2); return; } - switch (len) - { - case 21: arg = 0; break; - case 22: arg = pkt[21]; break; - case 23: arg = get_u16(pkt+21); break; - case 25: arg = get_u32(pkt+23); break; - default: DBG("BGP: NOTIFICATION with too much data\n"); /* FIXME */ arg = 0; - } - DBG("BGP: NOTIFICATION %d.%d %08x\n", pkt[19], pkt[20], arg); /* FIXME: Better reporting */ + bgp_log_error(conn->bgp, "Received error notification", pkt[19], pkt[20], pkt+21, len-21); conn->error_flag = 1; if (conn->primary) proto_notify_state(&conn->bgp->p, PS_STOP); @@ -447,7 +495,7 @@ bgp_rx_keepalive(struct bgp_conn *conn, byte *pkt, unsigned len) case BS_ESTABLISHED: break; default: - bgp_error(conn, 5, 0, conn->state, 0); + bgp_error(conn, 5, 0, NULL, 0); } } @@ -461,7 +509,7 @@ bgp_rx_packet(struct bgp_conn *conn, byte *pkt, unsigned len) case PKT_UPDATE: return bgp_rx_update(conn, pkt, len); case PKT_NOTIFICATION: return bgp_rx_notification(conn, pkt, len); case PKT_KEEPALIVE: return bgp_rx_keepalive(conn, pkt, len); - default: bgp_error(conn, 1, 3, pkt[18], 1); + default: bgp_error(conn, 1, 3, pkt+18, 1); } } @@ -478,19 +526,26 @@ bgp_rx(sock *sk, int size) { if (conn->error_flag) { + /* + * We still need to remember the erroneous packet, so that + * we can generate error notifications properly. To avoid + * subsequent reads rewriting the buffer, we just reset the + * rx_hook. + */ DBG("BGP: Error, dropping input\n"); - return 1; + sk->rx_hook = NULL; + return 0; } for(i=0; i<16; i++) if (pkt_start[i] != 0xff) { - bgp_error(conn, 1, 1, 0, 0); + bgp_error(conn, 1, 1, NULL, 0); break; } len = get_u16(pkt_start+16); if (len < BGP_HEADER_LENGTH || len > BGP_MAX_PACKET_LENGTH) { - bgp_error(conn, 1, 2, len, 2); + bgp_error(conn, 1, 2, pkt_start+16, 2); break; } if (end < pkt_start + len)