| Message ID | 1450186654-561-1-git-send-email-i.maximets@samsung.com |
|---|---|
| State | Changes Requested |
| Headers | show |
> -----Original Message----- > From: dev [mailto:dev-bounces@openvswitch.org] On Behalf Of Ilya Maximets > Sent: Tuesday, December 15, 2015 1:38 PM > To: dev@openvswitch.org; Ben Pfaff > Cc: Ilya Maximets; Pavel Fedin; Dyasly Sergey > Subject: [ovs-dev] [PATCH RFC] ovs-lib: try to call exit before killing > > 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. Have you tested the issue with this commit? commit e04f7e4f2f574500334326dbda1bb808cf25c721 Author: Ciara Loftus <ciara.loftus@intel.com> Date: Wed Oct 21 14:50:36 2015 +0100 netdev-dpdk: Clean-up after vHost User port delete Unregister and delete the socket associated with a vhost-user port when the port is deleted and/or the switch is brought down. Do not delete the socket if the vhost-user device is still attached to the guest. Signed-off-by: Ciara Loftus <ciara.loftus@intel.com> Acked-by: Daniele Di Proietto <diproiettod@vmware.com> > > Signed-off-by: Ilya Maximets <i.maximets@samsung.com> > --- > utilities/ovs-lib.in | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in > index dd8a1e9..8b371dd 100644 > --- a/utilities/ovs-lib.in > +++ b/utilities/ovs-lib.in > @@ -202,11 +202,14 @@ 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 exit > + ;; > TERM) > action "Killing $1 ($pid)" kill $pid > ;; > -- > 2.1.4 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev
Hello! > > Eample: > > Socket for vhost-user port will stay in a system > > after 'systemctl stop openvswitch' and opening > > that port after restart will fail. > > Have you tested the issue with this commit? > > commit e04f7e4f2f574500334326dbda1bb808cf25c721 > Author: Ciara Loftus <ciara.loftus@intel.com> > Date: Wed Oct 21 14:50:36 2015 +0100 > > netdev-dpdk: Clean-up after vHost User port delete > > Unregister and delete the socket associated with a vhost-user > port when the port is deleted and/or the switch is brought down. > Do not delete the socket if the vhost-user device is still attached > to the guest. > > Signed-off-by: Ciara Loftus <ciara.loftus@intel.com> > Acked-by: Daniele Di Proietto <diproiettod@vmware.com> Yes we did. The issue persists. Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia
Yes. Tested with last master
398e182 ("datapath-windows: remove ASSERT in OvsDoFlowLookupOutput()").
Problem is that ovs-vswitchd killed by 'ovs-ctl stop' called by systemd.
netdev_dpdk_vhost_destruct() never called.
Best regards, Ilya Maximets.
On 15.12.2015 16:52, Traynor, Kevin wrote:
>
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@openvswitch.org] On Behalf Of Ilya Maximets
>> Sent: Tuesday, December 15, 2015 1:38 PM
>> To: dev@openvswitch.org; Ben Pfaff
>> Cc: Ilya Maximets; Pavel Fedin; Dyasly Sergey
>> Subject: [ovs-dev] [PATCH RFC] ovs-lib: try to call exit before killing
>>
>> 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.
>
> Have you tested the issue with this commit?
>
> commit e04f7e4f2f574500334326dbda1bb808cf25c721
> Author: Ciara Loftus <ciara.loftus@intel.com>
> Date: Wed Oct 21 14:50:36 2015 +0100
>
> netdev-dpdk: Clean-up after vHost User port delete
>
> Unregister and delete the socket associated with a vhost-user
> port when the port is deleted and/or the switch is brought down.
> Do not delete the socket if the vhost-user device is still attached
> to the guest.
>
> Signed-off-by: Ciara Loftus <ciara.loftus@intel.com>
> Acked-by: Daniele Di Proietto <diproiettod@vmware.com>
>
>>
>> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
>> ---
>> utilities/ovs-lib.in | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in
>> index dd8a1e9..8b371dd 100644
>> --- a/utilities/ovs-lib.in
>> +++ b/utilities/ovs-lib.in
>> @@ -202,11 +202,14 @@ 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 exit
>> + ;;
>> TERM)
>> action "Killing $1 ($pid)" kill $pid
>> ;;
>> --
>> 2.1.4
>>
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev
fatal_signal_add_hook() in lib/fatal-signal.c is usually called if you need some function to be called after a signal is received.( I do not have any opinion with the proposed patch itself) On 15 December 2015 at 05:58, Ilya Maximets <i.maximets@samsung.com> wrote: > Yes. Tested with last master > 398e182 ("datapath-windows: remove ASSERT in OvsDoFlowLookupOutput()"). > > Problem is that ovs-vswitchd killed by 'ovs-ctl stop' called by systemd. > netdev_dpdk_vhost_destruct() never called. > > Best regards, Ilya Maximets. > > On 15.12.2015 16:52, Traynor, Kevin wrote: > > > >> -----Original Message----- > >> From: dev [mailto:dev-bounces@openvswitch.org] On Behalf Of Ilya > Maximets > >> Sent: Tuesday, December 15, 2015 1:38 PM > >> To: dev@openvswitch.org; Ben Pfaff > >> Cc: Ilya Maximets; Pavel Fedin; Dyasly Sergey > >> Subject: [ovs-dev] [PATCH RFC] ovs-lib: try to call exit before killing > >> > >> 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. > > > > Have you tested the issue with this commit? > > > > commit e04f7e4f2f574500334326dbda1bb808cf25c721 > > Author: Ciara Loftus <ciara.loftus@intel.com> > > Date: Wed Oct 21 14:50:36 2015 +0100 > > > > netdev-dpdk: Clean-up after vHost User port delete > > > > Unregister and delete the socket associated with a vhost-user > > port when the port is deleted and/or the switch is brought down. > > Do not delete the socket if the vhost-user device is still attached > > to the guest. > > > > Signed-off-by: Ciara Loftus <ciara.loftus@intel.com> > > Acked-by: Daniele Di Proietto <diproiettod@vmware.com> > > > >> > >> Signed-off-by: Ilya Maximets <i.maximets@samsung.com> > >> --- > >> utilities/ovs-lib.in | 5 ++++- > >> 1 file changed, 4 insertions(+), 1 deletion(-) > >> > >> diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in > >> index dd8a1e9..8b371dd 100644 > >> --- a/utilities/ovs-lib.in > >> +++ b/utilities/ovs-lib.in > >> @@ -202,11 +202,14 @@ 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 exit > >> + ;; > >> TERM) > >> action "Killing $1 ($pid)" kill $pid > >> ;; > >> -- > >> 2.1.4 > >> > >> _______________________________________________ > >> dev mailing list > >> dev@openvswitch.org > >> http://openvswitch.org/mailman/listinfo/dev > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev >
On Tue, Dec 15, 2015 at 04:37:34PM +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> > --- > utilities/ovs-lib.in | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in > index dd8a1e9..8b371dd 100644 > --- a/utilities/ovs-lib.in > +++ b/utilities/ovs-lib.in > @@ -202,11 +202,14 @@ 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 exit Probably should specify "-T 1" to make sure that ovs-appctl doesn't hang forever if the daemon is buggy. Might want to specify "-t $rundir/$1.pid" instead of "-t $1" so that the process we kill is certain to be the one in the right pidfile (in case of some weird configuration mismatch).
On 16.12.2015 07:21, Ben Pfaff wrote: > On Tue, Dec 15, 2015 at 04:37:34PM +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> >> --- >> utilities/ovs-lib.in | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in >> index dd8a1e9..8b371dd 100644 >> --- a/utilities/ovs-lib.in >> +++ b/utilities/ovs-lib.in >> @@ -202,11 +202,14 @@ 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 exit > > Probably should specify "-T 1" to make sure that ovs-appctl doesn't hang > forever if the daemon is buggy. > > Might want to specify "-t $rundir/$1.pid" instead of "-t $1" so that the > process we kill is certain to be the one in the right pidfile (in case > of some weird configuration mismatch). > > Thanks. New version posted. Best regards, Ilya Maximets.
diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in index dd8a1e9..8b371dd 100644 --- a/utilities/ovs-lib.in +++ b/utilities/ovs-lib.in @@ -202,11 +202,14 @@ 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 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> --- utilities/ovs-lib.in | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)