[LTP] [PATCH 1/3] network/tcp_cc: check sch_netem driver in the lib

[LTP] [PATCH 1/3] network/tcp_cc: check sch_netem driver in the lib

[Alexey Kodanev]

bbr01, bbr02 and dctcp01 are using netem.

Signed-off-by: Alexey Kodanev <aleksei.kodanev@bell-sw.com>
---
 testcases/network/tcp_cc/tcp_cc_lib.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/testcases/network/tcp_cc/tcp_cc_lib.sh b/testcases/network/tcp_cc/tcp_cc_lib.sh
index dff8cef19..1a6845664 100755
--- a/testcases/network/tcp_cc/tcp_cc_lib.sh
+++ b/testcases/network/tcp_cc/tcp_cc_lib.sh
@@ -6,6 +6,7 @@
 TST_NEEDS_TMPDIR=1
 TST_NEEDS_ROOT=1
 TST_NEEDS_CMDS="sysctl tc"
+TST_NEEDS_DRIVERS="sch_netem"
 
 . tst_net.sh
 
-- 
2.25.1

[LTP] [PATCH 2/3] network/tst_net.sh: fix busybox/sysctl in tst_set_sysctl()

[Alexey Kodanev]

busybox/sysctl expects -e option to be set before 'name=value'.
This can easily be fixed by splitting the string in tst_set_sysctl(),
so that 'rparam' with '-e' option is added in between.

Signed-off-by: Alexey Kodanev <aleksei.kodanev@bell-sw.com>
---
 testcases/lib/tst_net.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/testcases/lib/tst_net.sh b/testcases/lib/tst_net.sh
index 511fb7eb1..bb30c13ec 100644
--- a/testcases/lib/tst_net.sh
+++ b/testcases/lib/tst_net.sh
@@ -918,7 +918,7 @@ tst_set_sysctl()
 	local rparam=
 	[ "$TST_USE_NETNS" = "yes" ] && rparam="-r '-e'"
 
-	tst_net_run $safe $rparam "sysctl -q -w $name=$value"
+	tst_net_run $safe $rparam "sysctl -q -w" "$name=$value"
 }
 
 tst_cleanup_rhost()
-- 
2.25.1

[LTP] [PATCH 3/3] network/tst_net.sh: extend the pattern in tst_ping_opt_unsupported()

[Alexey Kodanev]

Signed-off-by: Alexey Kodanev <aleksei.kodanev@bell-sw.com>
---
 testcases/lib/tst_net.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/testcases/lib/tst_net.sh b/testcases/lib/tst_net.sh
index bb30c13ec..c15413a6d 100644
--- a/testcases/lib/tst_net.sh
+++ b/testcases/lib/tst_net.sh
@@ -811,7 +811,7 @@ tst_netload_compare()
 
 tst_ping_opt_unsupported()
 {
-	ping $@ 2>&1 | grep -q "invalid option"
+	ping $@ 2>&1 | grep -qE "(invalid|unrecognized) option"
 }
 
 # tst_ping -c COUNT -s MESSAGE_SIZES -p PATTERN -I IFACE -H HOST
-- 
2.25.1

[LTP] [PATCH 1/3] network/tcp_cc: check sch_netem driver in the lib

[Petr Vorel]

Hi Alexey,

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

Kind regards,
Petr

[LTP] [PATCH 2/3] network/tst_net.sh: fix busybox/sysctl in tst_set_sysctl()

[Petr Vorel]

Hi Alexey,

> busybox/sysctl expects -e option to be set before 'name=value'.
> This can easily be fixed by splitting the string in tst_set_sysctl(),
> so that 'rparam' with '-e' option is added in between.
Good catch.
Reviewed-by: Petr Vorel <pvorel@suse.cz>

...
> diff --git a/testcases/lib/tst_net.sh b/testcases/lib/tst_net.sh
> index 511fb7eb1..bb30c13ec 100644
> --- a/testcases/lib/tst_net.sh
> +++ b/testcases/lib/tst_net.sh
> @@ -918,7 +918,7 @@ tst_set_sysctl()
>  	local rparam=
>  	[ "$TST_USE_NETNS" = "yes" ] && rparam="-r '-e'"

> -	tst_net_run $safe $rparam "sysctl -q -w $name=$value"
> +	tst_net_run $safe $rparam "sysctl -q -w" "$name=$value"

