Hi Han: Have added the man section for ssl in v3. PTAL.
Thanks, On Mon, Oct 8, 2018 at 6:49 PM aginwala aginwala <[email protected]> wrote: > 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
