diff mbox series

lib: Add fifo library

Message ID 20191209064110.67975-1-lkml@jv-coder.de
State Changes Requested
Delegated to: Cyril Hrubis
Headers show
Series lib: Add fifo library | expand

Commit Message

Joerg Vehlow Dec. 9, 2019, 6:41 a.m. UTC
From: Joerg Vehlow <joerg.vehlow@aox-tech.de>

This is a proposal for a fifo library that can be used to synchronize
tests internally, e.g. between a shell script and c program or between forks.
If the proposal is accepted, I will also add documentation.

The library allows an arbitrary number of fifos to be created. The only requirement
is that the test needs TST_NEEDS_TMPDIR=1.
Reading and writing to the fifo is also a synchronization point.

Each fifo requires two queues: One for sending a message to the other process
and the second one is used for receive acknowledgement. Otherwise two messages
can be received at the same time, if the sender sends them too fast, before the receiver
has closed its file handle.

Signed-off-by: Joerg Vehlow <joerg.vehlow@aox-tech.de>
---
 include/tst_fifo.h                      |  12 +++
 lib/newlib_tests/.gitignore             |   2 +
 lib/newlib_tests/shell/tst_fifo_test.sh |  56 ++++++++++++
 lib/newlib_tests/tst_fifo.c             |  51 +++++++++++
 lib/newlib_tests/tst_fifo_proc.c        |  42 +++++++++
 lib/tst_fifo.c                          | 115 ++++++++++++++++++++++++
 testcases/Makefile                      |   2 +-
 testcases/lib/tst_fifo.sh               |  58 ++++++++++++
 testcases/lib/tst_test.sh               |   4 +-
 9 files changed, 340 insertions(+), 2 deletions(-)
 create mode 100644 include/tst_fifo.h
 create mode 100644 lib/newlib_tests/shell/tst_fifo_test.sh
 create mode 100644 lib/newlib_tests/tst_fifo.c
 create mode 100644 lib/newlib_tests/tst_fifo_proc.c
 create mode 100644 lib/tst_fifo.c
 create mode 100644 testcases/lib/tst_fifo.sh

Comments

Petr Vorel Dec. 9, 2019, 10:25 p.m. UTC | #1
Hi Joerg,

Reviewed-by: Petr Vorel <pvorel@suse.cz>

LGTM. Good work, thanks!

...
> +++ b/lib/newlib_tests/shell/tst_fifo_test.sh
> @@ -0,0 +1,56 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +# Copyright (c) 2019 Joerg Vehlow <joerg.vehlow@aox-tech.de>
> +
> +TST_TESTFUNC=do_test
> +TST_NEEDS_FIFO=1
> +TST_NEEDS_TMPDIR=1
> +
> +. tst_test.sh
> +
> +S2P="fifo_shell_to_proc"
> +P2S="fifo_proc_to_shell"
> +
> +do_test()
> +{
> +    tst_fifo_create $S2P
> +    tst_fifo_create $P2S
> +
> +    tst_fifo_proc &
> +    pid=$!
> +
> +    tst_fifo_send $S2P "init message"
> +    tst_res TPASS "init message send"
> +
> +    tst_fifo_send $S2P "second init message"
> +    tst_res TPASS "second init message send"
> +
> +    data=$(tst_fifo_receive $P2S)
> +    if [ "$data" == "answer 1" ]; then
Please use just = (bashism).

...
> +++ b/lib/newlib_tests/tst_fifo_proc.c
> @@ -0,0 +1,42 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2019 Joerg Vehlow <joerg.vehlow@aox-tech.de>
> + */
> +
> +
> +#define TST_NO_DEFAULT_MAIN
> +#include "tst_test.h"
> +#include "tst_fifo.h"
> +
> +#define S2P "fifo_shell_to_proc"
> +#define P2S "fifo_proc_to_shell"
> +
> +int main()
tst_fifo_proc.c:14:5: warning: old-style function definition [-Wold-style-definition]
=> int main(void)
> +{
> +    char data[128];
> +
> +    TCID = "tst_fifo_proc";
> +
> +    tst_fifo_init();
> +
> +    tst_fifo_receive(S2P, data, sizeof(data));
> +    tst_res(strcmp(data, "init message") == 0 ? TPASS : TFAIL,
> +            "Received expected init message (%s)", data);
> +
> +    // Wait a bit for asynchronous access to pipe
> +    sleep(1);
> +
> +    tst_fifo_receive(S2P, data, sizeof(data));
> +    tst_res(strcmp(data, "second init message") == 0 ? TPASS : TFAIL,
> +            "Received expected second init message");
> +
> +    tst_fifo_send(P2S, "answer 1");
> +    sleep(1);
> +    tst_fifo_send(P2S, "answer 2");
> +    sleep(1);
> +    tst_fifo_send(P2S, "answer 3");
> +
> +    tst_res(TPASS, "All tests passed");
> +
> +    return 0;
> +}
> \ No newline at end of file
> diff --git a/lib/tst_fifo.c b/lib/tst_fifo.c
> new file mode 100644
> index 000000000..7d139624b
> --- /dev/null
> +++ b/lib/tst_fifo.c
> @@ -0,0 +1,115 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2019 Joerg Vehlow <joerg.vehlow@aox-tech.de>
> + */
> +#define _GNU_SOURCE
If _GNU_SOURCE is not needed, drop it please.

> +#include <stddef.h>
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <unistd.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +
> +#define TST_NO_DEFAULT_MAIN
> +#include "tst_test.h"
> +#include "old_tmpdir.h"
> +#include "tst_fifo.h"
> +
> +#ifndef PATH_MAX
> +#ifdef MAXPATHLEN
> +#define PATH_MAX	MAXPATHLEN
> +#else
> +#define PATH_MAX	1024
> +#endif
> +#endif
Hm, this is copy paste from old tests (all use test.h, even quite new and clean tests/tst_tmpdir_test.c).
I wonder if this is still relevant, can't we use #include <limits.h>?

> +
> +#define FIFO_ENV_VAR "LTP_FIFO_PATH"
> +
> +static char *FIFO_DIR = NULL;
> +
> +#define INIT_FIFO_FUNCTION(req_mode, ack_mode) \
> +    char path_req[PATH_MAX]; \
> +    char path_ack[PATH_MAX]; \
> +    if (!FIFO_DIR) \
> +        tst_brk(TBROK, "you must call tst_fifo_init first"); \
> +    snprintf(path_req, sizeof(path_req), "%s/tst_fifo_%s.req", FIFO_DIR, name); \
> +    snprintf(path_ack, sizeof(path_ack), "%s/tst_fifo_%s.ack", FIFO_DIR, name);
> +    //if (required_mode && access(path, required_mode)) \
> +     //   tst_brk(TBROK, "cannot access '%s'", path);
Commented out code forgotten :).
...

> +++ b/testcases/Makefile
> @@ -28,7 +28,7 @@ include $(top_srcdir)/include/mk/env_pre.mk
>  # 1. kdump shouldn't be compiled by default, because it's runtime based and
>  #    WILL crash the build host (the tests need to be fixed to just build, not
>  #    run).
> -FILTER_OUT_DIRS		:= kdump
> +FILTER_OUT_DIRS		:= kdump open_posix_testsuite realtime kernel network misc
I guess this is unrelated change for your debug.