I wonder if it were a bit clearer if we moves all params to second arg:
	tst_net_run $safe "sysctl" "$name=$value -q -w"

Kind regards,
Petr

[LTP] [PATCH 3/3] network/tst_net.sh: extend the pattern in tst_ping_opt_unsupported()

[Petr Vorel]

Hi Alexey,

> -	ping $@ 2>&1 | grep -q "invalid option"
> +	ping $@ 2>&1 | grep -qE "(invalid|unrecognized) option"

Which implementation needs it? Both ping from iputils and busybox report
"invalid option".

Kind regards,
Petr

[LTP] [PATCH 3/3] network/tst_net.sh: extend the pattern in tst_ping_opt_unsupported()

[Alexey Kodanev]

Hi Petr,
On 20.08.2021 13:46, Petr Vorel wrote:
> Hi Alexey,
> 
>> -	ping $@ 2>&1 | grep -q "invalid option"
>> +	ping $@ 2>&1 | grep -qE "(invalid|unrecognized) option"
> 
> Which implementation needs it? Both ping from iputils and busybox report
> "invalid option".
> 

It's busybox/ping that is using musl libc getopt():
misc/getopt.c:			__getopt_msg(argv[0], ": unrecognized option: ", optchar, k);

[LTP] [PATCH 3/3] network/tst_net.sh: extend the pattern in tst_ping_opt_unsupported()

[Petr Vorel]

Hi Alexey,

> Hi Petr,
> On 20.08.2021 13:46, Petr Vorel wrote:
> > Hi Alexey,

> >> -	ping $@ 2>&1 | grep -q "invalid option"
> >> +	ping $@ 2>&1 | grep -qE "(invalid|unrecognized) option"

> > Which implementation needs it? Both ping from iputils and busybox report
> > "invalid option".


> It's busybox/ping that is using musl libc getopt():
> misc/getopt.c:			__getopt_msg(argv[0], ": unrecognized option: ", optchar, k);

Ah, thx for info.
Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

[LTP] [PATCH 2/3] network/tst_net.sh: fix busybox/sysctl in tst_set_sysctl()

[Alexey Kodanev]

On 20.08.2021 13:43, Petr Vorel wrote:
> Hi Alexey,
> 
>> busybox/sysctl expects -e option to be set before 'name=value'.
>> This can easily be fixed by splitting the string in tst_set_sysctl(),
>> so that 'rparam' with '-e' option is added in between.
> Good catch.
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
> 
> ...
>> diff --git a/testcases/lib/tst_net.sh b/testcases/lib/tst_net.sh
>> index 511fb7eb1..bb30c13ec 100644
>> --- a/testcases/lib/tst_net.sh
>> +++ b/testcases/lib/tst_net.sh
>> @@ -918,7 +918,7 @@ tst_set_sysctl()
>>  	local rparam=
>>  	[ "$TST_USE_NETNS" = "yes" ] && rparam="-r '-e'"
> 
>> -	tst_net_run $safe $rparam "sysctl -q -w $name=$value"
>> +	tst_net_run $safe $rparam "sysctl -q -w" "$name=$value"
> 
> I wonder if it were a bit clearer if we moves all params to second arg:
> 	tst_net_run $safe "sysctl" "$name=$value -q -w"

OK, but changed to "sysctl" "-q -w $name=$value" because the other
options should also be placed before the key/value pair:

    Usage: sysctl -p [-enq] [FILE...] / [-enqaw] [KEY[=VALUE]]...


Thanks for review Petr! Applied the patches.

[LTP] [PATCH 2/3] network/tst_net.sh: fix busybox/sysctl in tst_set_sysctl()

[Petr Vorel]

Hi Alexey,

..
> >> -	tst_net_run $safe $rparam "sysctl -q -w $name=$value"
> >> +	tst_net_run $safe $rparam "sysctl -q -w" "$name=$value"

> > I wonder if it were a bit clearer if we moves all params to second arg:
> > 	tst_net_run $safe "sysctl" "$name=$value -q -w"

> OK, but changed to "sysctl" "-q -w $name=$value" because the other
> options should also be placed before the key/value pair:

>     Usage: sysctl -p [-enq] [FILE...] / [-enqaw] [KEY[=VALUE]]...
Thanks for addressing it!

Kind regards,
Petr