* Process FIB update in bgp_zebra_route_notify_owner() and call
group_announce_route() if route is installed
* When bgp update is received for a route which is not installed earlier
(flag BGP_NODE_FIB_INSTALLED is not set) and suppress fib is enabled
set the flag BGP_NODE_FIB_INSTALL_PENDING to indicate fib install is
pending for the route. The route will be advertised when zebra send
ZAPI_ROUTE_INSTALLED status.
* The advertisement delay (BGP_DEFAULT_UPDATE_ADVERTISEMENT_TIME)
is added to allow more routes to be sent in single update message.
This is required since zebra sends route notify message for each route.
The delay will be applied to update group timer which advertises
routes to peers.
Signed-off-by: kssoman <somanks@gmail.com>
- tracepoint() -> frrtrace()
- tracelog() -> frrtracelog()
- tracepoint_enabled() -> frrtrace_enabled()
Also removes copypasta'd #ifdefs for those LTTng macros, those are
handled in lib/trace.h
Signed-off-by: Quentin Young <qlyoung@nvidia.com>
Replace all lib/thread cancel macros, use thread_cancel()
everywhere. Only the THREAD_OFF macro and thread_cancel() api are
supported. Also adjust thread_cancel_async() to NULL caller's pointer (if
present).
Signed-off-by: Mark Stapp <mjs@voltanet.io>
When received packet is processed in bgp_process_reads(), the data
is copied to static buffer and then copied to stream buffer.
The data can be copied directly to stream buffer which will avoid extra memcpy
Signed-off-by: kssoman <somanks@gmail.com>
bgp tcp connection.
When the BGP peer is configured between two bgp routes both routers would create
peer structure , when they receive each other’s open message. In this event both
speakers, open duplicate TCP sessions and send OPEN messages on each socket
simultaneously, the BGP Identifier is used to resolve which socket should be closed.
If BGP GR is enabled the old tcp session is dumped and the new session is retained.
So while this transfer of connection is happening, if all the bgp gr config
is not migrated to the new connection, the new bgp gr mode will never get applied.
Fix Summary:
1. Replicate GR configuration from the old session to the new session in bgp_accept().
2. Replicate GR configuration from stub to full-fledged peer in bgp_establish().
3. Disable all NSF flags, clear stale routes (if present), stop restart & stale timers
(if they are running) when the bgp GR mode is changed to “Disabled”.
4. Disable R-bit in cap, if it is not set the received open message.
Signed-off-by: Biswajit Sadhu <sadhub@vmware.com>
bgp_process_packets has an assert to make sure an appropriate amount of
working space in the input buffer has been freed up for future reads.
However, this assert shouldn't be made when we have encountered an error
that's going to tear down the session, because in this case we may not
be able to process the full contents of the input buffer.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
frr_with_mutex(...) { ... } locks and automatically unlocks the listed
mutex(es) when the block is exited. This adds a bit of safety against
forgetting the unlock in error paths & co. and makes the code a slight
bit more readable.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
All I can see is an unneccessary complication. If there's some purpose
here it needs to be documented...
Signed-off-by: David Lamparter <equinox@diac24.net>
The following types are nonstandard:
- u_char
- u_short
- u_int
- u_long
- u_int8_t
- u_int16_t
- u_int32_t
Replace them with the C99 standard types:
- uint8_t
- unsigned short
- unsigned int
- unsigned long
- uint8_t
- uint16_t
- uint32_t
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
Use the new threading facilities provided in lib/ to streamline the
threads used in bgpd. In particular, all of the lifecycle code has been
removed from the I/O thread and replaced with the default loop. Did not
do the same to the keepalives thread as it is much smaller (doesn't need
the event system).
Also cleaned up some comments to match the style guide.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
Condition needs to be set inside critical section, otherwise i/o thread
can deadlock. Also unlock mutex once finished with it, no need to hold
the lock for the life of the program.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
The BGP IO thread must be running before other threads
can start using it. So at startup check to see
that it running once, instead of before every
function call into.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
The multithreading code has a comment that reads:
"XXX: Heavy abuse of stream API. This needs a ring buffer."
This patch makes the relevant code use a ring buffer.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
Instead of checking whether the post-write number of updates sent was
greater than the pre-write number of updates sent, it was comparing post
to zero. In effect this meant every time we wrote a packet it was
counted as an update for route advertisement timer purposes.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
During initial session establishment, bgpd performs a "connection
transfer" to a new peer struct if the connection was initiated passively
(i.e. by the remote peer). With the addition of buffered input and a
reorganized packet processor, the following race condition manifests:
1. Remote peer initiates a connection. After exchanging OPEN messages,
we send them a KEEPALIVE. They send us a KEEPALIVE followed by
10,000 UPDATE messages. The I/O thread pushes these onto our local
peer's input buffer and schedules a packet processing job on the
main thread.
2. The packet job runs and processes the KEEPALIVE, which completes the
handshake on our end. As part of transferring to ESTABLISHED we
transfer all peer state to a new struct, as mentioned. Upon returning
from the KEEPALIVE processing routing, the peer context we had has
now been destroyed. We notice this and stop processing. Meanwhile
10k UPDATE messages are sitting on the input buffer.
3. N seconds later, the remote peer sends us a KEEPALIVE. The I/O thread
schedules another process job, which finds 10k UPDATEs waiting for
it. Convergence is achieved, but has been delayed by the value of the
KEEPALIVE timer.
The racey part is that if the remote peer takes a little bit of time to
send UPDATEs after KEEPALIVEs -- somewhere on the order of a few hundred
milliseconds -- we complete the transfer successfully and the packet
processing job is scheduled on the new peer upon arrival of the UPDATE
messages. Yuck.
The solution is to schedule a packet processing job on the new peer
struct after transferring state.
Lengthy commit message in case someone has to debug similar problems in
the future...
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
During initial session establishment, bgpd performs a "connection
transfer" to a new peer struct if the connection was initiated passively
(i.e. by the remote peer). With the addition of buffered input, I forgot
to transfer the raw input buffer to the new peer. This resulted in
infrequent failures during session handshaking whereby half of a packet
would be thrown away in the middle of a read causing us to send a NOTIFY
for an unsynchronized header. Usually the transfer coincided with a
clean input buffer, hence why it only showed up once in a while.
At some point when rearranging FSM code, bgpd lost the ability to
perform active opens because it was only paying attention to POLLIN and
not POLLOUT, when the latter is used to signify a successful connection
in the active case.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
Use best-performing memory orders where appropriate.
Also update some style and add missing comments.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
Slightly incorrect trigger for generating update group packets. In order
to match semantics of previous bgp_write() we need to trigger
update-group packet generation after every write operation, even if no
packets were written. Of course if we're tearing down the session we can
still skip this operation.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
Despaghettification of bgp_packet.c and bgp_fsm.c
Sometimes we call bgp_event_update() inline packet parsing.
Sometimes we post events instead.
Sometimes we increment packet counters in the FSM.
Sometimes we do it in packet routines.
Sometimes we update EOR's in FSM.
Sometimes we do it in packet routines.
Fix the madness.
bgp_process_packet() is now the centralized place to:
- Update message counters
- Execute FSM events in response to incoming packets
FSM events are now executed directly from this function instead of being
queued on the thread_master. This is to ensure that the FSM contains the
proper state after each packet is parsed. Otherwise there could be race
conditions where two packets are parsed in succession without the
appropriate FSM update in between, leading to session closure due to
receiving inappropriate messages for the current FSM state.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
When terminating I/O thread, just schedule an event to do any necessary
cleanup and gracefully exit instead of using a signal.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
* Start bit flags at 1, not 2
* Make run-flags atomic for i/o thread
* Remove work_cond mutex, it should no longer be necessary
* Add asserts to ensure proper ordering in bgp_connect()
* Use true/false with booleans, not 1/0
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
