Hi Daniel,

Thanks for the patch!  I have some comments for re-spin.

The subject includes "(Daniel Ding)" but that should be trimmed.  I'm
not sure if your email system added this.  If so, perhaps you can use
something like "git send-email" or attach a plaintext copy of the
patchfile on a new spin.

Additionally, I think $subject should be written something like:

"[PATCH] ovs-tcpdump: Destroy mirror port on SIGHUP/SIGTERM"

"Daniel Ding" <[email protected]> writes:

> 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.

Please keep commit messages to shorter lines.  I don't know if your
email client made this change or not.

> Signed-off-by: Daniel Ding <[email protected]>
> ---

Otherwise, change LGTM.  :)

> utilities/ovs-tcpdump.in | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/utilities/ovs-tcpdump.in b/utilities/ovs-tcpdump.in
> index 7fd26e405..211d50fc5 100755
> --- a/utilities/ovs-tcpdump.in
> +++ b/utilities/ovs-tcpdump.in
> @@ -17,6 +17,7 @@
>  import os
>  import pwd
>  from random import randint
> +import signal
>  import subprocess
>  import sys
>  import time
> @@ -489,6 +490,17 @@ def main():
>          print("ERROR: Mirror port (%s) exists for port %s." %
>                (mirror_interface, interface))
>          sys.exit(1)
> +
> +    def signal_handler(_signo, _stack_frame):
> +        ovsdb.destroy_mirror(interface, ovsdb.port_bridge(interface))
> +        ovsdb.destroy_port(mirror_interface, ovsdb.port_bridge(interface))
> +
> +        sys.exit(0)
> +
> +    if sys.platform in ['linux', 'linux2']:
> +        signal.signal(signal.SIGHUP, signal_handler)
> +    signal.signal(signal.SIGTERM, signal_handler)
> +
>      try:
>          ovsdb.make_port(mirror_interface, ovsdb.port_bridge(interface))
>          ovsdb.bridge_mirror(interface, mirror_interface,
>
> ---

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to