On 1/15/25 10:29, Frode Nordahl wrote: > On Wed, Jan 15, 2025 at 10:21 AM Frode Nordahl <[email protected]> wrote: >> >> On Wed, Jan 15, 2025 at 12:30 AM Ilya Maximets <[email protected]> wrote: >>> >>> Other services are running without monitors, so systemd can properly >>> track the pid. But ovs-monitor-ipsec is running with a monitor, so >>> there is one more fork and systemd complains about the pid file: >>> >>> systemd[1]: openvswitch-ipsec.service: Supervising process 1037185 >>> which is not our child. We'll most likely not notice >>> when it exits. >>> >>> This is also causing some spurious kills sent to the child on service >>> stop. >>> >>> Fix by running ovs-monitor-ipsec without a monitor as all other OVS >>> services. >>> >>> We can't use start_daemon, that would take care of this, because the >>> script is not on the PATH and we don't want to accidentally change >>> permissions for OVS directories (ipsec runs as root), but we can mimic >>> the behavior. >>> >>> Fixes: f385abded520 ("rhel: Use PIDFile on forking systemd service files") >>> Fixes: 9990322610f6 ("debian: Update packaging source from Debian/Ubuntu.") >>> Signed-off-by: Ilya Maximets <[email protected]> >>> --- >> >> Acked-by: Frode Nordahl <[email protected]> > > Forgot to highlight that with this change we should probably remove > debian/patches/ovs-ctl-ipsec.patch, since it is the only patch the > whole directory can be removed, i.e:
Hmm. Thanks for pointing out! I entirely forgot we have this... Will add this change in v3. Best regards, Ilya Maximets. > diff --git a/debian/automake.mk b/debian/automake.mk > index fe8febdd3..b86003f56 100644 > --- a/debian/automake.mk > +++ b/debian/automake.mk > @@ -56,8 +56,6 @@ EXTRA_DIST += \ > debian/openvswitch-vtep.init \ > debian/openvswitch-vtep.install \ > debian/ovs-systemd-reload \ > - debian/patches/ovs-ctl-ipsec.patch \ > - debian/patches/series \ > debian/python3-openvswitch.install \ > debian/rules \ > debian/source/format \ > diff --git a/debian/patches/ovs-ctl-ipsec.patch > b/debian/patches/ovs-ctl-ipsec.patch > deleted file mode 100644 > index 63375cd47..000000000 > --- a/debian/patches/ovs-ctl-ipsec.patch > +++ /dev/null > @@ -1,18 +0,0 @@ > -Description: Don't monitor ipsec daemon > - For Ubuntu systemd will monitor the ovs-monitor-ipsec daemon so > - there is no need to spawn a separate monitor thread to deal with > - restarts. Doing so has the side effect of confusing systemd into > - monitoring the wrong process. > -Author: James Page <[email protected]> > -Forwarded: not-needed > - > ---- a/utilities/ovs-ctl.in > -+++ b/utilities/ovs-ctl.in > -@@ -245,7 +245,7 @@ start_ovs_ipsec () { > - --pidfile=${rundir}/ovs-monitor-ipsec.pid \ > - --ike-daemon=$IKE_DAEMON \ > - $no_restart \ > -- --log-file --detach --monitor unix:${rundir}/db.sock || return 1 > -+ --log-file --detach unix:${rundir}/db.sock || return 1 > - return 0 > - } > diff --git a/debian/patches/series b/debian/patches/series > deleted file mode 100644 > index 87a2a1d97..000000000 > --- a/debian/patches/series > +++ /dev/null > @@ -1 +0,0 @@ > -ovs-ctl-ipsec.patch > > > >> >> >>> debian/openvswitch-ipsec.service | 2 +- >>> rhel/usr_lib_systemd_system_openvswitch-ipsec.service | 2 +- >>> utilities/ovs-ctl.in | 7 +++++-- >>> 3 files changed, 7 insertions(+), 4 deletions(-) >>> >>> diff --git a/debian/openvswitch-ipsec.service >>> b/debian/openvswitch-ipsec.service >>> index 608a6a618..dfb1e50d9 100644 >>> --- a/debian/openvswitch-ipsec.service >>> +++ b/debian/openvswitch-ipsec.service >>> @@ -6,7 +6,7 @@ After=openvswitch-switch.service >>> [Service] >>> Type=forking >>> PIDFile=/run/openvswitch/ovs-monitor-ipsec.pid >>> -ExecStart=/usr/share/openvswitch/scripts/ovs-ctl \ >>> +ExecStart=/usr/share/openvswitch/scripts/ovs-ctl --no-monitor \ >>> --ike-daemon=strongswan start-ovs-ipsec >>> ExecStop=/usr/share/openvswitch/scripts/ovs-ctl stop-ovs-ipsec >>> >>> diff --git a/rhel/usr_lib_systemd_system_openvswitch-ipsec.service >>> b/rhel/usr_lib_systemd_system_openvswitch-ipsec.service >>> index 92dad44f9..88a509662 100644 >>> --- a/rhel/usr_lib_systemd_system_openvswitch-ipsec.service >>> +++ b/rhel/usr_lib_systemd_system_openvswitch-ipsec.service >>> @@ -6,7 +6,7 @@ After=openvswitch.service >>> [Service] >>> Type=forking >>> PIDFile=/run/openvswitch/ovs-monitor-ipsec.pid >>> -ExecStart=/usr/share/openvswitch/scripts/ovs-ctl \ >>> +ExecStart=/usr/share/openvswitch/scripts/ovs-ctl --no-monitor \ >>> --ike-daemon=libreswan start-ovs-ipsec >>> ExecStop=/usr/share/openvswitch/scripts/ovs-ctl stop-ovs-ipsec >>> >>> diff --git a/utilities/ovs-ctl.in b/utilities/ovs-ctl.in >>> index 57abd3a5b..1f9ce1e6e 100644 >>> --- a/utilities/ovs-ctl.in >>> +++ b/utilities/ovs-ctl.in >>> @@ -245,12 +245,15 @@ start_ovs_ipsec () { >>> if test X$RESTART_IKE_DAEMON = Xno; then >>> no_restart="--no-restart-ike-daemon" >>> fi >>> + if test X"$MONITOR" != Xno; then >>> + monitor_arg="--monitor" >>> + fi >>> >>> ${datadir}/scripts/ovs-monitor-ipsec \ >>> --pidfile=${rundir}/ovs-monitor-ipsec.pid \ >>> --ike-daemon=$IKE_DAEMON \ >>> - $no_restart \ >>> - --log-file --detach --monitor unix:${rundir}/db.sock || return 1 >>> + $no_restart $monitor_arg \ >>> + --log-file --detach unix:${rundir}/db.sock || return 1 >>> return 0 >>> } >>> >>> -- >>> 2.47.0 >>> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
