| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
|
|
|
| |
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>
|
|
|
|
|
|
| |
inet_ntop expects network byte order.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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>
|
|
|
|
|
|
|
|
|
| |
* 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>
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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>
|
|
|
|
|
|
|
| |
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>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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 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>
|
|
|
|
|
|
|
| |
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>
|
|
|
|
|
|
|
| |
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>
|
|
|
|
|
| |
Signed-off-by: Jorge Boncompte [DTI2] <jorge@dti2.net>
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
|
|
|
|
|
|
|
| |
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>
|
|
|
|
|
| |
Signed-off-by: Jorge Boncompte [DTI2] <jorge@dti2.net>
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
|
|
|
|
|
|
|
|
|
| |
* 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>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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>
|
|
|
|
| |
* bgp_attr.c: (bgp_attr_unknown) total should be initialised from the args.
|
|
|
|
|
|
| |
* 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/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.
|
|
|
|
|
| |
* bgp_attr.c: (bgp_attr_flag_invalid) flags is meant to be masked
off with the mask variable...
|
|
|
|
|
|
|
|
|
| |
* 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.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* 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/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/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)
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* 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
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
(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
|
|
|
|
| |
this fixes commit b881c7074bb698aeb1b099175b325734fc6e44d2
|
|
|
|
|
| |
* bgp_attr.c (bgp_attr_originator_id, bgp_attr_cluster_list): provide
required arguments to bgp_attr_malformed()
|
|
|
|
|
| |
* bgp_attr.c
* bgp_attr_aggregator(): check Optional/Transitive flag bits
|
|
|
|
|
|
|
|
|
|
|
| |
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()
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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()
|
|
|
|
|
|
|
|
|
|
| |
* 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
|
| |
|
|
|
|
|
|
|
|
| |
* 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
|
|
|
|
|
|
|
|
| |
* 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
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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
|
|
|
|
|
|
|
|
|
|
|
|
| |
* 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
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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
|
|
|
|
|
|
|
|
|
|
|
|
| |
* 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
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
(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
|
|
|
|
|
|
|
|
| |
* 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
|
|
|
|
|
|
| |
* bgp_attr.c
* bgp_attr_med(): add checks for "optional", "transitive" and
"partial" bits, log each error condition independently
|
|
|
|
|
|
|
|
| |
* 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
|
| |
|
|\
| |
| |
| |
| |
| |
| |
| | |
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.
|