On Wed, Jan 15, 2025 at 3:00 PM 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. > > Debian package had a custom patch for this issue, removing now. > > 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]>
Thanks! Acked-by: Frode Nordahl <[email protected]> > --- > debian/automake.mk | 2 -- > debian/openvswitch-ipsec.service | 2 +- > debian/patches/ovs-ctl-ipsec.patch | 18 ------------------ > debian/patches/series | 1 - > ...ib_systemd_system_openvswitch-ipsec.service | 2 +- > utilities/ovs-ctl.in | 7 +++++-- > 6 files changed, 7 insertions(+), 25 deletions(-) > delete mode 100644 debian/patches/ovs-ctl-ipsec.patch > delete mode 100644 debian/patches/series > > 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/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/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 > 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
