summaryrefslogtreecommitdiffstats
path: root/bgpd/bgp_attr.c
Commit message (Collapse)AuthorAgeFilesLines
* bgpd: don't send NOTIFY twice for malformed attrsDavid Lamparter2014-06-291-12/+22
| | | | | | | Most of the attribute parsing functions were already sending a notify, let's clean up the code to make it happen only once. Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
* bgpd: fix IP endianness in debug messageDavid Lamparter2014-06-291-1/+1
| | | | | | inet_ntop expects network byte order. Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
* *: nuke ^L (page feed)David Lamparter2014-06-041-5/+5
| | | | | | | | | | | | | | Quagga sources have inherited a slew of Page Feed (^L, \xC) characters from ancient history. Among other things, these break patchwork's XML-RPC API because \xC is not a valid character in XML documents. Nuke them from high orbit. Patches can be adapted simply by: sed -e 's%^L%%' -i filename.patch (you can type page feeds in some environments with Ctrl-V Ctrl-L) Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
* bgpd: efficient NLRI packing for AFs != ipv4-unicastPradosh Mohapatra2014-06-031-119/+130
| | | | | | | | | | | | | | | | | | | | | ISSUE: Currently, for non-ipv4-unicast address families where prefixes are encoded in MP_REACH/MP_UNREACH attributes, BGP ends up sending one prefix per UPDATE message. This is quite inefficient. The patch addresses the issue. PATCH: We introduce a scratch buffer in the peer structure that stores the MP_REACH/MP_UNREACH attributes for non-ipv4-unicast families. This enables us to encode multiple prefixes. In the end, the two buffers are merged to create the UPDATE packet. Signed-off-by: Pradosh Mohapatra <pmohapat@cumulusnetworks.com> Reviewed-by: Daniel Walton <dwalton@cumulusnetworks.com> [DL: removed no longer existing bgp_packet_withdraw prototype] Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
* bgpd: track correct originator-id in reflected routesPradosh Mohapatra2014-05-151-1/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | ISSUE: Suppose route1 and route2 received from route-reflector-client1 and client2 respectively have identical attributes. The current logic of creating the adj-rib-out for a peer threads the 'adv' structures for both routes against the same attribute. This results in 'bgp_update_packet()' to pack those routes in the same UPDATE message with one attr structure formatted. The originator-id is thus set according to the first route's received router id. This is incorrect. PATCH: Fix bgp_announce_check() function to set the originator-id in the advertising attr structure. Also, fix the attribute hash function and compare function to consider originator-id. Otherwise attributes where all fields except the originator-id are identical get merged into one memory location. Signed-off-by: Pradosh Mohapatra <pmohapat at cumulusnetworks.com> Reviewed-by: Scott Feldman <sfeldma at cumulusnetworks.com> Reviewed-by: Ken Yin <kyin at cumulusnetworks.com> [DL: whitespace changes dropped] Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
* bgpd: use ATTR_FLAG_BIT() for BGP_ATTR_ valuesJorge Boncompte [DTI2]2014-05-151-4/+4
| | | | | | | | | * bgp_attr.c: this UNSET_FLAG()s are bogus. I did a quick review and I think that they could not cause any bug anyway. Signed-off-by: Jorge Boncompte [DTI2] <jorge@dti2.net> Acked-by: Feng Lu <lu.feng@6wind.com> Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
* bgpd: don't try to reconcile AS4_PATH with NULLDavid Lamparter2013-02-011-0/+3
| | | | | | | | | | | | | bgp_attr_munge_as4_attrs would previously try to reintegrate an AS4_PATH with a NULL AS_PATH, leading to a rather nasty SEGV. Let's go by RFC6793 and treat missing AS_PATH as 0-length AS_PATH, which in turn means discarding the AS4_PATH. [NB: we don't actually stick to the actual rule, which is discarding AS4_PATH if it's longer than AS_PATH; indeed we should probably fix that too] Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
* bgpd: fix a bug in bgp_attr_dupChristian Franke2013-01-141-4/+17
| | | | | | | | | | | | | | Commit 558d1fec11749d3257e improved bgp_attr_dup so it would be possible for the caller to provide attr_extra, allowing to use the stack instead of the heap for operations requiring only a short lived attr. However, this commit introduced a bug where bgp_attr_dup wouldn't copy attr_extra at all (but provide a reference to the original) if the caller provided attr_extra. Cc: Jorge Boncompte [DTI2] <jorge@dti2.net> Signed-off-by: Christian Franke <chris@opensourcerouting.org> Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
* bgpd: Fixed out-of-date commentAndrew Certain2012-11-301-1/+1
| | | | | | | When going through the code to write the documentation for local-as, I discovered that one of the comments was out-of-date. Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
* bgpd: add replace-as modifier for BGP neighborAndrew Certain2012-11-301-2/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Added replace-as modifier for BGP neighbors when using local-as. If the replace-as modifier is specified, only the replacement AS as specified by the local-as modifier is prepended to the AS_PATH, not the process's AS. In bgp_attr.c, I decided that if (peer->change_local_as) { /* If replace-as is specified, we only use the change_local_as when advertising routes. */ if( ! CHECK_FLAG (peer->flags, PEER_FLAG_LOCAL_AS_REPLACE_AS) ) { aspath = aspath_add_seq (aspath, peer->local_as); } aspath = aspath_add_seq (aspath, peer->change_local_as); } else { aspath = aspath_add_seq (aspath, peer->local_as); } was clearer than the alternative that didn't duplicate the prepending of the process's AS: /* First, append the process local AS unless we have an alternate local_as * and we're replacing it (as opposed to just prepending it). */ if (! (peer->change_local_as && CHECK_FLAG (peer->flags, PEER_FLAG_LOCAL_AS_REPLACE_AS) ) ) { aspath = aspath_add_seq (aspath, peer->local_as); } if (peer->change_local_as) aspath = aspath_add_seq (aspath, peer->change_local_as); } But I could be convinced otherwise. Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
* *: use array_size() helper macroBalaji.G2012-10-251-3/+2
| | | | | | | | Use the array_size() helper macro. Replaces several instances of local macros with the same definition. Reviewed-by: Scott Feldman <sfeldma@cumulusnetworks.com> Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
* bgpd: reduce struct attr_extra allocations/freeingJorge Boncompte [DTI2]2012-05-221-1/+5
| | | | | | | Try to use on stack structs for temporary uses. Signed-off-by: Jorge Boncompte [DTI2] <jorge@dti2.net> Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
* bgpd: use on stack struct attr_extra in bgp_attr_unintern()Jorge Boncompte [DTI2]2012-05-221-2/+2
| | | | | | | Reduce memory heap fragmentation and pressure on the memory allocator. Signed-off-by: Jorge Boncompte [DTI2] <jorge@dti2.net> Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
* bgpd: cleanup bgp_attr_unintern()Jorge Boncompte [DTI2]2012-05-221-11/+12
| | | | | Signed-off-by: Jorge Boncompte [DTI2] <jorge@dti2.net> Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
* bgpd: use on stack struct attr_extra on bgp_attr_aggregate_intern()Jorge Boncompte [DTI2]2012-05-221-10/+10
| | | | | | | Reduce memory heap fragmentation and pressure on the memory allocator. Signed-off-by: Jorge Boncompte [DTI2] <jorge@dti2.net> Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
* bgpd: reduce attrhash_make_key() indirectionsJorge Boncompte [DTI2]2012-05-221-16/+17
| | | | | Signed-off-by: Jorge Boncompte [DTI2] <jorge@dti2.net> Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
* bgpd: remove some useless initializationsJorge Boncompte [DTI2]2012-05-221-4/+1
| | | | | | | | | * bgp_attr.c: (bgp_attr_default_intern) bgp_attr_default_set() already initializes the memory. Fixes a struct attr_extra leak. * bgp_route.c: Remove useless on stack struct initializations. Signed-off-by: Jorge Boncompte [DTI2] <jorge@dti2.net> Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
* bgpd: remove calls to peer_sort() from fast-pathJorge Boncompte [DTI2]2012-05-221-16/+14
| | | | | | | | | | | | | | | | | peer_sort() it's called so much as to be annoying. In the assumption that the 'sort' of the peer doesn't change during an established session, I have changed all calls to peer_sort() in the 'fast-path' to only check the 'sort'. All the calls from the vty and such still recalculate the sort and store it in the peer. There's a lot of other calls to peer_sort() that could be changed but some maube tricky, someone more knowledgeable may try to reduce them. This hits peer_sort() from 5th out of the stadium^H^H list on a full internet table loading profiling session. Signed-off-by: Jorge Boncompte [DTI2] <jorge@dti2.net> Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
* bgpd: Fix regression in args consolidation, total should be inited from argsPaul Jakma2012-05-021-1/+1
| | | | * bgp_attr.c: (bgp_attr_unknown) total should be initialised from the args.
* bgpd: bgp_attr_flags_diagnose shouldn't assertPaul Jakma2012-03-271-1/+8
| | | | | | * bgpd/bgp_attr.c: (bgp_attr_flags_diagnose) debug code for error-handling paths probably shouldn't assert, instead it should just log that there was no problem.
* bgpd: attr_parse call to attr_malformed should deal with PROCEED error casePaul Jakma2012-03-271-8/+14
| | | | | | * bgpd/bgp_attr.c: (bgp_attr_parse) the invalid flag check call to bgp_attr_malformed is pretty useless if it doesn't actually allow for the PROCEED non-error case.
* bgpd: Fix silly mistake in bgp_attr_flag_invalidPaul Jakma2012-03-271-1/+1
| | | | | * bgp_attr.c: (bgp_attr_flag_invalid) flags is meant to be masked off with the mask variable...
* bgpd: malformed attribute error that can still proceed should fixup getpPaul Jakma2012-03-251-3/+10
| | | | | | | | | * bgp_attr.c: (bgp_attr_malformed) When a malformed attribute error can be ignored, and BGP message processing may still proceed, the stream getp should be adjusted to the end of the attribute - the caller may not have consumed all the attribute. Problem noted by Martin Winter in bug 678. Also, rename the 'startp' local to 'notify_datap', for clarity.
* bgpd: Move up flag-check calls, parcel up attr-parser args, and other cleanupsPaul Jakma2012-02-281-273/+194
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * bgp_attr.h: (struct bgp_attr_parser_args) Attribute parsing context, containing common arguments. * bgp_attr.c: (general) Move the bgp_attr_flag_invalid flag-check calls up, out of each individual attr parser function, to be done once in attr_parse. Similarly move the calculation of the 'total' attribute length field up to attr_parse. Bundle together common arguments to attr-parsing functions and helpers into (struct bgp_attr_parser_args), so it can be passed by reference down the stack & also de-clutter the argument lists & make it easier to add/modify the context for attr-parsing - add local const aliases to avoid modifying body of code too much. This also should help avoid cut & paste errors, where calls to helpers with hard-coded attribute types are pasted to other functions but the code isn't changed. (bgp_attr_flags_diagnose) as above. (bgp_attr_flag_invalid) as above. (bgp_attr_{origin,aspath,as4_path,nexthop,med,local_pref,atomic}) as above. (bgp_attr_{aggregator,as4_aggregator,community,originator_id}) as above (bgp_attr_{cluster_list,ext_communities},bgp_mp_{un,}reach_parse) as above (bgp_attr_unknown) as above. (bgp_attr_malformed) as above. Also, startp and length have to be special-cased, because whether or not to send attribute data depends on the particular error - a separate length argument, distinct from args->length, indicates whether or not the attribute data should be sent in the NOTIFY. (bgp_attr_aspath_check) Call to bgp_attr_malformed is wrong here, there is no attribute parsing context - e.g. the 'flag' argument is unlikely to be right, remove it. Explicitly handle the error instead. (bgp_attr_munge_as4_attrs) Flag argument is pointless. As the comment notes, the check here is pointless as AS_PATH presence already checked elsewhere. (bgp_attr_parse) Do bgp_attr_flag_invalid call here. Use (struct bgp_attr_parser_args) for args to attr parser functions. Remove out-of-context 'flag' argument to as4 checking functions.
* bgpd: consolidate attribute flag checksPaul Jakma2012-02-281-88/+133
| | | | | | | | | | | * bgpd/bgp_attr.c: (attr_flags_values []) array of required flags for attributes, EXTLEN & PARTIAL masked off as "dont care" as appropriate. (bgp_attr_flag_invalid) check if flags may be invalid, according to the above table & RFC rules. (bgp_attr_*) Use bgp_attr_flag_invalid. (bgp_attr_as4_aggregator) ditto, also take startp argument for the NOTIFY data. (bgp_attr_parse) pass startp to bgp_attr_as4_aggregator
* bgpd: Fix incorrect attribute type code in call to bgp_attr_malformedPaul Jakma2012-01-081-2/+2
|
* bgpd: Improve flag error messages in bgp_attr_aspathPaul Jakma2012-01-081-4/+13
| | | | | | | | * bgpd/bgp_attr.c: (bgp_attr_aspath) error message could be misleading, clearly log what flag was incorrect. (Problem noted in "bgpd: fix error message in bgp_attr_aspath()" in Quagga-RE)
* bgpd: rewrite attr flag error loggingDenis Ovsienko2012-01-081-54/+54
| | | | | | | | | | | | | | | | * bgp_attr.c * attr_flag_str: new message list * bgp_attr_flags_diagnose(): new function, implements previously added error logging in a generic way * bgp_attr_origin(): use bgp_attr_flags_diagnose() * bgp_attr_nexthop(): ditto * bgp_attr_med(): ditto * bgp_attr_local_pref(): ditto * bgp_attr_atomic(): ditto * bgp_attr_originator_id(): ditto * bgp_attr_cluster_list(): ditto * bgp_mp_reach_parse(): ditto * bgp_mp_unreach_parse(): ditto
* fix set never used warningsStephen Hemminger2011-12-181-16/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | (This patch was modified to leave calls to stream_getl() in place, they are necessary for the stream's internal pointer to advance to the correct position. -- Denis) Signed-off-by: Denis Ovsienko <infrastation@yandex.ru> Fix gcc warnings about varables that are set but never used. * bgpd/bgp_attr.c * cluster_unintern(): ret * transit_unintern(): ret * bgp_attr_default_intern(): attre * bgp_mp_reach_parse(): rd_high, rd_low * bgpd/bgp_route.c * bgp_announce_check_rsclient(): bgp * bgpd/bgp_zebra.c * zebra_read_ipv4(): ifindex * zebra_read_ipv6(): ifindex * bgpd/bgpd.c * bgp_config_write_peer(): filter * lib/distribute.c * distribute_list_all(): dist * distribute_list(): dist * distribute_list_prefix_all(): dist * distribute_list_prefix(): dist * lib/if_rmap.c * if_rmap(): if_rmap * lib/vty.c * vty_accept(): vty * lib/zclient.c * zclient_read(): ret * zebra/irdp_interface.c * if_group(): zi * zebra/rt_netlink.c * kernel_read(): ret, sock
* bgpd: fix memory leak for extra attributesOleg A. Arkhangelsky2011-12-031-0/+1
| | | | this fixes commit b881c7074bb698aeb1b099175b325734fc6e44d2
* bgpd: fix 2 more cases of length error reportingDenis Ovsienko2011-10-261-2/+2
| | | | | * bgp_attr.c (bgp_attr_originator_id, bgp_attr_cluster_list): provide required arguments to bgp_attr_malformed()
* bgpd: check AGGREGATOR attr flags (BZ#678)Denis Ovsienko2011-10-221-0/+17
| | | | | * bgp_attr.c * bgp_attr_aggregator(): check Optional/Transitive flag bits
* bgpd: fix more regressions in attr flag checksDenis Ovsienko2011-10-191-4/+4
| | | | | | | | | | | Commit 05a4936b713b9882171d0f7fb20b8439df23939e fixed some of the attributes involved, but not all. This commit should do it. * bgp_attr.c * bgp_attr_originator_id() * bgp_attr_cluster_list() * bgp_mp_reach_parse() * bgp_mp_unreach_parse()
* bgpd: use bgp_attr_malformed()Denis Ovsienko2011-10-181-17/+22
| | | | | | | | | | | | | | | Some of the recent attribute flags/length checks copied from QRE use bgp_notify_send_with_data() directly, but master branch assumes using bgp_attr_malformed(). * bgp_attr.c * bgp_attr_med() * bgp_attr_local_pref() * bgp_attr_atomic() * bgp_attr_originator_id() * bgp_attr_cluster_list() * bgp_mp_reach_parse() * bgp_mp_unreach_parse()
* bgpd: add flag checks for MP_(UN)REACH_NLRIDenis Ovsienko2011-10-171-6/+36
| | | | | | | | | | * bgp_attr.[ch] * bgp_mp_reach_parse(): add extra arguments and a uniform flag check block * bgp_mp_unreach_parse(): idem * bgp_attr_parse(): provide extra arguments * bgp_mp_attr_test.c * parse_test(): justify respective calls
* bgpd: fix spelling of CLUSTER_LISTDenis Ovsienko2011-10-171-1/+1
|
* bgpd: check CLUSTER_LIST attribute flagsDenis Ovsienko2011-10-171-2/+17
| | | | | | | | * bgp_attr.c * bgp_attr_cluster_list(): accept extra argument, add checks for "optional", "transitive" and "partial" bits, log each error condition independently * bgp_attr_parse(): provide extra arguments
* bgpd: check ORIGINATOR_ID attribute flagsDenis Ovsienko2011-10-171-2/+18
| | | | | | | | * bgp_attr.c * bgp_attr_originator_id(): accept extra argument, add checks for "optional", "transitive" and "partial" bits, log each error condition independently * bgp_attr_parse(): provide extra arguments
* bgpd: fix regression in improved attr flag checksDenis Ovsienko2011-10-121-5/+5
| | | | | | | | | | | | | Commit 2febf323411c1aed9d7694898f852ce2ef36a7e5 assumed every flag bit except optional/transitive/partial unset, which at times could not be true for "extended length" bit. * bgp_attr.c * bgp_attr_origin(): exclude BGP_ATTR_FLAG_EXTLEN from comparison * bgp_attr_nexthop(): idem * bgp_attr_med(): idem * bgp_attr_local_pref(): idem * bgp_attr_atomic(): idem
* bgpd: improve attr length error handling (BZ#679)Denis Ovsienko2011-10-081-12/+19
| | | | | | | | | | | | * bgp_attr.c * bgp_attr_parse(): provide extra argument to bgp_attr_aggregator() * bgp_attr_local_pref(): use bgp_notify_send_with_data() * bgp_attr_atomic(): idem * bgp_attr_aggregator(): idem Conflicts: bgpd/bgp_attr.c
* bgpd: improve attr flags checksDenis Ovsienko2011-09-301-138/+53
| | | | | | | | | | | | | | | | | | Do not check each of the Optional/Transitive/Partial attribute flag bits, when their only valid combination is known in advance, but still perform bit-deep error message logging. This change assumes unused (low-order) 4 bits of the flag octet cleared. * bgp_attr.c * bgp_attr_origin(): rewrite check * bgp_attr_nexthop(): idem * bgp_attr_med(): idem * bgp_attr_local_pref(): idem * bgp_attr_atomic(): idem Conflicts: bgpd/bgp_attr.c
* bgpd: ignore 4 bits of attribute flags byteDenis Ovsienko2011-09-301-1/+4
|
* bgpd: add missing "partial" flag checks (BZ#676)Denis Ovsienko2011-09-301-3/+38
| | | | | | | | | | | | | | | | | ORIGIN handling function used to have "partial" bit check and recent commits added it for NEXT_HOP, MULTI_EXIT_DISC and ATOMIC_AGGREGATE cases. This commit adds "partial" check for AS_PATH and LOCAL_PREF cases, which should leave attributes 1 through 6 inclusive completely covered with attribute flags checks. * bgp_attr.c * bgp_attr_origin(): use bit-by-bit checks for better diagnostics * bgp_attr_aspath(): add flag check * bgp_attr_local_pref(): idem Conflicts: bgpd/bgp_attr.c
* bgpd: improve NEXT_HOP attribute checks (BZ#680)Denis Ovsienko2011-09-301-6/+39
| | | | | | | | | | | | * lib/prefix.h * IPV4_CLASS_DE(): new helper macro * bgp_attr.c * bgp_attr_nexthop(): add check for "partial" bit, refresh flag error reporting, explain meaning of RFC4271 section 6.3 and implement it Conflicts: bgpd/bgp_attr.c
* bgpd: more SAFI fixesDenis Ovsienko2011-09-291-4/+4
| | | | | | | | | | | | | | | | | | | | | (with resolved conflict in bgpd/bgp_packet.c) Two macros resolving to the same integer constant broke a case block and a more thorough merge of BGP_SAFI_VPNV4 and BGP_SAFI_VPNV6 was performed. * bgpd.h: MPLS-labeled VPN SAFI is AFI-independent, switch to single * macro * bgp_capability_test.c: update test data * bgp_mp_attr_test.c: idem * bgp_route.c: (bgp_maximum_prefix_overflow, bgp_table_stats_vty) update macro and check conditions (where appropriate) * bgp_packet.c: (bgp_route_refresh_send, bgp_capability_send, bgp_update_receive, bgp_route_refresh_receive): idem * bgp_open.c: (bgp_capability_vty_out, bgp_afi_safi_valid_indices, bgp_open_capability_orf, bgp_open_capability): idem * bgp_attr.c: (bgp_mp_reach_parse, bgp_packet_attribute, bgp_packet_withdraw): idem
* bgpd: check ATOMIC_AGGREGATE attr flags (BZ#678)Denis Ovsienko2011-09-271-2/+38
| | | | | | | | * bgp_attr.c * bgp_attr_atomic(): accept extra argument, add checks for "optional", "transitive" and "partial" bits, log each error condition independently * bgp_attr_parse(): provide extra argument
* bgpd: check MULTI_EXIT_DISC attr flags (BZ#677)Denis Ovsienko2011-09-271-0/+32
| | | | | | * bgp_attr.c * bgp_attr_med(): add checks for "optional", "transitive" and "partial" bits, log each error condition independently
* bgpd: check LOCAL_PREF attribute flags (BZ#674)Denis Ovsienko2011-09-271-2/+27
| | | | | | | | * bgp_attr.c * bgp_attr_local_pref(): accept extra argument, add checks for "optional" and "transitive" bits, log each error condition independently * bgp_attr_parse(): provide extra argument
* bgpd: consistent log msg format (BZ#565)heasley2011-09-271-5/+5
|
* Merge branch 'attr-errors'Paul Jakma2011-07-291-258/+357
|\ | | | | | | | | | | | | | | Contains BGP fixes: - set extcommunity crash: tihs patch tries to make the refcounting more robust but does not fully solve the problem, sadly. - BGP attribute error handling: Little testing.