Rayhaan JaufeerallyandClaude Fable 5.1
a8aba52562
A review of the wire codec and the routing table, with the fixes. Bugs in the codec: - A NOTIFICATION with a code or subcode we have no name for (subcode 0, "unspecific", is common) failed to decode and was answered as a bad message. Every pair now reads, as BgpError::Other. - Optional parameters in the extended form of RFC 9072 were read and written with one-octet parameter lengths; they are two. - An ORF capability listing more than one address family made the whole OPEN malformed. - Bytes after an OPEN's optional parameters were accepted. - Extended communities were split into fields by rules that were wrong for the common types. They are kept as their eight octets. - Several length fields were computed in u16 and could wrap on encode. Simplifications in the codec: - Lengths are filled in after the value is written, for messages, attributes, capabilities and the UPDATE's two fields. The parallel wire_len functions, which had to be kept in step by hand, are gone. Message::to_wire no longer encodes into a second buffer. - Type codes for messages, attributes and capabilities are each defined once. The strum discriminant enums and their hand-written conversions are removed. - One function splits an attribute off a buffer; the UPDATE decoder, the plain attribute parser and the table's key reader all use it. - The UPDATE decoder's 185-line loop is an AttributeReader with one method that decides what becomes of an attribute. Duplicate detection is a bitset, not a HashSet allocated per UPDATE. - ParserContext::four_octet_asn is a bool. The "context not set" error path, which treated a programming error as a malformed route, is gone. - Codec functions take a context only if they read it, and return a Result only if they can fail. - MP_REACH_NLRI and MP_UNREACH_NLRI hold an AfiSafi. - AddressFamilyId is IpVersion, which is what it was. - Deserialize is no longer derived on codec types: nothing used it, and it bypassed IpPrefix's invariants. IpPrefix::to_wire, which was broken and unused, is removed. Bugs in the table: - The index of a row's best route was sixteen bits. A peer using ADD-PATH could send more routes for a prefix than that counts, and the index wrapped. It is 32 bits, at no cost in row size. - Shards and attribute-store stripes were picked from the top bits of the hash, which are the bits hashbrown keeps as the tag it compares first. With 64 shards each shard's table had 2 of 128 tag values. They are now picked from bits the table does not use. - remove_peer held the peer list's lock for the whole scan of the table, so no session could establish while a peer was removed. - When more prefixes went at once than were left, the forced trim dropped a fixed few entries, reset every reader, and re-ran the tidy on every later write. It drops the older half. - Readers were woken by every change, including a repeated announcement and changes their journal does not record. - Reader::poll with a budget of zero never made progress. Simplifications in the table: - The two journals of a shard are one Journals value written through three calls; the five places that kept them in step by hand are gone. Whether an entry is still live is decided in one place. - The shortcuts of route selection moved to select.rs, next to the invariant they depend on, which is now written down. - announce and withdraw take routes with or without path identifiers; the _paths variants are removed. - The selection key is read off the wire form only; the second implementation over parsed attributes is removed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
70 lines
2.4 KiB
Rust
70 lines
2.4 KiB
Rust
use nom::error::{ErrorKind, ParseError};
|
|
|
|
use crate::constants::AfiSafi;
|
|
use thiserror::Error;
|
|
|
|
/// What reading or writing a message has to know about the session it
|
|
/// belongs to. The default is a session before its OPENs are exchanged;
|
|
/// only an UPDATE depends on anything here, and there are none until then.
|
|
#[derive(Debug, Clone, Default)]
|
|
pub struct ParserContext {
|
|
/// Whether AS numbers are four octets wide on this session (RFC 6793).
|
|
pub four_octet_asn: bool,
|
|
/// Whether the peer is in our own AS. Some attributes mean something
|
|
/// only between internal peers and are dropped when an external one
|
|
/// sends them (RFC 7606 §7).
|
|
pub ibgp: bool,
|
|
/// The address families whose prefixes carry a path identifier in the
|
|
/// direction this context is for (RFC 7911). What a session sends and
|
|
/// what it receives are negotiated separately, so a session has one
|
|
/// context for reading and one for writing.
|
|
pub add_path: Vec<AfiSafi>,
|
|
}
|
|
|
|
impl ParserContext {
|
|
pub fn has_add_path(&self, family: AfiSafi) -> bool {
|
|
self.add_path.contains(&family)
|
|
}
|
|
}
|
|
|
|
/// Why a value could not be written to the wire.
|
|
#[derive(Debug, Clone, PartialEq, Eq, Error)]
|
|
pub enum EncodeError {
|
|
/// A length does not fit the field that has to carry it.
|
|
#[error("{what} is too long to encode: {len} exceeds {max}")]
|
|
TooLong {
|
|
what: &'static str,
|
|
len: usize,
|
|
max: usize,
|
|
},
|
|
}
|
|
|
|
// Custom error type for the parser.
|
|
#[derive(Debug)]
|
|
pub enum BgpParserError<I> {
|
|
/// ProtocolError encapsulates protocol level semantics problems that require
|
|
/// action from the caller of the parser.
|
|
ProtocolError(ProtocolErrorValues),
|
|
/// The input does not follow the format; the text says how.
|
|
Malformed(String),
|
|
Nom(I, ErrorKind),
|
|
}
|
|
|
|
impl<I> ParseError<I> for BgpParserError<I> {
|
|
fn from_error_kind(input: I, kind: ErrorKind) -> Self {
|
|
BgpParserError::Nom(input, kind)
|
|
}
|
|
fn append(_: I, _: ErrorKind, other: Self) -> Self {
|
|
other
|
|
}
|
|
}
|
|
|
|
#[derive(Debug)]
|
|
pub enum ProtocolErrorValues {
|
|
/// `UnsupportedVersion` is returned when the BGP Open message received is for a
|
|
/// version other than one we support. The unsupported version number is the value.
|
|
UnsupportedVersion(u8),
|
|
/// `UnknownOpenOption` is returned with the open option type code that is unknown.
|
|
UnknownOpenOption(u8),
|
|
}
|