>  ifneq ($(WITH_OPEN_POSIX_TESTSUITE),yes)
>  FILTER_OUT_DIRS		+= open_posix_testsuite
> diff --git a/testcases/lib/tst_fifo.sh b/testcases/lib/tst_fifo.sh
> new file mode 100644
> index 000000000..922b24059
> --- /dev/null
> +++ b/testcases/lib/tst_fifo.sh
> @@ -0,0 +1,58 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +# Copyright (c) 2019 Joerg Vehlow <joerg.vehlow@aox-tech.de>
tst_security.sh and tst_net.sh have each it's guard:
[ -n "$TST_SECURITY_LOADED" ] && return 0
tst_fifo.sh should probably have one as well.

> +[ "$TST_NEEDS_TMPDIR" != 1 ] && tst_brk TCONF "fifo library requires TST_NEEDS_TMPDIR=1"
If we apply https://patchwork.ozlabs.org/patch/1206399/, it should be
$TST_NEEDS_TMPDIR=1


> +[ -z "$TST_TMPDIR" ] && tst_brk TCONF "TST_TMPDIR is not supposed to be empty"
> +
> +export LTP_FIFO_PATH="$TST_TMPDIR"
> +
> +tst_fifo_create()
> +{
> +    [ $# -ne 1 ] && tst_brk TBROK "tst_fifo_create expects 1 parameter"
> +    local _tst_path_req="${TST_TMPDIR}/tst_fifo_$1.req"
> +    local _tst_path_ack="${TST_TMPDIR}/tst_fifo_$1.ack"
> +
> +    mkfifo "$_tst_path_req" || tst_brk TBROK "unable to create fifo '$_tst_path_req'"
> +    mkfifo "$_tst_path_ack" || tst_brk TBROK "unable to create fifo '$_tst_path_ack'"
> +}
> +
> +tst_fifo_destroy()
> +{
> +    [ $# -ne 1 ] && tst_brk TBROK "tst_fifo_destroy expects 1 parameter"
> +    local _tst_path_req="${TST_TMPDIR}/tst_fifo_$1.req"
> +    local _tst_path_ack="${TST_TMPDIR}/tst_fifo_$1.ack"
> +
> +    [ -p "$_tst_path_req" ] || tst_brk TBROK "tst_fifo_destroy fifo '$_tst_path_req' does not exist"
> +    [ -p "$_tst_path_ack" ] || tst_brk TBROK "tst_fifo_destroy fifo '$_tst_path_ack' does not exist"
> +
> +    rm "$_tst_path_req"  || tst_brk TBROK "unable to destroy fifo '$_tst_path_req'"
> +    rm "$_tst_path_ack"  || tst_brk TBROK "unable to destroy fifo '$_tst_path_ack'"
> +}
> +
> +tst_fifo_send()
> +{
> +    [ $# -ne 2 ] && tst_brk TBROK "tst_fifo_send expects 2 parameters"
> +    local _tst_path_req="${TST_TMPDIR}/tst_fifo_$1.req"
> +    local _tst_path_ack="${TST_TMPDIR}/tst_fifo_$1.ack"
> +    local _tst_msg="$2"
> +
> +    [ -p "$_tst_path_req" ] || tst_brk TBROK "tst_fifo_send fifo '$_tst_path_req' does not exist"
> +    [ -p "$_tst_path_ack" ] || tst_brk TBROK "tst_fifo_send fifo '$_tst_path_ack' does not exist"
> +
> +    echo -n "$_tst_msg" > "$_tst_path_req"
nit: I'd prefer printf
> +    cat "$_tst_path_ack" > /dev/null
> +}
> +

...
> +++ b/testcases/lib/tst_test.sh
> @@ -479,7 +479,7 @@ tst_run()
>  			NEEDS_DRIVERS|FS_TYPE|MNTPOINT|MNT_PARAMS);;
>  			IPV6|IPVER|TEST_DATA|TEST_DATA_IFS);;
>  			RETRY_FUNC|RETRY_FN_EXP_BACKOFF|TIMEOUT);;
> -			NET_MAX_PKT);;
> +			NET_MAX_PKT|NEEDS_FIFO);;
>  			*) tst_res TWARN "Reserved variable TST_$_tst_i used!";;
>  			esac
>  		done
> @@ -537,6 +537,8 @@ tst_run()
>  		cd "$TST_TMPDIR"
>  	fi

> +	[ "$TST_NEEDS_FIFO" = 1 ] && . tst_fifo.sh
I'd load it at the top, just below
. tst_ansi_color.sh
. tst_security.sh

Kind regards,
Petr
Joerg Vehlow Dec. 10, 2019, 5:53 a.m. UTC | #2
Hi,

thanks Petr.

Some comments:
>> +#include <stddef.h>
>> +#include <stdlib.h>
>> +#include <stdio.h>
>> +#include <unistd.h>
>> +#include <sys/types.h>
>> +#include <sys/stat.h>
>> +
>> +#define TST_NO_DEFAULT_MAIN
>> +#include "tst_test.h"
>> +#include "old_tmpdir.h"
>> +#include "tst_fifo.h"
>> +
>> +#ifndef PATH_MAX
>> +#ifdef MAXPATHLEN
>> +#define PATH_MAX	MAXPATHLEN
>> +#else
>> +#define PATH_MAX	1024
>> +#endif
>> +#endif
> Hm, this is copy paste from old tests (all use test.h, even quite new and clean tests/tst_tmpdir_test.c).
> I wonder if this is still relevant, can't we use #include <limits.h>?
Right, if there is nothing against using limits.h, I'll change it. I 
just used other code for reference for my code.
>> +++ b/testcases/Makefile
>> @@ -28,7 +28,7 @@ include $(top_srcdir)/include/mk/env_pre.mk
>>   # 1. kdump shouldn't be compiled by default, because it's runtime based and
>>   #    WILL crash the build host (the tests need to be fixed to just build, not
>>   #    run).
>> -FILTER_OUT_DIRS		:= kdump
>> +FILTER_OUT_DIRS		:= kdump open_posix_testsuite realtime kernel network misc
> I guess this is unrelated change for your debug.
Damn... This happens to me all the time, it's time for configure 
switches ;)
>
>> +[ "$TST_NEEDS_TMPDIR" != 1 ] && tst_brk TCONF "fifo library requires TST_NEEDS_TMPDIR=1"
> If we apply https://patchwork.ozlabs.org/patch/1206399/, it should be
> $TST_NEEDS_TMPDIR=1
I don't get it? The path you linked seems unrelated to me and did you mean
[  "$TST_NEEDS_TMPDIR" = 1 ] || ... ?
> +	[ "$TST_NEEDS_FIFO" = 1 ] && . tst_fifo.sh
> I'd load it at the top, just below
> . tst_ansi_color.sh
> . tst_security.sh
The way I implemented it, I can't load it at the top, because 
tst_fifo.sh requires
TST_TMPDIR to be already set, when it is included.
> Kind regards,
> Petr
Petr Vorel Dec. 10, 2019, 7:38 a.m. UTC | #3
Hi Joerg,

> > > +#ifndef PATH_MAX
> > > +#ifdef MAXPATHLEN
> > > +#define PATH_MAX	MAXPATHLEN
> > > +#else
> > > +#define PATH_MAX	1024
> > > +#endif
> > > +#endif
> > Hm, this is copy paste from old tests (all use test.h, even quite new and clean tests/tst_tmpdir_test.c).
> > I wonder if this is still relevant, can't we use #include <limits.h>?
> Right, if there is nothing against using limits.h, I'll change it. I just
> used other code for reference for my code.
I don't know the purpose of this code (anybody knows?), but unless there is good
reason for it, I'd be for using <limits.h>.

