On 9/8/22 09:37, Daniel Ding wrote:
> If ovs-tcpdump received HUP or TERM signal, mirror and mirror
> interface should be destroyed. This often happens, when
> controlling terminal is closed, like ssh session closed, and
> other users use kill to terminate it.
> 
> Acked-by: Mike Pattrick <[email protected]>
> Signed-off-by: Daniel Ding <[email protected]>
> ---
>  utilities/ovs-tcpdump.in | 27 +++++++++++++++------------
>  1 file changed, 15 insertions(+), 12 deletions(-)
> 
> diff --git a/utilities/ovs-tcpdump.in b/utilities/ovs-tcpdump.in
> index 7fd26e405..b3f764c87 100755
> --- a/utilities/ovs-tcpdump.in
> +++ b/utilities/ovs-tcpdump.in
> @@ -44,6 +44,7 @@ try:
>      from ovs import jsonrpc
>      from ovs.poller import Poller
>      from ovs.stream import Stream
> +    from ovs.fatal_signal import add_hook
>  except Exception:
>      print("ERROR: Please install the correct Open vSwitch python support")
>      print("       libraries (version @VERSION@).")
> @@ -405,6 +406,17 @@ def py_which(executable):
>                 for path in os.environ["PATH"].split(os.pathsep))
>  
>  
> +def teardown(db_sock, interface, mirror_interface, tap_created):
> +    def cleanup_mirror():
> +        ovsdb = OVSDB(db_sock)
> +        ovsdb.destroy_mirror(interface, ovsdb.port_bridge(interface))
> +        ovsdb.destroy_port(mirror_interface, ovsdb.port_bridge(interface))
> +        if tap_created is True:
> +            _del_taps[sys.platform](mirror_interface)
> +
> +    add_hook(cleanup_mirror, None, True)
> +
> +
>  def main():
>      rundir = os.environ.get('OVS_RUNDIR', '@RUNDIR@')
>      db_sock = 'unix:%s' % os.path.join(rundir, "db.sock")
> @@ -489,6 +501,9 @@ def main():
>          print("ERROR: Mirror port (%s) exists for port %s." %
>                (mirror_interface, interface))
>          sys.exit(1)
> +
> +    teardown(db_sock, interface, mirror_interface, tap_created)
> +
>      try:
>          ovsdb.make_port(mirror_interface, ovsdb.port_bridge(interface))
>          ovsdb.bridge_mirror(interface, mirror_interface,
> @@ -496,12 +511,6 @@ def main():
>                              mirror_select_all)
>      except OVSDBException as oe:
>          print("ERROR: Unable to properly setup the mirror: %s." % str(oe))
> -        try:
> -            ovsdb.destroy_port(mirror_interface, 
> ovsdb.port_bridge(interface))
> -            if tap_created is True:
> -                _del_taps[sys.platform](mirror_interface)
> -        except Exception:
> -            pass
>          sys.exit(1)
>  
>      ovsdb.close_idl()
> @@ -517,12 +526,6 @@ def main():
>      except KeyboardInterrupt:
>          if pipes.poll() is None:
>              pipes.terminate()
> -
> -        ovsdb = OVSDB(db_sock)
> -        ovsdb.destroy_mirror(interface, ovsdb.port_bridge(interface))
> -        ovsdb.destroy_port(mirror_interface, ovsdb.port_bridge(interface))
> -        if tap_created is True:
> -            _del_taps[sys.platform](mirror_interface)
>      except Exception:
>          print("Unable to tear down the ports and mirrors.")
>          print("Please use ovs-vsctl to remove the ports and mirrors 
> created.")

I think there was some misunderstanding here.  Sorry about that.
What I meant is that cleanup_mirror() function should have a
try-except block and have the 'print's above in the 'except' section.

i.e.

def teardown(...):
    def cleanup_mirror():
        try:
            <destroy a mirror>
        except Exception:
            print("Unable to tear down the ports and mirrors.")
            print("Please use ovs-vsctl to remove the ports and mirrors 
created.")
            print(" ex: ovs-vsctl --db=...

    add_hook(cleanup_mirror, None, True)

And for the pipes.poll() loop we only need to catch the KeyboardInterrupt.
No need to catch generic exceptions, IIUC.  The hook will be executed
anyway, even on the unexpected exception, right?

Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to