| Message ID | 202211240926557166012@chinatelecom.cn |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev] ovs-tcpdump:Stdout is shutdown before ovs-tcpdump exit | 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 11/24/22 02:26, Songtao Zhan wrote: > To: dev@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 <zhanst1@chinatelecom.cn> > --- > 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.
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()
To: dev@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 <zhanst1@chinatelecom.cn> --- utilities/ovs-tcpdump.in | 13 +++++++++++++ 1 file changed, 13 insertions(+)