Sure. I will add ssl usage example with some brief in the ovn-ctl.8.xml and send v3 for this patch . Does that sound good?
On Mon, Oct 8, 2018 at 5:32 PM Han Zhou <[email protected]> wrote: > > > On Mon, Oct 8, 2018 at 4:55 PM aginwala aginwala <[email protected]> > wrote: > > > > > > > > On Mon, Oct 8, 2018 at 10:47 AM Han Zhou <[email protected]> wrote: > >> > >> > >> > >> On Fri, Oct 5, 2018 at 6:48 PM aginwala aginwala <[email protected]> > wrote: > >> > > >> > Thanks for the review Han. Please find the comments inline below: > >> > On Thu, Oct 4, 2018 at 9:58 AM Han Zhou <[email protected]> wrote: > >> >> > >> >> Thanks Ali, please see my comments below > >> >> > >> >> On Fri, Sep 21, 2018 at 5:34 PM <[email protected]> wrote: > >> >> > > >> >> > For OVN DBs to work with SSL in HA, we need to have capability to > >> >> > pass ssl certs when starting OVN DBs. Say when starting OVN DBs > in active > >> >> > passive mode, in order for the standby DBs to sync from master > node, it > >> >> > cannot sync because the required ssl certs are not passed when > standby DBs > >> >> > are initialized. Hence, we need to have this option. > >> >> > > >> >> > e.g. start nb db with ssl certs as below: > >> >> > /usr/share/openvswitch/scripts/ovn-ctl > --ovn-nb-db-ssl-key=/etc/openvswitch/ovnnb-privkey.pem \ > >> >> > --ovn-nb-db-ssl-cert=/etc/openvswitch/ovnnb-cert.pem \ > >> >> > --ovn-nb-db-ssl-ca-cert=/etc/openvswitch/cacert.pem \ > >> >> > --db-nb-create-insecure-remote=no start_nb_ovsdb > >> >> > > >> >> > Certs can be generated based on ovs ssl docs: > >> >> > http://docs.openvswitch.org/en/latest/howto/ssl/ > >> >> > > >> >> > Signed-off-by: aginwala <[email protected]> > >> >> > --- > >> >> > ovn/utilities/ovn-ctl | 50 > +++++++++++++++++++++++++++++++++++++++++++------- > >> >> > 1 file changed, 43 insertions(+), 7 deletions(-) > >> >> > > >> >> > diff --git a/ovn/utilities/ovn-ctl b/ovn/utilities/ovn-ctl > >> >> > index 3ff0df6..4f45f3d 100755 > >> >> > --- a/ovn/utilities/ovn-ctl > >> >> > +++ b/ovn/utilities/ovn-ctl > >> >> > @@ -116,6 +116,9 @@ start_ovsdb__() { > >> >> > local addr > >> >> > local active_conf_file > >> >> > local use_remote_in_db > >> >> > + local ovn_db_ssl_key > >> >> > + local ovn_db_ssl_cert > >> >> > + local ovn_db_ssl_cacert > >> >> > eval pid=\$DB_${DB}_PID > >> >> > eval cluster_local_addr=\$DB_${DB}_CLUSTER_LOCAL_ADDR > >> >> > eval cluster_local_port=\$DB_${DB}_CLUSTER_LOCAL_PORT > >> >> > @@ -137,6 +140,9 @@ start_ovsdb__() { > >> >> > eval addr=\$DB_${DB}_ADDR > >> >> > eval active_conf_file=\$ovn${db}_active_conf_file > >> >> > eval use_remote_in_db=\$DB_${DB}_USE_REMOTE_IN_DB > >> >> > + eval ovn_db_ssl_key=\$OVN_${DB}_DB_SSL_KEY > >> >> > + eval ovn_db_ssl_cert=\$OVN_${DB}_DB_SSL_CERT > >> >> > + eval ovn_db_ssl_cacert=\$OVN_${DB}_DB_SSL_CA_CERT > >> >> > > >> >> > # Check and eventually start ovsdb-server for DB > >> >> > if pidfile_is_running $pid; then > >> >> > @@ -182,17 +188,32 @@ $cluster_remote_port > >> >> > > >> >> > if test X"$use_remote_in_db" != Xno; then > >> >> > set "$@" --remote=db:$schema_name,$table_name,connections > >> >> > + if test X"$create_insecure_remote" = Xno; then > >> >> > + set "$@" --remote=pssl:$port:$addr > >> >> > + elif test X"$create_insecure_remote" = Xyes; then > >> >> > + set "$@" --remote=ptcp:$port:$addr > >> >> > + fi > >> >> Why moving the logic here? This if block only says if the connection > settings in DB table should be used. Whether insecure mode is allowed was > supposed to be independent with this condition. Could you explain the > reason behind the change? > >> >> >> I moved it because remote=db is needed if ovsdb is running as a > standalone node or an active ovsdb server node. For standby nodes in case > of active_passive mode, remote=db will not be there because it uses > --sync-from. Hope its clear. > >> > >> As discussed, $use_remote_in_db and $create_insecure_remote were > independent. Moving $create_insecure_remote logic here make it useful only > when $use_remote_in_db is "yes", which is not how it was supposed to be. > >> > >> I understand that for standby node, we will set $use_remote_in_db as > "no". Is this a problem? > >> > > >> Just verified and have kept the logic intact as even for ssl, we have > connection table set for 0.0.0.0 for LB use case and it works fine. Hence, > have updated it in v2. Please take a look and let me know. > >> > >> --sync-from has nothing to do with "remote". > >> > > >> > > >> >> > >> >> > fi > >> >> > - set "$@" --private-key=db:$schema_name,SSL,private_key > >> >> > - set "$@" --certificate=db:$schema_name,SSL,certificate > >> >> > - set "$@" --ca-cert=db:$schema_name,SSL,ca_cert > >> >> > - set "$@" --ssl-protocols=db:$schema_name,SSL,ssl_protocols > >> >> > - set "$@" --ssl-ciphers=db:$schema_name,SSL,ssl_ciphers > >> >> > >> >> So it will not use the settings in DB any more? It seems this change > removed support for "--use-remote-in-db=yes", which is the default behavior > that should be kept. The DB settings should not be used only if > "--use-remote-in-db=no" > >> > > >> > >> As discussed, this is similar support that we have for > ovn-controller with ssl. If the key is passed from cli, it will use the key > else fall back to default setters for ssl. > >> >> > >> I missed the "else" below. So it does fallback to use DB config when > command line option is not specified. It looks good for me, but it would be > better to clarify the behavior in help message of this tool that command > line options for these SSL related parameters overrides any DB setting. > > > > >>> Cool. So it does shows ssl key in options section when you type > help similar to ovn-controller ssl support and hope that suffices. Please > ack if all ok in v2 patch that I sent. > >> > I meant documenting the behavior that command line option always override > DB settings if both exist. > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
