diff mbox series

[v1] lib/tst_test.sh: skip test if ip returns "Error: Unknown device type"

Message ID 20210722063422.18059-1-radoslav.kolev@suse.com
State Changes Requested
Headers show
Series [v1] lib/tst_test.sh: skip test if ip returns "Error: Unknown device type" | expand

Commit Message

Radoslav Kolev July 22, 2021, 6:34 a.m. UTC
In network stress test groups there are tests expecting
CONFIG_NET_IPVTI to be enabled in the kernel, and if it's not they
fail. There is a check for VTI support in the ip utility, but not
for the kernel. Skip these tests if vti device type is not known by
the kernel.

Signed-off-by: Radoslav Kolev <radoslav.kolev@suse.com>
---
 testcases/lib/tst_test.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Petr Vorel July 22, 2021, 7:49 a.m. UTC | #1
Hi Radoslav,

> In network stress test groups there are tests expecting
> CONFIG_NET_IPVTI to be enabled in the kernel, and if it's not they
> fail. There is a check for VTI support in the ip utility, but not
> for the kernel. Skip these tests if vti device type is not known by
> the kernel.

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

Kind regards,
Petr

> Signed-off-by: Radoslav Kolev <radoslav.kolev@suse.com>
> ---
>  testcases/lib/tst_test.sh | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

> diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
> index c6aa2c487..0458c90c2 100644
> --- a/testcases/lib/tst_test.sh
> +++ b/testcases/lib/tst_test.sh
> @@ -241,12 +241,13 @@ TST_RTNL_CHK()
>  	local msg1="RTNETLINK answers: Function not implemented"
>  	local msg2="RTNETLINK answers: Operation not supported"
>  	local msg3="RTNETLINK answers: Protocol not supported"
> +	local msg4="Error: Unknown device type"
>  	local output="$($@ 2>&1 || echo 'LTP_ERR')"
>  	local msg

>  	echo "$output" | grep -q "LTP_ERR" || return 0

> -	for msg in "$msg1" "$msg2" "$msg3"; do
> +	for msg in "$msg1" "$msg2" "$msg3" "$msg4"; do
>  		echo "$output" | grep -q "$msg" && tst_brk TCONF "'$@': $msg"
>  	done
Radoslav Kolev July 27, 2021, 8:20 a.m. UTC | #2
On 7/22/21 10:49 AM, Petr Vorel wrote:
> Hi Radoslav,
>
>> In network stress test groups there are tests expecting
>> CONFIG_NET_IPVTI to be enabled in the kernel, and if it's not they
>> fail. There is a check for VTI support in the ip utility, but not
>> for the kernel. Skip these tests if vti device type is not known by
>> the kernel.
> LGTM.
> Reviewed-by: Petr Vorel <pvorel@suse.cz>

Thanks for the review, Petr!

Alexey, please let me know if you have any comments.


Regards,

Radoslav
Alexey Kodanev July 27, 2021, 8:44 a.m. UTC | #3
Hi Radoslav,
On 27.07.2021 11:20, Radoslav Kolev wrote:
> On 7/22/21 10:49 AM, Petr Vorel wrote:
>> Hi Radoslav,
>>
>>> In network stress test groups there are tests expecting
>>> CONFIG_NET_IPVTI to be enabled in the kernel, and if it's not they
>>> fail. There is a check for VTI support in the ip utility, but not
>>> for the kernel. Skip these tests if vti device type is not known by
>>> the kernel.
>> LGTM.
>> Reviewed-by: Petr Vorel <pvorel@suse.cz>
> 
> Thanks for the review, Petr!
> 
> Alexey, please let me know if you have any comments.
> 
> 

What about checking vti drivers in stress/ipsec/ipsec_lib.sh:tst_ipsec_setup_vti()
Similar to the checks for xfrm_user driver there...

For example:

tst_net_run -q "tst_check_drivers ip_vti ip6_vti" || \
    tst_brk TCONF "vti driver not available on lhost or rhost"


I think this should work for wireguard02 test as well.
Petr Vorel July 27, 2021, 10:02 a.m. UTC | #4
Hi Alexey, Radoslav,

> Hi Radoslav,

> On 27.07.2021 11:20, Radoslav Kolev wrote:
> > On 7/22/21 10:49 AM, Petr Vorel wrote:
> >> Hi Radoslav,

> >>> In network stress test groups there are tests expecting
> >>> CONFIG_NET_IPVTI to be enabled in the kernel, and if it's not they
> >>> fail. There is a check for VTI support in the ip utility, but not
> >>> for the kernel. Skip these tests if vti device type is not known by
> >>> the kernel.
> >> LGTM.
> >> Reviewed-by: Petr Vorel <pvorel@suse.cz>

> > Thanks for the review, Petr!

> > Alexey, please let me know if you have any comments.



> What about checking vti drivers in stress/ipsec/ipsec_lib.sh:tst_ipsec_setup_vti()
> Similar to the checks for xfrm_user driver there...

> For example:

> tst_net_run -q "tst_check_drivers ip_vti ip6_vti" || \
>     tst_brk TCONF "vti driver not available on lhost or rhost"


> I think this should work for wireguard02 test as well.

The above LGTM, Radoslav, do you have time to look into it?
Alexey, do we also accept this patch? IMHO this error should be mostly TCONF and
it'd work for other possible drivers.

