On 9/24/26 9:32 PM, Numan Siddique wrote: > On Thu, Sep 24, 2026, 9:35 a.m. Ales Musil <[email protected]> wrote: > >> >> >> On Thu, Sep 24, 2026 at 3:20 PM Dumitru Ceara <[email protected]> wrote: >> >>> On 9/24/26 7:55 AM, Ales Musil wrote: >>>> The bridge controller was started with the generic controller's >>> priority and wrapper settings, and its logging option was ignored. Use the >>> bridge controller-specific variables so these accepted options affect the >>> daemon that they configure. >>>> >>>> Keep the usage output consistent with the accepted option names and >>> variables. >>>> >>>> Assisted-by: GPT-5.6-Luna, OpenCode >>>> >>>> Signed-off-by: Ales Musil <[email protected]> >>>> --- >> >> >>> Hi Ales, >>> >> >> Hi Dumitru, >> >> I have also noticed the commit message is a bit broken, so that will need >> some adjustments. >>
Hi Ales, Numan, I took care of the commit message. > > > Thanks for fixing this. LGTM. > > Acked-by: Numan Siddique <[email protected]> > Thanks for the review and for the patch! Applied to main, 26.09 and 26.03. Please see below for another minor finding though. > Numan > > >> >>> >>>> utilities/ovn-ctl | 36 ++++++++++++++++++------------------ >>>> 1 file changed, 18 insertions(+), 18 deletions(-) >>>> >>>> diff --git a/utilities/ovn-ctl b/utilities/ovn-ctl >>>> index 3b62ca9b7..5aaf6932e 100755 >>>> --- a/utilities/ovn-ctl >>>> +++ b/utilities/ovn-ctl >>>> @@ -759,7 +759,7 @@ start_controller_vtep () { >>>> >>>> start_ovnbr_controller () { >>>> set ovn-br-controller >>>> - set "$@" -vconsole:emer -vsyslog:err -vfile:info >>>> + set "$@" $OVNBR_CONTROLLER_LOG >>>> if test X"$OVNBR_CONTROLLER_SSL_KEY" != X; then >>>> set "$@" --private-key=$OVNBR_CONTROLLER_SSL_KEY >>>> fi >>>> @@ -788,7 +788,7 @@ start_ovnbr_controller () { >>>> set "$@" $extra_args >>>> fi >>>> >>>> - OVS_RUNDIR=${OVS_RUNDIR} start_ovn_daemon >>> "$OVN_CONTROLLER_PRIORITY" "$OVN_CONTROLLER_WRAPPER" "$@" >>>> + OVS_RUNDIR=${OVS_RUNDIR} start_ovn_daemon >>> "$OVNBR_CONTROLLER_PRIORITY" "$OVNBR_CONTROLLER_WRAPPER" "$@" >>> >>> CC-ing Numan because he might be using ovn-br. Just to make sure this >>> fix doesn't affect his deployment. >>> >>> Regards, >>> Dumitru >>> >>>> } >>>> >>>> ## ---- ## >>>> @@ -1319,22 +1319,22 @@ Options: >>>> --db-sb-relay-remote Specifies upstream cluster/server >>> remote for ovsdb relay >>>> --db-sb-relay-use-remote-in-db=no|yes >>>> OVN_Sorthbound db listen on target >>> connection table (default: $DB_SB_RELAY_USE_REMOTE_IN_DB) >>>> - --ovn-br-controller-priority=NICE set ovn-br-controller's >>> niceness (default: $OVN_CONTROLLER_PRIORITY) >>>> - --ovn-br-controller-wrapper=WRAPPER run with a wrapper like >>> valgrind for debugging >>>> - --ovn-br-controller-ssl-key=KEY OVN Bridge Controller SSL/TLS >>> private key file >>>> - --ovn-br-controller-ssl-cert=CERT OVN Bridge Controller SSL/TLS >>> certificate file >>>> - --ovn-br-controller-ssl-ca-cert=CERT OVN Bridge Controller SSL/TLS >>> CA certificate file >>>> - --ovn-br-controller-ssl-bootstrap-ca-cert=CERT Bootstrapped OVN >>> Bridge Controller SSL/TLS CA certificate file >>>> - --ovn-br-controller-ssl-protocols=PROTOCOLS OVN Bridge Controller >>> SSL/TLS protocols >>>> - --ovn-br-controller-ssl-ciphers=CIPHERS OVN Bridge Controller >>> SSL/TLS cipher list >>>> - --ovn-br-controller-ssl-ciphersuites=CIPHERSUITES OVN Bridge >>> Controller TLSv1.3+ ciphersuite list >>>> - --ovn-br-db-ssl-key=KEY OVN Bridge Controller DB SSL/TLS private key >>> file >>>> - --ovn-br-db-ssl-cert=CERT OVN Bridge Controller DB SSL/TLS >>> certificate file >>>> - --ovn-br-db-ssl-ca-cert=CERT OVN Bridge Controller DB SSL/TLS CA >>> certificate file >>>> - --ovn-br-db-ssl-protocols=PROTOCOLS OVN Bridge Controller DB SSL/TLS >>> protocols >>>> - --ovn-br-db-ssl-ciphers=CIPHERS OVN Bridge Controller DB SSL/TLS >>> cipher list >>>> - --ovn-br-db-ssl-ciphersuites=CIPHERSUITES OVN Bridge Controller DB >>> TLSv1.3+ ciphersuite list >>>> - --ovn-br-controller-log=STRING ovn controller process logging >>> params (default: $OVN_CONTROLLER_LOG) >>>> + --ovnbr-controller-priority=NICE set ovn-br-controller's >>> niceness (default: $OVNBR_CONTROLLER_PRIORITY) >>>> + --ovnbr-controller-wrapper=WRAPPER run with a wrapper like >>> valgrind for debugging >>>> + --ovnbr-controller-ssl-key=KEY OVN Bridge Controller SSL/TLS private >>> key file >>>> + --ovnbr-controller-ssl-cert=CERT OVN Bridge Controller SSL/TLS >>> certificate file >>>> + --ovnbr-controller-ssl-ca-cert=CERT OVN Bridge Controller SSL/TLS CA >>> certificate file >>>> + --ovnbr-controller-ssl-bootstrap-ca-cert=CERT Bootstrapped OVN >>> Bridge Controller SSL/TLS CA certificate file >>>> + --ovnbr-controller-ssl-protocols=PROTOCOLS OVN Bridge Controller >>> SSL/TLS protocols >>>> + --ovnbr-controller-ssl-ciphers=CIPHERS OVN Bridge Controller SSL/TLS >>> cipher list >>>> + --ovnbr-controller-ssl-ciphersuites=CIPHERSUITES OVN Bridge >>> Controller TLSv1.3+ ciphersuite list >>>> + --ovnbr-db-ssl-key=KEY OVN Bridge Controller DB SSL/TLS private key >>> file >>>> + --ovnbr-db-ssl-cert=CERT OVN Bridge Controller DB SSL/TLS >>> certificate file >>>> + --ovnbr-db-ssl-ca-cert=CERT OVN Bridge Controller DB SSL/TLS CA >>> certificate file >>>> + --ovnbr-db-ssl-protocols=PROTOCOLS OVN Bridge Controller DB SSL/TLS >>> protocols >>>> + --ovnbr-db-ssl-ciphers=CIPHERS OVN Bridge Controller DB SSL/TLS >>> cipher list >>>> + --ovnbr-db-ssl-ciphersuites=CIPHERSUITES OVN Bridge Controller DB >>> TLSv1.3+ ciphersuite list >>>> + --ovnbr-controller-log=STRING ovn-br-controller process >>> logging params (default: $OVNBR_CONTROLLER_LOG) >>>> --ovn-br-db-log=STRING ovn brdb ovsdb-server processes >>> logging params (default: $OVN_BR_DB_LOG) A note from my AI helper: The --ovn-br-db-log option on the very next line (not changed by this patch) still references $OVN_BR_DB_LOG, which is never defined in set_defaults(). The option name --ovn-br-db-log maps via tr to OVN_BR_DB_LOG which also does not exist. Should this be renamed to --ovnbr-db-log and reference a variable that is actually defined? Right now this option appears non-functional for the same reason the options this patch fixes were non-functional. Similarly, start_ovsdb__() with DB=OVNBR evaluates $OVN_OVNBR_LOG for the logging parameters, but that variable is never initialized either. Might be worth defining it in set_defaults() alongside the other OVNBR defaults so that --ovnbr-db-log can actually take effect. This is pre-existing, so not a blocker for this patch, but it would be nice to fix it while we are in the area. Regards, Dumitru >>>> --ovsdb-br-wrapper=WRAPPER run with a wrapper like valgrind for >>> debugging >>>> -h, --help display this help message >>> >>> >> Regards, >> Ales >> > > > > > numan > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
