Hi Martin, Frode, Thanks for the patch and reviews!
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. >>> 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. 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
