On Mon, Feb 28, 2022 at 12:35:09PM +0100, Claudio Jeker wrote: > From the start bgpd had prefix_link and prefix_unlink to link all the > various data objects together to build an actual prefix. Now prefix_move() > tries to be smart and reimplemented prefix_link and prefix_unlink as > inline versions (with minimal differences). Later the prefix_adjout_* > functions were added and again similar code to prefix_link / prefix_unlink > was added. > > This diff removes the code duplication and uses prefix_link and > prefix_unlink in all those extra places. This removes a common error when > changing the object layout because one of the many codepaths was often > missed. > > To make this happen prefix_link() gained an extra argument for the > pt_entry (the adjout has no rib_entry and so no way to get the pt from > there). It also moves the pftable handling and prefix_evaluate() call to > the currently only caller (prefix_add). Add an extra check in nexthop_link > to skip prefixes in the adj-rib-out since those are not linked up. > With this prefix_link can be used in all other places. > > prefix_unlink can be almost used as is only the pt_unref() needs attention > (or actually setting p->pt to NULL). In the Adj-RIB-Out prefixes are > unlinked but remain on the RB tree to act as withdraw requests. For this > prefix_adjout_update() adds an extra reference to the pt_entry so that > after calling prefix_unlink() the pt_entry is still valid. > prefix_adjout_destroy() takes this last reference before freeing the > prefix again. > > The result is a much cleaner handling of prefixes and far fewer places to > introduce new bugs.
It's a tricky diff to review, but it looks all correct. The result is indeed much cleaner ok tb
