| Message ID | 1450248656-15574-1-git-send-email-i.maximets@samsung.com |
|---|---|
| State | Changes Requested |
| Headers | show |
On Wed, Dec 16, 2015 at 09:50:56AM +0300, Ilya Maximets wrote: > While killing OVS may not free all allocated resources. > > Eample: > Socket for vhost-user port will stay in a system > after 'systemctl stop openvswitch' and opening > that port after restart will fail. > > Signed-off-by: Ilya Maximets <i.maximets@samsung.com> > --- > > version 2: > * added '-T 1' > * '-t $1' --> '-t $rundir/$1.$pid.ctl' Thanks. Have you tested it?
On 16.12.2015 09:53, Ben Pfaff wrote: > On Wed, Dec 16, 2015 at 09:50:56AM +0300, Ilya Maximets wrote: >> While killing OVS may not free all allocated resources. >> >> Eample: >> Socket for vhost-user port will stay in a system >> after 'systemctl stop openvswitch' and opening >> that port after restart will fail. >> >> Signed-off-by: Ilya Maximets <i.maximets@samsung.com> >> --- >> >> version 2: >> * added '-T 1' >> * '-t $1' --> '-t $rundir/$1.$pid.ctl' > > Thanks. > > Have you tested it? > Yes. On my setup it works. Only reason why it put RFC to that patch: Is all daemons have unixctl exit command? Or we can just let ovs-appctl to fail here in that case? Best regards, Ilya Maximets.
On Wed, Dec 16, 2015 at 09:58:56AM +0300, Ilya Maximets wrote: > > > On 16.12.2015 09:53, Ben Pfaff wrote: > > On Wed, Dec 16, 2015 at 09:50:56AM +0300, Ilya Maximets wrote: > >> While killing OVS may not free all allocated resources. > >> > >> Eample: > >> Socket for vhost-user port will stay in a system > >> after 'systemctl stop openvswitch' and opening > >> that port after restart will fail. > >> > >> Signed-off-by: Ilya Maximets <i.maximets@samsung.com> > >> --- > >> > >> version 2: > >> * added '-T 1' > >> * '-t $1' --> '-t $rundir/$1.$pid.ctl' > > > > Thanks. > > > > Have you tested it? > > > > Yes. On my setup it works. > Only reason why it put RFC to that patch: > Is all daemons have unixctl exit command? Or we can just let ovs-appctl to fail here > in that case? I think that all of our current daemons do have an "exit" command, but it might be worth removing the "2" after EXIT. The purpose of the delays in this chain is to wait a bit for a given signal to be processed through the operating system and the daemon, but EXIT should wait until the process has actually exited, or at least get very close to that. Maybe a short delay like .1 would be reasonable, to allow for a brief race window.
On 16.12.2015 11:27, Ben Pfaff wrote: > On Wed, Dec 16, 2015 at 09:58:56AM +0300, Ilya Maximets wrote: >> >> >> On 16.12.2015 09:53, Ben Pfaff wrote: >>> On Wed, Dec 16, 2015 at 09:50:56AM +0300, Ilya Maximets wrote: >>>> While killing OVS may not free all allocated resources. >>>> >>>> Eample: >>>> Socket for vhost-user port will stay in a system >>>> after 'systemctl stop openvswitch' and opening >>>> that port after restart will fail. >>>> >>>> Signed-off-by: Ilya Maximets <i.maximets@samsung.com> >>>> --- >>>> >>>> version 2: >>>> * added '-T 1' >>>> * '-t $1' --> '-t $rundir/$1.$pid.ctl' >>> >>> Thanks. >>> >>> Have you tested it? >>> >> >> Yes. On my setup it works. >> Only reason why it put RFC to that patch: >> Is all daemons have unixctl exit command? Or we can just let ovs-appctl to fail here >> in that case? > > I think that all of our current daemons do have an "exit" command, but > it might be worth removing the "2" after EXIT. The purpose of the > delays in this chain is to wait a bit for a given signal to be processed > through the operating system and the daemon, but EXIT should wait until > the process has actually exited, or at least get very close to that. > Maybe a short delay like .1 would be reasonable, to allow for a brief > race window. I don't think so, because ovs-vswitchd replies on connection to ovs-appctl immediately, but actual exiting performed while stopping execution in main loop. Best regards, Ilya Maximets.
On Wed, Dec 16, 2015 at 12:07:23PM +0300, Ilya Maximets wrote: > On 16.12.2015 11:27, Ben Pfaff wrote: > > On Wed, Dec 16, 2015 at 09:58:56AM +0300, Ilya Maximets wrote: > >> > >> > >> On 16.12.2015 09:53, Ben Pfaff wrote: > >>> On Wed, Dec 16, 2015 at 09:50:56AM +0300, Ilya Maximets wrote: > >>>> While killing OVS may not free all allocated resources. > >>>> > >>>> Eample: > >>>> Socket for vhost-user port will stay in a system > >>>> after 'systemctl stop openvswitch' and opening > >>>> that port after restart will fail. > >>>> > >>>> Signed-off-by: Ilya Maximets <i.maximets@samsung.com> > >>>> --- > >>>> > >>>> version 2: > >>>> * added '-T 1' > >>>> * '-t $1' --> '-t $rundir/$1.$pid.ctl' > >>> > >>> Thanks. > >>> > >>> Have you tested it? > >>> > >> > >> Yes. On my setup it works. > >> Only reason why it put RFC to that patch: > >> Is all daemons have unixctl exit command? Or we can just let ovs-appctl to fail here > >> in that case? > > > > I think that all of our current daemons do have an "exit" command, but > > it might be worth removing the "2" after EXIT. The purpose of the > > delays in this chain is to wait a bit for a given signal to be processed > > through the operating system and the daemon, but EXIT should wait until > > the process has actually exited, or at least get very close to that. > > Maybe a short delay like .1 would be reasonable, to allow for a brief > > race window. > > I don't think so, because ovs-vswitchd replies on connection to ovs-appctl > immediately, but actual exiting performed while stopping execution in > main loop. Then that means that this patch will generally make it take about 4 longer to stop OVS, since killing each daemon will sleep for 2 seconds.
On 16.12.2015 13:02, Ben Pfaff wrote: > On Wed, Dec 16, 2015 at 12:07:23PM +0300, Ilya Maximets wrote: >> On 16.12.2015 11:27, Ben Pfaff wrote: >>> On Wed, Dec 16, 2015 at 09:58:56AM +0300, Ilya Maximets wrote: >>>> >>>> >>>> On 16.12.2015 09:53, Ben Pfaff wrote: >>>>> On Wed, Dec 16, 2015 at 09:50:56AM +0300, Ilya Maximets wrote: >>>>>> While killing OVS may not free all allocated resources. >>>>>> >>>>>> Eample: >>>>>> Socket for vhost-user port will stay in a system >>>>>> after 'systemctl stop openvswitch' and opening >>>>>> that port after restart will fail. >>>>>> >>>>>> Signed-off-by: Ilya Maximets <i.maximets@samsung.com> >>>>>> --- >>>>>> >>>>>> version 2: >>>>>> * added '-T 1' >>>>>> * '-t $1' --> '-t $rundir/$1.$pid.ctl' >>>>> >>>>> Thanks. >>>>> >>>>> Have you tested it? >>>>> >>>> >>>> Yes. On my setup it works. >>>> Only reason why it put RFC to that patch: >>>> Is all daemons have unixctl exit command? Or we can just let ovs-appctl to fail here >>>> in that case? >>> >>> I think that all of our current daemons do have an "exit" command, but >>> it might be worth removing the "2" after EXIT. The purpose of the >>> delays in this chain is to wait a bit for a given signal to be processed >>> through the operating system and the daemon, but EXIT should wait until >>> the process has actually exited, or at least get very close to that. >>> Maybe a short delay like .1 would be reasonable, to allow for a brief >>> race window. >> >> I don't think so, because ovs-vswitchd replies on connection to ovs-appctl >> immediately, but actual exiting performed while stopping execution in >> main loop. > > Then that means that this patch will generally make it take about 4 > longer to stop OVS, since killing each daemon will sleep for 2 seconds. Ok. May be something like 'EXIT .1 .25 .65 1 1 TERM' will be better?
On Wed, Dec 16, 2015 at 01:31:03PM +0300, Ilya Maximets wrote: > > > On 16.12.2015 13:02, Ben Pfaff wrote: > > On Wed, Dec 16, 2015 at 12:07:23PM +0300, Ilya Maximets wrote: > >> On 16.12.2015 11:27, Ben Pfaff wrote: > >>> On Wed, Dec 16, 2015 at 09:58:56AM +0300, Ilya Maximets wrote: > >>>> > >>>> > >>>> On 16.12.2015 09:53, Ben Pfaff wrote: > >>>>> On Wed, Dec 16, 2015 at 09:50:56AM +0300, Ilya Maximets wrote: > >>>>>> While killing OVS may not free all allocated resources. > >>>>>> > >>>>>> Eample: > >>>>>> Socket for vhost-user port will stay in a system > >>>>>> after 'systemctl stop openvswitch' and opening > >>>>>> that port after restart will fail. > >>>>>> > >>>>>> Signed-off-by: Ilya Maximets <i.maximets@samsung.com> > >>>>>> --- > >>>>>> > >>>>>> version 2: > >>>>>> * added '-T 1' > >>>>>> * '-t $1' --> '-t $rundir/$1.$pid.ctl' > >>>>> > >>>>> Thanks. > >>>>> > >>>>> Have you tested it? > >>>>> > >>>> > >>>> Yes. On my setup it works. > >>>> Only reason why it put RFC to that patch: > >>>> Is all daemons have unixctl exit command? Or we can just let ovs-appctl to fail here > >>>> in that case? > >>> > >>> I think that all of our current daemons do have an "exit" command, but > >>> it might be worth removing the "2" after EXIT. The purpose of the > >>> delays in this chain is to wait a bit for a given signal to be processed > >>> through the operating system and the daemon, but EXIT should wait until > >>> the process has actually exited, or at least get very close to that. > >>> Maybe a short delay like .1 would be reasonable, to allow for a brief > >>> race window. > >> > >> I don't think so, because ovs-vswitchd replies on connection to ovs-appctl > >> immediately, but actual exiting performed while stopping execution in > >> main loop. > > > > Then that means that this patch will generally make it take about 4 > > longer to stop OVS, since killing each daemon will sleep for 2 seconds. > > Ok. May be something like 'EXIT .1 .25 .65 1 1 TERM' will be better? I'm more comfortable with that.
On 16.12.2015 15:21, Ben Pfaff wrote: > On Wed, Dec 16, 2015 at 01:31:03PM +0300, Ilya Maximets wrote: >> >> >> On 16.12.2015 13:02, Ben Pfaff wrote: >>> On Wed, Dec 16, 2015 at 12:07:23PM +0300, Ilya Maximets wrote: >>>> On 16.12.2015 11:27, Ben Pfaff wrote: >>>>> On Wed, Dec 16, 2015 at 09:58:56AM +0300, Ilya Maximets wrote: >>>>>> >>>>>> >>>>>> On 16.12.2015 09:53, Ben Pfaff wrote: >>>>>>> On Wed, Dec 16, 2015 at 09:50:56AM +0300, Ilya Maximets wrote: >>>>>>>> While killing OVS may not free all allocated resources. >>>>>>>> >>>>>>>> Eample: >>>>>>>> Socket for vhost-user port will stay in a system >>>>>>>> after 'systemctl stop openvswitch' and opening >>>>>>>> that port after restart will fail. >>>>>>>> >>>>>>>> Signed-off-by: Ilya Maximets <i.maximets@samsung.com> >>>>>>>> --- >>>>>>>> >>>>>>>> version 2: >>>>>>>> * added '-T 1' >>>>>>>> * '-t $1' --> '-t $rundir/$1.$pid.ctl' >>>>>>> >>>>>>> Thanks. >>>>>>> >>>>>>> Have you tested it? >>>>>>> >>>>>> >>>>>> Yes. On my setup it works. >>>>>> Only reason why it put RFC to that patch: >>>>>> Is all daemons have unixctl exit command? Or we can just let ovs-appctl to fail here >>>>>> in that case? >>>>> >>>>> I think that all of our current daemons do have an "exit" command, but >>>>> it might be worth removing the "2" after EXIT. The purpose of the >>>>> delays in this chain is to wait a bit for a given signal to be processed >>>>> through the operating system and the daemon, but EXIT should wait until >>>>> the process has actually exited, or at least get very close to that. >>>>> Maybe a short delay like .1 would be reasonable, to allow for a brief >>>>> race window. >>>> >>>> I don't think so, because ovs-vswitchd replies on connection to ovs-appctl >>>> immediately, but actual exiting performed while stopping execution in >>>> main loop. >>> >>> Then that means that this patch will generally make it take about 4 >>> longer to stop OVS, since killing each daemon will sleep for 2 seconds. >> >> Ok. May be something like 'EXIT .1 .25 .65 1 1 TERM' will be better? > > I'm more comfortable with that. OK. New version posted. Best regards, Ilya Maximets.
diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in index dd8a1e9..ace3e0d 100644 --- a/utilities/ovs-lib.in +++ b/utilities/ovs-lib.in @@ -202,11 +202,15 @@ start_daemon () { stop_daemon () { if test -e "$rundir/$1.pid"; then if pid=`cat "$rundir/$1.pid"`; then - for action in TERM .1 .25 .65 1 1 1 1 KILL 1 1 1 2 10 15 30 FAIL; do + for action in EXIT 2 TERM .1 .25 .65 1 1 1 1 KILL 1 1 1 2 10 15 30 FAIL; do if pid_exists "$pid" >/dev/null 2>&1; then :; else return 0 fi case $action in + EXIT) + action "Exiting $1 ($pid)" \ + ${bindir}/ovs-appctl -T 1 -t $rundir/$1.$pid.ctl exit + ;; TERM) action "Killing $1 ($pid)" kill $pid ;;
While killing OVS may not free all allocated resources. Eample: Socket for vhost-user port will stay in a system after 'systemctl stop openvswitch' and opening that port after restart will fail. Signed-off-by: Ilya Maximets <i.maximets@samsung.com> --- version 2: * added '-T 1' * '-t $1' --> '-t $rundir/$1.$pid.ctl' utilities/ovs-lib.in | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)