Better reporting of both local and remote errors.

This commit is contained in:
Martin Mares 2000-04-25 21:13:25 +00:00
parent a47a01083b
commit efcece2da3
4 changed files with 109 additions and 52 deletions

View file

@ -22,7 +22,7 @@
#include "bgp.h" #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 { struct attr_desc {
char *name; /* FIXME: Use the same names as in filters */ 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 */ /* Check if all mandatory attributes are present */
if (mandatory) 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]; code = bgp_mandatory_attrs[i];
if (!(seen[code/8] & (1 << (code%8)))) 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; return NULL;
} }
} }
@ -854,11 +854,11 @@ bgp_decode_attrs(struct bgp_conn *conn, byte *attr, unsigned int len, struct lin
return a; return a;
malformed: malformed:
bgp_error(conn, 3, 1, len, 0); bgp_error(conn, 3, 1, NULL, 0);
return NULL; return NULL;
err: err:
bgp_error(conn, 3, errcode, code, 0); /* FIXME: Return attribute data! */ bgp_error(conn, 3, errcode, z-2, l+2);
return NULL; return NULL;
} }
@ -868,7 +868,7 @@ bgp_get_attr(eattr *a, byte *buf)
unsigned int i = EA_ID(a->id); unsigned int i = EA_ID(a->id);
struct attr_desc *d; 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]; d = &bgp_attr_table[i];
buf += bsprintf(buf, "%s", d->name); buf += bsprintf(buf, "%s", d->name);

View file

@ -94,7 +94,7 @@ bgp_graceful_close_conn(struct bgp_conn *c)
case BS_OPENSENT: case BS_OPENSENT:
case BS_OPENCONFIRM: case BS_OPENCONFIRM:
case BS_ESTABLISHED: case BS_ESTABLISHED:
bgp_error(c, 6, 0, 0, 0); bgp_error(c, 6, 0, NULL, 0);
return 1; return 1;
default: default:
bug("bgp_graceful_close_conn: Unknown state %d", c->state); 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; struct bgp_conn *conn = t->data;
DBG("BGP: Hold timeout, closing connection\n"); DBG("BGP: Hold timeout, closing connection\n");
bgp_error(conn, 4, 0, 0, 0); bgp_error(conn, 4, 0, NULL, 0);
} }
static void static void
@ -408,16 +408,16 @@ bgp_init(struct proto_config *C)
} }
void 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) if (c->error_flag)
return; return;
bgp_log_error(c->bgp, "Error", code, subcode, data, (len > 0) ? len : -len);
c->error_flag = 1; c->error_flag = 1;
c->notify_code = code; c->notify_code = code;
c->notify_subcode = subcode; c->notify_subcode = subcode;
c->notify_arg = data; c->notify_data = data;
c->notify_arg_size = len; c->notify_size = (len > 0) ? len : 0;
if (c->primary) if (c->primary)
proto_notify_state(&c->bgp->p, PS_STOP); proto_notify_state(&c->bgp->p, PS_STOP);
bgp_schedule_packet(c, PKT_NOTIFICATION); bgp_schedule_packet(c, PKT_NOTIFICATION);

View file

@ -38,7 +38,8 @@ struct bgp_conn {
struct timer *hold_timer; struct timer *hold_timer;
struct timer *keepalive_timer; struct timer *keepalive_timer;
int packets_to_send; /* Bitmap of packet types to be sent */ 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 error_flag; /* Error state, ignore all input */
int primary; /* This connection is primary */ int primary; /* This connection is primary */
unsigned hold_time, keepalive_time; /* Times calculated from my and neighbor's requirements */ 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_start_timer(struct timer *t, int value);
void bgp_check(struct bgp_config *c); 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); void bgp_close_conn(struct bgp_conn *c);
/* attrs.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_schedule_packet(struct bgp_conn *conn, int type);
void bgp_tx(struct birdsock *sk); void bgp_tx(struct birdsock *sk);
int bgp_rx(struct birdsock *sk, int size); 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 */ /* Packet types */

View file

