Ondrej Zajicek <santi...@crfreenet.org> writes: > On Tue, Jan 31, 2023 at 11:55:50AM +0100, Toke Høiland-Jørgensen via > Bird-users wrote: >> When creating a new babel_source object we initialise the seqno to 0. The >> caller will update the source object with the right metric and seqno value, >> for both newly created and old source objects. However if we initialise the >> source object seqno to 0 that may actually turn out to be a valid (higher) >> seqno than the one in the routing table, because of seqno wrapping. In this >> case the source metric will not be set properly, which breaks feasibility >> tracking for subsequent updates. >> >> To fix this, add a new initial_seqno argument to babel_get_source() which >> is used when allocating a new object, and set that to the seqno value of >> the update we're sending. > > Thanks, merged: > > https://gitlab.nic.cz/labs/bird/-/commit/dc4c5f51f83f97100b207136ecfde8ff94e597e6 > > The find/get function pair pattern is often used in BIRD, it has some > advantages in keeping invariants, but it is cumbersome when nontrivial > initialization of new objects is used. Sometimes it leads to branching in > caller and doing additional initialization there, in such cases it make > sense to switch to find/add function pair. (just a general remark)
Noted. I think find/get works fine here, though :) > btw, the section 3.7.3 says: > > "Before sending an update (prefix, plen, router-id, seqno, metric) with > finite metric (i.e., not a route retraction)" and "Note that the > garbage-collection timer is not reset when a retraction is sent." > > But the whole section 'Update feasibility distance for redistributed routes' > is done in all cases? This has been clarified in RFC8966 as: "Note that the feasibility distance is not updated and the garbage-collection timer is not reset when a retraction (an update with infinite metric) is sent." The feasibility distance is only updated if the metric is lower, which is never true for an (infinite-metric) retraction, so that's kinda implicit anyway. We do actually update the garbage collection time regardless of the route metric in the regular update function (but not when sending an explicit retraction). I think that's OK, though? > Also, did you review BIRD Babel code whether we could switch RFC > references from RFC 6126 to RFC 8966? I vaguely remember that there > were such a patch. Yeah, it should be updated to RFC8966. We implement sub-TLVs as specified in that, and the extensions (source-specific routing, MAC) are not compatible with a pure RFC6126 implementation anyway. -Toke
