[bgpd] cleanup, compact and consolidate capability parsing code
2007-07-26 Paul Jakma <paul.jakma@sun.com>
* (general) Clean up and compact capability parsing slightly.
Consolidate validation of length and logging of generic TLV, and
memcpy of capability data, thus removing such from cap specifc
code (not always present or correct).
* bgp_open.h: Add structures for the generic capability TLV header
and for the data formats of the various specific capabilities we
support. Hence remove the badly named, or else misdefined, struct
capability.
* bgp_open.c: (bgp_capability_vty_out) Use struct capability_mp_data.
Do the length checks *before* memcpy()'ing based on that length
(stored capability - should have been validated anyway on input,
but..).
(bgp_afi_safi_valid_indices) new function to validate (afi,safi)
which is about to be used as index into arrays, consolidates
several instances of same, at least one of which appeared to be
incomplete..
(bgp_capability_mp) Much condensed.
(bgp_capability_orf_entry) New, process one ORF entry
(bgp_capability_orf) Condensed. Fixed to process all ORF entries.
(bgp_capability_restart) Condensed, and fixed to use a
cap-specific type, rather than abusing capability_mp.
(struct message capcode_str) added to aid generic logging.
(size_t cap_minsizes[]) added to aid generic validation of
capability length field.
(bgp_capability_parse) Generic logging and validation of TLV
consolidated here. Code compacted as much as possible.
* bgp_packet.c: (bgp_open_receive) Capability parsers now use
streams, so no more need here to manually fudge the input stream
getp.
(bgp_capability_msg_parse) use struct capability_mp_data. Validate
lengths /before/ memcpy. Use bgp_afi_safi_valid_indices.
(bgp_capability_receive) Exported for use by test harness.
* bgp_vty.c: (bgp_show_summary) fix conversion warning
(bgp_show_peer) ditto
* bgp_debug.h: Fix storage 'extern' after type 'const'.
* lib/log.c: (mes_lookup) warning about code not being in
same-number array slot should be debug, not warning. E.g. BGP
has several discontigious number spaces, allocating from
different parts of a space is not uncommon (e.g. IANA
assigned versus vendor-assigned code points in some number
space).
diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c
index 2653201..17ac1f7 100644
--- a/bgpd/bgp_packet.c
+++ b/bgpd/bgp_packet.c
@@ -1371,8 +1371,6 @@
ret = bgp_open_option_parse (peer, optlen, &capability);
if (ret < 0)
return ret;
-
- stream_forward_getp (peer->ibuf, optlen);
}
else
{
@@ -1991,7 +1989,8 @@
bgp_capability_msg_parse (struct peer *peer, u_char *pnt, bgp_size_t length)
{
u_char *end;
- struct capability cap;
+ struct capability_mp_data mpc;
+ struct capability_header *hdr;
u_char action;
struct bgp *bgp;
afi_t afi;
@@ -2001,7 +2000,7 @@
end = pnt + length;
while (pnt < end)
- {
+ {
/* We need at least action, capability code and capability length. */
if (pnt + 3 > end)
{
@@ -2009,12 +2008,9 @@
bgp_notify_send (peer, BGP_NOTIFY_CEASE, 0);
return -1;
}
-
action = *pnt;
-
- /* Fetch structure to the byte stream. */
- memcpy (&cap, pnt + 1, sizeof (struct capability));
-
+ hdr = (struct capability_header *)(pnt + 1);
+
/* Action value check. */
if (action != CAPABILITY_ACTION_SET
&& action != CAPABILITY_ACTION_UNSET)
@@ -2027,77 +2023,77 @@
if (BGP_DEBUG (normal, NORMAL))
zlog_debug ("%s CAPABILITY has action: %d, code: %u, length %u",
- peer->host, action, cap.code, cap.length);
+ peer->host, action, hdr->code, hdr->length);
/* Capability length check. */
- if (pnt + (cap.length + 3) > end)
+ if ((pnt + hdr->length + 3) > end)
{
zlog_info ("%s Capability length error", peer->host);
bgp_notify_send (peer, BGP_NOTIFY_CEASE, 0);
return -1;
}
+ /* Fetch structure to the byte stream. */
+ memcpy (&mpc, pnt + 3, sizeof (struct capability_mp_data));
+
/* We know MP Capability Code. */
- if (cap.code == CAPABILITY_CODE_MP)
+ if (hdr->code == CAPABILITY_CODE_MP)
{
- afi = ntohs (cap.mpc.afi);
- safi = cap.mpc.safi;
+ afi = ntohs (mpc.afi);
+ safi = mpc.safi;
/* Ignore capability when override-capability is set. */
if (CHECK_FLAG (peer->flags, PEER_FLAG_OVERRIDE_CAPABILITY))
continue;
-
+
+ if (!bgp_afi_safi_valid_indices (afi, &safi))
+ {
+ if (BGP_DEBUG (normal, NORMAL))
+ zlog_debug ("%s Dynamic Capability MP_EXT afi/safi invalid",
+ peer->host, afi, safi);
+ continue;
+ }
+
/* Address family check. */
- if ((afi == AFI_IP
- || afi == AFI_IP6)
- && (safi == SAFI_UNICAST
- || safi == SAFI_MULTICAST
- || safi == BGP_SAFI_VPNV4))
- {
- if (BGP_DEBUG (normal, NORMAL))
- zlog_debug ("%s CAPABILITY has %s MP_EXT CAP for afi/safi: %u/%u",
- peer->host,
- action == CAPABILITY_ACTION_SET
- ? "Advertising" : "Removing",
- ntohs(cap.mpc.afi) , cap.mpc.safi);
-
- /* Adjust safi code. */
- if (safi == BGP_SAFI_VPNV4)
- safi = SAFI_MPLS_VPN;
-
- if (action == CAPABILITY_ACTION_SET)
- {
- peer->afc_recv[afi][safi] = 1;
- if (peer->afc[afi][safi])
- {
- peer->afc_nego[afi][safi] = 1;
- bgp_announce_route (peer, afi, safi);
- }
- }
- else
- {
- peer->afc_recv[afi][safi] = 0;
- peer->afc_nego[afi][safi] = 0;
+ if (BGP_DEBUG (normal, NORMAL))
+ zlog_debug ("%s CAPABILITY has %s MP_EXT CAP for afi/safi: %u/%u",
+ peer->host,
+ action == CAPABILITY_ACTION_SET
+ ? "Advertising" : "Removing",
+ ntohs(mpc.afi) , mpc.safi);
+
+ if (action == CAPABILITY_ACTION_SET)
+ {
+ peer->afc_recv[afi][safi] = 1;
+ if (peer->afc[afi][safi])
+ {
+ peer->afc_nego[afi][safi] = 1;
+ bgp_announce_route (peer, afi, safi);
+ }
+ }
+ else
+ {
+ peer->afc_recv[afi][safi] = 0;
+ peer->afc_nego[afi][safi] = 0;
- if (peer_active_nego (peer))
- bgp_clear_route (peer, afi, safi);
- else
- BGP_EVENT_ADD (peer, BGP_Stop);
- }
- }
+ if (peer_active_nego (peer))
+ bgp_clear_route (peer, afi, safi);
+ else
+ BGP_EVENT_ADD (peer, BGP_Stop);
+ }
}
else
{
zlog_warn ("%s unrecognized capability code: %d - ignored",
- peer->host, cap.code);
+ peer->host, hdr->code);
}
- pnt += cap.length + 3;
+ pnt += hdr->length + 3;
}
return 0;
}
/* Dynamic Capability is received. */
-static void
+int
bgp_capability_receive (struct peer *peer, bgp_size_t size)
{
u_char *pnt;
@@ -2130,7 +2126,7 @@
}
/* Parse packet. */
- ret = bgp_capability_msg_parse (peer, pnt, size);
+ return bgp_capability_msg_parse (peer, pnt, size);
}
/* BGP read utility function. */