On Thu, Oct 03, 2019 at 02:49:10PM +0200, Jan Tluka wrote:
Thu, Oct 03, 2019 at 02:25:15PM CEST, olichtne(a)redhat.com wrote:
>On Thu, Oct 03, 2019 at 01:21:44PM +0200, Jan Tluka wrote:
>> Thu, Oct 03, 2019 at 11:33:45AM CEST, olichtne(a)redhat.com wrote:
>> >From: Ondrej Lichtner <olichtne(a)redhat.com>
>> >
>> >When not flushed, the pipe is buffered and the testpmd process doesn't
>> >actually start forwarding packets as expected.
>> >
>> >Signed-off-by: Ondrej Lichtner <olichtne(a)redhat.com>
>> >---
>> > lnst/Tests/TestPMD.py | 3 +++
>> > 1 file changed, 3 insertions(+)
>> >
>> >diff --git a/lnst/Tests/TestPMD.py b/lnst/Tests/TestPMD.py
>> >index 59c69aa..a92a42a 100644
>> >--- a/lnst/Tests/TestPMD.py
>> >+++ b/lnst/Tests/TestPMD.py
>> >@@ -39,11 +39,14 @@ def run(self):
>> > close_fds=True)
>> >
>> > process.stdin.write(str.encode("start tx_first\n"))
>> >+ process.stdin.flush()
>> >
>> > self.wait_for_interrupt()
>> >
>> > process.stdin.write(str.encode("stop\n"))
>> >+ process.stdin.flush()
>> > process.stdin.write(str.encode("quit\n"))
>> >+ process.stdin.flush()
>> >
>> > out, err = process.communicate()
>> > self._res_data = {"stdout": out, "stderr":
err}
>>
>> Hmm, isn't it possible to configure the stdin to be unbuffered?
>> J.
>
>well, the open() call does have a "buffering" option that can configure
>it, in our case the pipe is opened in binary mode which falls back to a
>"io.DEFAULT_BUFFER_SIZE" buffering. Unfortunatelly I can't find a
simple
>way to change the buffer size once the file is already opened, it might
>be possible via ioctl, but it could be messy.
>
>We could also just create the pipe manually, and be able to change the
>buffering parameter for the open() call.
>
>Either way I don't think it's that important for this use case where we
>just have 3 simple writes, calling flush 3 times is ok. We can look into
>a better way later if we extend this to more write operations and it
>gets tedious to flush every time. What do you think?
>
>-Ondrej
I looked briefly into subprocess.Popen() that is called few lines
before. We could add bufsize parameter, if set to bufsize=0 then __all__
of stdin, stdout, stderr will get unbuffered. Not sure if we want to
touch anything other than stdin though.
If that does not fit, let's proceed with the patch you've posted.
J.
Got a little confused while I was pushing the next patchset and also
ended up pushing this... I guess it shouldn't be a big problem, we can
work on improvements later.
-Ondrej