<div dir="ltr"><div dir="ltr"><div><div><div dir="ltr" class="gmail_attr">On Mon, 2 Mar 2026 at 06:54, Ondrej Zajicek <<a href="mailto:santiago@crfreenet.org">santiago@crfreenet.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hello<br>
<br>
I finally got to finish reviewing and merging this patchset, sorry for delay.<br>
<br>
Here are the modified versions:<br>
<br>
<a href="https://gitlab.nic.cz/labs/bird/-/commits/dynamic-nbrs-2" rel="noreferrer" target="_blank">https://gitlab.nic.cz/labs/bird/-/commits/dynamic-nbrs-2</a><br>
<a href="https://gitlab.nic.cz/labs/bird/-/commit/4327fabff1db9b78e5e80bb8d22dc848fbf9a6b9" rel="noreferrer" target="_blank">https://gitlab.nic.cz/labs/bird/-/commit/4327fabff1db9b78e5e80bb8d22dc848fbf9a6b9</a><br>
<a href="https://gitlab.nic.cz/labs/bird/-/commit/7eda3e381414f97caed5b2cd02d6b4b3b4554ee0" rel="noreferrer" target="_blank">https://gitlab.nic.cz/labs/bird/-/commit/7eda3e381414f97caed5b2cd02d6b4b3b4554ee0</a><br>
<a href="https://gitlab.nic.cz/labs/bird/-/commit/0ee9f93bd076c5cc425ceaec9acedbbb7c9021ec" rel="noreferrer" target="_blank">https://gitlab.nic.cz/labs/bird/-/commit/0ee9f93bd076c5cc425ceaec9acedbbb7c9021ec</a><br>
<br>
(They are backported to BIRD v2, i will port them to BIRD v3 soon)<br>
</blockquote><br>Hi Ondrej,<br><br></div>First of all, thank you very much for taking on this very thorough rework, I really appreciate it.<br><br><div dir="ltr" class="gmail_attr">On Mon, 2 Mar 2026 at 06:54, Ondrej Zajicek <<a href="mailto:santiago@crfreenet.org">santiago@crfreenet.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Please take a look at these patches and reply if you are okay with them<br>
after my modifications. If you have any questions, feel free to ask.</blockquote><br></div><div>Overall, I have no objections to the changes to my original patches.<br></div><div>I think the added consistency with the existing pieces is a clear improvement, so thank you for that.<br><br><div dir="ltr" class="gmail_attr">On Mon, 2 Mar 2026 at 06:54, Ondrej Zajicek <<a href="mailto:santiago@crfreenet.org">santiago@crfreenet.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">The most visible change is naming, using neighbor instead of peer. That<br>
is consistent with how equivalent structures are named in other<br>
protocols in peer.</blockquote><br></div><div>I went back and forth on the naming for a while, at the start of the implementation, so I am entirely fine settling for 'neighbor'.<br><br><div dir="ltr" class="gmail_attr">On Mon, 2 Mar 2026 at 06:54, Ondrej Zajicek <<a href="mailto:santiago@crfreenet.org">santiago@crfreenet.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Otherwise in the first two patches (Nest and RADv) there are<br>
just minor changes:<br>
<br>
Nest:<br>
- Neighbor entry formatting change<br>
- Parser grammar for neighbor entries<br>
- Filter tests<br>
- Minor fixes<br>
- Some documentation</blockquote><br></div><div>Sorry for missing on adding the filter tests in the first place, it is really something I should have included myself.</div></div><br><div class="gmail_quote gmail_quote_container"><div dir="ltr" class="gmail_attr"><div dir="ltr" class="gmail_attr">On Mon, 2 Mar 2026 at 06:54, Ondrej Zajicek <<a href="mailto:santiago@crfreenet.org">santiago@crfreenet.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">RAdv:<br>
- Moving neighbor prune to separate timer unrelated to ifaces<br>
- More idiomatic log messages, incluing TBF for warning<br>
- Reusing existing 'lifetime' attribute for neighbor entries<br>
(it still stores expire time but plan to change it in the future)</blockquote><br></div><div class="gmail_attr">Moving neighbor prune to separate timer is a good idea, my original reuse of the timer was a little too "hacky", in hindsight.<br><br><div dir="ltr" class="gmail_attr">On Mon, 2 Mar 2026 at 06:54, Ondrej Zajicek <<a href="mailto:santiago@crfreenet.org">santiago@crfreenet.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">With the BGP patch the situation was more complicated, i noticed several<br>
bugs in our dynamic BGP code in BIRD 3 and some parts of your patch looked<br>
like a workaround for some of these issues.<br>
<br>
- Refactoring BGP spawning code to avoid duplication<br>
<br>
- No disable and re-spawn of existing spawned protocols,<br>
this should not be necessary. We just claim existing sessions<br>
in this case.<br><br>
- Simplified some parts of channel handling</blockquote><br></div><div class="gmail_attr">Yes, I had to workaround some of the original dynamic BGP behavior (especially when it came to session duplication), so my implementation came out a lot more complicated than I would have liked.</div><div class="gmail_attr">Thank you very much for reworking the spawning behavior, I have seen the patch and the automatic peering implementation is much cleaner now.</div><div dir="ltr" class="gmail_attr"><br><div dir="ltr" class="gmail_attr">On Mon, 2 Mar 2026 at 06:54, Ondrej Zajicek <<a href="mailto:santiago@crfreenet.org">santiago@crfreenet.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">- I removed the 'persist' option. We just keep track whether the session<br>
is claimed (there is existing neighbor entry) or unclaimed. This is<br>
just makeshift now, there is a good question how to properly handle<br>
unclaimed sessions, but i think it should be consistent with how we<br>
handle dynamic BPG sessions from incoming connections, so i would prefer<br>
to keep it simple now.</blockquote><br></div><div class="gmail_attr">I am ok with dropping the persist option. The main intent was always to keep the behavior aligned with the dynamic BGP one (this is why I set persist=on as the default option).<br></div><div class="gmail_attr">The automatic teardown was a cool option but not essential to the feature and I can see the benefit of keeping things as simple as possible.<br><br></div><div class="gmail_attr">Thanks again for the work on this. Let me know if you need anything or have further comments for the BIRD 3 port.</div><div dir="ltr" class="gmail_attr"><br></div><div class="gmail_attr">Best Regards,<br></div><div class="gmail_attr">Matteo</div></div></div>