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