Merge remote-tracking branch 'origin/master'
diff --git a/bgpd/bgp_advertise.c b/bgpd/bgp_advertise.c
index 87eb7ac..666218f 100644
--- a/bgpd/bgp_advertise.c
+++ b/bgpd/bgp_advertise.c
@@ -140,13 +140,13 @@
baa->refcnt--;
if (baa->refcnt && baa->attr)
- bgp_attr_unintern (baa->attr);
+ bgp_attr_unintern (&baa->attr);
else
{
if (baa->attr)
{
hash_release (hash, baa);
- bgp_attr_unintern (baa->attr);
+ bgp_attr_unintern (&baa->attr);
}
baa_free (baa);
}
@@ -319,7 +319,7 @@
struct peer *peer, afi_t afi, safi_t safi)
{
if (adj->attr)
- bgp_attr_unintern (adj->attr);
+ bgp_attr_unintern (&adj->attr);
if (adj->adv)
bgp_advertise_clean (peer, adj, afi, safi);
@@ -339,7 +339,7 @@
{
if (adj->attr != attr)
{
- bgp_attr_unintern (adj->attr);
+ bgp_attr_unintern (&adj->attr);
adj->attr = bgp_attr_intern (attr);
}
return;
@@ -355,7 +355,7 @@
void
bgp_adj_in_remove (struct bgp_node *rn, struct bgp_adj_in *bai)
{
- bgp_attr_unintern (bai->attr);
+ bgp_attr_unintern (&bai->attr);
BGP_ADJ_IN_DEL (rn, bai);
peer_unlock (bai->peer); /* adj_in peer reference */
XFREE (MTYPE_BGP_ADJ_IN, bai);
diff --git a/bgpd/bgp_aspath.c b/bgpd/bgp_aspath.c
index cf93042..ba4f5b4 100644
--- a/bgpd/bgp_aspath.c
+++ b/bgpd/bgp_aspath.c
@@ -340,19 +340,21 @@
/* Unintern aspath from AS path bucket. */
void
-aspath_unintern (struct aspath *aspath)
+aspath_unintern (struct aspath **aspath)
{
struct aspath *ret;
+ struct aspath *asp = *aspath;
+
+ if (asp->refcnt)
+ asp->refcnt--;
- if (aspath->refcnt)
- aspath->refcnt--;
-
- if (aspath->refcnt == 0)
+ if (asp->refcnt == 0)
{
/* This aspath must exist in aspath hash table. */
- ret = hash_release (ashash, aspath);
+ ret = hash_release (ashash, asp);
assert (ret != NULL);
- aspath_free (aspath);
+ aspath_free (asp);
+ *aspath = NULL;
}
}
@@ -671,79 +673,78 @@
return aspath;
}
-/* parse as-segment byte stream in struct assegment
- *
- * Returns NULL if the AS_PATH or AS4_PATH is not valid.
- */
-static struct assegment *
-assegments_parse (struct stream *s, size_t length, int use32bit, int as4_path)
+/* parse as-segment byte stream in struct assegment */
+static int
+assegments_parse (struct stream *s, size_t length,
+ struct assegment **result, int use32bit)
{
struct assegment_header segh;
struct assegment *seg, *prev = NULL, *head = NULL;
+ size_t bytes = 0;
- assert (length > 0); /* does not expect empty AS_PATH or AS4_PATH */
+ /* empty aspath (ie iBGP or somesuch) */
+ if (length == 0)
+ return 0;
if (BGP_DEBUG (as4, AS4_SEGMENT))
zlog_debug ("[AS4SEG] Parse aspath segment: got total byte length %lu",
(unsigned long) length);
-
- /* double check that length does not exceed stream */
- if (STREAM_READABLE(s) < length)
- return NULL;
+ /* basic checks */
+ if ((STREAM_READABLE(s) < length)
+ || (STREAM_READABLE(s) < AS_HEADER_SIZE)
+ || (length % AS16_VALUE_SIZE ))
+ return -1;
- /* deal with each segment in turn */
- while (length > 0)
+ while (bytes < length)
{
int i;
size_t seg_size;
- /* softly softly, get the header first on its own */
- if (length < AS_HEADER_SIZE)
+ if ((length - bytes) <= AS_HEADER_SIZE)
{
- assegment_free_all (head);
- return NULL;
+ if (head)
+ assegment_free_all (head);
+ return -1;
}
+ /* softly softly, get the header first on its own */
segh.type = stream_getc (s);
segh.length = stream_getc (s);
seg_size = ASSEGMENT_SIZE(segh.length, use32bit);
- /* includes the header bytes */
if (BGP_DEBUG (as4, AS4_SEGMENT))
zlog_debug ("[AS4SEG] Parse aspath segment: got type %d, length %d",
segh.type, segh.length);
+ /* check it.. */
+ if ( ((bytes + seg_size) > length)
+ /* 1771bis 4.3b: seg length contains one or more */
+ || (segh.length == 0)
+ /* Paranoia in case someone changes type of segment length.
+ * Shift both values by 0x10 to make the comparison operate
+ * on more, than 8 bits (otherwise it's a warning, bug #564).
+ */
+ || ((sizeof segh.length > 1)
+ && (0x10 + segh.length > 0x10 + AS_SEGMENT_MAX)))
+ {
+ if (head)
+ assegment_free_all (head);
+ return -1;
+ }
+
switch (segh.type)
{
case AS_SEQUENCE:
case AS_SET:
- break ;
-
case AS_CONFED_SEQUENCE:
case AS_CONFED_SET:
- if (!as4_path)
- break ;
- /* RFC4893 3: "invalid for the AS4_PATH attribute" */
- /* fall through */
-
- default: /* reject unknown or invalid AS_PATH segment types */
- seg_size = 0 ;
+ break;
+ default:
+ if (head)
+ assegment_free_all (head);
+ return -1;
}
-
- /* Stop now if segment is not valid (discarding anything collected to date)
- *
- * RFC4271 4.3, Path Attributes, b) AS_PATH:
- *
- * "path segment value field contains one or more AS numbers"
- */
- if ((seg_size == 0) || (seg_size > length) || (segh.length == 0))
- {
- assegment_free_all (head);
- return NULL;
- }
-
- length -= seg_size ;
/* now its safe to trust lengths */
seg = assegment_new (segh.type, segh.length);
@@ -756,52 +757,47 @@
for (i = 0; i < segh.length; i++)
seg->as[i] = (use32bit) ? stream_getl (s) : stream_getw (s);
+ bytes += seg_size;
+
if (BGP_DEBUG (as4, AS4_SEGMENT))
- zlog_debug ("[AS4SEG] Parse aspath segment: length left: %lu",
- (unsigned long) length);
+ zlog_debug ("[AS4SEG] Parse aspath segment: Bytes now: %lu",
+ (unsigned long) bytes);
prev = seg;
}
- return assegment_normalise (head);
+ *result = assegment_normalise (head);
+ return 0;
}
-/* AS path parse function -- parses AS_PATH and AS4_PATH attributes
- *
- * Requires: s -- stream, currently positioned before first segment
- * of AS_PATH or AS4_PATH (ie after attribute header)
- * length -- length of the value of the AS_PATH or AS4_PATH
- * use32bit -- true <=> 4Byte ASN, otherwise 2Byte ASN
- * as4_path -- true <=> AS4_PATH, otherwise AS_PATH
- *
- * Returns: if valid: address of struct aspath in the hash of known aspaths,
- * with reference count incremented.
- * else: NULL
- *
- * NB: empty AS path (length == 0) is valid. The returned struct aspath will
- * have segments == NULL and str == zero length string (unique).
+/* AS path parse function. pnt is a pointer to byte stream and length
+ is length of byte stream. If there is same AS path in the the AS
+ path hash then return it else make new AS path structure.
+
+ On error NULL is returned.
*/
struct aspath *
-aspath_parse (struct stream *s, size_t length, int use32bit, int as4_path)
+aspath_parse (struct stream *s, size_t length, int use32bit)
{
struct aspath as;
struct aspath *find;
- /* Parse each segment and construct normalised list of struct assegment */
- memset (&as, 0, sizeof (struct aspath));
- if (length != 0)
- {
- as.segments = assegments_parse (s, length, use32bit, as4_path);
+ /* If length is odd it's malformed AS path. */
+ /* Nit-picking: if (use32bit == 0) it is malformed if odd,
+ * otherwise its malformed when length is larger than 2 and (length-2)
+ * is not dividable by 4.
+ * But... this time we're lazy
+ */
+ if (length % AS16_VALUE_SIZE )
+ return NULL;
- if (as.segments == NULL)
- return NULL ; /* Invalid AS_PATH or AS4_PATH */
- } ;
+ memset (&as, 0, sizeof (struct aspath));
+ if (assegments_parse (s, length, &as.segments, use32bit) < 0)
+ return NULL;
/* If already same aspath exist then return it. */
find = hash_get (ashash, &as, aspath_hash_alloc);
- assert(find) ; /* valid aspath, so must find or create */
-
/* aspath_hash_alloc dupes segments too. that probably could be
* optimised out.
*/
@@ -809,6 +805,8 @@
if (as.str)
XFREE (MTYPE_AS_STR, as.str);
+ if (! find)
+ return NULL;
find->refcnt++;
return find;
@@ -1632,7 +1630,7 @@
struct aspath *
aspath_empty (void)
{
- return aspath_parse (NULL, 0, 1, 0); /* 32Bit ;-) not AS4_PATH */
+ return aspath_parse (NULL, 0, 1); /* 32Bit ;-) */
}
struct aspath *
diff --git a/bgpd/bgp_aspath.h b/bgpd/bgp_aspath.h
index d63b914..d55f9ce 100644
--- a/bgpd/bgp_aspath.h
+++ b/bgpd/bgp_aspath.h
@@ -65,7 +65,7 @@
/* Prototypes. */
extern void aspath_init (void);
extern void aspath_finish (void);
-extern struct aspath *aspath_parse (struct stream *, size_t, int, int);
+extern struct aspath *aspath_parse (struct stream *, size_t, int);
extern struct aspath *aspath_dup (struct aspath *);
extern struct aspath *aspath_aggregate (struct aspath *, struct aspath *);
extern struct aspath *aspath_prepend (struct aspath *, struct aspath *);
@@ -80,7 +80,7 @@
extern struct aspath *aspath_str2aspath (const char *);
extern void aspath_free (struct aspath *);
extern struct aspath *aspath_intern (struct aspath *);
-extern void aspath_unintern (struct aspath *);
+extern void aspath_unintern (struct aspath **);
extern const char *aspath_print (struct aspath *);
extern void aspath_print_vty (struct vty *, const char *, struct aspath *, const char *);
extern void aspath_print_all_vty (struct vty *);
diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c
index 01598c8..d43c104 100644
--- a/bgpd/bgp_attr.c
+++ b/bgpd/bgp_attr.c
@@ -500,6 +500,7 @@
attre->ecommunity = ecommunity_intern (attre->ecommunity);
else
attre->ecommunity->refcnt++;
+
}
if (attre->cluster)
{
@@ -516,10 +517,10 @@
attre->transit->refcnt++;
}
}
-
+
find = (struct attr *) hash_get (attrhash, attr, bgp_attr_hash_alloc);
find->refcnt++;
-
+
return find;
}
@@ -561,7 +562,7 @@
new = bgp_attr_intern (&attr);
bgp_attr_extra_free (&attr);
- aspath_unintern (new->aspath);
+ aspath_unintern (&new->aspath);
return new;
}
@@ -613,52 +614,67 @@
new = bgp_attr_intern (&attr);
bgp_attr_extra_free (&attr);
- aspath_unintern (new->aspath);
+ aspath_unintern (&new->aspath);
return new;
}
+/* Unintern just the sub-components of the attr, but not the attr */
+void
+bgp_attr_unintern_sub (struct attr *attr)
+{
+ /* aspath refcount shoud be decrement. */
+ if (attr->aspath)
+ aspath_unintern (&attr->aspath);
+ UNSET_FLAG(attr->flag, BGP_ATTR_AS_PATH);
+
+ if (attr->community)
+ community_unintern (&attr->community);
+ UNSET_FLAG(attr->flag, BGP_ATTR_COMMUNITIES);
+
+ if (attr->extra)
+ {
+ if (attr->extra->ecommunity)
+ ecommunity_unintern (&attr->extra->ecommunity);
+ UNSET_FLAG(attr->flag, BGP_ATTR_EXT_COMMUNITIES);
+
+ if (attr->extra->cluster)
+ cluster_unintern (attr->extra->cluster);
+ UNSET_FLAG(attr->flag, BGP_ATTR_CLUSTER_LIST);
+
+ if (attr->extra->transit)
+ transit_unintern (attr->extra->transit);
+ }
+}
+
/* Free bgp attribute and aspath. */
void
-bgp_attr_unintern (struct attr *attr)
+bgp_attr_unintern (struct attr **attr)
{
struct attr *ret;
- struct aspath *aspath;
- struct community *community;
- struct ecommunity *ecommunity = NULL;
- struct cluster_list *cluster = NULL;
- struct transit *transit = NULL;
-
+ struct attr tmp;
+
/* Decrement attribute reference. */
- attr->refcnt--;
- aspath = attr->aspath;
- community = attr->community;
- if (attr->extra)
+ (*attr)->refcnt--;
+
+ tmp = *(*attr);
+
+ if ((*attr)->extra)
{
- ecommunity = attr->extra->ecommunity;
- cluster = attr->extra->cluster;
- transit = attr->extra->transit;
+ tmp.extra = bgp_attr_extra_new ();
+ memcpy (tmp.extra, (*attr)->extra, sizeof (struct attr_extra));
}
-
+
/* If reference becomes zero then free attribute object. */
- if (attr->refcnt == 0)
+ if ((*attr)->refcnt == 0)
{
- ret = hash_release (attrhash, attr);
+ ret = hash_release (attrhash, *attr);
assert (ret != NULL);
- bgp_attr_extra_free (attr);
- XFREE (MTYPE_ATTR, attr);
+ bgp_attr_extra_free (*attr);
+ XFREE (MTYPE_ATTR, *attr);
+ *attr = NULL;
}
- /* aspath refcount shoud be decrement. */
- if (aspath)
- aspath_unintern (aspath);
- if (community)
- community_unintern (community);
- if (ecommunity)
- ecommunity_unintern (ecommunity);
- if (cluster)
- cluster_unintern (cluster);
- if (transit)
- transit_unintern (transit);
+ bgp_attr_unintern_sub (&tmp);
}
void
@@ -671,8 +687,9 @@
if (attr->extra)
{
struct attr_extra *attre = attr->extra;
+
if (attre->ecommunity && ! attre->ecommunity->refcnt)
- ecommunity_free (attre->ecommunity);
+ ecommunity_free (&attre->ecommunity);
if (attre->cluster && ! attre->cluster->refcnt)
cluster_free (attre->cluster);
if (attre->transit && ! attre->transit->refcnt)
@@ -680,8 +697,69 @@
}
}
+/* Implement draft-scudder-idr-optional-transitive behaviour and
+ * avoid resetting sessions for malformed attributes which are
+ * are partial/optional and hence where the error likely was not
+ * introduced by the sending neighbour.
+ */
+static bgp_attr_parse_ret_t
+bgp_attr_malformed (struct peer *peer, u_char type, u_char flag,
+ u_char subcode, u_char *startp, bgp_size_t length)
+{
+ /* Only relax error handling for eBGP peers */
+ if (peer_sort (peer) != BGP_PEER_EBGP)
+ {
+ bgp_notify_send_with_data (peer, BGP_NOTIFY_UPDATE_ERR, subcode,
+ startp, length);
+ return BGP_ATTR_PARSE_ERROR;
+
+ }
+
+ switch (type) {
+ /* where an optional attribute is inconsequential, e.g. it does not affect
+ * route selection, and can be safely ignored then any such attributes
+ * which are malformed should just be ignored and the route processed as
+ * normal.
+ */
+ case BGP_ATTR_AS4_AGGREGATOR:
+ case BGP_ATTR_AGGREGATOR:
+ case BGP_ATTR_ATOMIC_AGGREGATE:
+ return BGP_ATTR_PARSE_PROCEED;
+
+ /* Core attributes, particularly ones which may influence route
+ * selection should always cause session resets
+ */
+ case BGP_ATTR_ORIGIN:
+ case BGP_ATTR_AS_PATH:
+ case BGP_ATTR_NEXT_HOP:
+ case BGP_ATTR_MULTI_EXIT_DISC:
+ case BGP_ATTR_LOCAL_PREF:
+ case BGP_ATTR_COMMUNITIES:
+ case BGP_ATTR_ORIGINATOR_ID:
+ case BGP_ATTR_CLUSTER_LIST:
+ case BGP_ATTR_MP_REACH_NLRI:
+ case BGP_ATTR_MP_UNREACH_NLRI:
+ case BGP_ATTR_EXT_COMMUNITIES:
+ bgp_notify_send_with_data (peer, BGP_NOTIFY_UPDATE_ERR, subcode,
+ startp, length);
+ return BGP_ATTR_PARSE_ERROR;
+ }
+
+ /* Partial optional attributes that are malformed should not cause
+ * the whole session to be reset. Instead treat it as a withdrawal
+ * of the routes, if possible.
+ */
+ if (CHECK_FLAG (flag, BGP_ATTR_FLAG_TRANS)
+ && CHECK_FLAG (flag, BGP_ATTR_FLAG_OPTIONAL)
+ && CHECK_FLAG (flag, BGP_ATTR_FLAG_PARTIAL))
+ return BGP_ATTR_PARSE_WITHDRAW;
+
+ /* default to reset */
+ return BGP_ATTR_PARSE_ERROR;
+}
+
/* Get origin attribute of the update message. */
-static int
+static bgp_attr_parse_ret_t
bgp_attr_origin (struct peer *peer, bgp_size_t length,
struct attr *attr, u_char flag, u_char *startp)
{
@@ -699,11 +777,9 @@
{
zlog (peer->log, LOG_ERR,
"Origin attribute flag isn't transitive %d", flag);
- bgp_notify_send_with_data (peer,
- BGP_NOTIFY_UPDATE_ERR,
- BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
- startp, total);
- return -1;
+ return bgp_attr_malformed (peer, BGP_ATTR_ORIGIN, flag,
+ BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
+ startp, total);
}
/* If any recognized attribute has Attribute Length that conflicts
@@ -715,10 +791,9 @@
{
zlog (peer->log, LOG_ERR, "Origin attribute length is not one %d",
length);
- bgp_notify_send_with_data (peer, BGP_NOTIFY_UPDATE_ERR,
- BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
- startp, total);
- return -1;
+ return bgp_attr_malformed (peer, BGP_ATTR_ORIGIN, flag,
+ BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
+ startp, total);
}
/* Fetch origin attribute. */
@@ -733,12 +808,9 @@
{
zlog (peer->log, LOG_ERR, "Origin attribute value is invalid %d",
attr->origin);
-
- bgp_notify_send_with_data (peer,
- BGP_NOTIFY_UPDATE_ERR,
- BGP_NOTIFY_UPDATE_INVAL_ORIGIN,
- startp, total);
- return -1;
+ return bgp_attr_malformed (peer, BGP_ATTR_ORIGIN, flag,
+ BGP_NOTIFY_UPDATE_INVAL_ORIGIN,
+ startp, total);
}
/* Set oring attribute flag. */
@@ -746,82 +818,54 @@
return 0;
}
-/* Parse AS path information. This function is wrapper of aspath_parse.
- *
- * Parses AS_PATH or AS4_PATH.
- *
- * Returns: if valid: address of struct aspath in the hash of known aspaths,
- * with reference count incremented.
- * else: NULL
- *
- * NB: empty AS path (length == 0) is valid. The returned struct aspath will
- * have segments == NULL and str == zero length string (unique).
- */
-static struct aspath *
+
+/* Parse AS path information. This function is wrapper of
+ aspath_parse. */
+static int
bgp_attr_aspath (struct peer *peer, bgp_size_t length,
- struct attr *attr, u_char flag, u_char *startp, int as4_path)
+ struct attr *attr, u_char flag, u_char *startp)
{
- u_char require ;
- struct aspath *asp ;
+ bgp_size_t total;
- /* Check the attribute flags */
- require = as4_path ? BGP_ATTR_FLAG_OPTIONAL | BGP_ATTR_FLAG_TRANS
- : BGP_ATTR_FLAG_TRANS ;
+ total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3);
- if ((flag & (BGP_ATTR_FLAG_OPTIONAL | BGP_ATTR_FLAG_TRANS)) != require)
+ /* Flag check. */
+ if (CHECK_FLAG (flag, BGP_ATTR_FLAG_OPTIONAL)
+ || ! CHECK_FLAG (flag, BGP_ATTR_FLAG_TRANS))
{
- const char* path_type ;
- bgp_size_t total;
-
- path_type = as4_path ? "AS4_PATH" : "AS_PATH" ;
-
- if (!CHECK_FLAG(flag, BGP_ATTR_FLAG_TRANS))
zlog (peer->log, LOG_ERR,
- "%s attribute flag isn't transitive %d", path_type, flag) ;
-
- if ((flag & BGP_ATTR_FLAG_OPTIONAL) != (require & BGP_ATTR_FLAG_OPTIONAL))
- zlog (peer->log, LOG_ERR,
- "%s attribute flag must %sbe optional %d", path_type,
- (flag & BGP_ATTR_FLAG_OPTIONAL) ? "not " : "", flag) ;
-
- total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3);
-
- bgp_notify_send_with_data (peer,
- BGP_NOTIFY_UPDATE_ERR,
- BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
- startp, total);
-
- return NULL ;
- } ;
-
- /* Parse the AS_PATH/AS4_PATH body.
- *
- * For AS_PATH peer with AS4 => 4Byte ASN otherwise 2Byte ASN
- * AS4_PATH 4Byte ASN
- */
- asp = aspath_parse (peer->ibuf, length,
- as4_path || CHECK_FLAG (peer->cap, PEER_CAP_AS4_RCV), as4_path) ;
-
- if (asp != NULL)
- {
- attr->flag |= ATTR_FLAG_BIT (as4_path ? BGP_ATTR_AS4_PATH
- : BGP_ATTR_AS_PATH) ;
+ "As-Path attribute flag isn't transitive %d", flag);
+ return bgp_attr_malformed (peer, BGP_ATTR_AS_PATH, flag,
+ BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
+ startp, total);
}
- else
+
+ /*
+ * peer with AS4 => will get 4Byte ASnums
+ * otherwise, will get 16 Bit
+ */
+ attr->aspath = aspath_parse (peer->ibuf, length,
+ CHECK_FLAG (peer->cap, PEER_CAP_AS4_RCV));
+
+ /* In case of IBGP, length will be zero. */
+ if (! attr->aspath)
{
- zlog (peer->log, LOG_ERR, "Malformed AS path length is %d", length);
+ zlog (peer->log, LOG_ERR,
+ "Malformed AS path from %s, length is %d",
+ peer->host, length);
+ return bgp_attr_malformed (peer, BGP_ATTR_AS_PATH, flag,
+ BGP_NOTIFY_UPDATE_MAL_AS_PATH,
+ NULL, 0);
+ }
- /* TODO: should BGP_NOTIFY_UPDATE_MAL_AS_PATH be sent for AS4_PATH ?? */
- bgp_notify_send (peer,
- BGP_NOTIFY_UPDATE_ERR,
- BGP_NOTIFY_UPDATE_MAL_AS_PATH);
- } ;
+ /* Set aspath attribute flag. */
+ attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_AS_PATH);
- return asp ;
+ return BGP_ATTR_PARSE_PROCEED;
}
-static int bgp_attr_aspath_check( struct peer *peer,
- struct attr *attr)
+static bgp_attr_parse_ret_t
+bgp_attr_aspath_check (struct peer *peer, struct attr *attr, u_char flag)
{
/* These checks were part of bgp_attr_aspath, but with
* as4 we should to check aspath things when
@@ -840,10 +884,9 @@
(peer_sort (peer) == BGP_PEER_EBGP && aspath_confed_check (attr->aspath)))
{
zlog (peer->log, LOG_ERR, "Malformed AS path from %s", peer->host);
- bgp_notify_send (peer,
- BGP_NOTIFY_UPDATE_ERR,
- BGP_NOTIFY_UPDATE_MAL_AS_PATH);
- return -1;
+ return bgp_attr_malformed (peer, BGP_ATTR_AS_PATH, flag,
+ BGP_NOTIFY_UPDATE_MAL_AS_PATH,
+ NULL, 0);
}
/* First AS check for EBGP. */
@@ -854,10 +897,9 @@
{
zlog (peer->log, LOG_ERR,
"%s incorrect first AS (must be %u)", peer->host, peer->as);
- bgp_notify_send (peer,
- BGP_NOTIFY_UPDATE_ERR,
- BGP_NOTIFY_UPDATE_MAL_AS_PATH);
- return -1;
+ return bgp_attr_malformed (peer, BGP_ATTR_AS_PATH, flag,
+ BGP_NOTIFY_UPDATE_MAL_AS_PATH,
+ NULL, 0);
}
}
@@ -867,16 +909,57 @@
{
aspath = aspath_dup (attr->aspath);
aspath = aspath_add_seq (aspath, peer->change_local_as);
- aspath_unintern (attr->aspath);
+ aspath_unintern (&attr->aspath);
attr->aspath = aspath_intern (aspath);
}
- return 0;
+ return BGP_ATTR_PARSE_PROCEED;
+}
+/* Parse AS4 path information. This function is another wrapper of
+ aspath_parse. */
+static int
+bgp_attr_as4_path (struct peer *peer, bgp_size_t length,
+ struct attr *attr, u_char flag, u_char *startp,
+ struct aspath **as4_path)
+{
+ bgp_size_t total;
+
+ total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3);
+
+ /* Flag check. */
+ if (!CHECK_FLAG (flag, BGP_ATTR_FLAG_OPTIONAL)
+ || !CHECK_FLAG (flag, BGP_ATTR_FLAG_TRANS))
+ {
+ zlog (peer->log, LOG_ERR,
+ "As4-Path attribute flag isn't optional/transitive %d", flag);
+ return bgp_attr_malformed (peer, BGP_ATTR_AS_PATH, flag,
+ BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
+ startp, total);
+ }
+
+ *as4_path = aspath_parse (peer->ibuf, length, 1);
+
+ /* In case of IBGP, length will be zero. */
+ if (!*as4_path)
+ {
+ zlog (peer->log, LOG_ERR,
+ "Malformed AS4 path from %s, length is %d",
+ peer->host, length);
+ return bgp_attr_malformed (peer, BGP_ATTR_AS4_PATH, flag,
+ BGP_NOTIFY_UPDATE_MAL_AS_PATH,
+ NULL, 0);
+ }
+
+ /* Set aspath attribute flag. */
+ if (as4_path)
+ attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_AS4_PATH);
+
+ return BGP_ATTR_PARSE_PROCEED;
}
/* Nexthop attribute. */
-static int
+static bgp_attr_parse_ret_t
bgp_attr_nexthop (struct peer *peer, bgp_size_t length,
struct attr *attr, u_char flag, u_char *startp)
{
@@ -890,11 +973,9 @@
{
zlog (peer->log, LOG_ERR,
"Origin attribute flag isn't transitive %d", flag);
- bgp_notify_send_with_data (peer,
- BGP_NOTIFY_UPDATE_ERR,
- BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
- startp, total);
- return -1;
+ return bgp_attr_malformed (peer, BGP_ATTR_NEXT_HOP, flag,
+ BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
+ startp, total);
}
/* Check nexthop attribute length. */
@@ -903,21 +984,19 @@
zlog (peer->log, LOG_ERR, "Nexthop attribute length isn't four [%d]",
length);
- bgp_notify_send_with_data (peer,
- BGP_NOTIFY_UPDATE_ERR,
- BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
- startp, total);
- return -1;
+ return bgp_attr_malformed (peer, BGP_ATTR_NEXT_HOP, flag,
+ BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
+ startp, total);
}
attr->nexthop.s_addr = stream_get_ipv4 (peer->ibuf);
attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_NEXT_HOP);
- return 0;
+ return BGP_ATTR_PARSE_PROCEED;
}
/* MED atrribute. */
-static int
+static bgp_attr_parse_ret_t
bgp_attr_med (struct peer *peer, bgp_size_t length,
struct attr *attr, u_char flag, u_char *startp)
{
@@ -930,23 +1009,21 @@
{
zlog (peer->log, LOG_ERR,
"MED attribute length isn't four [%d]", length);
-
- bgp_notify_send_with_data (peer,
- BGP_NOTIFY_UPDATE_ERR,
- BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
- startp, total);
- return -1;
+
+ return bgp_attr_malformed (peer, BGP_ATTR_MULTI_EXIT_DISC, flag,
+ BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
+ startp, total);
}
attr->med = stream_getl (peer->ibuf);
attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_MULTI_EXIT_DISC);
- return 0;
+ return BGP_ATTR_PARSE_PROCEED;
}
/* Local preference attribute. */
-static int
+static bgp_attr_parse_ret_t
bgp_attr_local_pref (struct peer *peer, bgp_size_t length,
struct attr *attr, u_char flag)
{
@@ -956,7 +1033,7 @@
if (peer_sort (peer) == BGP_PEER_EBGP)
{
stream_forward_getp (peer->ibuf, length);
- return 0;
+ return BGP_ATTR_PARSE_PROCEED;
}
if (length == 4)
@@ -967,7 +1044,7 @@
/* Set atomic aggregate flag. */
attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_LOCAL_PREF);
- return 0;
+ return BGP_ATTR_PARSE_PROCEED;
}
/* Atomic aggregate. */
@@ -979,16 +1056,15 @@
{
zlog (peer->log, LOG_ERR, "Bad atomic aggregate length %d", length);
- bgp_notify_send (peer,
- BGP_NOTIFY_UPDATE_ERR,
- BGP_NOTIFY_UPDATE_ATTR_LENG_ERR);
- return -1;
+ return bgp_attr_malformed (peer, BGP_ATTR_ATOMIC_AGGREGATE, flag,
+ BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
+ NULL, 0);
}
/* Set atomic aggregate flag. */
attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_ATOMIC_AGGREGATE);
- return 0;
+ return BGP_ATTR_PARSE_PROCEED;
}
/* Aggregator attribute */
@@ -1000,17 +1076,16 @@
struct attr_extra *attre = bgp_attr_extra_get (attr);
/* peer with AS4 will send 4 Byte AS, peer without will send 2 Byte */
- if ( CHECK_FLAG (peer->cap, PEER_CAP_AS4_RCV ) )
+ if (CHECK_FLAG (peer->cap, PEER_CAP_AS4_RCV))
wantedlen = 8;
if (length != wantedlen)
{
zlog (peer->log, LOG_ERR, "Aggregator length is not %d [%d]", wantedlen, length);
- bgp_notify_send (peer,
- BGP_NOTIFY_UPDATE_ERR,
- BGP_NOTIFY_UPDATE_ATTR_LENG_ERR);
- return -1;
+ return bgp_attr_malformed (peer, BGP_ATTR_AGGREGATOR, flag,
+ BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
+ NULL, 0);
}
if ( CHECK_FLAG (peer->cap, PEER_CAP_AS4_RCV ) )
@@ -1022,36 +1097,35 @@
/* Set atomic aggregate flag. */
attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_AGGREGATOR);
- return 0;
+ return BGP_ATTR_PARSE_PROCEED;
}
/* New Aggregator attribute */
-static int
+static bgp_attr_parse_ret_t
bgp_attr_as4_aggregator (struct peer *peer, bgp_size_t length,
- struct attr *attr, as_t *as4_aggregator_as,
+ struct attr *attr, u_char flag,
+ as_t *as4_aggregator_as,
struct in_addr *as4_aggregator_addr)
{
if (length != 8)
{
zlog (peer->log, LOG_ERR, "New Aggregator length is not 8 [%d]", length);
-
- bgp_notify_send (peer,
- BGP_NOTIFY_UPDATE_ERR,
- BGP_NOTIFY_UPDATE_ATTR_LENG_ERR);
- return -1;
+ return bgp_attr_malformed (peer, BGP_ATTR_AS4_AGGREGATOR, flag,
+ BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
+ NULL, 0);
}
*as4_aggregator_as = stream_getl (peer->ibuf);
as4_aggregator_addr->s_addr = stream_get_ipv4 (peer->ibuf);
attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_AS4_AGGREGATOR);
- return 0;
+ return BGP_ATTR_PARSE_PROCEED;
}
/* Munge Aggregator and New-Aggregator, AS_PATH and NEW_AS_PATH.
*/
-static int
-bgp_attr_munge_as4_attrs (struct peer *peer, struct attr *attr,
+static bgp_attr_parse_ret_t
+bgp_attr_munge_as4_attrs (struct peer *peer, struct attr *attr, u_char flag,
struct aspath *as4_path, as_t as4_aggregator,
struct in_addr *as4_aggregator_addr)
{
@@ -1059,7 +1133,7 @@
struct aspath *newpath;
struct attr_extra *attre = attr->extra;
- if ( CHECK_FLAG (peer->cap, PEER_CAP_AS4_RCV) )
+ if (CHECK_FLAG (peer->cap, PEER_CAP_AS4_RCV))
{
/* peer can do AS4, so we ignore AS4_PATH and AS4_AGGREGATOR
* if given.
@@ -1077,11 +1151,11 @@
peer->host, "AS4 capable peer, yet it sent");
}
- return 0;
+ return BGP_ATTR_PARSE_PROCEED;
}
- if (attr->flag & ( ATTR_FLAG_BIT( BGP_ATTR_AS4_PATH))
- && !(attr->flag & ( ATTR_FLAG_BIT( BGP_ATTR_AS_PATH))))
+ if (attr->flag & (ATTR_FLAG_BIT (BGP_ATTR_AS4_PATH))
+ && !(attr->flag & (ATTR_FLAG_BIT (BGP_ATTR_AS_PATH))))
{
/* Hu? This is not supposed to happen at all!
* got as4_path and no aspath,
@@ -1093,10 +1167,9 @@
zlog (peer->log, LOG_ERR,
"%s BGP not AS4 capable peer sent AS4_PATH but"
" no AS_PATH, cant do anything here", peer->host);
- bgp_notify_send (peer,
- BGP_NOTIFY_UPDATE_ERR,
- BGP_NOTIFY_UPDATE_MAL_ATTR);
- return -1;
+ return bgp_attr_malformed (peer, BGP_ATTR_AS_PATH, flag,
+ BGP_NOTIFY_UPDATE_MAL_ATTR,
+ NULL, 0);
}
/* We have a asn16 peer. First, look for AS4_AGGREGATOR
@@ -1104,7 +1177,7 @@
*/
if (attr->flag & (ATTR_FLAG_BIT (BGP_ATTR_AS4_AGGREGATOR) ) )
{
- if ( attr->flag & (ATTR_FLAG_BIT (BGP_ATTR_AGGREGATOR) ) )
+ if (attr->flag & (ATTR_FLAG_BIT (BGP_ATTR_AGGREGATOR) ) )
{
assert (attre);
@@ -1120,7 +1193,7 @@
* Aggregating node and the AS_PATH is to be
* constructed "as in all other cases"
*/
- if ( attre->aggregator_as != BGP_AS_TRANS )
+ if (attre->aggregator_as != BGP_AS_TRANS)
{
/* ignore */
if ( BGP_DEBUG(as4, AS4))
@@ -1155,24 +1228,27 @@
}
/* need to reconcile NEW_AS_PATH and AS_PATH */
- if ( !ignore_as4_path && (attr->flag & ( ATTR_FLAG_BIT( BGP_ATTR_AS4_PATH))) )
+ if (!ignore_as4_path && (attr->flag & (ATTR_FLAG_BIT( BGP_ATTR_AS4_PATH))))
{
newpath = aspath_reconcile_as4 (attr->aspath, as4_path);
- aspath_unintern (attr->aspath);
+ aspath_unintern (&attr->aspath);
attr->aspath = aspath_intern (newpath);
}
- return 0;
+ return BGP_ATTR_PARSE_PROCEED;
}
/* Community attribute. */
-static int
+static bgp_attr_parse_ret_t
bgp_attr_community (struct peer *peer, bgp_size_t length,
- struct attr *attr, u_char flag)
+ struct attr *attr, u_char flag, u_char *startp)
{
+ bgp_size_t total
+ = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3);
+
if (length == 0)
{
attr->community = NULL;
- return 0;
+ return BGP_ATTR_PARSE_PROCEED;
}
attr->community =
@@ -1182,15 +1258,17 @@
stream_forward_getp (peer->ibuf, length);
if (!attr->community)
- return -1;
+ return bgp_attr_malformed (peer, BGP_ATTR_COMMUNITIES, flag,
+ BGP_NOTIFY_UPDATE_OPT_ATTR_ERR,
+ startp, total);
attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_COMMUNITIES);
- return 0;
+ return BGP_ATTR_PARSE_PROCEED;
}
/* Originator ID attribute. */
-static int
+static bgp_attr_parse_ret_t
bgp_attr_originator_id (struct peer *peer, bgp_size_t length,
struct attr *attr, u_char flag)
{
@@ -1198,10 +1276,9 @@
{
zlog (peer->log, LOG_ERR, "Bad originator ID length %d", length);
- bgp_notify_send (peer,
- BGP_NOTIFY_UPDATE_ERR,
- BGP_NOTIFY_UPDATE_ATTR_LENG_ERR);
- return -1;
+ return bgp_attr_malformed (peer, BGP_ATTR_ORIGINATOR_ID, flag,
+ BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
+ NULL, 0);
}
(bgp_attr_extra_get (attr))->originator_id.s_addr
@@ -1209,11 +1286,11 @@
attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_ORIGINATOR_ID);
- return 0;
+ return BGP_ATTR_PARSE_PROCEED;
}
/* Cluster list attribute. */
-static int
+static bgp_attr_parse_ret_t
bgp_attr_cluster_list (struct peer *peer, bgp_size_t length,
struct attr *attr, u_char flag)
{
@@ -1222,20 +1299,20 @@
{
zlog (peer->log, LOG_ERR, "Bad cluster list length %d", length);
- bgp_notify_send (peer,
- BGP_NOTIFY_UPDATE_ERR,
- BGP_NOTIFY_UPDATE_ATTR_LENG_ERR);
- return -1;
+ return bgp_attr_malformed (peer, BGP_ATTR_CLUSTER_LIST, flag,
+ BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
+ NULL, 0);
}
(bgp_attr_extra_get (attr))->cluster
= cluster_parse ((struct in_addr *)stream_pnt (peer->ibuf), length);
-
- stream_forward_getp (peer->ibuf, length);;
+
+ /* XXX: Fix cluster_parse to use stream API and then remove this */
+ stream_forward_getp (peer->ibuf, length);
attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_CLUSTER_LIST);
- return 0;
+ return BGP_ATTR_PARSE_PROCEED;
}
/* Multiprotocol reachability information parse. */
@@ -1262,7 +1339,7 @@
{
zlog_info ("%s: %s sent invalid length, %lu",
__func__, peer->host, (unsigned long)length);
- return -1;
+ return BGP_ATTR_PARSE_ERROR;
}
/* Load AFI, SAFI. */
@@ -1276,7 +1353,7 @@
{
zlog_info ("%s: %s, MP nexthop length, %u, goes past end of attribute",
__func__, peer->host, attre->mp_nexthop_len);
- return -1;
+ return BGP_ATTR_PARSE_ERROR;
}
/* Nexthop length check. */
@@ -1324,14 +1401,14 @@
default:
zlog_info ("%s: (%s) Wrong multiprotocol next hop length: %d",
__func__, peer->host, attre->mp_nexthop_len);
- return -1;
+ return BGP_ATTR_PARSE_ERROR;
}
if (!LEN_LEFT)
{
zlog_info ("%s: (%s) Failed to read SNPA and NLRI(s)",
__func__, peer->host);
- return -1;
+ return BGP_ATTR_PARSE_ERROR;
}
{
@@ -1347,7 +1424,7 @@
{
zlog_info ("%s: (%s) Failed to read NLRI",
__func__, peer->host);
- return -1;
+ return BGP_ATTR_PARSE_ERROR;
}
if (safi != BGP_SAFI_VPNV4)
@@ -1357,7 +1434,7 @@
{
zlog_info ("%s: (%s) NLRI doesn't pass sanity check",
__func__, peer->host);
- return -1;
+ return BGP_ATTR_PARSE_ERROR;
}
}
@@ -1368,7 +1445,7 @@
stream_forward_getp (s, nlri_len);
- return 0;
+ return BGP_ATTR_PARSE_PROCEED;
#undef LEN_LEFT
}
@@ -1387,7 +1464,7 @@
#define BGP_MP_UNREACH_MIN_SIZE 3
if ((length > STREAM_READABLE(s)) || (length < BGP_MP_UNREACH_MIN_SIZE))
- return -1;
+ return BGP_ATTR_PARSE_ERROR;
afi = stream_getw (s);
safi = stream_getc (s);
@@ -1398,7 +1475,7 @@
{
ret = bgp_nlri_sanity_check (peer, afi, stream_pnt (s), withdraw_len);
if (ret < 0)
- return -1;
+ return BGP_ATTR_PARSE_ERROR;
}
mp_withdraw->afi = afi;
@@ -1408,20 +1485,23 @@
stream_forward_getp (s, withdraw_len);
- return 0;
+ return BGP_ATTR_PARSE_PROCEED;
}
/* Extended Community attribute. */
-static int
+static bgp_attr_parse_ret_t
bgp_attr_ext_communities (struct peer *peer, bgp_size_t length,
- struct attr *attr, u_char flag)
+ struct attr *attr, u_char flag, u_char *startp)
{
+ bgp_size_t total
+ = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3);
+
if (length == 0)
{
if (attr->extra)
attr->extra->ecommunity = NULL;
/* Empty extcomm doesn't seem to be invalid per se */
- return 0;
+ return BGP_ATTR_PARSE_PROCEED;
}
(bgp_attr_extra_get (attr))->ecommunity =
@@ -1430,15 +1510,17 @@
stream_forward_getp (peer->ibuf, length);
if (!attr->extra->ecommunity)
- return -1;
+ return bgp_attr_malformed (peer, BGP_ATTR_EXT_COMMUNITIES,
+ flag, BGP_NOTIFY_UPDATE_OPT_ATTR_ERR,
+ startp, total);
attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_EXT_COMMUNITIES);
- return 0;
+ return BGP_ATTR_PARSE_PROCEED;
}
/* BGP unknown attribute treatment. */
-static int
+static bgp_attr_parse_ret_t
bgp_attr_unknown (struct peer *peer, struct attr *attr, u_char flag,
u_char type, bgp_size_t length, u_char *startp)
{
@@ -1464,20 +1546,17 @@
then the Error Subcode is set to Unrecognized Well-known
Attribute. The Data field contains the unrecognized attribute
(type, length and value). */
- if (! CHECK_FLAG (flag, BGP_ATTR_FLAG_OPTIONAL))
+ if (!CHECK_FLAG (flag, BGP_ATTR_FLAG_OPTIONAL))
{
- /* Adjust startp to do not include flag value. */
- bgp_notify_send_with_data (peer,
- BGP_NOTIFY_UPDATE_ERR,
- BGP_NOTIFY_UPDATE_UNREC_ATTR,
- startp, total);
- return -1;
+ return bgp_attr_malformed (peer, type, flag,
+ BGP_NOTIFY_UPDATE_UNREC_ATTR,
+ startp, total);
}
/* Unrecognized non-transitive optional attributes must be quietly
ignored and not passed along to other BGP peers. */
if (! CHECK_FLAG (flag, BGP_ATTR_FLAG_TRANS))
- return 0;
+ return BGP_ATTR_PARSE_PROCEED;
/* If a path with recognized transitive optional attribute is
accepted and passed along to other BGP peers and the Partial bit
@@ -1500,17 +1579,17 @@
memcpy (transit->val + transit->length, startp, total);
transit->length += total;
- return 0;
+ return BGP_ATTR_PARSE_PROCEED;
}
/* Read attribute of update packet. This function is called from
bgp_update() in bgpd.c. */
-int
+bgp_attr_parse_ret_t
bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size,
struct bgp_nlri *mp_update, struct bgp_nlri *mp_withdraw)
{
int ret;
- u_char flag;
+ u_char flag = 0;
u_char type = 0;
bgp_size_t length;
u_char *startp, *endp;
@@ -1527,7 +1606,7 @@
/* End pointer of BGP attribute. */
endp = BGP_INPUT_PNT (peer) + size;
-
+
/* Get attributes to the end of attribute length. */
while (BGP_INPUT_PNT (peer) < endp)
{
@@ -1543,7 +1622,7 @@
bgp_notify_send (peer,
BGP_NOTIFY_UPDATE_ERR,
BGP_NOTIFY_UPDATE_ATTR_LENG_ERR);
- return -1;
+ return BGP_ATTR_PARSE_ERROR;
}
/* Fetch attribute flag and type. */
@@ -1563,7 +1642,7 @@
bgp_notify_send (peer,
BGP_NOTIFY_UPDATE_ERR,
BGP_NOTIFY_UPDATE_ATTR_LENG_ERR);
- return -1;
+ return BGP_ATTR_PARSE_ERROR;
}
/* Check extended attribue length bit. */
@@ -1585,7 +1664,7 @@
bgp_notify_send (peer,
BGP_NOTIFY_UPDATE_ERR,
BGP_NOTIFY_UPDATE_MAL_ATTR);
- return -1;
+ return BGP_ATTR_PARSE_ERROR;
}
/* Set type to bitmap to check duplicate attribute. `type' is
@@ -1603,7 +1682,7 @@
bgp_notify_send (peer,
BGP_NOTIFY_UPDATE_ERR,
BGP_NOTIFY_UPDATE_ATTR_LENG_ERR);
- return -1;
+ return BGP_ATTR_PARSE_ERROR;
}
/* OK check attribute and store it's value. */
@@ -1613,12 +1692,10 @@
ret = bgp_attr_origin (peer, length, attr, flag, startp);
break;
case BGP_ATTR_AS_PATH:
- attr->aspath = bgp_attr_aspath (peer, length, attr, flag, startp, 0);
- ret = attr->aspath ? 0 : -1 ;
+ ret = bgp_attr_aspath (peer, length, attr, flag, startp);
break;
case BGP_ATTR_AS4_PATH:
- as4_path = bgp_attr_aspath (peer, length, attr, flag, startp, 1);
- ret = as4_path ? 0 : -1 ;
+ ret = bgp_attr_as4_path (peer, length, attr, flag, startp, &as4_path);
break;
case BGP_ATTR_NEXT_HOP:
ret = bgp_attr_nexthop (peer, length, attr, flag, startp);
@@ -1636,10 +1713,12 @@
ret = bgp_attr_aggregator (peer, length, attr, flag);
break;
case BGP_ATTR_AS4_AGGREGATOR:
- ret = bgp_attr_as4_aggregator (peer, length, attr, &as4_aggregator, &as4_aggregator_addr);
+ ret = bgp_attr_as4_aggregator (peer, length, attr, flag,
+ &as4_aggregator,
+ &as4_aggregator_addr);
break;
case BGP_ATTR_COMMUNITIES:
- ret = bgp_attr_community (peer, length, attr, flag);
+ ret = bgp_attr_community (peer, length, attr, flag, startp);
break;
case BGP_ATTR_ORIGINATOR_ID:
ret = bgp_attr_originator_id (peer, length, attr, flag);
@@ -1654,26 +1733,39 @@
ret = bgp_mp_unreach_parse (peer, length, mp_withdraw);
break;
case BGP_ATTR_EXT_COMMUNITIES:
- ret = bgp_attr_ext_communities (peer, length, attr, flag);
+ ret = bgp_attr_ext_communities (peer, length, attr, flag, startp);
break;
default:
ret = bgp_attr_unknown (peer, attr, flag, type, length, startp);
break;
}
-
- /* If error occured immediately return to the caller. */
- if (ret < 0)
+
+ /* If hard error occured immediately return to the caller. */
+ if (ret == BGP_ATTR_PARSE_ERROR)
{
zlog (peer->log, LOG_WARNING,
"%s: Attribute %s, parse error",
peer->host,
LOOKUP (attr_str, type));
- bgp_notify_send (peer,
- BGP_NOTIFY_UPDATE_ERR,
- BGP_NOTIFY_UPDATE_MAL_ATTR);
- return ret;
+ bgp_notify_send (peer,
+ BGP_NOTIFY_UPDATE_ERR,
+ BGP_NOTIFY_UPDATE_MAL_ATTR);
+ if (as4_path)
+ aspath_unintern (&as4_path);
+ return ret;
}
-
+ if (ret == BGP_ATTR_PARSE_WITHDRAW)
+ {
+
+ zlog (peer->log, LOG_WARNING,
+ "%s: Attribute %s, parse error - treating as withdrawal",
+ peer->host,
+ LOOKUP (attr_str, type));
+ if (as4_path)
+ aspath_unintern (&as4_path);
+ return ret;
+ }
+
/* Check the fetched length. */
if (BGP_INPUT_PNT (peer) != attr_endp)
{
@@ -1683,7 +1775,9 @@
bgp_notify_send (peer,
BGP_NOTIFY_UPDATE_ERR,
BGP_NOTIFY_UPDATE_ATTR_LENG_ERR);
- return -1;
+ if (as4_path)
+ aspath_unintern (&as4_path);
+ return BGP_ATTR_PARSE_ERROR;
}
}
@@ -1696,7 +1790,9 @@
bgp_notify_send (peer,
BGP_NOTIFY_UPDATE_ERR,
BGP_NOTIFY_UPDATE_ATTR_LENG_ERR);
- return -1;
+ if (as4_path)
+ aspath_unintern (&as4_path);
+ return BGP_ATTR_PARSE_ERROR;
}
/*
@@ -1710,19 +1806,22 @@
* all attributes first, including these 32bit ones, and now,
* afterwards, we look what and if something is to be done for as4.
*/
- if (bgp_attr_munge_as4_attrs (peer, attr, as4_path,
+ if (bgp_attr_munge_as4_attrs (peer, attr, flag, as4_path,
as4_aggregator, &as4_aggregator_addr))
- return -1;
+ {
+ if (as4_path)
+ aspath_unintern (&as4_path);
+ return BGP_ATTR_PARSE_ERROR;
+ }
/* At this stage, we have done all fiddling with as4, and the
* resulting info is in attr->aggregator resp. attr->aspath
* so we can chuck as4_aggregator and as4_path alltogether in
* order to save memory
*/
- if ( as4_path )
+ if (as4_path)
{
- aspath_unintern( as4_path ); /* unintern - it is in the hash */
- as4_path = NULL;
+ aspath_unintern (&as4_path); /* unintern - it is in the hash */
/* The flag that we got this is still there, but that does not
* do any trouble
*/
@@ -1737,10 +1836,10 @@
* Finally do the checks on the aspath we did not do yet
* because we waited for a potentially synthesized aspath.
*/
- if ( attr->flag & ( ATTR_FLAG_BIT( BGP_ATTR_AS_PATH)))
+ if (attr->flag & (ATTR_FLAG_BIT(BGP_ATTR_AS_PATH)))
{
- ret = bgp_attr_aspath_check( peer, attr );
- if ( ret < 0 )
+ ret = bgp_attr_aspath_check (peer, attr, flag);
+ if (ret != BGP_ATTR_PARSE_PROCEED)
return ret;
}
@@ -1748,7 +1847,7 @@
if (attr->extra && attr->extra->transit)
attr->extra->transit = transit_intern (attr->extra->transit);
- return 0;
+ return BGP_ATTR_PARSE_PROCEED;
}
/* Well-known attribute check. */
@@ -1779,9 +1878,9 @@
BGP_NOTIFY_UPDATE_ERR,
BGP_NOTIFY_UPDATE_MISS_ATTR,
&type, 1);
- return -1;
+ return BGP_ATTR_PARSE_ERROR;
}
- return 0;
+ return BGP_ATTR_PARSE_PROCEED;
}
int stream_put_prefix (struct stream *, struct prefix *);
diff --git a/bgpd/bgp_attr.h b/bgpd/bgp_attr.h
index af9dcf5..c011251 100644
--- a/bgpd/bgp_attr.h
+++ b/bgpd/bgp_attr.h
@@ -132,17 +132,25 @@
#define ATTR_FLAG_BIT(X) (1 << ((X) - 1))
+typedef enum {
+ BGP_ATTR_PARSE_PROCEED = 0,
+ BGP_ATTR_PARSE_ERROR = -1,
+ BGP_ATTR_PARSE_WITHDRAW = -2,
+} bgp_attr_parse_ret_t;
+
/* Prototypes. */
extern void bgp_attr_init (void);
extern void bgp_attr_finish (void);
-extern int bgp_attr_parse (struct peer *, struct attr *, bgp_size_t,
- struct bgp_nlri *, struct bgp_nlri *);
+extern bgp_attr_parse_ret_t bgp_attr_parse (struct peer *, struct attr *,
+ bgp_size_t, struct bgp_nlri *,
+ struct bgp_nlri *);
extern int bgp_attr_check (struct peer *, struct attr *);
extern struct attr_extra *bgp_attr_extra_get (struct attr *);
extern void bgp_attr_extra_free (struct attr *);
extern void bgp_attr_dup (struct attr *, struct attr *);
extern struct attr *bgp_attr_intern (struct attr *attr);
-extern void bgp_attr_unintern (struct attr *);
+extern void bgp_attr_unintern_sub (struct attr *);
+extern void bgp_attr_unintern (struct attr **);
extern void bgp_attr_flush (struct attr *);
extern struct attr *bgp_attr_default_set (struct attr *attr, u_char);
extern struct attr *bgp_attr_default_intern (u_char);
diff --git a/bgpd/bgp_clist.c b/bgpd/bgp_clist.c
index d660167..6c9976e 100644
--- a/bgpd/bgp_clist.c
+++ b/bgpd/bgp_clist.c
@@ -70,7 +70,7 @@
if (entry->config)
XFREE (MTYPE_ECOMMUNITY_STR, entry->config);
if (entry->u.ecom)
- ecommunity_free (entry->u.ecom);
+ ecommunity_free (&entry->u.ecom);
break;
case COMMUNITY_LIST_EXPANDED:
case EXTCOMMUNITY_LIST_EXPANDED:
@@ -806,7 +806,7 @@
entry = community_list_entry_lookup (list, str, direct);
if (ecom)
- ecommunity_free (ecom);
+ ecommunity_free (&ecom);
if (regex)
bgp_regex_free (regex);
diff --git a/bgpd/bgp_community.c b/bgpd/bgp_community.c
index ae1d7a1..2ba45f6 100644
--- a/bgpd/bgp_community.c
+++ b/bgpd/bgp_community.c
@@ -321,21 +321,22 @@
/* Free community attribute. */
void
-community_unintern (struct community *com)
+community_unintern (struct community **com)
{
struct community *ret;
- if (com->refcnt)
- com->refcnt--;
+ if ((*com)->refcnt)
+ (*com)->refcnt--;
/* Pull off from hash. */
- if (com->refcnt == 0)
+ if ((*com)->refcnt == 0)
{
/* Community value com must exist in hash. */
- ret = (struct community *) hash_release (comhash, com);
+ ret = (struct community *) hash_release (comhash, *com);
assert (ret != NULL);
- community_free (com);
+ community_free (*com);
+ *com = NULL;
}
}
diff --git a/bgpd/bgp_community.h b/bgpd/bgp_community.h
index bc1e56e..9e48377 100644
--- a/bgpd/bgp_community.h
+++ b/bgpd/bgp_community.h
@@ -57,7 +57,7 @@
extern struct community *community_uniq_sort (struct community *);
extern struct community *community_parse (u_int32_t *, u_short);
extern struct community *community_intern (struct community *);
-extern void community_unintern (struct community *);
+extern void community_unintern (struct community **);
extern char *community_str (struct community *);
extern unsigned int community_hash_make (struct community *);
extern struct community *community_str2com (const char *);
diff --git a/bgpd/bgp_ecommunity.c b/bgpd/bgp_ecommunity.c
index 8d5fa74..8d91c74 100644
--- a/bgpd/bgp_ecommunity.c
+++ b/bgpd/bgp_ecommunity.c
@@ -42,13 +42,14 @@
/* Allocate ecommunities. */
void
-ecommunity_free (struct ecommunity *ecom)
+ecommunity_free (struct ecommunity **ecom)
{
- if (ecom->val)
- XFREE (MTYPE_ECOMMUNITY_VAL, ecom->val);
- if (ecom->str)
- XFREE (MTYPE_ECOMMUNITY_STR, ecom->str);
- XFREE (MTYPE_ECOMMUNITY, ecom);
+ if ((*ecom)->val)
+ XFREE (MTYPE_ECOMMUNITY_VAL, (*ecom)->val);
+ if ((*ecom)->str)
+ XFREE (MTYPE_ECOMMUNITY_STR, (*ecom)->str);
+ XFREE (MTYPE_ECOMMUNITY, *ecom);
+ ecom = NULL;
}
/* Add a new Extended Communities value to Extended Communities
@@ -197,7 +198,7 @@
find = (struct ecommunity *) hash_get (ecomhash, ecom, hash_alloc_intern);
if (find != ecom)
- ecommunity_free (ecom);
+ ecommunity_free (&ecom);
find->refcnt++;
@@ -209,18 +210,18 @@
/* Unintern Extended Communities Attribute. */
void
-ecommunity_unintern (struct ecommunity *ecom)
+ecommunity_unintern (struct ecommunity **ecom)
{
struct ecommunity *ret;
- if (ecom->refcnt)
- ecom->refcnt--;
-
+ if ((*ecom)->refcnt)
+ (*ecom)->refcnt--;
+
/* Pull off from hash. */
- if (ecom->refcnt == 0)
+ if ((*ecom)->refcnt == 0)
{
/* Extended community must be in the hash. */
- ret = (struct ecommunity *) hash_release (ecomhash, ecom);
+ ret = (struct ecommunity *) hash_release (ecomhash, *ecom);
assert (ret != NULL);
ecommunity_free (ecom);
@@ -516,7 +517,7 @@
if (! keyword_included || keyword)
{
if (ecom)
- ecommunity_free (ecom);
+ ecommunity_free (&ecom);
return NULL;
}
keyword = 1;
@@ -536,7 +537,7 @@
if (! keyword)
{
if (ecom)
- ecommunity_free (ecom);
+ ecommunity_free (&ecom);
return NULL;
}
keyword = 0;
@@ -549,7 +550,7 @@
case ecommunity_token_unknown:
default:
if (ecom)
- ecommunity_free (ecom);
+ ecommunity_free (&ecom);
return NULL;
}
}
diff --git a/bgpd/bgp_ecommunity.h b/bgpd/bgp_ecommunity.h
index 942fdc7..2f59dc4 100644
--- a/bgpd/bgp_ecommunity.h
+++ b/bgpd/bgp_ecommunity.h
@@ -67,13 +67,13 @@
extern void ecommunity_init (void);
extern void ecommunity_finish (void);
-extern void ecommunity_free (struct ecommunity *);
+extern void ecommunity_free (struct ecommunity **);
extern struct ecommunity *ecommunity_parse (u_int8_t *, u_short);
extern struct ecommunity *ecommunity_dup (struct ecommunity *);
extern struct ecommunity *ecommunity_merge (struct ecommunity *, struct ecommunity *);
extern struct ecommunity *ecommunity_intern (struct ecommunity *);
extern int ecommunity_cmp (const void *, const void *);
-extern void ecommunity_unintern (struct ecommunity *);
+extern void ecommunity_unintern (struct ecommunity **);
extern unsigned int ecommunity_hash_make (void *);
extern struct ecommunity *ecommunity_str2com (const char *, int, int);
extern char *ecommunity_ecom2str (struct ecommunity *, int);
diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c
index ed2cb73..1d9fcc9 100644
--- a/bgpd/bgp_packet.c
+++ b/bgpd/bgp_packet.c
@@ -206,7 +206,7 @@
/* Synchnorize attribute. */
if (adj->attr)
- bgp_attr_unintern (adj->attr);
+ bgp_attr_unintern (&adj->attr);
else
peer->scount[afi][safi]++;
@@ -1583,26 +1583,47 @@
BGP_NOTIFY_UPDATE_MAL_ATTR);
return -1;
}
+
+ /* Certain attribute parsing errors should not be considered bad enough
+ * to reset the session for, most particularly any partial/optional
+ * attributes that have 'tunneled' over speakers that don't understand
+ * them. Instead we withdraw only the prefix concerned.
+ *
+ * Complicates the flow a little though..
+ */
+ bgp_attr_parse_ret_t attr_parse_ret = BGP_ATTR_PARSE_PROCEED;
+ /* This define morphs the update case into a withdraw when lower levels
+ * have signalled an error condition where this is best.
+ */
+#define NLRI_ATTR_ARG (attr_parse_ret != BGP_ATTR_PARSE_WITHDRAW ? &attr : NULL)
/* Parse attribute when it exists. */
if (attribute_len)
{
- ret = bgp_attr_parse (peer, &attr, attribute_len,
+ attr_parse_ret = bgp_attr_parse (peer, &attr, attribute_len,
&mp_update, &mp_withdraw);
- if (ret < 0)
+ if (attr_parse_ret == BGP_ATTR_PARSE_ERROR)
return -1;
}
-
+
/* Logging the attribute. */
- if (BGP_DEBUG (update, UPDATE_IN))
+ if (attr_parse_ret == BGP_ATTR_PARSE_WITHDRAW
+ || BGP_DEBUG (update, UPDATE_IN))
{
ret= bgp_dump_attr (peer, &attr, attrstr, BUFSIZ);
+ int lvl = (attr_parse_ret == BGP_ATTR_PARSE_WITHDRAW)
+ ? LOG_ERR : LOG_DEBUG;
+
+ if (attr_parse_ret == BGP_ATTR_PARSE_WITHDRAW)
+ zlog (peer->log, LOG_ERR,
+ "%s rcvd UPDATE with errors in attr(s)!! Withdrawing route.",
+ peer->host);
if (ret)
- zlog (peer->log, LOG_DEBUG, "%s rcvd UPDATE w/ attr: %s",
+ zlog (peer->log, lvl, "%s rcvd UPDATE w/ attr: %s",
peer->host, attrstr);
}
-
+
/* Network Layer Reachability Information. */
update_len = end - stream_pnt (s);
@@ -1611,7 +1632,12 @@
/* Check NLRI packet format and prefix length. */
ret = bgp_nlri_sanity_check (peer, AFI_IP, stream_pnt (s), update_len);
if (ret < 0)
- return -1;
+ {
+ bgp_attr_unintern_sub (&attr);
+ if (attr.extra)
+ bgp_attr_extra_free (&attr);
+ return -1;
+ }
/* Set NLRI portion to structure. */
update.afi = AFI_IP;
@@ -1634,15 +1660,20 @@
update. */
ret = bgp_attr_check (peer, &attr);
if (ret < 0)
- return -1;
+ {
+ bgp_attr_unintern_sub (&attr);
+ if (attr.extra)
+ bgp_attr_extra_free (&attr);
+ return -1;
+ }
- bgp_nlri_parse (peer, &attr, &update);
+ bgp_nlri_parse (peer, NLRI_ATTR_ARG, &update);
}
if (mp_update.length
&& mp_update.afi == AFI_IP
&& mp_update.safi == SAFI_UNICAST)
- bgp_nlri_parse (peer, &attr, &mp_update);
+ bgp_nlri_parse (peer, NLRI_ATTR_ARG, &mp_update);
if (mp_withdraw.length
&& mp_withdraw.afi == AFI_IP
@@ -1669,7 +1700,7 @@
if (mp_update.length
&& mp_update.afi == AFI_IP
&& mp_update.safi == SAFI_MULTICAST)
- bgp_nlri_parse (peer, &attr, &mp_update);
+ bgp_nlri_parse (peer, NLRI_ATTR_ARG, &mp_update);
if (mp_withdraw.length
&& mp_withdraw.afi == AFI_IP
@@ -1699,7 +1730,7 @@
if (mp_update.length
&& mp_update.afi == AFI_IP6
&& mp_update.safi == SAFI_UNICAST)
- bgp_nlri_parse (peer, &attr, &mp_update);
+ bgp_nlri_parse (peer, NLRI_ATTR_ARG, &mp_update);
if (mp_withdraw.length
&& mp_withdraw.afi == AFI_IP6
@@ -1728,7 +1759,7 @@
if (mp_update.length
&& mp_update.afi == AFI_IP6
&& mp_update.safi == SAFI_MULTICAST)
- bgp_nlri_parse (peer, &attr, &mp_update);
+ bgp_nlri_parse (peer, NLRI_ATTR_ARG, &mp_update);
if (mp_withdraw.length
&& mp_withdraw.afi == AFI_IP6
@@ -1756,7 +1787,7 @@
if (mp_update.length
&& mp_update.afi == AFI_IP
&& mp_update.safi == BGP_SAFI_VPNV4)
- bgp_nlri_parse_vpnv4 (peer, &attr, &mp_update);
+ bgp_nlri_parse_vpnv4 (peer, NLRI_ATTR_ARG, &mp_update);
if (mp_withdraw.length
&& mp_withdraw.afi == AFI_IP
@@ -1778,21 +1809,10 @@
/* Everything is done. We unintern temporary structures which
interned in bgp_attr_parse(). */
- if (attr.aspath)
- aspath_unintern (attr.aspath);
- if (attr.community)
- community_unintern (attr.community);
+ bgp_attr_unintern_sub (&attr);
if (attr.extra)
- {
- if (attr.extra->ecommunity)
- ecommunity_unintern (attr.extra->ecommunity);
- if (attr.extra->cluster)
- cluster_unintern (attr.extra->cluster);
- if (attr.extra->transit)
- transit_unintern (attr.extra->transit);
- bgp_attr_extra_free (&attr);
- }
-
+ bgp_attr_extra_free (&attr);
+
/* If peering is stopped due to some reason, do not generate BGP
event. */
if (peer->status != Established)
diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
index 5c516f0..aabd264 100644
--- a/bgpd/bgp_route.c
+++ b/bgpd/bgp_route.c
@@ -137,7 +137,7 @@
bgp_info_free (struct bgp_info *binfo)
{
if (binfo->attr)
- bgp_attr_unintern (binfo->attr);
+ bgp_attr_unintern (&binfo->attr);
bgp_info_extra_free (&binfo->extra);
@@ -1802,14 +1802,14 @@
/* Apply import policy. */
if (bgp_import_modifier (rsclient, peer, p, &new_attr, afi, safi) == RMAP_DENY)
{
- bgp_attr_unintern (attr_new2);
+ bgp_attr_unintern (&attr_new2);
reason = "import-policy;";
goto filtered;
}
attr_new = bgp_attr_intern (&new_attr);
- bgp_attr_unintern (attr_new2);
+ bgp_attr_unintern (&attr_new2);
/* IPv4 unicast next hop check. */
if (afi == AFI_IP && safi == SAFI_UNICAST)
@@ -1818,7 +1818,7 @@
if (new_attr.nexthop.s_addr == 0
|| ntohl (new_attr.nexthop.s_addr) >= 0xe0000000)
{
- bgp_attr_unintern (attr_new);
+ bgp_attr_unintern (&attr_new);
reason = "martian next-hop;";
goto filtered;
@@ -1848,7 +1848,7 @@
p->prefixlen, rsclient->host);
bgp_unlock_node (rn);
- bgp_attr_unintern (attr_new);
+ bgp_attr_unintern (&attr_new);
return;
}
@@ -1868,7 +1868,7 @@
bgp_info_set_flag (rn, ri, BGP_INFO_ATTR_CHANGED);
/* Update to new attribute. */
- bgp_attr_unintern (ri->attr);
+ bgp_attr_unintern (&ri->attr);
ri->attr = attr_new;
/* Update MPLS tag. */
@@ -2128,7 +2128,7 @@
}
bgp_unlock_node (rn);
- bgp_attr_unintern (attr_new);
+ bgp_attr_unintern (&attr_new);
bgp_attr_extra_free (&new_attr);
return 0;
@@ -2175,7 +2175,7 @@
}
/* Update to new attribute. */
- bgp_attr_unintern (ri->attr);
+ bgp_attr_unintern (&ri->attr);
ri->attr = attr_new;
/* Update MPLS tag. */
@@ -2467,7 +2467,7 @@
}
bgp_attr_extra_free (&attr);
- aspath_unintern (aspath);
+ aspath_unintern (&aspath);
}
static void
@@ -3215,7 +3215,7 @@
bgp_attr_flush (&attr_tmp);
/* Unintern original. */
- aspath_unintern (attr.aspath);
+ aspath_unintern (&attr.aspath);
bgp_static_withdraw_rsclient (bgp, rsclient, p, afi, safi);
bgp_attr_extra_free (&attr);
@@ -3242,8 +3242,8 @@
bgp->peer_self->rmap_type = 0;
- bgp_attr_unintern (attr_new);
- aspath_unintern (attr.aspath);
+ bgp_attr_unintern (&attr_new);
+ aspath_unintern (&attr.aspath);
bgp_attr_extra_free (&attr);
bgp_static_withdraw_rsclient (bgp, rsclient, p, afi, safi);
@@ -3253,7 +3253,7 @@
bgp->peer_self->rmap_type = 0;
- bgp_attr_unintern (attr_new);
+ bgp_attr_unintern (&attr_new);
attr_new = bgp_attr_intern (&new_attr);
bgp_attr_extra_free (&new_attr);
@@ -3268,8 +3268,8 @@
!CHECK_FLAG(ri->flags, BGP_INFO_REMOVED))
{
bgp_unlock_node (rn);
- bgp_attr_unintern (attr_new);
- aspath_unintern (attr.aspath);
+ bgp_attr_unintern (&attr_new);
+ aspath_unintern (&attr.aspath);
bgp_attr_extra_free (&attr);
return;
}
@@ -3281,14 +3281,14 @@
/* Rewrite BGP route information. */
if (CHECK_FLAG(ri->flags, BGP_INFO_REMOVED))
bgp_info_restore(rn, ri);
- bgp_attr_unintern (ri->attr);
+ bgp_attr_unintern (&ri->attr);
ri->attr = attr_new;
ri->uptime = bgp_clock ();
/* Process change. */
bgp_process (bgp, rn, afi, safi);
bgp_unlock_node (rn);
- aspath_unintern (attr.aspath);
+ aspath_unintern (&attr.aspath);
bgp_attr_extra_free (&attr);
return;
}
@@ -3313,7 +3313,7 @@
bgp_process (bgp, rn, afi, safi);
/* Unintern original. */
- aspath_unintern (attr.aspath);
+ aspath_unintern (&attr.aspath);
bgp_attr_extra_free (&attr);
}
@@ -3363,7 +3363,7 @@
bgp_attr_flush (&attr_tmp);
/* Unintern original. */
- aspath_unintern (attr.aspath);
+ aspath_unintern (&attr.aspath);
bgp_attr_extra_free (&attr);
bgp_static_withdraw (bgp, p, afi, safi);
return;
@@ -3384,8 +3384,8 @@
!CHECK_FLAG(ri->flags, BGP_INFO_REMOVED))
{
bgp_unlock_node (rn);
- bgp_attr_unintern (attr_new);
- aspath_unintern (attr.aspath);
+ bgp_attr_unintern (&attr_new);
+ aspath_unintern (&attr.aspath);
bgp_attr_extra_free (&attr);
return;
}
@@ -3399,7 +3399,7 @@
bgp_info_restore(rn, ri);
else
bgp_aggregate_decrement (bgp, p, ri, afi, safi);
- bgp_attr_unintern (ri->attr);
+ bgp_attr_unintern (&ri->attr);
ri->attr = attr_new;
ri->uptime = bgp_clock ();
@@ -3407,7 +3407,7 @@
bgp_aggregate_increment (bgp, p, ri, afi, safi);
bgp_process (bgp, rn, afi, safi);
bgp_unlock_node (rn);
- aspath_unintern (attr.aspath);
+ aspath_unintern (&attr.aspath);
bgp_attr_extra_free (&attr);
return;
}
@@ -3435,7 +3435,7 @@
bgp_process (bgp, rn, afi, safi);
/* Unintern original. */
- aspath_unintern (attr.aspath);
+ aspath_unintern (&attr.aspath);
bgp_attr_extra_free (&attr);
}
@@ -5299,7 +5299,7 @@
bgp_attr_extra_free (&attr_new);
/* Unintern original. */
- aspath_unintern (attr.aspath);
+ aspath_unintern (&attr.aspath);
bgp_attr_extra_free (&attr);
bgp_redistribute_delete (p, type);
return;
@@ -5322,8 +5322,8 @@
if (attrhash_cmp (bi->attr, new_attr) &&
!CHECK_FLAG(bi->flags, BGP_INFO_REMOVED))
{
- bgp_attr_unintern (new_attr);
- aspath_unintern (attr.aspath);
+ bgp_attr_unintern (&new_attr);
+ aspath_unintern (&attr.aspath);
bgp_attr_extra_free (&attr);
bgp_unlock_node (bn);
return;
@@ -5338,7 +5338,7 @@
bgp_info_restore(bn, bi);
else
bgp_aggregate_decrement (bgp, p, bi, afi, SAFI_UNICAST);
- bgp_attr_unintern (bi->attr);
+ bgp_attr_unintern (&bi->attr);
bi->attr = new_attr;
bi->uptime = bgp_clock ();
@@ -5346,7 +5346,7 @@
bgp_aggregate_increment (bgp, p, bi, afi, SAFI_UNICAST);
bgp_process (bgp, bn, afi, SAFI_UNICAST);
bgp_unlock_node (bn);
- aspath_unintern (attr.aspath);
+ aspath_unintern (&attr.aspath);
bgp_attr_extra_free (&attr);
return;
}
@@ -5368,7 +5368,7 @@
}
/* Unintern original. */
- aspath_unintern (attr.aspath);
+ aspath_unintern (&attr.aspath);
bgp_attr_extra_free (&attr);
}
diff --git a/bgpd/bgp_routemap.c b/bgpd/bgp_routemap.c
index 255b25a..178af60 100644
--- a/bgpd/bgp_routemap.c
+++ b/bgpd/bgp_routemap.c
@@ -1482,10 +1482,10 @@
else
new_ecom = ecommunity_dup (ecom);
- bgp_info->attr->extra->ecommunity = new_ecom;
+ bgp_info->attr->extra->ecommunity = ecommunity_intern (new_ecom);
if (old_ecom)
- ecommunity_free (old_ecom);
+ ecommunity_unintern (&old_ecom);
bgp_info->attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_EXT_COMMUNITIES);
}
@@ -1501,7 +1501,7 @@
ecom = ecommunity_str2com (arg, ECOMMUNITY_ROUTE_TARGET, 0);
if (! ecom)
return NULL;
- return ecom;
+ return ecommunity_intern (ecom);
}
/* Free function for set community. */
@@ -1509,7 +1509,7 @@
route_set_ecommunity_rt_free (void *rule)
{
struct ecommunity *ecom = rule;
- ecommunity_free (ecom);
+ ecommunity_unintern (&ecom);
}
/* Set community rule structure. */
@@ -1528,7 +1528,7 @@
route_set_ecommunity_soo (void *rule, struct prefix *prefix,
route_map_object_t type, void *object)
{
- struct ecommunity *ecom;
+ struct ecommunity *ecom, *old_ecom, *new_ecom;
struct bgp_info *bgp_info;
if (type == RMAP_BGP)
@@ -1539,8 +1539,19 @@
if (! ecom)
return RMAP_OKAY;
+ old_ecom = (bgp_attr_extra_get (bgp_info->attr))->ecommunity;
+
+ if (old_ecom)
+ new_ecom = ecommunity_merge (ecommunity_dup (old_ecom), ecom);
+ else
+ new_ecom = ecommunity_dup (ecom);
+
+ bgp_info->attr->extra->ecommunity = ecommunity_intern (new_ecom);
+
+ if (old_ecom)
+ ecommunity_unintern (&old_ecom);
+
bgp_info->attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_EXT_COMMUNITIES);
- (bgp_attr_extra_get (bgp_info->attr))->ecommunity = ecommunity_dup (ecom);
}
return RMAP_OKAY;
}
@@ -1555,7 +1566,7 @@
if (! ecom)
return NULL;
- return ecom;
+ return ecommunity_intern (ecom);
}
/* Free function for set community. */
@@ -1563,7 +1574,7 @@
route_set_ecommunity_soo_free (void *rule)
{
struct ecommunity *ecom = rule;
- ecommunity_free (ecom);
+ ecommunity_unintern (&ecom);
}
/* Set community rule structure. */
diff --git a/tests/aspath_test.c b/tests/aspath_test.c
index 9e51e8d..4a2ce9a 100644
--- a/tests/aspath_test.c
+++ b/tests/aspath_test.c
@@ -6,6 +6,7 @@
#include "bgpd/bgpd.h"
#include "bgpd/bgp_aspath.h"
+#include "bgpd/bgp_attr.h"
#define VT100_RESET "\x1b[0m"
#define VT100_RED "\x1b[31m"
@@ -407,7 +408,7 @@
"#ASNs = 0, data = seq(8466 3 52737 4096 3456)",
{ 0x2,0x0, 0x21,0x12, 0x00,0x03, 0xce,0x01, 0x10,0x00, 0x0d,0x80 },
12,
- { "", "",
+ { NULL, NULL,
0, 0, 0, 0, 0, 0 },
},
{ /* 26 */
@@ -417,10 +418,190 @@
0x2,0x2, 0x10,0x00, 0x0d,0x80 },
14
,
- { "", "",
+ { NULL, NULL,
0, 0, 0, 0, 0, 0 },
},
- { NULL, NULL, {0}, 0, { NULL, 0, 0 } }
+ { /* 27 */
+ "invalid segment type",
+ "type=8(4096 3456)",
+ { 0x8,0x2, 0x10,0x00, 0x0d,0x80 },
+ 14
+ ,
+ { NULL, NULL,
+ 0, 0, 0, 0, 0, 0 },
+ }, { NULL, NULL, {0}, 0, { NULL, 0, 0 } }
+};
+
+/* */
+static struct aspath_tests {
+ const char *desc;
+ const struct test_segment *segment;
+ const char *shouldbe; /* String it should evaluate to */
+ const enum as4 { AS4_DATA, AS2_DATA }
+ as4; /* whether data should be as4 or not (ie as2) */
+ const int result; /* expected result for bgp_attr_parse */
+ const int cap; /* capabilities to set for peer */
+ const char attrheader [1024];
+ size_t len;
+} aspath_tests [] =
+{
+ /* 0 */
+ {
+ "basic test",
+ &test_segments[0],
+ "8466 3 52737 4096",
+ AS2_DATA, 0,
+ 0,
+ { BGP_ATTR_FLAG_TRANS,
+ BGP_ATTR_AS_PATH,
+ 10,
+ },
+ 3,
+ },
+ /* 1 */
+ {
+ "length too short",
+ &test_segments[0],
+ "8466 3 52737 4096",
+ AS2_DATA, -1,
+ 0,
+ { BGP_ATTR_FLAG_TRANS,
+ BGP_ATTR_AS_PATH,
+ 8,
+ },
+ 3,
+ },
+ /* 2 */
+ {
+ "length too long",
+ &test_segments[0],
+ "8466 3 52737 4096",
+ AS2_DATA, -1,
+ 0,
+ { BGP_ATTR_FLAG_TRANS,
+ BGP_ATTR_AS_PATH,
+ 12,
+ },
+ 3,
+ },
+ /* 3 */
+ {
+ "incorrect flag",
+ &test_segments[0],
+ "8466 3 52737 4096",
+ AS2_DATA, -1,
+ 0,
+ { BGP_ATTR_FLAG_TRANS|BGP_ATTR_FLAG_OPTIONAL,
+ BGP_ATTR_AS_PATH,
+ 10,
+ },
+ 3,
+ },
+ /* 4 */
+ {
+ "as4_path, with as2 format data",
+ &test_segments[0],
+ "8466 3 52737 4096",
+ AS2_DATA, -1,
+ 0,
+ { BGP_ATTR_FLAG_TRANS|BGP_ATTR_FLAG_OPTIONAL,
+ BGP_ATTR_AS4_PATH,
+ 10,
+ },
+ 3,
+ },
+ /* 5 */
+ {
+ "as4, with incorrect attr length",
+ &test_segments[0],
+ "8466 3 52737 4096",
+ AS4_DATA, -1,
+ PEER_CAP_AS4_RCV,
+ { BGP_ATTR_FLAG_TRANS|BGP_ATTR_FLAG_OPTIONAL,
+ BGP_ATTR_AS4_PATH,
+ 10,
+ },
+ 3,
+ },
+ /* 6 */
+ {
+ "basic 4-byte as-path",
+ &test_segments[0],
+ "8466 3 52737 4096",
+ AS4_DATA, 0,
+ PEER_CAP_AS4_RCV|PEER_CAP_AS4_ADV,
+ { BGP_ATTR_FLAG_TRANS,
+ BGP_ATTR_AS_PATH,
+ 18,
+ },
+ 3,
+ },
+ /* 7 */
+ {
+ "4b AS_PATH: too short",
+ &test_segments[0],
+ "8466 3 52737 4096",
+ AS4_DATA, -1,
+ PEER_CAP_AS4_RCV|PEER_CAP_AS4_ADV,
+ { BGP_ATTR_FLAG_TRANS,
+ BGP_ATTR_AS_PATH,
+ 16,
+ },
+ 3,
+ },
+ /* 8 */
+ {
+ "4b AS_PATH: too long",
+ &test_segments[0],
+ "8466 3 52737 4096",
+ AS4_DATA, -1,
+ PEER_CAP_AS4_RCV|PEER_CAP_AS4_ADV,
+ { BGP_ATTR_FLAG_TRANS,
+ BGP_ATTR_AS_PATH,
+ 20,
+ },
+ 3,
+ },
+ /* 9 */
+ {
+ "4b AS_PATH: too long2",
+ &test_segments[0],
+ "8466 3 52737 4096",
+ AS4_DATA, -1,
+ PEER_CAP_AS4_RCV|PEER_CAP_AS4_ADV,
+ { BGP_ATTR_FLAG_TRANS,
+ BGP_ATTR_AS_PATH,
+ 22,
+ },
+ 3,
+ },
+ /* 10 */
+ {
+ "4b AS_PATH: bad flags",
+ &test_segments[0],
+ "8466 3 52737 4096",
+ AS4_DATA, -1,
+ PEER_CAP_AS4_RCV|PEER_CAP_AS4_ADV,
+ { BGP_ATTR_FLAG_TRANS|BGP_ATTR_FLAG_OPTIONAL,
+ BGP_ATTR_AS_PATH,
+ 18,
+ },
+ 3,
+ },
+ /* 11 */
+ {
+ "4b AS_PATH: confed",
+ &test_segments[6],
+ "8466 3 52737 4096",
+ AS4_DATA, -1,
+ PEER_CAP_AS4_ADV,
+ { BGP_ATTR_FLAG_TRANS|BGP_ATTR_FLAG_OPTIONAL,
+ BGP_ATTR_AS4_PATH,
+ 14,
+ },
+ 3,
+ },
+ { NULL, NULL, NULL, 0, 0, 0, { 0 }, 0 },
};
/* prepending tests */
@@ -430,21 +611,25 @@
struct test_spec sp;
} prepend_tests[] =
{
+ /* 0 */
{ &test_segments[0], &test_segments[1],
{ "8466 3 52737 4096 8722 4",
"8466 3 52737 4096 8722 4",
6, 0, NOT_ALL_PRIVATE, 4096, 1, 8466 },
},
+ /* 1 */
{ &test_segments[1], &test_segments[3],
{ "8722 4 8482 51457 {5204}",
"8722 4 8482 51457 {5204}",
5, 0, NOT_ALL_PRIVATE, 5204, 1, 8722 }
},
+ /* 2 */
{ &test_segments[3], &test_segments[4],
{ "8482 51457 {5204} 8467 59649 {4196,48658} {17322,30745}",
"8482 51457 {5204} 8467 59649 {4196,48658} {17322,30745}",
7, 0, NOT_ALL_PRIVATE, 5204, 1, 8482 },
},
+ /* 3 */
{ &test_segments[4], &test_segments[5],
{ "8467 59649 {4196,48658} {17322,30745} 6435 59408 21665"
" {2457,4369,61697} 1842 41590 51793",
@@ -452,11 +637,13 @@
" {2457,4369,61697} 1842 41590 51793",
11, 0, NOT_ALL_PRIVATE, 61697, 1, 8467 }
},
+ /* 4 */
{ &test_segments[5], &test_segments[6],
- { "6435 59408 21665 {2457,4369,61697} 1842 41590 51793 (123 456 789)",
- "6435 59408 21665 {2457,4369,61697} 1842 41590 51793 (123 456 789)",
- 7, 3, NOT_ALL_PRIVATE, 123, 1, 6435 },
+ { "6435 59408 21665 {2457,4369,61697} 1842 41590 51793",
+ "6435 59408 21665 {2457,4369,61697} 1842 41590 51793",
+ 7, 0, NOT_ALL_PRIVATE, 1842, 1, 6435 },
},
+ /* 5 */
{ &test_segments[6], &test_segments[7],
{ "(123 456 789) (123 456 789) (111 222)",
"",
@@ -649,7 +836,7 @@
s = stream_new (len);
stream_put (s, data, len);
}
- as = aspath_parse (s, len, use32bit, 0);
+ as = aspath_parse (s, len, use32bit);
if (s)
stream_free (s);
@@ -682,6 +869,12 @@
static struct stream *s;
struct aspath *asinout, *asconfeddel, *asstr, *as4;
+ if (as == NULL && sp->shouldbe == NULL)
+ {
+ printf ("Correctly failed to parse\n");
+ return fails;
+ }
+
out = aspath_snmp_pathseg (as, &bytes);
asinout = make_aspath (out, bytes, 0);
@@ -826,7 +1019,7 @@
printf ("%s: %s\n", t->name, t->desc);
asp = make_aspath (t->asdata, t->len, 0);
-
+
printf ("aspath: %s\nvalidating...:\n", aspath_print (asp));
if (!validate (asp, &t->sp))
@@ -835,7 +1028,9 @@
printf (FAILED "\n");
printf ("\n");
- aspath_unintern (asp);
+
+ if (asp)
+ aspath_unintern (asp);
}
/* prepend testing */
@@ -891,7 +1086,8 @@
printf (FAILED "!\n");
printf ("\n");
- aspath_unintern (asp1);
+ if (asp1)
+ aspath_unintern (asp1);
aspath_free (asp2);
}
@@ -993,30 +1189,106 @@
aspath_unintern (asp2);
}
}
-
+
+static int
+handle_attr_test (struct aspath_tests *t)
+{
+ struct bgp bgp = { 0 };
+ struct peer peer = { 0 };
+ struct attr attr = { 0 };
+ int ret;
+ int initfail = failed;
+ struct aspath *asp;
+ size_t datalen;
+
+ asp = make_aspath (t->segment->asdata, t->segment->len, 0);
+
+ peer.ibuf = stream_new (BGP_MAX_PACKET_SIZE);
+ peer.obuf = stream_fifo_new ();
+ peer.bgp = &bgp;
+ peer.host = (char *)"none";
+ peer.fd = -1;
+ peer.cap = t->cap;
+
+ stream_write (peer.ibuf, t->attrheader, t->len);
+ datalen = aspath_put (peer.ibuf, asp, t->as4 == AS4_DATA);
+
+ ret = bgp_attr_parse (&peer, &attr, t->len + datalen, NULL, NULL);
+
+ if (ret != t->result)
+ {
+ printf ("bgp_attr_parse returned %d, expected %d\n", ret, t->result);
+ printf ("datalen %d\n", datalen);
+ failed++;
+ }
+ if (ret != 0)
+ goto out;
+
+ if (attr.aspath == NULL)
+ {
+ printf ("aspath is NULL!\n");
+ failed++;
+ }
+ if (attr.aspath && strcmp (attr.aspath->str, t->shouldbe))
+ {
+ printf ("attr str and 'shouldbe' mismatched!\n"
+ "attr str: %s\n"
+ "shouldbe: %s\n",
+ attr.aspath->str, t->shouldbe);
+ failed++;
+ }
+
+out:
+ if (attr.aspath)
+ aspath_unintern (attr.aspath);
+ if (asp)
+ aspath_unintern (asp);
+ return failed - initfail;
+}
+
+static void
+attr_test (struct aspath_tests *t)
+{
+ printf ("%s\n", t->desc);
+ printf ("%s\n\n", handle_attr_test (t) ? FAILED : OK);
+}
+
int
main (void)
{
int i = 0;
- aspath_init();
+ bgp_master_init ();
+ master = bm->master;
+ bgp_attr_init ();
+
while (test_segments[i].name)
{
+ printf ("test %u\n", i);
parse_test (&test_segments[i]);
empty_prepend_test (&test_segments[i++]);
}
i = 0;
while (prepend_tests[i].test1)
- prepend_test (&prepend_tests[i++]);
+ {
+ printf ("prepend test %u\n", i);
+ prepend_test (&prepend_tests[i++]);
+ }
i = 0;
while (aggregate_tests[i].test1)
- aggregate_test (&aggregate_tests[i++]);
+ {
+ printf ("aggregate test %u\n", i);
+ aggregate_test (&aggregate_tests[i++]);
+ }
i = 0;
while (reconcile_tests[i].test1)
- as4_reconcile_test (&reconcile_tests[i++]);
+ {
+ printf ("reconcile test %u\n", i);
+ as4_reconcile_test (&reconcile_tests[i++]);
+ }
i = 0;
@@ -1026,6 +1298,14 @@
empty_get_test();
+ i = 0;
+
+ while (aspath_tests[i].desc)
+ {
+ printf ("aspath_attr test %d\n", i);
+ attr_test (&aspath_tests[i++]);
+ }
+
printf ("failures: %d\n", failed);
printf ("aspath count: %ld\n", aspath_count());
diff --git a/tools/multiple-bgpd.sh b/tools/multiple-bgpd.sh
index 028ad69..d6a38ed 100644
--- a/tools/multiple-bgpd.sh
+++ b/tools/multiple-bgpd.sh
@@ -25,13 +25,14 @@
NEXTAS=$((${ASBASE} + $NEXT))
PREVADDR="${PREFIX}${PREV}"
PREVAS=$((${ASBASE} + $PREV))
+ ASN=$((64560+${H}))
# Edit config to suit.
cat > "$CONF" <<- EOF
password whatever
service advanced-vty
!
- router bgp $((64560+${H}))
+ router bgp ${ASN}
bgp router-id ${ADDR}
network 10.${H}.1.0/24 pathlimit 1
network 10.${H}.2.0/24 pathlimit 2
@@ -40,6 +41,7 @@
neighbor default update-source ${ADDR}
neighbor default capability orf prefix-list both
neighbor default soft-reconfiguration inbound
+ neighbor default route-map test out
neighbor ${NEXTADDR} remote-as ${NEXTAS}
neighbor ${NEXTADDR} peer-group default
neighbor ${PREVADDR} remote-as ${PREVAS}
@@ -53,10 +55,15 @@
neighbor default activate
neighbor default capability orf prefix-list both
neighbor default default-originate
+ neighbor default route-map test out
neighbor ${NEXTADDR} peer-group default
neighbor ${PREVADDR} peer-group default
exit-address-family
!
+ route-map test permit 10
+ set extcommunity rt ${ASN}:1
+ set extcommunity soo ${ASN}:2
+ set community ${ASN}:1
line vty
!
end