@ -21,17 +21,11 @@
static byte * static byte *
bgp_create_notification(struct bgp_conn *conn, byte *buf) 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[0] = conn->notify_code;
buf[1] = conn->notify_subcode; buf[1] = conn->notify_subcode;
switch (conn->notify_arg_size) memcpy(buf+2, conn->notify_data, conn->notify_size);
{ return buf + 2 + conn->notify_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");
}
} }
static byte * static byte *
@ -205,7 +199,7 @@ bgp_parse_options(struct bgp_conn *conn, byte *opt, int len)
while (len > 0) while (len > 0)
{ {
if (len < 2 || len < 2 + opt[1]) 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 #ifdef LOCAL_DEBUG
{ {
int i; int i;
@ -228,7 +222,7 @@ bgp_parse_options(struct bgp_conn *conn, byte *opt, int len)
* to do so. Also, capability negotiation with * to do so. Also, capability negotiation with
* Cisco routers doesn't work without that. * 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; return 0;
} }
len -= 2 + opt[1]; len -= 2 + opt[1];
@ -248,26 +242,26 @@ bgp_rx_open(struct bgp_conn *conn, byte *pkt, int len)
/* Check state */ /* Check state */
if (conn->state != BS_OPENSENT) if (conn->state != BS_OPENSENT)
{ bgp_error(conn, 5, 0, conn->state, 0); } { bgp_error(conn, 5, 0, NULL, 0); }
/* Check message contents */ /* Check message contents */
if (len < 29 || len != 29 + pkt[28]) 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) 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); as = get_u16(pkt+20);
hold = get_u16(pkt+22); hold = get_u16(pkt+22);
id = get_u32(pkt+24); id = get_u32(pkt+24);
DBG("BGP: OPEN as=%d hold=%d id=%08x\n", as, hold, id); DBG("BGP: OPEN as=%d hold=%d id=%08x\n", as, hold, id);
if (cf->remote_as && as != p->remote_as) 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) 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; p->remote_id = id;
if (bgp_parse_options(conn, pkt+29, pkt[28])) if (bgp_parse_options(conn, pkt+29, pkt[28]))
return; return;
if (!id || id == 0xffffffff || id == p->local_id) 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 */ /* Check the other connection */
other = (conn == &p->outgoing_conn) ? &p->incoming_conn : &p->outgoing_conn; 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 */ /* Should close the other connection */
DBG("BGP: Collision, closing the other connection\n"); DBG("BGP: Collision, closing the other connection\n");
bgp_error(other, 6, 0, 0, 0); bgp_error(other, 6, 0, NULL, 0);
break; break;
} }
/* Fall thru */ /* Fall thru */
case BS_ESTABLISHED: case BS_ESTABLISHED:
/* Should close this connection */ /* Should close this connection */
DBG("BGP: Collision, closing this connection\n"); DBG("BGP: Collision, closing this connection\n");
bgp_error(conn, 6, 0, 0, 0); bgp_error(conn, 6, 0, NULL, 0);
return; return;
default: default:
bug("bgp_rx_open: Unknown state"); 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 b = *pp++; \
int q; \ int q; \
ll--; \ 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; \ q = (b+7) / 8; \
if (ll < q) goto malformed; \ if (ll < q) goto malformed; \
memcpy(&prefix, pp, q); \ memcpy(&prefix, pp, q); \
@ -348,13 +342,13 @@ bgp_rx_update(struct bgp_conn *conn, byte *pkt, int len)
DBG("BGP: UPDATE\n"); DBG("BGP: UPDATE\n");
if (conn->state != BS_ESTABLISHED) 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); bgp_start_timer(conn->hold_timer, conn->hold_time);
/* Find parts of the packet and check sizes */ /* Find parts of the packet and check sizes */
if (len < 23) if (len < 23)
{ {
bgp_error(conn, 1, 2, len, 2); bgp_error(conn, 1, 2, pkt+16, 2);
return; return;
} }
withdrawn = pkt + 21; withdrawn = pkt + 21;
@ -403,7 +397,69 @@ bgp_rx_update(struct bgp_conn *conn, byte *pkt, int len)
malformed: malformed:
if (a) if (a)
rta_free(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; i<len; i++)
t += bsprintf(t, "%02x", data[i]);
}
*t = 0;
log(L_REMOTE "%s: %s: %s%s", p->p.name, msg, name, argbuf);
} }
static void static void
@ -413,18 +469,10 @@ bgp_rx_notification(struct bgp_conn *conn, byte *pkt, int len)
if (len < 21) if (len < 21)
{ {
bgp_error(conn, 1, 2, len, 2); bgp_error(conn, 1, 2, pkt+16, 2);
return; return;
} }
switch (len) bgp_log_error(conn->bgp, "Received error notification", pkt[19], pkt[20], pkt+21, len-21);
{
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 */
conn->error_flag = 1; conn->error_flag = 1;
if (conn->primary) if (conn->primary)
proto_notify_state(&conn->bgp->p, PS_STOP); 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: case BS_ESTABLISHED:
break; break;
default: 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_UPDATE: return bgp_rx_update(conn, pkt, len);
case PKT_NOTIFICATION: return bgp_rx_notification(conn, pkt, len); case PKT_NOTIFICATION: return bgp_rx_notification(conn, pkt, len);
case PKT_KEEPALIVE: return bgp_rx_keepalive(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) 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"); DBG("BGP: Error, dropping input\n");
return 1; sk->rx_hook = NULL;
return 0;
} }
for(i=0; i<16; i++) for(i=0; i<16; i++)
if (pkt_start[i] != 0xff) if (pkt_start[i] != 0xff)
{ {
bgp_error(conn, 1, 1, 0, 0); bgp_error(conn, 1, 1, NULL, 0);
break; break;
} }
len = get_u16(pkt_start+16); len = get_u16(pkt_start+16);
if (len < BGP_HEADER_LENGTH || len > BGP_MAX_PACKET_LENGTH) 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; break;
} }
if (end < pkt_start + len) if (end < pkt_start + len)