BGP uses KEEPALIVE packets and the HOLD timer to detect stalled sessions.
The problem is that this timer only looks at the receive side of the TCP
session. If for some reason the send side stalls the

BGP uses KEEPALIVE packets and the HOLD timer to detect stalled sessions.
The problem is that this timer only looks at the receive side of the TCP
session. If for some reason the send side stalls the system fully depends
on the remote BGP peer to reset the session. As seen in an ever growing
OutQ and as a result important changes can get stalled and cause routing
troubles.

This change introduces a SEND HOLD timer. The timer is reset whenever the
session engine was able to write data to the TCP socket. If the send hold
timer expires bgpd was not able to send any data to that neighbor for at
least 90 seconds and therefor the session is forcefully closed with a hold
timer expired notification.

The send hold timer acts as a last resort to detect faulty peers. On an
idle session it can take a long time until this timer triggers but the
main goal here is to reset a stuck session at some point which did not
happen before.

With and OK job@

show more ...

Make the timer code independent of struct peer this way it can be used
in other places as well.
ok procter@

Refactor the control code to be more like a module.
Change control_dispatch_msg() to return the change of control connection
count since the return value was not used before.
Add control_fill_pfds()

Refactor the control code to be more like a module.
Change control_dispatch_msg() to return the change of control connection
count since the return value was not used before.
Add control_fill_pfds() to replace the TAILQ_FOREACH loop in session.c.
This allows to move the ctl_conns symbol to control.c (from session.h
where it caused issues when compiled with -fno-common).
OK benno@

show more ...

In bgpctl argument parser, re-arrange 'reason' parsing ('nei action [reason]')
to be more generic, then change 'reload' to take take a '[reason]' also,
which will be logged by bgpd.
ok kn claudio

Move the code to figure out the alternate IP address (IPv6 addr for IPv4
sessions and vice versa) from the RDE to the SE. The SE is the right place
for this since there getsockname(2) fetches the loc

Move the code to figure out the alternate IP address (IPv6 addr for IPv4
sessions and vice versa) from the RDE to the SE. The SE is the right place
for this since there getsockname(2) fetches the local address and so the
alternate one can be fetched there as well.
With this the route pledge is no longer needed in the RDE and the pledge
is now just "stdio recvfd".
OK benno@

show more ...

Implement 'max-prefix NUM out' to limit the number of announced prefixes.
This is an easy safety switch to not leak full tables to upstreams and
peers. If the limit is hit a Cease notification is sen

Implement 'max-prefix NUM out' to limit the number of announced prefixes.
This is an easy safety switch to not leak full tables to upstreams and
peers. If the limit is hit a Cease notification is sent and the session
is closed.
This implements most of https://tools.ietf.org/html/draft-sa-idr-maxprefix-00
OK job@

show more ...

Add last_rcvd_errcode and last_rcvd_suberr to the peer_stats so that
not only sent errors can be reported but also received ones.
OK denis@ deraadt@

Convert last_updown and last_read from time(3) to use getmonotime()
which returns clock_gettime(2) with CLOCK_MONOTONIC. While doing that
introduce last_write for symetry.
OK denis@

Add some TCP MD5SUM specific calls needed by protable. On Linux systems
there is no pfkey interface and instead the loading is done via setsockopts.
To make this work correctly there are some additio

Add some TCP MD5SUM specific calls needed by protable. On Linux systems
there is no pfkey interface and instead the loading is done via setsockopts.
To make this work correctly there are some additional functions needed when
peers get reconfigured or when new listeners show up.
OK benno@

show more ...

Pass a struct listen_addr pointer to tcp_md5_listen and not just the fd,
the linux shim needs this.

Switch the peer TAILQ to a RB tree indexed by the peer id. This way
getpeerbyid() gets a lot quicker at finding the peer when many peers
are configured. In my test case the difference is around 20% r

Switch the peer TAILQ to a RB tree indexed by the peer id. This way
getpeerbyid() gets a lot quicker at finding the peer when many peers
are configured. In my test case the difference is around 20% runtime.
OK denis@

show more ...

Change timer_nextisdue() and timer_nextduein() to take the current time
as an argument. This way getmonotime() can be called once at the start
of looping over all peers instead of twice during the lo

Change timer_nextisdue() and timer_nextduein() to take the current time
as an argument. This way getmonotime() can be called once at the start
of looping over all peers instead of twice during the loop.
Makes a big difference with many peers.
OK florian@ sthen@

show more ...

Rework the TCP md5sig and IKE handling. Move the pfkey socket to the parent
process in this process. The refreshing of the keys is done whenever the
session state is changes to state IDLE or ACTIVE.

Rework the TCP md5sig and IKE handling. Move the pfkey socket to the parent
process in this process. The refreshing of the keys is done whenever the
session state is changes to state IDLE or ACTIVE. This should behave better
when reloading configs with auth changes.
OK benno@

