Due to an oversight, the provider lists weren't compared on reconfiguration.

Reported-By: Ralph Covelli <rcovelli@he.net>

Maria Matejka authored 4 months ago and Ondřej Zajíček committed 4 months ago

Several users build BIRD with excluded support for protocols they don't
need. Testing single-protocol builds; the assumption is that if single
protocols and all protocols are buildable, then possibly any reasonable
combination is buildable as well.

Minor change by committer.

Maria Matejka authored 4 months ago and Ondřej Zajíček committed 4 months ago

Now they need device protocol which is always built.

The commit 21213be5 expanded private_id
in route source to u64, but forgot to modify function arguments, so it
was still cropped at 32-bit, which may cause some collisions for L3VPN.
This patch fixes that.

Protocol outlines should not contain specific values.

Also fix some space intendation in code sections.

Also weaken the explicit alignment of net_addr to be u32, as VPN
is already u32-aligned.

Use a distinct data structure for VPN route distinguishers instead
of just u64.

On BSD, the onlink flag is not tracked or reported by kernel. We are
using an heuristic that assigns the onlink flag to routes scanned from
the kernel. We should use the same heuristic even in BSD-Netlink
case, as the onlink flag is not reported here too.

Thanks to Björn König for the original patch.

Add newlines to the body of proto-build.c . Use printf instead of echo
for portable handling of backslash sequences.

Thanks to CCX for the change.

Fabian Bläse authored 4 months ago and Ondřej Zajíček committed 4 months ago

The Babel seqno wraps around when reaching its maximum value (UINT16_MAX).
When comparing seqnos, this has to be taken into account. Therefore,
plain number comparisons do not work.

Implement several options (min/max graceful restart time, min/max long
lived stale time) to override graceful restart and long-lived graceful
restart timer values, as suggested by RFC 9494.

(broken by previous change)

It seems that it should be 'providers' instead of 'provider'.

It matches keyword declaration and documentation. Mismatch beween keyword
declaration also breaks static-only builds.

Maria Matejka authored 5 months ago and Ondřej Zajíček committed 4 months ago

Minor changes by committer.

Fix several errors including:
 - Unaligned memory access to 'Length of Error Text' field
 - No validation of 'Length of Encapsulated PDU' field
 - No validation of 'Error Code' field
 - No validation of characters in diagnostic message

APSA records can be arbitrarily large, let's use 64k as
a reasonable limit.

Maria Matejka authored 5 months ago and Ondřej Zajíček committed 4 months ago

All the 'dump something' CLI commands now have a new mandatory
argument -- name of the file where to dump the data. This allows
for more flexible dumping even for production deployments where
the debug output is by default off.

Also the dump commands are now restricted (they weren't before)
to assure that only the appropriate users can run these time consuming
commands.

Maria Matejka authored 5 months ago and Ondřej Zajíček committed 4 months ago

When printing near the end of the buffer, there was an overflow in two cases:

(1) %c and size is zero
(2) %1N, %1I, %1I4, %1I6 (auto-fill field_width for Net or IP), size is
    more than actual length of the net/ip but less than the auto-filled
    field width.

Manual code examination showed that nothing could have ever triggered
this behavior. All older versions of BIRD, including BIRD 3 development
versions, are totally safe. This exact overflow has been found while
implementing a new feature in later commits.

Instead of several levels of functions, just have two functions
(one for routes, the other for end-of-rib), this allows to create
messages in a simple linear fashion.

Also reduce three duplicite functions to construct BGP header for
BMP messages to just one.

Fill message length after the message is created instead of computing
it beforehand. Also rename some functions to fit common pattern.

Put all arguments for per-peer header into one structure instead of
passing them around as a lump of arguments. Also simplify code for
peer flags.

Maria Matejka authored 7 months ago and Ondřej Zajíček committed 4 months ago

This commit is quite a substantial rework of the underlying layers in
BMP TX:

- several unnecessary layers of indirection dropped, including most of
  the original BMP's buffer machinery
- all messages are now written directly into one protocol's buffer
  allocated for the whole time big enough to fit every possible message
- output blocks are allocated by pages and immediately returned when
  used, improving the overall memory footprint
- no intermediary allocation is done from the heap altogether
- there is a documented and configurable limit on the TX queue size

Maria Matejka authored 4 months ago and Ondřej Zajíček committed 4 months ago

Unit tests by Ondrej Zajicek.

We shouldn't convert bytes 2 and 3 of the PDU blindly, there are several
cases where these are used by bytes. Instead, the conversion is done
only where needed.

This fixes misinterpretation bug of ASPA PDU flags on little endian
architectures.

The strcmp function is not guaranteed to return -1 or +1
but any negative or positive value if the input strings
are different. Fixed the false assumption which triggered
a build bug on emulated arm64.

The patch initializes logging in unit tests. Previously, unit tests did
not initialize logging and other subsystems, just resources. But
resource_init() could under certain circumstances trigger logging and
cause crash.

The bug was Found by Jakub Ruzicka, dissected by David Petera and Maria
Matejka, disguised as failing build for Debian arm64 in pbuilder
emulation which did not like disabling THP.

Fixes #42.

The _EMPTY and _CONFED variants are easy to spot bare-eyed from the AS path.

The END_OF_DATA PDU was extended in version 1, so it has different length
in different versions. We should do the PDU length check according to its
version.