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. > > 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
