On Thu, Aug 25, 2022 at 12:29:36PM +0200, Ilya Maximets wrote: > On 8/24/22 21:29, Aaron Conole wrote: > > lic121 <lic...@chinatelecom.cn> 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 <lic...@chinatelecom.cn> > >> --- > > > > 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.
Ilya, thanks for your review on this patch. Would you mind have a look at the patch that persistent max ct connection configureation. https://patchwork.ozlabs.org/project/openvswitch/patch/1653105300-28434-1-git-send-email-lic...@chinatelecom.cn/ > > 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 d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
