Re: [ovs-dev] [PATCH] ovs-tcpdump:Stdout is shutdown before ovs-tcpdump exit

2023-03-17 Thread Adrian Moreno



On 11/24/22 02:26, Songtao Zhan wrote:

To: d...@openvswitch.org

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 
---
  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
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH] ovs-tcpdump:Stdout is shutdown before ovs-tcpdump exit

2022-11-23 Thread Songtao Zhan
To: d...@openvswitch.org

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.

Signed-off-by: Songtao Zhan 
---
 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
+# -i eth0 | grep "192.168.1.1"), the pipe is no longer available
+# after received ctrl+c
+# 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
+try:
+sys.stdout.close()
+# The shutdown operation brushes stdout into the pipe, so a pipe
+# error may be reported
+except IOError:
+pass
+
 if pipes.poll() is None:
 pipes.terminate()
 
-- 
2.31.1




zhan...@chinatelecom.cn
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev