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.

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

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

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

Reply via email to