| Message ID | 1661422414-18677-1-git-send-email-zhihui.ding@easystack.cn |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev,v2] ovs-tcpdump: Cleanup mirror port on SIGHUP/SIGTERM | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/intel-ovs-compilation | success | test: success |
On Thu, Aug 25, 2022 at 6:14 AM Daniel Ding <zhihui.ding@easystack.cn> 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. > > Signed-off-by: Daniel Ding <zhihui.ding@easystack.cn> > --- > utilities/ovs-tcpdump.in | 30 ++++++++++++++++++------------ > 1 file changed, 18 insertions(+), 12 deletions(-) > > diff --git a/utilities/ovs-tcpdump.in b/utilities/ovs-tcpdump.in > index 7fd26e405..1d88a4666 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,20 @@ 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 as e: > + print("ERROR: Unable to clean mirror: %s" % str(e)) > + > + 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 +504,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 +514,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 +529,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.") Nit: This exception text is no longer relevant. That said, it probably wasn't even relevant before, as it wouldn't have caught any exceptions in the KeyboardInterrupt code above. Otherwise, it looks good! Acked-by: Mike Pattrick <mkp@redhat.com> > -- > 2.27.0 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On 8/31/22 21:45, Mike Pattrick wrote: > On Thu, Aug 25, 2022 at 6:14 AM Daniel Ding <zhihui.ding@easystack.cn> 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. >> >> Signed-off-by: Daniel Ding <zhihui.ding@easystack.cn> >> --- >> utilities/ovs-tcpdump.in | 30 ++++++++++++++++++------------ >> 1 file changed, 18 insertions(+), 12 deletions(-) >> >> diff --git a/utilities/ovs-tcpdump.in b/utilities/ovs-tcpdump.in >> index 7fd26e405..1d88a4666 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,20 @@ 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 as e: >> + print("ERROR: Unable to clean mirror: %s" % str(e)) >> + >> + 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 +504,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 +514,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 +529,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.") > > Nit: This exception text is no longer relevant. That said, it probably > wasn't even relevant before, as it wouldn't have caught any exceptions > in the KeyboardInterrupt code above. I guess, it was supposed to print warnings in case of any other exception during the actual tcpdump operation. So, we should move these warnings to the new handler in case of exception there. > > Otherwise, it looks good! > > Acked-by: Mike Pattrick <mkp@redhat.com> > >> -- >> 2.27.0 >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> >
diff --git a/utilities/ovs-tcpdump.in b/utilities/ovs-tcpdump.in index 7fd26e405..1d88a4666 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,20 @@ 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 as e: + print("ERROR: Unable to clean mirror: %s" % str(e)) + + 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 +504,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 +514,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 +529,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.")
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. Signed-off-by: Daniel Ding <zhihui.ding@easystack.cn> --- utilities/ovs-tcpdump.in | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-)