On 11/24/22 02:26, Songtao Zhan wrote:
To: [email protected]

If there is a pipe behind ovs-tcpdump(such as ovs-tcpdump -i eth0
| grep "192.168.1.1"), the child process (grep "192.168.1.1") may
exit first and close the pipe when received SIGTERM. When farther
process(ovs-tcpdump) exit, stdout is flushed into broken pipe, and
then received a exception IOError. To avoid such problems, ovs-tcp
dump first close stdout before exit.


Thanks for the patch. I've tested it and it seems to solve the identified problems. I only have some nits on the comments.


Signed-off-by: Songtao Zhan <[email protected]>
---
  utilities/ovs-tcpdump.in | 13 +++++++++++++
  1 file changed, 13 insertions(+)

diff --git a/utilities/ovs-tcpdump.in b/utilities/ovs-tcpdump.in
index a49ec9f94..c8a10c727 100755
--- a/utilities/ovs-tcpdump.in
+++ b/utilities/ovs-tcpdump.in
@@ -538,6 +538,19 @@ def main():
              print(data.decode('utf-8'))
          raise KeyboardInterrupt
      except KeyboardInterrupt:
+        # If there is a pipe behind ovs-tcpdump(such as ovs-tcpdump

Add a space before the "(".

+        # -i eth0 | grep "192.168.1.1"), the pipe is no longer available
+        # after received ctrl+c

End the sentence in a full stop.

+        # If we write data to an unavailable pipe, a pipe error will be
+        # reported, so we turn off stdout to avoid subsequence flushing
+        # of data into the pipe

End the sentence in a full stop.

+        try:
+            sys.stdout.close()
+        # The shutdown operation brushes stdout into the pipe, so a pipe
+        # error may be reported

I don't really understand this sentence. Can you rephrase? Or maybe just remove it as the above comment is clear enough.

+        except IOError:
+            pass
+
          if pipes.poll() is None:
              pipes.terminate()

Thanks.
--
Adrián Moreno

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

Reply via email to