On 2/27/25 12:12 PM, [email protected] wrote: > On Thu, 2025-02-13 at 11:16 +0100, Dumitru Ceara wrote: >> Hi Martin, Frode, >> >> Thanks for the patch and reviews! > > Hi Frode and Dumitru, > thanks for the feedback and sorry that this patch fallen, once again, a > bit low on my TODO list. However, now it's back in my focus, so I'll > post v2 asap. >
Thanks, Martin! While we're at it, I forgot who was supposed to work on removing the "experimental" note for the "routing-protocol-redirect" and "routing-protocols" NB options in 25.03.. Would you be able to include a patch that does that too? Thanks, Dumitru >> >> On 2/7/25 4:04 PM, Frode Nordahl wrote: >>> On Fri, Feb 7, 2025 at 1:41 PM <[email protected]> wrote: >>>> >>>> Thanks for the feedback Frode, >>>> >>>> On Fri, 2025-02-07 at 10:23 +0100, Frode Nordahl wrote: >>>>> On Thu, Feb 6, 2025 at 5:43 PM Martin Kalcok >>>>> <[email protected]> wrote: >>>>>> >>>>>> BGP daemon behind the "routing-protocol-redirect" port can >>>>>> now >>>>>> successfully >>>>>> form BGP unnumbered sessions if LRP sends out periodic RAs. >>>>>> This >>>>>> was not >>>>>> previously possible, but it was fixed in [0] and [1]. >>>>>> >>>>>> [0] >>>>>> https://github.com/ovn-org/ovn/commit/ebe5d70122ce0f74067858f5cb19276c852a81da >>>>>> [1] >>>>>> https://github.com/ovn-org/ovn/commit/744340f701b0449be7737f0d388af940fd117dc4 >>>>>> >>>>>> Signed-off-by: Martin Kalcok <[email protected]> >>>>>> --- >>>>> >>>>> Thanks for the patch! >>>>> >>>>> While not a problem of this patch, reading the nice subject of >>>>> it >>>>> spurs the desire for topic and/or tutorial type docs for this >>>>> epic, >>>>> and we could even update the OVN Gateway High Availability Plan >>>>> document [2], but that is of course separate patches. >>>>> >>>>> 2: >>>>> https://github.com/ovn-org/ovn/blob/main/Documentation/topics/high-availability.rst >>>>> >>>> >>>> +1 tutorial for this setup would be nice. >>>> >> >> I agree but I guess that can also be a follow up. >> >>>>>> ovn-nb.xml | 6 ++++++ >>>>>> 1 file changed, 6 insertions(+) >>>>>> >>>>>> diff --git a/ovn-nb.xml b/ovn-nb.xml >>>>>> index 9873ff5e7..584cda6db 100644 >>>>>> --- a/ovn-nb.xml >>>>>> +++ b/ovn-nb.xml >>>>>> @@ -3752,6 +3752,12 @@ or >>>>>> <code>BFD</code> (forwards UDP port 3784) >>>>>> </li> >>>>>> </ul> >>>>>> + <p> >>>>>> + Note that for BGP to work in "unnumbered mode" >>>>>> (automatic on-link >>>>>> + peer discovery), Logical Router Port needs to >>>>>> enable >>>>>> sending of >>>>> >>>>> There are many references to RFCs in this document, does >>>>> perhaps the >>>>> "BGP unnumbered" high level description deserve some RFC >>>>> references? >>>> >>>> Yup, I can add that. >>> >>> Thx! >>> >>>>> >>>>>> + periodic IPv6 Router Announcements (see the <ref >>>>>> + column="ipv6_ra_configs" key="send_periodic"/>). >>>>>> + </p> >>>>> >>>>> I believe we found some additional configuration required when >>>>> enabling this downstream, such as ipv6_ra_configs:address_mode. >>>>> >>>> >>>> Potentially, but I wouldn't put it here. The minimal set of >>>> options >>>> required for enabling IPV6 RAs should be added to the >>>> "ipv6_ra_configs" >>>> section in LRP documentation. >>> >>> Sure, but it would be nice to have some consolidated view of how to >>> use the high level feature, without having to know the details of >>> how >>> it actually works, CMS developers deserve UX too! :) >>> >>> If not here then please add that somewhere. >>> >> >> I actually agree with the consolidated view. We have a lot of >> documentation about all the various knobs and features in OVN but >> it's >> not always easy to have an overview. > > Ack. > >> >>>>> The default values for ipv6_ra_configs:max_interval and >>>>> ipv6_ra_configs:min_interval also do not work well with this >>>>> feature, >>>>> and need to be set for proper operation. >>>> >>>> I was thinking whether to include note about the intervals, but >>>> then >>>> for some reason I didn't. I can add a note that intervals have >>>> effect >>>> on how fast the session forms. >>> >>> With default settings, it takes 9 _minutes_ for a BGP session to >>> form, >>> a CMS developer serendippently walking by this feature would most >>> likely assume it does not work unless they are made aware of having >>> to >>> set those values. >>> >> >> Is this something we can alter defaults for automatically, e.g., >> determine that dynamic routing / routing-protocol-redirect was >> configured and automatically change intervals? It might be risky >> though, in which case we should at least document some sane value >> recommendations. > > Yeah, I'd stick with the documentation here. Aside from the reconfig > being bit unexpected, not everyone will be using BGP unnumbered and we > can't really tell the difference. > > Thanks, > Martin. > >> >> I'm going to move this patch to "Changes Requested": >> https://patchwork.ozlabs.org/project/ovn/patch/[email protected]/ >> >> Looking forward to v2. >> >> Regards, >> Dumitru >> >>> -- >>> Frode Nordahl >>> >>>> Thanks for the review, >>>> Martin. >>>>> >>>>> Should these perhaps be explained here? >>>>> >>>>> -- >>>>> Frode Nordahl >>>>> >>>>>> </column> >>>>>> >>>>>> <column name="options" key="gateway_mtu_bypass"> >>>>>> -- >>>>>> 2.43.0 >>>>>> >>>>>> _______________________________________________ >>>>>> dev mailing list >>>>>> [email protected] >>>>>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >>>> >>> _______________________________________________ >>> dev mailing list >>> [email protected] >>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