show more ...

Be more careful when setting timeout to 0 because there is pending work
on a peer. Just checking the peer read buffer size is not enough since
the data present could be a partial message and so the S

Be more careful when setting timeout to 0 because there is pending work
on a peer. Just checking the peer read buffer size is not enough since
the data present could be a partial message and so the SE should sleep
until a new POLLIN event fires. Adjust the logic by adding a rpending
flag that is only set if reading the session buffer was exited early
because MSG_PROCESS_LIMIT was hit.
OK benno@

show more ...

Move the struct peer into bgpd_config and switch it to a TAILQ instead of
the hand-rolled list. This changes the way peers are reloaded since now
both parent and session engine are now merging the li

Move the struct peer into bgpd_config and switch it to a TAILQ instead of
the hand-rolled list. This changes the way peers are reloaded since now
both parent and session engine are now merging the lists.
OK denis@

show more ...

Do a better job at cleaning up the config on shutdown. Remove bits that
were missed before (e.g. network related objects). This helps to detect
memory leaks.
Start using new_config() and free_config(

Do a better job at cleaning up the config on shutdown. Remove bits that
were missed before (e.g. network related objects). This helps to detect
memory leaks.
Start using new_config() and free_config() in all places where bgpd_config
structure are used. This way the struct is properly initialised and cleaned
up. Introduce copy_config() to only copy the values into the other struct
leaving the pointers as they were.
Looks good to benno@

show more ...

Convert the remote and local addresses in struct peer to be bgpd_addrs
instead of sockaddr_storage. This again helps protability and simplifies
some code. sa2addr now takes an optional pointer to ret

Convert the remote and local addresses in struct peer to be bgpd_addrs
instead of sockaddr_storage. This again helps protability and simplifies
some code. sa2addr now takes an optional pointer to return the port of
the sockaddr.
OK benno@

show more ...

Convert some (u_)int64_t to (unsigned) long long. Makes portability a bit
nicer since it requires less casts.
sigh deraadt@

Move get_mpe_config() to kroute.c so that config.c does not depend on
system specific code.

Do not depend on the length field of struct sockaddr instead pass the
length to the various functions needing it. Helps portability.
OK benno@

The definition of VPNs in bgpd was never super elegant. The 'depend on
mpeX' config was a bit redundant. Also to make it more flexible (e.g. having
more than one mpeX interface per rdomain the syntax

The definition of VPNs in bgpd was never super elegant. The 'depend on
mpeX' config was a bit redundant. Also to make it more flexible (e.g. having
more than one mpeX interface per rdomain the syntax was changed.

To make this possible especially the network distribution logic had to be
adjusted and cleaned up. This should in general make network statements
well defined and conflicts between 'network A.B.C.D/N' and e.g. 'network static'
are handled in a well defined way ('network A.B.C.D/N' has preference).

With and OK dlg@, OK denis@

show more ...

Support group descriptions in control messages that accept a neighbor
description. With this it is possible to show rib a group of peers, to show
or signal a group of peers all at once.
OK sthen@ ben

Support group descriptions in control messages that accept a neighbor
description. With this it is possible to show rib a group of peers, to show
or signal a group of peers all at once.
OK sthen@ benno@

show more ...

Check if a control socket or address is already in use befor using it.
If it is used abort startup or let a reload fail.
Sockets are now not unlinked anymore on regular shutdown.

This helps a lot wh

Check if a control socket or address is already in use befor using it.
If it is used abort startup or let a reload fail.
Sockets are now not unlinked anymore on regular shutdown.

This helps a lot when one tries to do a config check without -n.

Inputs and OK claudio@

show more ...

bgpctl can cause bgpd to do a lot of work (e.g. dumping all of the RIB)
but then bgpctl can quickly exit and bgpd still has to do all the work.
Instead introduce a terminate imsg to stop such long ru

bgpctl can cause bgpd to do a lot of work (e.g. dumping all of the RIB)
but then bgpctl can quickly exit and bgpd still has to do all the work.
Instead introduce a terminate imsg to stop such long running commands if
bgpctl closes the connection before the run is over.
OK benno@, sthen@, deraadt@

show more ...

Major refactoring of the RIB handling code. Mainly change how the RIB is
walked. rib_dump_r() is now an internal function and instead the code gets
an additional callback for throttling the rib_dump

Major refactoring of the RIB handling code. Mainly change how the RIB is
walked. rib_dump_r() is now an internal function and instead the code gets
an additional callback for throttling the rib_dump code. This removes a
lot of similar code used to make sure the RDE is not walking to fast and
replaces it with simpler callbacks. The other big change is the removal
of struct rib pointers in other data structures. The rib pointers are not
stable because of a realloc() call happening when extending the array so
instead use the RIB ID as a reference.
Tested and OK denis@ and benno@

show more ...