| Age | Commit message (Collapse) | Author |
|---|
|
* if.c: (if_lookup_by_name) shouldn't crash just cause we got a NULL name
|
|
Start BGP listener only after first instance is started. This helps the
security if BGP is not used but daemon is started. It also addresses some
issues like MD5 not working on listener unless IPV6 configured (because
listener was not in list); as well as compiler warnings.
* bgp_network.c: (bgp_listener) listen socket creation consolidated here
(bgp_socket) Use bgp_listener
* bgpd.c: (bgp_get) call bgp_socket on creation of first struct bgp.
(bgp_init) remove bgp_socket call.
* memtypes.c: Add MTYPE_BGP_LISTENER
|
|
I've spent the last several weeks working on stability fixes to bgpd.
These patches fix all of the numerous crashes, assertion failures, memory
leaks and memory stomping I could find. Valgrind was used extensively.
Added new function bgp_exit() to help catch problems. If "debug bgp" is
configured and bgpd exits with status of 0, statistics on remaining
lib/memory.c allocations are printed to stderr. It is my hope that other
developers will use this to stay on top of memory issues.
Example questionable exit:
bgpd: memstats: Current memory utilization in module LIB:
bgpd: memstats: Link List : 6
bgpd: memstats: Link Node : 5
bgpd: memstats: Hash : 8
bgpd: memstats: Hash Bucket : 2
bgpd: memstats: Hash Index : 8
bgpd: memstats: Work queue : 3
bgpd: memstats: Work queue item : 2
bgpd: memstats: Work queue name string : 3
bgpd: memstats: Current memory utilization in module BGP:
bgpd: memstats: BGP instance : 1
bgpd: memstats: BGP peer : 1
bgpd: memstats: BGP peer hostname : 1
bgpd: memstats: BGP attribute : 1
bgpd: memstats: BGP extra attributes : 1
bgpd: memstats: BGP aspath : 1
bgpd: memstats: BGP aspath str : 1
bgpd: memstats: BGP table : 24
bgpd: memstats: BGP node : 1
bgpd: memstats: BGP route : 1
bgpd: memstats: BGP synchronise : 8
bgpd: memstats: BGP Process queue : 1
bgpd: memstats: BGP node clear queue : 1
bgpd: memstats: NOTE: If configuration exists, utilization may be expected.
Example clean exit:
bgpd: memstats: No remaining tracked memory utilization.
This patch fixes bug #397: "Invalid free in bgp_announce_check()".
This patch fixes bug #492: "SIGBUS in bgpd/bgp_route.c:
bgp_clear_route_node()".
My apologies for not separating out these changes into individual patches.
The complexity of doing so boggled what is left of my brain. I hope this
is all still useful to the community.
This code has been production tested, in non-route-server-client mode, on
a linux 32-bit box and a 64-bit box.
Release/reset functions, used by bgp_exit(), added to:
bgpd/bgp_attr.c,h
bgpd/bgp_community.c,h
bgpd/bgp_dump.c,h
bgpd/bgp_ecommunity.c,h
bgpd/bgp_filter.c,h
bgpd/bgp_nexthop.c,h
bgpd/bgp_route.c,h
lib/routemap.c,h
File by file analysis:
* bgpd/bgp_aspath.c: Prevent re-use of ashash after it is released.
* bgpd/bgp_attr.c: #if removed uncalled cluster_dup().
* bgpd/bgp_clist.c,h: Allow community_list_terminate() to be called from
bgp_exit().
* bgpd/bgp_filter.c: Fix aslist->name use without allocation check, and
also fix memory leak.
* bgpd/bgp_main.c: Created bgp_exit() exit routine. This function frees
allocations made as part of bgpd initialization and, to some extent,
configuration. If "debug bgp" is configured, memory stats are printed
as described above.
* bgpd/bgp_nexthop.c: zclient_new() already allocates stream for
ibuf/obuf, so bgp_scan_init() shouldn't do it too. Also, made it so
zlookup is global so bgp_exit() can use it.
* bgpd/bgp_packet.c: bgp_capability_msg_parse() call to bgp_clear_route()
adjusted to use new BGP_CLEAR_ROUTE_NORMAL flag.
* bgpd/bgp_route.h: Correct reference counter "lock" to be signed.
bgp_clear_route() now accepts a bgp_clear_route_type of either
BGP_CLEAR_ROUTE_NORMAL or BGP_CLEAR_ROUTE_MY_RSCLIENT.
* bgpd/bgp_route.c:
- bgp_process_rsclient(): attr was being zero'ed and then
bgp_attr_extra_free() was being called with it, even though it was
never filled with valid data.
- bgp_process_rsclient(): Make sure rsclient->group is not NULL before
use.
- bgp_processq_del(): Add call to bgp_table_unlock().
- bgp_process(): Add call to bgp_table_lock().
- bgp_update_rsclient(): memset clearing of new_attr not needed since
declarationw with "= { 0 }" does it. memset was already commented
out.
- bgp_update_rsclient(): Fix screwed up misleading indentation.
- bgp_withdraw_rsclient(): Fix screwed up misleading indentation.
- bgp_clear_route_node(): Support BGP_CLEAR_ROUTE_MY_RSCLIENT.
- bgp_clear_node_queue_del(): Add call to bgp_table_unlock() and also
free struct bgp_clear_node_queue used for work item.
- bgp_clear_node_complete(): Do peer_unlock() after BGP_EVENT_ADD() in
case peer is released by peer_unlock() call.
- bgp_clear_route_table(): Support BGP_CLEAR_ROUTE_MY_RSCLIENT. Use
struct bgp_clear_node_queue to supply data to worker. Add call to
bgp_table_lock().
- bgp_clear_route(): Add support for BGP_CLEAR_ROUTE_NORMAL or
BGP_CLEAR_ROUTE_MY_RSCLIENT.
- bgp_clear_route_all(): Use BGP_CLEAR_ROUTE_NORMAL.
Bug 397 fixes:
- bgp_default_originate()
- bgp_announce_table()
* bgpd/bgp_table.h:
- struct bgp_table: Added reference count. Changed type of owner to be
"struct peer *" rather than "void *".
- struct bgp_node: Correct reference counter "lock" to be signed.
* bgpd/bgp_table.c:
- Added bgp_table reference counting.
- bgp_table_free(): Fixed cleanup code. Call peer_unlock() on owner if
set.
- bgp_unlock_node(): Added assertion.
- bgp_node_get(): Added call to bgp_lock_node() to code path that it was
missing from.
* bgpd/bgp_vty.c:
- peer_rsclient_set_vty(): Call peer_lock() as part of peer assignment
to owner. Handle failure gracefully.
- peer_rsclient_unset_vty(): Add call to bgp_clear_route() with
BGP_CLEAR_ROUTE_MY_RSCLIENT purpose.
* bgpd/bgp_zebra.c: Made it so zclient is global so bgp_exit() can use it.
* bgpd/bgpd.c:
- peer_lock(): Allow to be called when status is "Deleted".
- peer_deactivate(): Supply BGP_CLEAR_ROUTE_NORMAL purpose to
bgp_clear_route() call.
- peer_delete(): Common variable listnode pn. Fix bug in which rsclient
was only dealt with if not part of a peer group. Call
bgp_clear_route() for rsclient, if appropriate, and do so with
BGP_CLEAR_ROUTE_MY_RSCLIENT purpose.
- peer_group_get(): Use XSTRDUP() instead of strdup() for conf->host.
- peer_group_bind(): Call bgp_clear_route() for rsclient, and do so with
BGP_CLEAR_ROUTE_MY_RSCLIENT purpose.
- bgp_create(): Use XSTRDUP() instead of strdup() for peer_self->host.
- bgp_delete(): Delete peers before groups, rather than after. And then
rather than deleting rsclients, verify that there are none at this
point.
- bgp_unlock(): Add assertion.
- bgp_free(): Call bgp_table_finish() rather than doing XFREE() itself.
* lib/command.c,h: Compiler warning fixes. Add cmd_terminate(). Fixed
massive leak in install_element() in which cmd_make_descvec() was being
called more than once for the same cmd->strvec/string/doc.
* lib/log.c: Make closezlog() check fp before calling fclose().
* lib/memory.c: Catch when alloc count goes negative by using signed
counts. Correct #endif comment. Add log_memstats_stderr().
* lib/memory.h: Add log_memstats_stderr().
* lib/thread.c: thread->funcname was being accessed in thread_call() after
it had been freed. Rearranged things so that thread_call() frees
funcname. Also made it so thread_master_free() cleans up cpu_record.
* lib/vty.c,h: Use global command_cr. Add vty_terminate().
* lib/zclient.c,h: Re-enable zclient_free().
|
|
* lib/sockunion.c: (sockunion_normalise_mapped) The code to normalize
address was not copying port value - probably reason why IPV4 in IPV6
never worked right.
|
|
* lib/thread.{c,h}: As per subject. This will avoid head-scratching for next
person who adds a thread-type and gets strange breakage.
|
|
* bgpd/bgp_network.c: Fix MD5 listen in IPv4 version of bgp_socket() by
adding listen socket to listen_sockets list so that MD5 passwords can
get set.
* lib/sockopt.c: (sockopt_tcp_signature) Fix bogus "% Error while applying
TCP-Sig to session(s)" / "can't set TCP_MD5SIG option" startup error
messages by not returning error when there isn't one.
|
|
* configure.ac: Check for the actual CLOCK_MONOTONIC symbol, as clock_gettime
need not imply that clock type is supported
* lib/zebra.h: use the method given in autoconf docs for sys/time.h
inclusion
|
|
Avoid bugs, and improve cache locality by allowing message lists
to be read/only.
|
|
smux_trap and oid_copy should allow read-only source.
|
|
When BGP crashes, sometimes all we get to see is syslog.
|
|
An external user was building on system without IPv6 and
it wouldn't work.
|
|
|
|
The cmd_nodes used to configure vty, can mostly be static so
(basic data hiding 101).
|
|
Use Ansi-C prototypes rather than old K&R method of declaring
function without arguments
|
|
|
|
Simple conversion of XMALLOC/memset to XCALLOC
|
|
* configure.ac: Add a --enable-linux24-tcp-md5 argument, to compile in
support for the old TCP-MD5 patch for Linux 2.4 systems. This overrides
auto-detection of TCP-MD5 supported by the target system.
* lib/sockopt.c: (sockopt_tcp_signature) add in a variant for the old
Linux 2.4, TCP_MD5_AUTH variant of TCP-MD5 support, conditional
on the previous configure arg.
|
|
Compiled on 32-bit and 64-bit linux gcc 4.1.2 platforms.
No run-time testing on 32-bit and limited run-time testing on 64-bit.
|
|
Macro SNMP_INTEGER() prepares data which is eventually processed
by asn_build_int(). SNMP_INTEGER() was using "int32_t" whereas
asn_build_int() uses "long". On 32-bit systems these are the
same, both 4 bytes, but on x86 64-bit systems "long" is 8 bytes.
asn_build_int()'s reaction to an improperly sized value is to return
a NULL pointer. Quagga's smux.c would eventually get this NULL
pointer and use it in calculations to determine how much data to send
over the smux connection, resulting in garbage being sent to the
SNMP agent.
Corrected SNMP_INTEGER() to use "long".
Tested on 32-bit and 64-bit x86 Linux 2.6.27.10 systems running
Quagga 0.99.11 with bgpd smux.
|
|
* lib/workqueue.c: free-ing workqueues had never been tested obviously,
queue's thread was not being stopped
* bgpd/bgpd.c: null out freed workqueues, to prevent acting on freed
workqueues
|
|
Whenever a thread adds an timer funcname_thread_add_timer_timeval() gets called
to add the timer. Before adding the timer a quagga_gettimeofday() call
is made to do some time house keeping. However quagga_gettimeofday() only
updates recent_time, not relative_time that is used to calculate
the alarm_time. Replace with quagga_get_relative (NULL)
|
|
The int type cast should be on the whole expression
passed to the mod operator. Otherwise it won't work
when/if c0/c1 is unsigned.
Making c0/c1 unsigned makes it possible to use 5802
as MODX value.
|
|
* lib/sockunion.c: (sockunion_accept) sockunion needs to be normalised here,
as it used to before this was moved to a dedicated function..
|
|
* lib/checksum.c: (fletcher_checksum) Switch the second phase of the checksum
back to the old ospfd logic.
The isisd-derived version:
a) is very hard to follow
b) had some kind of subtle bug that caused it be wrong when c0=0 and c1=254
(potentially fixable by doing the mods before adjusting x and y)
Additionally:
- explicitely cast expressions using non-internal variables to int, to ensure
the result is signed.
- defensively change the length argument to 'size_t', to ensure the code
works with that argument being unsigned..
Thanks to Joakim Tjernlund for the investigative work into this bug.
* tests/test-checksum.c: new file to exercise the checksum code.
|
|
* lib/sockunion.c: Re-enable conversion of v6mapped INET6 sockunions to
regular INET sockunions, which was disabled in 0df7c9, as it leads to
failures in sockunion_cmp.
(sockunion_normalise_mapped) consolidate here
(sockunion_accept) remove conversion code altogether here - no sockunion
is created
(sockunion_get{sockname,peername}) normalise newly created sockunion.
|
|
|
|
--without-crypto
Autoconfig work by me, the rest was done by
"Kirill K. Smirnov" <lich@math.spbu.ru>
|
|
* lib/command.h: Add a RESTRICTED_NODE, intended for use with
anonymous, 'no login' vtys, to provide a subset of 'view' mode
commands.
* lib/command.c: Add RESTRICTED_NODE bits, nothing special, just
following VIEW_NODE.
* lib/vty.c: (vty_auth) enable authentication should fall back to
restricted/view node as appropriate.
(vty_create) init vty's to restricted/view node as appropriate,
for the 'no login' case.
(vty_{no_,}restricted_mode_cmd) config commands to enable
'anonymous restricted' in vty configuration.
(vty_config_write) 'anonymous restricted' config.
(vty_init) Install some commands to restricted mode, and the
'anonymous restricted' config commands into VTY_NODE.
* bgpd/*.c: Install some of the safe(r) BGP commands into
'restricted mode', i.e. lookup commands of non-sensitive data.
Useful with looking-glass route-servers.
|
|
|
|
|
|
2008-08-15 Paul Jakma <paul.jakma@sun.com>
* */*: Fix various problems flagged by Sun Studio compiler.
- '<qualifier> <storage>' obsolescent in declarations
- empty statements (';' after ALIAS definitions)
- implicit declarations (e.g printstack in lib/log.c)
- "\%" in printf string instead of "%%"
- loops that return on the first iteration (legitimately, but
compiler can't really know), e.g. bgp_routemap.c
- internal declarations which mask prototypes.
|
|
2008-08-15 Paul Jakma <paul.jakma@sun.com>
* lib/sockopt.c: (sockopt_tcp_signature) ifdef'ing hid variable
declaration for HAVE_DECL_TCP_MD5SIG && !GNU_LINUX, breaking
compile - fix.
|
|
2008-08-14 Stephen Hemminger <stephen.hemminger@vyatta.com>
* lib/log.{c,h}: struct message argument should point to const
* */*.c: adjust to suit,
Signed-off-by: Paul Jakma <paul@quagga.net>
|
|
2008-08-14 Stephen Hemminger <stephen.hemminger@vyatta.com>
* lib/hash.h: (struct hash) Hash comparator callback really
ought to treat storage behind arguments as constant - a compare
function with side-effects would be evil.
* */*.c: Adjust comparator functions similarly, thus fixing at least
a few compiler warnings about const qualifier being dropped.
Signed-off-by: Paul Jakma <paul@quagga.net>
|
|
2008-08-13 Jingjing Duan <Jingjing.Duan@sun.com>
* lib/checksum.?: (fletcher_checksum) implementation of
Fletcher checksum, as per RFC1008.
Signed-off-by: Paul Jakma <paul@quagga.net>
|
|
2008-08-13 roy <quagga@de.vio.us>
* lib/vty.c: (vty_delete_char) move check for authentication
down a bit, so we do the delete, but still not re-write of line.
Signed-off-by: Paul Jakma <paul@quagga.net>
|
|
|
|
2008-07-21 Paul Jakma <paul.jakma@sun.com>
* bgp_packet.c: (bgp_open_receive) fix warning in a zlog call
* bgp_vty.c: (bgp_vty_return) add return code
* bgpd.c: (bgp_master_init) setup the socket list.
* bgp_network.c: Remove the dual IPv4/6 socket thing for now, which
was implemented by Michael, until such time as its clear its
required for Linux (see sockopt comments). IPv6 support, including
IPv4 sessions on AF_INET6 sockets, therefore is broken, and the
'-l 0.0.0.0' arguments would need to be given to bgpd to make
things work here.
2008-07-21 Michael H. Warfield <mhw@wittsend.com>
YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
Tomohiko Kusuda <kusuda@inetcore.com>
Leigh Brown <leigh@solinno.co.uk>
* bgp_network.c: (bgp_md5_set_one) shim between libzebra tcp-md5
sockopt and bgpd.
(bgp_md5_set_socket) Helper for bgp_connect
(bgp_md5_set) setup TCP-MD5SIG for the given peer.
(bgp_connect) call out to bgp_md5_set_socket for the outgoing
connect socket.
(bgp_socket) save references to the listen sockets, needed if
TCP-MD5SIG is applied later or changed.
* bgp_vty.c: (*neighbor_password_cmd) New 'neighbor ... password'
commands.
* bgpd.c: (peer_{new,delete) manage TCP-MD5 password
(peer_group2peer_config_copy) inherit TCP-MD5 password
(peer_password_{un,}set) orchestrate the whole add/remove of TCP-MD5
passwords: applying checks, stopping peers, and trying to return
errors to UI, etc.
(bgp_config_write_peer) save password.
Fix missing newline in writeout of neighbor ... port.
2008-07-21 Paul Jakma <paul.jakma@sun.com>
* sockunion.c: ifdef out various places that converted
v4mapped sockets to pure v4. Doesn't seem necessary at all,
presumably a workaround for now historical inet_ntop bugs (?)
2008-07-21 Michael H. Warfield <mhw@wittsend.com>
YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
* sockopt.{c,h}: (sockopt_tcp_signature) Add TCP-MD5SIG support.
|
|
2008-06-07 Paul Jakma <paul@jakma.org>
* stream.{c,h}: (stream_{put,write}) add const qualifier to source
argument. Change u_char to void *.
|
|
|
|
2008-05-29 Stephen Hemminger <stephen.hemminger@vyatta.com>
* rt_netlink.c: (netlink_install_filter) BPF filter to catch and
drop responses to zebra's own route messages.
(kernel_init) add BPF filter on the netlink socket.
|
|
2008-02-28 Paul Jakma <paul.jakma@sun.com>
* log.c: (mes_lookup) Sowmini Varadhan diagnosed a problem where
this function can cause a NULL dereference, on lookups for unknown
indices, or messages with NULL strings. Can occur, e.g., debug
logging code when processing received messages. Fixed to accept a
pointer to a default string to be used if there is no match.
* log.h: LOOKUP adjusted to match
|
|
2008-02-28 Paul Jakma <paul.jakma@sun.com>
* linklist.c: This implementation expects that the data pointer not
be null, e.g. listgetdata() asserts this. The list add methods
don't apply the same sanity check.
Noted by Jim Carlson in bug #437.
|
|
2008-01-30 Peter Szilagyi <sp615@hszk.bme.hu>
* lib/stream.h: Remove named 'new' parameter in prototype
for c++ header compatibility.
* ospfd/ospf_opaque.h: ditto
* ospfd/ospfd.h: Renamed struct export to _export for c++
header compatibility.
* ospf6d/ospf6_area.h: ditto
|
|
2008-01-11 Andrew J. Schorr <ajschorr@alumni.princeton.edu>
* lib/zebra.h: Revert previous change, no need to include
<net/if_media.h> here.
* zebra/ioctl.c: If HAVE_BSD_LINK_DETECT is defined, include
<net/if_media.h>
(if_get_flags) Remove debug messages about BSD link state.
* zebra/kernel_socket.c: (bsd_linkdetect_translate) If link state
is unknown, we should set the IFF_RUNNING flag.
|
|
2008-01-10 Ingo Flaschberger <if@xip.at>
* configure.ac: Define HAVE_BSD_LINK_DETECT if <net/if_media.h> is
present.
* lib/zebra.h: If HAVE_BSD_LINK_DETECT is defined,
include <net/if_media.h>.
* zebra/ioctl.c: (if_get_flags) If HAVE_BSD_LINK_DETECT, use the
SIOCGIFMEDIA ioctl to ascertain link state.
* zebra/kernel_socket.c: (bsd_linkdetect_translate) New function to
map the ifm_data.ifi_link_state value into the IFF_RUNNING flag.
(ifm_read) Call bsd_linkdetect_translate to fix the IFF_RUNNING
flag before calling if_flags_update.
|
|
2008-01-08 Pavol Rusnak <prusnak@suse.cz>
* memory.c: (mtype_memstr) Fix accidental shift past width of type,
constant should have been forced to UL, rather than being left to
default to int.
|
|
existing static routes to be revalidated)
|
|
2007-10-22 Lorenzo Colitti <lorenzo@colitti.com>
* smux.c: (smux_stop) Avoid cancelling a defunct thread pointer
(smux_start) Stop smux before trying to start it, possibly again.
|
|
|