> > > +++ b/testcases/Makefile
> > > @@ -28,7 +28,7 @@ include $(top_srcdir)/include/mk/env_pre.mk
> > >   # 1. kdump shouldn't be compiled by default, because it's runtime based and
> > >   #    WILL crash the build host (the tests need to be fixed to just build, not
> > >   #    run).
> > > -FILTER_OUT_DIRS		:= kdump
> > > +FILTER_OUT_DIRS		:= kdump open_posix_testsuite realtime kernel network misc
> > I guess this is unrelated change for your debug.
> Damn... This happens to me all the time, it's time for configure switches ;)
Yep, I have it on my todo list, but haven't forced to do it. Feel free to
implement it.
https://github.com/linux-test-project/ltp/issues/617

> > > +[ "$TST_NEEDS_TMPDIR" != 1 ] && tst_brk TCONF "fifo library requires TST_NEEDS_TMPDIR=1"
> > If we apply https://patchwork.ozlabs.org/patch/1206399/, it should be
> > $TST_NEEDS_TMPDIR=1
> I don't get it? The path you linked seems unrelated to me and did you mean
> [  "$TST_NEEDS_TMPDIR" = 1 ] || ... ?
> > +	[ "$TST_NEEDS_FIFO" = 1 ] && . tst_fifo.sh
> > I'd load it at the top, just below
> > . tst_ansi_color.sh
> > . tst_security.sh
> The way I implemented it, I can't load it at the top, because tst_fifo.sh
> requires
> TST_TMPDIR to be already set, when it is included.
Right, sorry. BTW, am I missing something or you don't use this variable in
shell? I see it ony defined in tst_fifo.sh, then you use $TST_TMPDIR.
(+ defined and used in tst_fifo.c)
+ local _tst_path_req="${TST_TMPDIR}/tst_fifo_$1.req"
So what is the purpose of the variable in shell?

