On Tue, 4 Nov 2025 at 05:26, Ilya Maximets <[email protected]> wrote:
> On 10/31/25 9:37 PM, Guru Shetty wrote: > > > > > > On Fri, 24 Oct 2025 at 14:19, Ilya Maximets <[email protected] <mailto: > [email protected]>> wrote: > > > > On 10/24/25 3:28 AM, Numan Siddique wrote: > > > On Wed, Oct 22, 2025 at 2:10 PM Mark Michelson via dev > > > <[email protected] <mailto:[email protected]>> wrote: > > >> > > >> Thanks for this patch too, and thanks especially for adding a > test! > > >> > > >> Acked-by: Mark Michelson <[email protected] <mailto: > [email protected]>> > > > > > > Thanks Guru and Mark. > > > > > > I applied both the patches to the main. > > > > Hi. This change technically enables the option for more than just > dbctl. > > The option is now available and documented for all of these: > > > > $ git grep STREAM_SSL_LONG_OPTIONS > > controller-vtep/ovn-controller-vtep.c: > STREAM_SSL_LONG_OPTIONS, > > controller/ovn-controller.c: STREAM_SSL_LONG_OPTIONS, > > ic/ovn-ic.c: STREAM_SSL_LONG_OPTIONS, > > northd/ovn-northd.c: STREAM_SSL_LONG_OPTIONS, > > utilities/ovn-dbctl.c: STREAM_SSL_LONG_OPTIONS, > > utilities/ovn-ic-nbctl.c: STREAM_SSL_LONG_OPTIONS, > > utilities/ovn-ic-sbctl.c: STREAM_SSL_LONG_OPTIONS, > > utilities/ovn-trace.c: STREAM_SSL_LONG_OPTIONS, > > > > $ git grep ssl.xml > > controller-vtep/ovn-controller-vtep.8.xml: <xi:include > href="lib/ssl.xml" xmlns:xi="http://www.w3.org/2003/XInclude < > http://www.w3.org/2003/XInclude>"/> > > controller/ovn-controller.8.xml: <xi:include href="lib/ssl.xml" > xmlns:xi="http://www.w3.org/2003/XInclude <http://www.w3.org/2003/XInclude > >"/> > > ic/ovn-ic.8.xml: <xi:include href="lib/ssl.xml" xmlns:xi=" > http://www.w3.org/2003/XInclude <http://www.w3.org/2003/XInclude>"/> > > northd/ovn-northd.8.xml: <xi:include href="lib/ssl.xml" xmlns:xi=" > http://www.w3.org/2003/XInclude <http://www.w3.org/2003/XInclude>"/> > > utilities/ovn-ic-nbctl.8.xml: <xi:include href="lib/ssl.xml" > xmlns:xi="http://www.w3.org/2003/XInclude <http://www.w3.org/2003/XInclude > >"/> > > utilities/ovn-ic-sbctl.8.xml: <xi:include href="lib/ssl.xml" > xmlns:xi="http://www.w3.org/2003/XInclude <http://www.w3.org/2003/XInclude > >"/> > > utilities/ovn-nbctl.8.xml: <xi:include href="lib/ssl.xml" > xmlns:xi="http://www.w3.org/2003/XInclude <http://www.w3.org/2003/XInclude > >"/> > > utilities/ovn-sbctl.8.xml: <xi:include href="lib/ssl.xml" > xmlns:xi="http://www.w3.org/2003/XInclude <http://www.w3.org/2003/XInclude > >"/> > > utilities/ovn-trace.8.xml: <xi:include href="lib/ssl.xml" xmlns:xi=" > http://www.w3.org/2003/XInclude <http://www.w3.org/2003/XInclude>"/> > > > > But ovn-ic, ovn-northd and ovn-controller do not implement the > handlers for it: > > > > $ git grep STREAM_SSL_OPTION_HANDLERS > > controller-vtep/ovn-controller-vtep.c: > STREAM_SSL_OPTION_HANDLERS > > utilities/ovn-ic-nbctl.c: STREAM_SSL_OPTION_HANDLERS > > utilities/ovn-ic-sbctl.c: STREAM_SSL_OPTION_HANDLERS > > utilities/ovn-trace.c: STREAM_SSL_OPTION_HANDLERS > > > > So they will accept the option and crash: > > > > $ ovn-controller --ssl-server-name=qwe > > Aborted (core dumped) > > > > Could you, please, add the handlers for the programs that are > missing them? > > A small NEWS entry for the change would also be good to have. > > > > > > The patch "ovn: Fix SNI support in a few utilities." that I sent for > review just now is > > intended to fix the above situation. > > > > For the situation below, do we want to go all-in with support for > ovn-ctl and > > set-ssl commands right now (because it involves schema changes), > > or should we wait till an actual use case for someone so that we can > also add > > appropriate unit tests? > > I agree that the set-ssl command and the schema changes are likely not > needed. > I'm not sure what the use case for these would be as in vast majority of > the > cases connections stored in the database are passive and hence do not need > the > SNI support. > > But I think, since we're adding command line support for all the daemons, > we > should be able to pass the command line configuration via ovn-ctl. So, > IMO, we > need to implement the following: > > --ovn-controller-ssl-server-name=NAME > --ovn-nb-db-... > --ovn-sb-db-... > --ovn-northd-... > --ovn-ic-... > --ovn-ic-nb-db-... > --ovn-ic-sb-db-... > --ovn-sb-relay-... > > All of these are or can be client connections, for which SNI makes sense > (on > databases, client connections would be backup or relay servers). > I made an attempt and sent a patch > > The relay one is currently missing ciphers and protocols for some reason, > but > it's a separate issue. > > Best regards, Ilya Maximets. > > > > > > > > > > > We may also need to extend the ovn-ctl to support passing the new > option to > > the daemons. Not sure if need to extend the Nb and Sb schema to > allow > > databases to override SNI while connecting to each other, but seems > like an > > unnecessary restriction or an incomplete support if we do not allow > that. > > This will also include extension of the dbctl set-ssl commands. > > > > You may use the following commit as a reference: > > dbdd8eaaf556 ("treewide: Update OVS submodule to modernize SSL/TLS > support.") > > > > Best regards, Ilya Maximets. > > > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