Kind regards,
Petr
Alexey Kodanev July 28, 2021, 11:38 a.m. UTC | #5
On 27.07.2021 13:02, Petr Vorel wrote:
> Hi Alexey, Radoslav,
> 
>> Hi Radoslav,
> 
>> On 27.07.2021 11:20, Radoslav Kolev wrote:
>>> On 7/22/21 10:49 AM, Petr Vorel wrote:
>>>> Hi Radoslav,
> 
>>>>> In network stress test groups there are tests expecting
>>>>> CONFIG_NET_IPVTI to be enabled in the kernel, and if it's not they
>>>>> fail. There is a check for VTI support in the ip utility, but not
>>>>> for the kernel. Skip these tests if vti device type is not known by
>>>>> the kernel.
>>>> LGTM.
>>>> Reviewed-by: Petr Vorel <pvorel@suse.cz>
> 
>>> Thanks for the review, Petr!
> 
>>> Alexey, please let me know if you have any comments.
> 
> 
> 
>> What about checking vti drivers in stress/ipsec/ipsec_lib.sh:tst_ipsec_setup_vti()
>> Similar to the checks for xfrm_user driver there...
> 
>> For example:
> 
>> tst_net_run -q "tst_check_drivers ip_vti ip6_vti" || \
>>     tst_brk TCONF "vti driver not available on lhost or rhost"
> 
> 
>> I think this should work for wireguard02 test as well.
> 
> The above LGTM, Radoslav, do you have time to look into it?
> Alexey, do we also accept this patch? IMHO this error should be mostly TCONF and
> it'd work for other possible drivers.
> 

Not sure if we really want to add the new patterns every time the
error message from ip changes. For example depending on the ip/libc
the error can be "Error: Unknown device type." or "RTNETLINK answers:
Not supported".

We could also save the error message in setup by passing the wrong
type and then compare it during the test:

no_dev_msg="$(ip link add ltp0 type ltp0 2>&1)"
Petr Vorel July 28, 2021, 1:06 p.m. UTC | #6
> On 27.07.2021 13:02, Petr Vorel wrote:
> > Hi Alexey, Radoslav,

> >> Hi Radoslav,

> >> On 27.07.2021 11:20, Radoslav Kolev wrote:
> >>> On 7/22/21 10:49 AM, Petr Vorel wrote:
> >>>> Hi Radoslav,

> >>>>> In network stress test groups there are tests expecting
> >>>>> CONFIG_NET_IPVTI to be enabled in the kernel, and if it's not they
> >>>>> fail. There is a check for VTI support in the ip utility, but not
> >>>>> for the kernel. Skip these tests if vti device type is not known by
> >>>>> the kernel.
> >>>> LGTM.
> >>>> Reviewed-by: Petr Vorel <pvorel@suse.cz>

> >>> Thanks for the review, Petr!

> >>> Alexey, please let me know if you have any comments.



> >> What about checking vti drivers in stress/ipsec/ipsec_lib.sh:tst_ipsec_setup_vti()
> >> Similar to the checks for xfrm_user driver there...

> >> For example:

> >> tst_net_run -q "tst_check_drivers ip_vti ip6_vti" || \
> >>     tst_brk TCONF "vti driver not available on lhost or rhost"


> >> I think this should work for wireguard02 test as well.

> > The above LGTM, Radoslav, do you have time to look into it?
> > Alexey, do we also accept this patch? IMHO this error should be mostly TCONF and
> > it'd work for other possible drivers.


> Not sure if we really want to add the new patterns every time the
> error message from ip changes. For example depending on the ip/libc
> the error can be "Error: Unknown device type." or "RTNETLINK answers:
> Not supported".
Sure, more effective ways are always welcome.

> We could also save the error message in setup by passing the wrong
> type and then compare it during the test:

> no_dev_msg="$(ip link add ltp0 type ltp0 2>&1)"
Yep, that'd be safer. But your original proposal to check ip_vti ip6_vti
is IMHO the best solution. Radoslav, would you send a new patch with it?

Kind regards,
Petr
Radoslav Kolev July 28, 2021, 2 p.m. UTC | #7
On 7/28/21 4:06 PM, Petr Vorel wrote:
> Yep, that'd be safer. But your original proposal to check ip_vti ip6_vti
> is IMHO the best solution. Radoslav, would you send a new patch with it?

Agreed that the proposed by Alexey is a better approach. Yes, patch is 
coming.
diff mbox series

Patch

diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
index c6aa2c487..0458c90c2 100644
--- a/testcases/lib/tst_test.sh
+++ b/testcases/lib/tst_test.sh
@@ -241,12 +241,13 @@  TST_RTNL_CHK()
 	local msg1="RTNETLINK answers: Function not implemented"
 	local msg2="RTNETLINK answers: Operation not supported"
 	local msg3="RTNETLINK answers: Protocol not supported"
+	local msg4="Error: Unknown device type"
 	local output="$($@ 2>&1 || echo 'LTP_ERR')"
 	local msg
 
 	echo "$output" | grep -q "LTP_ERR" || return 0
 
-	for msg in "$msg1" "$msg2" "$msg3"; do
+	for msg in "$msg1" "$msg2" "$msg3" "$msg4"; do
 		echo "$output" | grep -q "$msg" && tst_brk TCONF "'$@': $msg"
 	done