And maybe the test for $TST_TMPDIR might not be needed (it's not supposed to be
used without tst_test.sh, but there is no harm to have it (maybe the error
message should be something like "tst_test.sh is part of shell API and shouldn't
be loaded separately").

Kind regards,
Petr
Cyril Hrubis Dec. 10, 2019, 2:32 p.m. UTC | #4
Hi!
> This is a proposal for a fifo library that can be used to synchronize
> tests internally, e.g. between a shell script and c program or between forks.
> If the proposal is accepted, I will also add documentation.
> 
> The library allows an arbitrary number of fifos to be created. The only requirement
> is that the test needs TST_NEEDS_TMPDIR=1.
> Reading and writing to the fifo is also a synchronization point.
> 
> Each fifo requires two queues: One for sending a message to the other process
> and the second one is used for receive acknowledgement. Otherwise two messages
> can be received at the same time, if the sender sends them too fast, before the receiver
> has closed its file handle.
> 
> Signed-off-by: Joerg Vehlow <joerg.vehlow@aox-tech.de>
> ---
>  include/tst_fifo.h                      |  12 +++
>  lib/newlib_tests/.gitignore             |   2 +
>  lib/newlib_tests/shell/tst_fifo_test.sh |  56 ++++++++++++
>  lib/newlib_tests/tst_fifo.c             |  51 +++++++++++
>  lib/newlib_tests/tst_fifo_proc.c        |  42 +++++++++
>  lib/tst_fifo.c                          | 115 ++++++++++++++++++++++++
>  testcases/Makefile                      |   2 +-
>  testcases/lib/tst_fifo.sh               |  58 ++++++++++++
>  testcases/lib/tst_test.sh               |   4 +-
>  9 files changed, 340 insertions(+), 2 deletions(-)
>  create mode 100644 include/tst_fifo.h
>  create mode 100644 lib/newlib_tests/shell/tst_fifo_test.sh
>  create mode 100644 lib/newlib_tests/tst_fifo.c
>  create mode 100644 lib/newlib_tests/tst_fifo_proc.c
>  create mode 100644 lib/tst_fifo.c
>  create mode 100644 testcases/lib/tst_fifo.sh
> 
> diff --git a/include/tst_fifo.h b/include/tst_fifo.h
> new file mode 100644
> index 000000000..f5f61dee7
> --- /dev/null
> +++ b/include/tst_fifo.h
> @@ -0,0 +1,12 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2019 Joerg Vehlow <joerg.vehlow@aox-tech.de>
> + */
> +
> +void tst_fifo_init(void);
> +
> +void tst_fifo_create(const char *name);
> +void tst_fifo_destroy(const char *name);
> +
> +void tst_fifo_send(const char *name, const char *data);
> +int tst_fifo_receive(const char *name, char *data, int maxlen);
> \ No newline at end of file

You are missing guards here. I.e. the #ifdef TST_FIFO__, #define ...

> diff --git a/lib/newlib_tests/.gitignore b/lib/newlib_tests/.gitignore
> index d4aa4935f..1d887e0ab 100644
> --- a/lib/newlib_tests/.gitignore
> +++ b/lib/newlib_tests/.gitignore
> @@ -17,6 +17,8 @@ test16
>  tst_capability01
>  tst_capability02
>  tst_device
> +tst_fifo
> +tst_fifo_proc
>  tst_safe_fileops
>  tst_res_hexd
>  tst_strstatus
> diff --git a/lib/newlib_tests/shell/tst_fifo_test.sh b/lib/newlib_tests/shell/tst_fifo_test.sh
> new file mode 100644
> index 000000000..f124a5f0a
> --- /dev/null
> +++ b/lib/newlib_tests/shell/tst_fifo_test.sh
> @@ -0,0 +1,56 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +# Copyright (c) 2019 Joerg Vehlow <joerg.vehlow@aox-tech.de>
> +
> +TST_TESTFUNC=do_test
> +TST_NEEDS_FIFO=1
> +TST_NEEDS_TMPDIR=1
> +
> +. tst_test.sh
> +
> +S2P="fifo_shell_to_proc"
> +P2S="fifo_proc_to_shell"
> +
> +do_test()
> +{
> +    tst_fifo_create $S2P
> +    tst_fifo_create $P2S
> +
> +    tst_fifo_proc &
> +    pid=$!
> +
> +    tst_fifo_send $S2P "init message"
> +    tst_res TPASS "init message send"
> +
> +    tst_fifo_send $S2P "second init message"
> +    tst_res TPASS "second init message send"
> +
> +    data=$(tst_fifo_receive $P2S)
> +    if [ "$data" == "answer 1" ]; then
> +        tst_res TPASS "Received first expected answer"
> +    else
> +        tst_res TFAIL "First expected answer mismatch ('$data')"
> +    fi
> +
> +    data=$(tst_fifo_receive $P2S)
> +    if [ "$data" == "answer 2" ]; then
> +        tst_res TPASS "Received second expected answer"
> +    else
> +        tst_res TFAIL "Second expected answer mismatch ('$data')"
> +    fi
> +
> +    data=$(tst_fifo_receive $P2S)
> +    if [ "$data" == "answer 3" ]; then
> +        tst_res TPASS "Received third expected answer"
> +    else
> +        tst_res TFAIL "Third expected answer mismatch ('$data')"
> +    fi
> +
> +    tst_res TINFO "Waiting for tst_fifo_proc to terminate"
> +    wait $pid
> +
> +    tst_res TPASS "All tests passed"
> +}
> +
> +
> +tst_run
> \ No newline at end of file
> diff --git a/lib/newlib_tests/tst_fifo.c b/lib/newlib_tests/tst_fifo.c
> new file mode 100644
> index 000000000..c7d68cb08
> --- /dev/null
> +++ b/lib/newlib_tests/tst_fifo.c
> @@ -0,0 +1,51 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2019 Joerg Vehlow <joerg.vehlow@aox-tech.de>
> + */
> +
> +#include "tst_test.h"
> +#include "tst_fifo.h"
> +
> +#define MSG_P2C "AbcD"
> +#define MSG_C2P "Hello World"
> +
> +static void do_setup(void)
> +{
> +    tst_fifo_init();
> +}
> +
> +static void do_test(void)
> +{
> +    tst_fifo_create("p2c");
> +    tst_fifo_create("c2p");
> +
> +    pid_t pid = SAFE_FORK();
> +    if (pid == 0) {
> +        char data[sizeof(MSG_P2C)];
> +        if (tst_fifo_receive("p2c", data, sizeof(data)) != strlen(MSG_P2C))
> +            tst_res(TFAIL, "Child did not receive expected length");
> +        if (strcmp(data, MSG_P2C) != 0)
> +            tst_res(TFAIL, "Child did not receive expected value ('%s' != '%s')", MSG_P2C, data);
> +        else
> +            tst_res(TPASS, "Child received expected value");
> +        
> +        tst_fifo_send("c2p", MSG_C2P);
> +    } else {
> +        tst_fifo_send("p2c", MSG_P2C);
> +
> +        char data[sizeof(MSG_C2P)];
> +        if (tst_fifo_receive("c2p", data, sizeof(data)) != strlen(MSG_C2P))
> +            tst_res(TFAIL, "Parent did not receive expected length");
> +        if (strcmp(data, MSG_C2P) != 0)
> +            tst_res(TFAIL, "Parent did not receive expected value ('%s' != '%s')", MSG_C2P, data);
> +        else
> +            tst_res(TPASS, "Parent received expected value");
> +    }
> +}
> +
> +static struct tst_test test = {
> +    .needs_tmpdir = 1,
> +    .forks_child = 1,
> +    .setup = do_setup,
> +	.test_all = do_test
> +};

It's a minor thing but the whitespaces are all mangled in this source
and in the others. Have you used checkpatch.pl (which is shipped along
with kernel sources) to check for common mistakes in a patch?

> diff --git a/lib/newlib_tests/tst_fifo_proc.c b/lib/newlib_tests/tst_fifo_proc.c
> new file mode 100644
> index 000000000..bd9604741
> --- /dev/null
> +++ b/lib/newlib_tests/tst_fifo_proc.c
> @@ -0,0 +1,42 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2019 Joerg Vehlow <joerg.vehlow@aox-tech.de>
> + */
> +
> +
> +#define TST_NO_DEFAULT_MAIN
> +#include "tst_test.h"
> +#include "tst_fifo.h"
> +
> +#define S2P "fifo_shell_to_proc"
> +#define P2S "fifo_proc_to_shell"
> +
> +int main()
> +{
> +    char data[128];
> +
> +    TCID = "tst_fifo_proc";
> +
> +    tst_fifo_init();
> +
> +    tst_fifo_receive(S2P, data, sizeof(data));
> +    tst_res(strcmp(data, "init message") == 0 ? TPASS : TFAIL,
> +            "Received expected init message (%s)", data);
> +
> +    // Wait a bit for asynchronous access to pipe
> +    sleep(1);
> +
> +    tst_fifo_receive(S2P, data, sizeof(data));
> +    tst_res(strcmp(data, "second init message") == 0 ? TPASS : TFAIL,
> +            "Received expected second init message");
> +
> +    tst_fifo_send(P2S, "answer 1");
> +    sleep(1);
> +    tst_fifo_send(P2S, "answer 2");
> +    sleep(1);
> +    tst_fifo_send(P2S, "answer 3");
> +
> +    tst_res(TPASS, "All tests passed");
> +
> +    return 0;
> +}
> \ No newline at end of file
> diff --git a/lib/tst_fifo.c b/lib/tst_fifo.c
> new file mode 100644
> index 000000000..7d139624b
> --- /dev/null
> +++ b/lib/tst_fifo.c
> @@ -0,0 +1,115 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2019 Joerg Vehlow <joerg.vehlow@aox-tech.de>
> + */
> +#define _GNU_SOURCE
> +#include <stddef.h>
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <unistd.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +
> +#define TST_NO_DEFAULT_MAIN
> +#include "tst_test.h"
> +#include "old_tmpdir.h"
> +#include "tst_fifo.h"
> +
> +#ifndef PATH_MAX
> +#ifdef MAXPATHLEN
> +#define PATH_MAX	MAXPATHLEN
> +#else
> +#define PATH_MAX	1024
> +#endif
> +#endif
> +
> +#define FIFO_ENV_VAR "LTP_FIFO_PATH"
> +
> +static char *FIFO_DIR = NULL;
> +
> +#define INIT_FIFO_FUNCTION(req_mode, ack_mode) \
> +    char path_req[PATH_MAX]; \
> +    char path_ack[PATH_MAX]; \
> +    if (!FIFO_DIR) \
> +        tst_brk(TBROK, "you must call tst_fifo_init first"); \
> +    snprintf(path_req, sizeof(path_req), "%s/tst_fifo_%s.req", FIFO_DIR, name); \
> +    snprintf(path_ack, sizeof(path_ack), "%s/tst_fifo_%s.ack", FIFO_DIR, name);
> +    //if (required_mode && access(path, required_mode)) \
> +     //   tst_brk(TBROK, "cannot access '%s'", path);

If nothing else this macro is a reason to nack this patchset, it's quite
ugly as it is.

I was wondering if we can could keep the fds open, but I guess that the
code is depending on a fact that the processes are blocked and
synchronized on open() on the fifo, right?

But if that's true we do not need full blown bidirectional communication
for the memcg tests. We just need one fifo, where the parent reads from
it and the C child writes there after it finished allocation. The parent
will either get read some data or the read in the parent will return
with 0 (EOF), if the other side of the fifo has been closed (the process
killed).

> +void tst_fifo_init(void)
> +{
> +    if (tst_tmpdir_created()) {
> +        FIFO_DIR = tst_get_tmpdir();
> +        setenv(FIFO_ENV_VAR, FIFO_DIR, 1);
> +    } else {
> +        FIFO_DIR = getenv(FIFO_ENV_VAR);
> +    }

Shouldn't the env variable take precedence?

I wonder if there would be a case where two tests each with it's own
temporary directory would need to communicate. But I guess that we can
fix this at any point.

> +    if (!FIFO_DIR)
> +        tst_brk(TBROK, "no tempdir and " FIFO_ENV_VAR " not set");
> +}
> +
> +void tst_fifo_create(const char *name)
> +{
> +    INIT_FIFO_FUNCTION(0, 0);
> +
> +    if (mkfifo(path_req, S_IRWXU | S_IRWXG | S_IRWXO)) 
> +        tst_brk(TBROK, "mkfifo(%s) failed with %s", path_req, tst_strerrno(errno));
> +
> +    if (mkfifo(path_ack, S_IRWXU | S_IRWXG | S_IRWXO)) 
> +        tst_brk(TBROK, "mkfifo(%s) failed with %s", path_ack, tst_strerrno(errno));
> +}
> +
> +void tst_fifo_destroy(const char *name)
> +{
> +    INIT_FIFO_FUNCTION(R_OK | W_OK, R_OK | W_OK);
> +
> +    if (remove(path_req))
> +        tst_brk(TBROK, "unable to remove fifo '%s'", path_req);
> +    if (remove(path_ack))
> +        tst_brk(TBROK, "unable to remove fifo '%s'", path_ack);
> +}
> +
> +void tst_fifo_send(const char *name, const char *data)
> +{
> +    int fd;
> +    char ack[2];
> +    INIT_FIFO_FUNCTION(W_OK, R_OK);
> +
> +    fd = SAFE_OPEN(path_req, O_WRONLY);
> +    SAFE_WRITE(1, fd, data, strlen(data));
> +    SAFE_CLOSE(fd);
> +
> +    fd = SAFE_OPEN(path_ack, O_RDONLY);
> +    SAFE_READ(1, fd, ack, 2);
> +    SAFE_CLOSE(fd);
> +}
> +
> +int tst_fifo_receive(const char *name, char *data, int maxlen)
> +{
> +    int fd;
> +    int nbyte;
> +    int pos;
> +    INIT_FIFO_FUNCTION(R_OK, W_OK);
> +
> +    fd = SAFE_OPEN(path_req, O_RDONLY);
> +    pos = 0;
> +    while (1) {
> +        nbyte = SAFE_READ(0, fd, data + pos, maxlen - pos);
> +        if (nbyte == 0)
> +            break;
> +        pos += nbyte;
> +        if (pos == maxlen)
> +            tst_brk(TBROK, "buffer is not big enough");
> +    }
> +
> +    SAFE_CLOSE(fd);
> +
> +    fd = SAFE_OPEN(path_ack, O_WRONLY);
> +    SAFE_WRITE(1, fd, "OK", 2);
> +    SAFE_CLOSE(fd);

I'm not sure we want to implement automatic ack like this. Why can't we
just keep this as generic two-directional communication. I would also
say that code that explicitly reads acks from the pipe would be easier
to read as well, because there will be less hidden in the library.

> +    data[pos] = 0;
> +    return pos;
> +}

These functions would be better if they both included a timeout
parameter. The fifo fds would have to be opened in non-blocking
and we would have to use poll(). That way we wouldn't end up stuck if
the other side is not dead but got stuck somehow.

> diff --git a/testcases/Makefile b/testcases/Makefile
> index b04e6309f..47c110972 100644
> --- a/testcases/Makefile
> +++ b/testcases/Makefile
> @@ -28,7 +28,7 @@ include $(top_srcdir)/include/mk/env_pre.mk
>  # 1. kdump shouldn't be compiled by default, because it's runtime based and
>  #    WILL crash the build host (the tests need to be fixed to just build, not
>  #    run).
> -FILTER_OUT_DIRS		:= kdump
> +FILTER_OUT_DIRS		:= kdump open_posix_testsuite realtime kernel network misc
>  
>  ifneq ($(WITH_OPEN_POSIX_TESTSUITE),yes)
>  FILTER_OUT_DIRS		+= open_posix_testsuite
> diff --git a/testcases/lib/tst_fifo.sh b/testcases/lib/tst_fifo.sh
> new file mode 100644
> index 000000000..922b24059
> --- /dev/null
> +++ b/testcases/lib/tst_fifo.sh
> @@ -0,0 +1,58 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +# Copyright (c) 2019 Joerg Vehlow <joerg.vehlow@aox-tech.de>
> +
> +[ "$TST_NEEDS_TMPDIR" != 1 ] && tst_brk TCONF "fifo library requires TST_NEEDS_TMPDIR=1"
> +[ -z "$TST_TMPDIR" ] && tst_brk TCONF "TST_TMPDIR is not supposed to be empty"
> +
> +export LTP_FIFO_PATH="$TST_TMPDIR"
> +
> +tst_fifo_create()
> +{
> +    [ $# -ne 1 ] && tst_brk TBROK "tst_fifo_create expects 1 parameter"
> +    local _tst_path_req="${TST_TMPDIR}/tst_fifo_$1.req"
> +    local _tst_path_ack="${TST_TMPDIR}/tst_fifo_$1.ack"
> +
> +    mkfifo "$_tst_path_req" || tst_brk TBROK "unable to create fifo '$_tst_path_req'"
> +    mkfifo "$_tst_path_ack" || tst_brk TBROK "unable to create fifo '$_tst_path_ack'"
> +}
> +
> +tst_fifo_destroy()
> +{
> +    [ $# -ne 1 ] && tst_brk TBROK "tst_fifo_destroy expects 1 parameter"
> +    local _tst_path_req="${TST_TMPDIR}/tst_fifo_$1.req"
> +    local _tst_path_ack="${TST_TMPDIR}/tst_fifo_$1.ack"
> +
> +    [ -p "$_tst_path_req" ] || tst_brk TBROK "tst_fifo_destroy fifo '$_tst_path_req' does not exist"
> +    [ -p "$_tst_path_ack" ] || tst_brk TBROK "tst_fifo_destroy fifo '$_tst_path_ack' does not exist"
> +
> +    rm "$_tst_path_req"  || tst_brk TBROK "unable to destroy fifo '$_tst_path_req'"
> +    rm "$_tst_path_ack"  || tst_brk TBROK "unable to destroy fifo '$_tst_path_ack'"
> +}
> +
> +tst_fifo_send()
> +{
> +    [ $# -ne 2 ] && tst_brk TBROK "tst_fifo_send expects 2 parameters"
> +    local _tst_path_req="${TST_TMPDIR}/tst_fifo_$1.req"
> +    local _tst_path_ack="${TST_TMPDIR}/tst_fifo_$1.ack"
> +    local _tst_msg="$2"
> +
> +    [ -p "$_tst_path_req" ] || tst_brk TBROK "tst_fifo_send fifo '$_tst_path_req' does not exist"
> +    [ -p "$_tst_path_ack" ] || tst_brk TBROK "tst_fifo_send fifo '$_tst_path_ack' does not exist"
> +
> +    echo -n "$_tst_msg" > "$_tst_path_req"
> +    cat "$_tst_path_ack" > /dev/null
> +}
> +
> +tst_fifo_receive()
> +{
> +    [ $# -ne 1 ] && tst_brk TBROK "tst_fifo_receive expects 1 parameter"
> +    local _tst_path_req="${TST_TMPDIR}/tst_fifo_$1.req"
> +    local _tst_path_ack="${TST_TMPDIR}/tst_fifo_$1.ack"
> +
> +    [ -p "$_tst_path_req" ] || tst_brk TBROK "tst_fifo_receive fifo '$_tst_path_req' does not exist"
> +    [ -p "$_tst_path_ack" ] || tst_brk TBROK "tst_fifo_receive fifo '$_tst_path_ack' does not exist"
> +
> +    cat "$_tst_path_req"
> +    echo -n "OK" > "$_tst_path_ack"
> +}

Can we please create a small C helper, as we do for checkpoints, so that
we will have only one implementation of the FIFO?

> diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
> index 70c1ef2e3..2352c5eea 100644
> --- a/testcases/lib/tst_test.sh
> +++ b/testcases/lib/tst_test.sh
> @@ -479,7 +479,7 @@ tst_run()
>  			NEEDS_DRIVERS|FS_TYPE|MNTPOINT|MNT_PARAMS);;
>  			IPV6|IPVER|TEST_DATA|TEST_DATA_IFS);;
>  			RETRY_FUNC|RETRY_FN_EXP_BACKOFF|TIMEOUT);;
> -			NET_MAX_PKT);;
> +			NET_MAX_PKT|NEEDS_FIFO);;
>  			*) tst_res TWARN "Reserved variable TST_$_tst_i used!";;
>  			esac
>  		done
> @@ -537,6 +537,8 @@ tst_run()
>  		cd "$TST_TMPDIR"
>  	fi
>  
> +	[ "$TST_NEEDS_FIFO" = 1 ] && . tst_fifo.sh
> +
>  	TST_MNTPOINT="${TST_MNTPOINT:-mntpoint}"
>  	if [ "$TST_NEEDS_DEVICE" = 1 ]; then
>  		if [ -z ${TST_TMPDIR} ]; then
> -- 
> 2.20.1
>
Joerg Vehlow Dec. 11, 2019, 2:43 p.m. UTC | #5
Hi Cyril,

