summaryrefslogtreecommitdiffstats
path: root/tests
diff options
context:
space:
mode:
authorPaul Jakma <paul@quagga.net>2010-11-23 17:35:42 +0100
committerPaul Jakma <paul@quagga.net>2011-03-21 14:51:14 +0100
commitb881c7074bb698aeb1b099175b325734fc6e44d2 (patch)
tree70b4816a083166bbf00c1f85f19a67df0c0a5948 /tests
parenttools/multiple-bgpd.sh: set some community attributes to help test them (diff)
downloadfrr-b881c7074bb698aeb1b099175b325734fc6e44d2.tar.xz
frr-b881c7074bb698aeb1b099175b325734fc6e44d2.zip
bgpd: Implement revised error handling for partial optional/trans. attributes
* BGP error handling generally boils down to "reset session". This was fine when all BGP speakers pretty much understood all BGP messages. However the increasing deployment of new attribute types has shown this approach to cause problems, in particular where a new attribute type is "tunneled" over some speakers which do not understand it, and then arrives at a speaker which does but considers it malformed (e.g. corruption along the way, or because of early implementation bugs/interop issues). To mitigate this drafts before the IDR (likely to be adopted) propose to treat errors in partial (i.e. not understood by neighbour), optional transitive attributes, when received from eBGP peers, as withdrawing only the NLRIs in the affected UPDATE, rather than causing the entire session to be reset. See: http://tools.ietf.org/html/draft-scudder-idr-optional-transitive * bgp_aspath.c: (assegments_parse) Replace the "NULL means valid, 0-length OR an error" return value with an error code - instead taking pointer to result structure as arg. (aspath_parse) adjust to suit previous change, but here NULL really does mean error in the external interface. * bgp_attr.h (bgp_attr_parse) use an explictly typed and enumerated value to indicate return result. (bgp_attr_unintern_sub) cleans up just the members of an attr, but not the attr itself, for benefit of those who use a stack-local attr. * bgp_attr.c: (bgp_attr_unintern_sub) split out from bgp_attr_unintern (bgp_attr_unintern) as previous. (bgp_attr_malformed) helper function to centralise decisions on how to handle errors in attributes. (bgp_attr_{aspathlimit,origin,etc..}) Use bgp_attr_malformed. (bgp_attr_aspathlimit) Subcode for error specifc to this attr should be BGP_NOTIFY_UPDATE_OPT_ATTR_ERR. (bgp_attr_as4_path) be more rigorous about checks, ala bgp_attr_as_path. (bgp_attr_parse) Adjust to deal with the additional error level that bgp_attr_ parsers can raise, and also similarly return appropriate error back up to (bgp_update_receive). Try to avoid leaking as4_path. * bgp_packet.c: (bgp_update_receive) Adjust to deal with BGP_ATTR_PARSE_WITHDRAW error level from bgp_attr_parse, which should lead to a withdraw, by making the attribute parameter in call to (bgp_nlri_parse) conditional on the error, so the update case morphs also into a withdraw. Use bgp_attr_unintern_sub from above, instead of doing this itself. Fix error case returns which were not calling bgp_attr_unintern_sub and probably leaking memory. * tests/aspath_test.c: Fix to work for null return with bad segments
Diffstat (limited to 'tests')
-rw-r--r--tests/aspath_test.c21
1 files changed, 15 insertions, 6 deletions
diff --git a/tests/aspath_test.c b/tests/aspath_test.c
index ade60c67c..4a2ce9aa0 100644
--- a/tests/aspath_test.c
+++ b/tests/aspath_test.c
@@ -408,7 +408,7 @@ static struct test_segment {
"#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 */
@@ -418,7 +418,7 @@ static struct test_segment {
0x2,0x2, 0x10,0x00, 0x0d,0x80 },
14
,
- { "", "",
+ { NULL, NULL,
0, 0, 0, 0, 0, 0 },
},
{ /* 27 */
@@ -427,7 +427,7 @@ static struct test_segment {
{ 0x8,0x2, 0x10,0x00, 0x0d,0x80 },
14
,
- { "", "",
+ { NULL, NULL,
0, 0, 0, 0, 0, 0 },
}, { NULL, NULL, {0}, 0, { NULL, 0, 0 } }
};
@@ -869,6 +869,12 @@ validate (struct aspath *as, const struct test_spec *sp)
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);
@@ -1013,7 +1019,7 @@ parse_test (struct test_segment *t)
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))
@@ -1022,7 +1028,9 @@ parse_test (struct test_segment *t)
printf (FAILED "\n");
printf ("\n");
- aspath_unintern (asp);
+
+ if (asp)
+ aspath_unintern (asp);
}
/* prepend testing */
@@ -1078,7 +1086,8 @@ empty_prepend_test (struct test_segment *t)
printf (FAILED "!\n");
printf ("\n");
- aspath_unintern (asp1);
+ if (asp1)
+ aspath_unintern (asp1);
aspath_free (asp2);
}