On 8/24/22 21:29, Aaron Conole wrote:
> lic121 <[email protected]> writes:
> 
>> Support running a list of scripts after vswitchd start, so that we can
>> make some runtime config persistence across ovs restart. For example,
>> the config that `ovs-appctl dpctl` makes.
>>
>> Signed-off-by: lic121 <[email protected]>
>> ---
> 
> This is an interesting patch.  I'm thinking that there is some value in
> having a post-start mechanism like this because we talked years ago on
> the list about having a way to do things like changing thread
> priorities to RT[1], etc.  These are things that OVS might not want to
> have the complicated configuration knobs required to provide, but a
> post-start script could perform and would be easy to see just by looking
> at the post-start directory.

Hmm.  that might be useful, but I'm not sure if ovn-ctl is a right place.
On most of the systems OVS is managed by systemd or some other init system.
And most of such systems has post-start hooks.  For example, users may
add their scripts into ExecStartPost of a systemd service file.  This
should also give more visibility to the end user and, probably, more
flexibility in the configuration by having also pre-start hooks and other
tuning options.

> 
> OTOH, I think that if there are ovs-appctl settings that we want to make
> persistent, then it would make a lot more sense to just expose those
> settings via the database (which is persistent).  Maybe there is a
> reason to keep some as post-start configurable only, but I can't think
> of any that we wouldn't also want to just put into the DB.

That's a good point.  If you want something to be preserved across restarts,
the database is exactly the place to store the configuration.

For the mentioned vlog configuration, we already have ovs-ctl knobs that
will place desired logging options into cmdline, IIRC.

Best regards, Ilya Maximets.

> 
> [1]: 
> https://lists.linuxfoundation.org/pipermail/ovs-dev/2017-January/327025.html
> 
>>  Documentation/ref/ovs-ctl.8.rst |  9 +++++++++
>>  NEWS                            |  2 ++
>>  utilities/ovs-ctl.in            | 11 +++++++++++
>>  3 files changed, 22 insertions(+)
>>
>> diff --git a/Documentation/ref/ovs-ctl.8.rst 
>> b/Documentation/ref/ovs-ctl.8.rst
>> index 7cfc413..380ef06 100644
>> --- a/Documentation/ref/ovs-ctl.8.rst
>> +++ b/Documentation/ref/ovs-ctl.8.rst
>> @@ -37,6 +37,15 @@ system administrators but to be called internally by 
>> system startup
>>  scripts.
>>  
>>  
>> +Ovs has a lot of runtime configurations which can't survive from
>> +service restart. For example, the vlog level config, dpctl/set
>> +config and flows. To persistent runtime configurations, we now
>> +support running a list of scripts after vswitchd process. Just
>> +put your scripts under ``etcdir/post_scripts_vswitchd/`` and with
>> +``.sh`` suffix, ovs-ctl start/restart will auto execute your
>> +scripts one by one.
>> +
>> +
>>  Each ``ovs-ctl`` command is described separately below.
>>  
>>  The ``start`` command
>> diff --git a/NEWS b/NEWS
>> index 024fa44..9387d37 100644
>> --- a/NEWS
>> +++ b/NEWS
>> @@ -81,6 +81,8 @@ v3.0.0 - xx xxx xxxx
>>     - Linux TC offload:
>>       * Add support for offloading meters via tc police.
>>       * Add support for offloading the check_pkt_len action.
>> +   - ovs-ctl:
>> +     * Support ovs post start scripts
>>     - New configuration knob 'other_config:all-members-active' for
>>       balance-slb bonds.
>>     - Previously deprecated Linux kernel module is now fully removed from
>> diff --git a/utilities/ovs-ctl.in b/utilities/ovs-ctl.in
>> index e6e07f4..a765ef3 100644
>> --- a/utilities/ovs-ctl.in
>> +++ b/utilities/ovs-ctl.in
>> @@ -226,9 +226,18 @@ do_start_forwarding () {
>>      fi
>>  }
>>  
>> +run_post_start_scripts () {
>> +    mkdir -p $POST_SCRIPTS_DIR_VSWITCHD
> 
> I'm not thrilled about this.  We shouldn't be generating this
> directory.  If it exists, then we can use, otherwise, we should bail.
> 
>> +    for f in `ls $POST_SCRIPTS_DIR_VSWITCHD/*.sh 2>/dev/null|| true`
>> +    do
> 
> I'm not a scripting expert, but I don't think we want to work it like
> this.  Further, we should consider that we can use some information that
> the script has to pass on to the post-script.  As an example, we might
> want to pass the OVS_RUNDIR (which makes determining which daemons are
> running a bit easier).
> 
>> +        action "Running post script $f" bash $f
>> +    done
>> +}
>> +
>>  start_forwarding () {
>>      if test X"$OVS_VSWITCHD" = Xyes; then
>>          do_start_forwarding || return 1
>> +        run_post_start_scripts || return 1
>>      fi
>>      if test X"$RECORD_HOSTNAME" = Xyes; then
>>          set_hostname &
>> @@ -348,6 +357,8 @@ set_defaults () {
>>      DB_SCHEMA=$datadir/vswitch.ovsschema
>>      EXTRA_DBS=
>>  
>> +    POST_SCRIPTS_DIR_VSWITCHD=$etcdir/post_scripts_vswitchd
>> +
>>      PROTOCOL=gre
>>      DPORT=
>>      SPORT=
> 

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to