I was expecting some kind of objection from you (and maybe others),
that is why I send this patch before implementing
everything (e.g. timeouts and documentation).
>> +
>> +#define INIT_FIFO_FUNCTION(req_mode, ack_mode) \
>> +    char path_req[PATH_MAX]; \
>> +    char path_ack[PATH_MAX]; \
>> +    if (!FIFO_DIR) \
>> +        tst_brk(TBROK, "you must call tst_fifo_init first"); \
>> +    snprintf(path_req, sizeof(path_req), "%s/tst_fifo_%s.req", FIFO_DIR, name); \
>> +    snprintf(path_ack, sizeof(path_ack), "%s/tst_fifo_%s.ack", FIFO_DIR, name);
>> +    //if (required_mode && access(path, required_mode)) \
>> +     //   tst_brk(TBROK, "cannot access '%s'", path);
> If nothing else this macro is a reason to nack this patchset, it's quite
> ugly as it is.
You are right, I will probably turn it into a function. I don't like it 
either.
> I was wondering if we can could keep the fds open, but I guess that the
> code is depending on a fact that the processes are blocked and
> synchronized on open() on the fifo, right?
Yes that, and the fact, that keeping the handle open in shell script is 
harder.
>
> But if that's true we do not need full blown bidirectional communication
> for the memcg tests. We just need one fifo, where the parent reads from
> it and the C child writes there after it finished allocation. The parent
> will either get read some data or the read in the parent will return
> with 0 (EOF), if the other side of the fifo has been closed (the process
> killed).
At first I tired it unidirectional, but that has the disadvantage, that 
the receiver has
to be able to split messages. I explained it shortly in the commit message:

