[ovs-dev] [PATCH v3] ovn-ctl: add support for SSL nb/sb db connections

[Ben Pfaff]

On Thu, Dec 22, 2016 at 08:07:02AM -0500, Lance Richardson wrote:
> > From: "Lance Richardson" <lrichard at redhat.com>
> > To: "Ben Pfaff" <blp at ovn.org>, nusiddiq at redhat.com, "Russell Bryant" <russell at ovn.org>
> > Cc: dev at openvswitch.org
> > Sent: Thursday, December 22, 2016 7:51:16 AM
> > Subject: Re: [PATCH v3] ovn-ctl: add support for SSL nb/sb db connections
> > 
> > > From: "Ben Pfaff" <blp at ovn.org>
> > > To: "Lance Richardson" <lrichard at redhat.com>
> > > Cc: dev at openvswitch.org, nusiddiq at redhat.com
> > > Sent: Thursday, December 22, 2016 12:04:05 AM
> > > Subject: Re: [PATCH v3] ovn-ctl: add support for SSL nb/sb db connections
> > > 
> > > On Wed, Dec 21, 2016 at 06:35:43PM -0500, Lance Richardson wrote:
> > > > Add support for SSL connections to OVN northbound and/or
> > > > southbound databases.
> > > > 
> > > > To improve security, the NB and SB ovsdb daemons no longer
> > > > have open ptcp connections by default.  This is a change in
> > > > behavior from previous versions, users wishing to use TCP
> > > > connections to the NB/SB daemons can either request that
> > > > a passive TCP connection be used via ovn-ctl command-line
> > > > options (e.g. via OVN_CTL_OPTS/OVN_NORTHD_OPTS in startup
> > > > scripts):
> > > > 
> > > >     --db-sb-create-remote=yes
> > > >     --db-nb-create-remote=yes
> > > 
> > > Thanks for writing this, and for rebasing.
> > > 
> > > I don't yet understand the design choices for the --db-?b-create-remote
> > > options.  The names seem odd to me, since these options are particularly
> > > about adding insecure remotes, and so I would expect the names to say
> > > something about "legacy" or "insecure".  I'm also puzzled why these
> > > options, which I'd expect to be supplied time after time to ovn-ctl if
> > > they are necessary at all, make a stateful database change.  I would
> > > have guessed, instead, that they add another --remote option to daemon
> > > invocations.
> > > 
> > > Can you help me understand better?
> > > 
> > > Thanks,
> > > 
> > > Ben.
> > > 
> > 
> > OK, picking names is hard (I've heard that somewhere recently :-))...
> > --db-?b-create-insecure-remote does seem to be a better name.
> > --db-?b-create-legacy-remote also seems better, but I wonder if "legacy"
> > might become less meaningful over time if other changes are made in this
> > area. Maybe --db-?sb-create-default-tcp-remote would also be worth
> > considering.
> > 
> > v1 of this series created the tcp remote using command-line options.
> > v2 changed the default remote scheme to use remote configuration from
> > the database and added the ability to configure the default remote's
> > inactivity time based on feedback from Rusell and Numan. In retrospect,
> > maybe those changes should have been deferred to a separate patch set.
> > 
> > How about this: I rebase v1 of this patch, renaming --db-?b-default-remote
> > to --db-?b-create-insecure-remote in the process? This would require
> > users wanting to configure inactivity probe time for TCP connections
> > to configure the connection in the db, a future patch could add support
> > for configuring the inactivity probe time from the ovsdb-server command
> > line if needed.
> > 
> > Thanks,
> > 
> >    Lance
> 
> v1 of this patch is here:
> 
>     http://patchwork.ozlabs.org/patch/701571/
> 
> Also, it's probably not clear from the above, the motivation for changing from
> command-line remote configuration to db remote configuration for the "legacy"
> remote was solely to allow the inactivity probe time to be configured for
> the connection.

I am happy with this, but I'd like to hear from Russell and Numan, since
I either forgot about or never read their feedback on v1.