Daniel Ding <[email protected]> writes: > On 2022/9/10 1:43, Ilya Maximets wrote: > > 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.") > > The above 'except' section, should be remove it, if I understand correctlly. > > > > 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) > > Okay, I will add try-except block in cleanup_mirror() function again, and > prints > warning. > > > > 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? > > Right. The third parameter of add_hook indicats the hook function need be run > at exiting even on the unexpeted exception. > > > > Best regards, Ilya Maximets. > > Thanks for your review, @Ilya Maximets. And the following is newer patch > according to your suggestions. > > --- > utilities/ovs-tcpdump.in | 40 ++++++++++++++++++++++------------------ > 1 file changed, 22 insertions(+), 18 deletions(-) > > diff --git a/utilities/ovs-tcpdump.in b/utilities/ovs-tcpdump.in > index 7fd26e405..92cff6d2c 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,24 @@ 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(): > + try: > + 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.") > + print(" ex: ovs-vsctl --db=%s del-port %s" % (db_sock, > + 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 +508,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 +518,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() > @@ -518,18 +534,6 @@ def main(): > 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.") > - print(" ex: ovs-vsctl --db=%s del-port %s" % (db_sock, > - mirror_interface)) > - sys.exit(1) > - > sys.exit(0) > > Best regards, Daniel Ding.
Hi Daniel, Please submit new version as a formal [PATCH v5] _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