If a writer writes two messages without waiting for the receiver to 
receive it completely,
the receiver can read both messages with one read from the queue.

A sends 'foo' and blocks
B opens the fifio
A unblocks and sends 'bar'
B reads from the queue -> 'foobar'

To me the usage of two queues has two advantages:
1. No need to split messages on the receiver side
2. We have a full synchronization point, which makes reasoning a lot simpler

Regarding the memcg implementation:
I thought just replacing the current signal mechanism with new api 
should work:
The c child blocks on a pipe it reads from, the shell script sends a 
command through
this pipe. If the shell script tries to send the next command before the 
c child is ready
to process another command, the script blocks at the fifo open and waits 
for the child.
(This is where the race condition is in memcg_process).
The c child does not have to tell the shell script, that it is finished, 
the current polling
implemented works good enough. The advantage here is, that a dying child 
during
fifo communication is not a use case.

If the child can die while the shell script waits for fifo 
communication, the only
possible solution would to keep the fifo open in the child and parent, 
because
otherwise if the child dies, while the parent has not yet opened the 
pipe, it
will still block forever. -> This would require always open handle on 
client side
and prevent any kind of synchronization.

>> +void tst_fifo_init(void)
>> +{
>> +    if (tst_tmpdir_created()) {
>> +        FIFO_DIR = tst_get_tmpdir();
>> +        setenv(FIFO_ENV_VAR, FIFO_DIR, 1);
>> +    } else {
>> +        FIFO_DIR = getenv(FIFO_ENV_VAR);
>> +    }
> Shouldn't the env variable take precedence?
> I wonder if there would be a case where two tests each with it's own
> temporary directory would need to communicate. But I guess that we can
> fix this at any point

Yesno: I expect that there is only one "top-level-test". This is either 
a c test or
a shell script. If the shell script executes a c program, it is most 
likely a helper
(like tst_fifo_proc in my test case) and this helper does not use the 
main function
from tst_test and is supposed to use the temporary directory of it's 
parent and thus
it does not create it's own temporary directory.
If the main test is a c program, it either uses helpers like described 
above or it forks itself.
The fork does not need to call tst_fifo_init anyway.

So I see these two options as mutually exclusive anyway. But you may be 
right.

>> +int tst_fifo_receive(const char *name, char *data, int maxlen)
>> +{
>> +    int fd;
>> +    int nbyte;
>> +    int pos;
>> +    INIT_FIFO_FUNCTION(R_OK, W_OK);
>> +
>> +    fd = SAFE_OPEN(path_req, O_RDONLY);
>> +    pos = 0;
>> +    while (1) {
>> +        nbyte = SAFE_READ(0, fd, data + pos, maxlen - pos);
>> +        if (nbyte == 0)
>> +            break;
>> +        pos += nbyte;
>> +        if (pos == maxlen)
>> +            tst_brk(TBROK, "buffer is not big enough");
>> +    }
>> +
>> +    SAFE_CLOSE(fd);
>> +
>> +    fd = SAFE_OPEN(path_ack, O_WRONLY);
>> +    SAFE_WRITE(1, fd, "OK", 2);
>> +    SAFE_CLOSE(fd);
> I'm not sure we want to implement automatic ack like this. Why can't we
> just keep this as generic two-directional communication. I would also
> say that code that explicitly reads acks from the pipe would be easier
> to read as well, because there will be less hidden in the library.
That depends on the semantic of the functions.
As written above, I like to see them as synchronization points
>
>> +    data[pos] = 0;
>> +    return pos;
>> +}
> These functions would be better if they both included a timeout
> parameter. The fifo fds would have to be opened in non-blocking
> and we would have to use poll(). That way we wouldn't end up stuck if
> the other side is not dead but got stuck somehow.
Yes, completely right, I forgot to write that I was planing to implement 
this and just
haven't done it to discuss the basic api first.
> Can we please create a small C helper, as we do for checkpoints, so that
> we will have only one implementation of the FIFO?
Makes sense, but makes "keeping file handles open" even more impossible.

Jörg
diff mbox series

Patch

diff --git a/include/tst_fifo.h b/include/tst_fifo.h
new file mode 100644
index 000000000..f5f61dee7
--- /dev/null
+++ b/include/tst_fifo.h
@@ -0,0 +1,12 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2019 Joerg Vehlow <joerg.vehlow@aox-tech.de>
+ */
+
+void tst_fifo_init(void);
+
+void tst_fifo_create(const char *name);
+void tst_fifo_destroy(const char *name);
+
+void tst_fifo_send(const char *name, const char *data);
+int tst_fifo_receive(const char *name, char *data, int maxlen);
\ No newline at end of file
diff --git a/lib/newlib_tests/.gitignore b/lib/newlib_tests/.gitignore
index d4aa4935f..1d887e0ab 100644
--- a/lib/newlib_tests/.gitignore
+++ b/lib/newlib_tests/.gitignore
@@ -17,6 +17,8 @@  test16
 tst_capability01
 tst_capability02
 tst_device
+tst_fifo
+tst_fifo_proc
 tst_safe_fileops
 tst_res_hexd
 tst_strstatus
diff --git a/lib/newlib_tests/shell/tst_fifo_test.sh b/lib/newlib_tests/shell/tst_fifo_test.sh
new file mode 100644
index 000000000..f124a5f0a
--- /dev/null
+++ b/lib/newlib_tests/shell/tst_fifo_test.sh
@@ -0,0 +1,56 @@ 
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (c) 2019 Joerg Vehlow <joerg.vehlow@aox-tech.de>
+
+TST_TESTFUNC=do_test
+TST_NEEDS_FIFO=1
+TST_NEEDS_TMPDIR=1
+
+. tst_test.sh
+
+S2P="fifo_shell_to_proc"
+P2S="fifo_proc_to_shell"
+
+do_test()
+{
+    tst_fifo_create $S2P
+    tst_fifo_create $P2S
+
+    tst_fifo_proc &
+    pid=$!
+
+    tst_fifo_send $S2P "init message"
+    tst_res TPASS "init message send"
+
+    tst_fifo_send $S2P "second init message"
+    tst_res TPASS "second init message send"
+
+    data=$(tst_fifo_receive $P2S)
+    if [ "$data" == "answer 1" ]; then
+        tst_res TPASS "Received first expected answer"
+    else
+        tst_res TFAIL "First expected answer mismatch ('$data')"
+    fi
+
+    data=$(tst_fifo_receive $P2S)
+    if [ "$data" == "answer 2" ]; then
+        tst_res TPASS "Received second expected answer"
+    else
+        tst_res TFAIL "Second expected answer mismatch ('$data')"
+    fi
+
+    data=$(tst_fifo_receive $P2S)
+    if [ "$data" == "answer 3" ]; then
+        tst_res TPASS "Received third expected answer"
+    else
+        tst_res TFAIL "Third expected answer mismatch ('$data')"
+    fi
+
+    tst_res TINFO "Waiting for tst_fifo_proc to terminate"
+    wait $pid
+
+    tst_res TPASS "All tests passed"
+}
+
+
+tst_run
\ No newline at end of file
diff --git a/lib/newlib_tests/tst_fifo.c b/lib/newlib_tests/tst_fifo.c
new file mode 100644
index 000000000..c7d68cb08
--- /dev/null
+++ b/lib/newlib_tests/tst_fifo.c
@@ -0,0 +1,51 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2019 Joerg Vehlow <joerg.vehlow@aox-tech.de>
+ */
+
+#include "tst_test.h"
+#include "tst_fifo.h"
+
+#define MSG_P2C "AbcD"
+#define MSG_C2P "Hello World"
+
+static void do_setup(void)
+{
+    tst_fifo_init();
+}
+
+static void do_test(void)
+{
+    tst_fifo_create("p2c");
+    tst_fifo_create("c2p");
+
+    pid_t pid = SAFE_FORK();
+    if (pid == 0) {
+        char data[sizeof(MSG_P2C)];
+        if (tst_fifo_receive("p2c", data, sizeof(data)) != strlen(MSG_P2C))
+            tst_res(TFAIL, "Child did not receive expected length");
+        if (strcmp(data, MSG_P2C) != 0)
+            tst_res(TFAIL, "Child did not receive expected value ('%s' != '%s')", MSG_P2C, data);
+        else
+            tst_res(TPASS, "Child received expected value");
+        
+        tst_fifo_send("c2p", MSG_C2P);
+    } else {
+        tst_fifo_send("p2c", MSG_P2C);
+
+        char data[sizeof(MSG_C2P)];
+        if (tst_fifo_receive("c2p", data, sizeof(data)) != strlen(MSG_C2P))
+            tst_res(TFAIL, "Parent did not receive expected length");
+        if (strcmp(data, MSG_C2P) != 0)
+            tst_res(TFAIL, "Parent did not receive expected value ('%s' != '%s')", MSG_C2P, data);
+        else
+            tst_res(TPASS, "Parent received expected value");
+    }
+}
+
+static struct tst_test test = {
+    .needs_tmpdir = 1,
+    .forks_child = 1,
+    .setup = do_setup,
+	.test_all = do_test
+};
diff --git a/lib/newlib_tests/tst_fifo_proc.c b/lib/newlib_tests/tst_fifo_proc.c
new file mode 100644
index 000000000..bd9604741
--- /dev/null
+++ b/lib/newlib_tests/tst_fifo_proc.c
@@ -0,0 +1,42 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2019 Joerg Vehlow <joerg.vehlow@aox-tech.de>
+ */
+
+
+#define TST_NO_DEFAULT_MAIN
+#include "tst_test.h"
+#include "tst_fifo.h"
+
+#define S2P "fifo_shell_to_proc"
+#define P2S "fifo_proc_to_shell"
+
+int main()
+{
+    char data[128];
+
+    TCID = "tst_fifo_proc";
+
+    tst_fifo_init();
+
+    tst_fifo_receive(S2P, data, sizeof(data));
+    tst_res(strcmp(data, "init message") == 0 ? TPASS : TFAIL,
+            "Received expected init message (%s)", data);
+
+    // Wait a bit for asynchronous access to pipe
+    sleep(1);
+
+    tst_fifo_receive(S2P, data, sizeof(data));
+    tst_res(strcmp(data, "second init message") == 0 ? TPASS : TFAIL,
+            "Received expected second init message");
+
+    tst_fifo_send(P2S, "answer 1");
+    sleep(1);
+    tst_fifo_send(P2S, "answer 2");
+    sleep(1);
+    tst_fifo_send(P2S, "answer 3");
+
+    tst_res(TPASS, "All tests passed");
+
+    return 0;
+}
\ No newline at end of file
diff --git a/lib/tst_fifo.c b/lib/tst_fifo.c
new file mode 100644
index 000000000..7d139624b
--- /dev/null
+++ b/lib/tst_fifo.c
@@ -0,0 +1,115 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2019 Joerg Vehlow <joerg.vehlow@aox-tech.de>
+ */
+#define _GNU_SOURCE
+#include <stddef.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+
+#define TST_NO_DEFAULT_MAIN
+#include "tst_test.h"
+#include "old_tmpdir.h"
+#include "tst_fifo.h"
+
+#ifndef PATH_MAX
+#ifdef MAXPATHLEN
+#define PATH_MAX	MAXPATHLEN
+#else
+#define PATH_MAX	1024
+#endif
+#endif
+
+#define FIFO_ENV_VAR "LTP_FIFO_PATH"
+
+static char *FIFO_DIR = NULL;
+
+#define INIT_FIFO_FUNCTION(req_mode, ack_mode) \
+    char path_req[PATH_MAX]; \
+    char path_ack[PATH_MAX]; \
+    if (!FIFO_DIR) \
+        tst_brk(TBROK, "you must call tst_fifo_init first"); \
+    snprintf(path_req, sizeof(path_req), "%s/tst_fifo_%s.req", FIFO_DIR, name); \
+    snprintf(path_ack, sizeof(path_ack), "%s/tst_fifo_%s.ack", FIFO_DIR, name);
+    //if (required_mode && access(path, required_mode)) \
+     //   tst_brk(TBROK, "cannot access '%s'", path);
+
+void tst_fifo_init(void)
+{
+    if (tst_tmpdir_created()) {
+        FIFO_DIR = tst_get_tmpdir();
+        setenv(FIFO_ENV_VAR, FIFO_DIR, 1);
+    } else {
+        FIFO_DIR = getenv(FIFO_ENV_VAR);
+    }
+
+    if (!FIFO_DIR)
+        tst_brk(TBROK, "no tempdir and " FIFO_ENV_VAR " not set");
+}
+
+void tst_fifo_create(const char *name)
+{
+    INIT_FIFO_FUNCTION(0, 0);
+
+    if (mkfifo(path_req, S_IRWXU | S_IRWXG | S_IRWXO)) 
+        tst_brk(TBROK, "mkfifo(%s) failed with %s", path_req, tst_strerrno(errno));
+
+    if (mkfifo(path_ack, S_IRWXU | S_IRWXG | S_IRWXO)) 
+        tst_brk(TBROK, "mkfifo(%s) failed with %s", path_ack, tst_strerrno(errno));
+}
+
+void tst_fifo_destroy(const char *name)
+{
+    INIT_FIFO_FUNCTION(R_OK | W_OK, R_OK | W_OK);
+
+    if (remove(path_req))
+        tst_brk(TBROK, "unable to remove fifo '%s'", path_req);
+    if (remove(path_ack))
+        tst_brk(TBROK, "unable to remove fifo '%s'", path_ack);
+}
+
+void tst_fifo_send(const char *name, const char *data)
+{
+    int fd;
+    char ack[2];
+    INIT_FIFO_FUNCTION(W_OK, R_OK);
+
+    fd = SAFE_OPEN(path_req, O_WRONLY);
+    SAFE_WRITE(1, fd, data, strlen(data));
+    SAFE_CLOSE(fd);
+
+    fd = SAFE_OPEN(path_ack, O_RDONLY);
+    SAFE_READ(1, fd, ack, 2);
+    SAFE_CLOSE(fd);
+}
+
+int tst_fifo_receive(const char *name, char *data, int maxlen)
+{
+    int fd;
+    int nbyte;
+    int pos;
+    INIT_FIFO_FUNCTION(R_OK, W_OK);
+
+    fd = SAFE_OPEN(path_req, O_RDONLY);
+    pos = 0;
+    while (1) {
+        nbyte = SAFE_READ(0, fd, data + pos, maxlen - pos);
+        if (nbyte == 0)
+            break;
+        pos += nbyte;
+        if (pos == maxlen)
+            tst_brk(TBROK, "buffer is not big enough");
+    }
+
+    SAFE_CLOSE(fd);
+
+    fd = SAFE_OPEN(path_ack, O_WRONLY);
+    SAFE_WRITE(1, fd, "OK", 2);
+    SAFE_CLOSE(fd);
+
+    data[pos] = 0;
+    return pos;
+}
diff --git a/testcases/Makefile b/testcases/Makefile
index b04e6309f..47c110972 100644
--- a/testcases/Makefile
+++ b/testcases/Makefile
@@ -28,7 +28,7 @@  include $(top_srcdir)/include/mk/env_pre.mk
 # 1. kdump shouldn't be compiled by default, because it's runtime based and
 #    WILL crash the build host (the tests need to be fixed to just build, not
 #    run).
-FILTER_OUT_DIRS		:= kdump
+FILTER_OUT_DIRS		:= kdump open_posix_testsuite realtime kernel network misc
 
 ifneq ($(WITH_OPEN_POSIX_TESTSUITE),yes)
 FILTER_OUT_DIRS		+= open_posix_testsuite
diff --git a/testcases/lib/tst_fifo.sh b/testcases/lib/tst_fifo.sh
new file mode 100644
index 000000000..922b24059
--- /dev/null
+++ b/testcases/lib/tst_fifo.sh
@@ -0,0 +1,58 @@ 
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (c) 2019 Joerg Vehlow <joerg.vehlow@aox-tech.de>
+
+[ "$TST_NEEDS_TMPDIR" != 1 ] && tst_brk TCONF "fifo library requires TST_NEEDS_TMPDIR=1"
+[ -z "$TST_TMPDIR" ] && tst_brk TCONF "TST_TMPDIR is not supposed to be empty"
+
+export LTP_FIFO_PATH="$TST_TMPDIR"
+
+tst_fifo_create()
+{
+    [ $# -ne 1 ] && tst_brk TBROK "tst_fifo_create expects 1 parameter"
+    local _tst_path_req="${TST_TMPDIR}/tst_fifo_$1.req"
+    local _tst_path_ack="${TST_TMPDIR}/tst_fifo_$1.ack"
+
+    mkfifo "$_tst_path_req" || tst_brk TBROK "unable to create fifo '$_tst_path_req'"
+    mkfifo "$_tst_path_ack" || tst_brk TBROK "unable to create fifo '$_tst_path_ack'"
+}
+
+tst_fifo_destroy()
+{
+    [ $# -ne 1 ] && tst_brk TBROK "tst_fifo_destroy expects 1 parameter"
+    local _tst_path_req="${TST_TMPDIR}/tst_fifo_$1.req"
+    local _tst_path_ack="${TST_TMPDIR}/tst_fifo_$1.ack"
+
+    [ -p "$_tst_path_req" ] || tst_brk TBROK "tst_fifo_destroy fifo '$_tst_path_req' does not exist"
+    [ -p "$_tst_path_ack" ] || tst_brk TBROK "tst_fifo_destroy fifo '$_tst_path_ack' does not exist"
+
+    rm "$_tst_path_req"  || tst_brk TBROK "unable to destroy fifo '$_tst_path_req'"
+    rm "$_tst_path_ack"  || tst_brk TBROK "unable to destroy fifo '$_tst_path_ack'"
+}
+
+tst_fifo_send()
+{
+    [ $# -ne 2 ] && tst_brk TBROK "tst_fifo_send expects 2 parameters"
+    local _tst_path_req="${TST_TMPDIR}/tst_fifo_$1.req"
+    local _tst_path_ack="${TST_TMPDIR}/tst_fifo_$1.ack"
+    local _tst_msg="$2"
+
+    [ -p "$_tst_path_req" ] || tst_brk TBROK "tst_fifo_send fifo '$_tst_path_req' does not exist"
+    [ -p "$_tst_path_ack" ] || tst_brk TBROK "tst_fifo_send fifo '$_tst_path_ack' does not exist"
+
+    echo -n "$_tst_msg" > "$_tst_path_req"
+    cat "$_tst_path_ack" > /dev/null
+}
+
+tst_fifo_receive()
+{
+    [ $# -ne 1 ] && tst_brk TBROK "tst_fifo_receive expects 1 parameter"
+    local _tst_path_req="${TST_TMPDIR}/tst_fifo_$1.req"
+    local _tst_path_ack="${TST_TMPDIR}/tst_fifo_$1.ack"
+
+    [ -p "$_tst_path_req" ] || tst_brk TBROK "tst_fifo_receive fifo '$_tst_path_req' does not exist"
+    [ -p "$_tst_path_ack" ] || tst_brk TBROK "tst_fifo_receive fifo '$_tst_path_ack' does not exist"
+
+    cat "$_tst_path_req"
+    echo -n "OK" > "$_tst_path_ack"
+}
diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
index 70c1ef2e3..2352c5eea 100644
--- a/testcases/lib/tst_test.sh
+++ b/testcases/lib/tst_test.sh
@@ -479,7 +479,7 @@  tst_run()
 			NEEDS_DRIVERS|FS_TYPE|MNTPOINT|MNT_PARAMS);;
 			IPV6|IPVER|TEST_DATA|TEST_DATA_IFS);;
 			RETRY_FUNC|RETRY_FN_EXP_BACKOFF|TIMEOUT);;
-			NET_MAX_PKT);;
+			NET_MAX_PKT|NEEDS_FIFO);;
 			*) tst_res TWARN "Reserved variable TST_$_tst_i used!";;
 			esac
 		done
@@ -537,6 +537,8 @@  tst_run()
 		cd "$TST_TMPDIR"
 	fi
 
+	[ "$TST_NEEDS_FIFO" = 1 ] && . tst_fifo.sh
+
 	TST_MNTPOINT="${TST_MNTPOINT:-mntpoint}"
 	if [ "$TST_NEEDS_DEVICE" = 1 ]; then
 		if [ -z ${TST_TMPDIR} ]; then