2017-09-07 08:19:12

by Naresh Kamboju

[permalink] [raw]
Subject: [PATCH v2 1/2] selftests: bpf: test_kmod.sh: check if module is present in the path before insert

From: Naresh Kamboju <[email protected]>

The test script works when kernel source and build module test_bpf.ko
present on the machine. This patch will check if module is present in
the path.

Signed-off-by: Naresh Kamboju <[email protected]>
---
tools/testing/selftests/bpf/test_kmod.sh | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_kmod.sh b/tools/testing/selftests/bpf/test_kmod.sh
index 6d58cca8e235..a53eb1cb54ef 100755
--- a/tools/testing/selftests/bpf/test_kmod.sh
+++ b/tools/testing/selftests/bpf/test_kmod.sh
@@ -9,9 +9,11 @@ test_run()

echo "[ JIT enabled:$1 hardened:$2 ]"
dmesg -C
- insmod $SRC_TREE/lib/test_bpf.ko 2> /dev/null
- if [ $? -ne 0 ]; then
- rc=1
+ if [ -f $SRC_TREE/lib/test_bpf.ko ]; then
+ insmod $SRC_TREE/lib/test_bpf.ko 2> /dev/null
+ if [ $? -ne 0 ]; then
+ rc=1
+ fi
fi
rmmod test_bpf 2> /dev/null
dmesg | grep FAIL
--
2.13.0


2017-09-07 08:19:19

by Naresh Kamboju

[permalink] [raw]
Subject: [PATCH v2 2/2] selftests: bpf: test_kmod.sh: use modprobe on target device

From: Naresh Kamboju <[email protected]>

on ARM and ARM64 devices kernel source tree is not available so
insmod "$SRC_TREE/lib/test_bpf.ko" is not working.

on these target devices the test_bpf.ko is installed under
/lib/modules/`uname -r`/kernel/lib/
so use modprobe dry run to check for missing test_bpf.ko module and
insert for testing.

Signed-off-by: Naresh Kamboju <[email protected]>
---
tools/testing/selftests/bpf/test_kmod.sh | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/tools/testing/selftests/bpf/test_kmod.sh b/tools/testing/selftests/bpf/test_kmod.sh
index a53eb1cb54ef..eab9a970d742 100755
--- a/tools/testing/selftests/bpf/test_kmod.sh
+++ b/tools/testing/selftests/bpf/test_kmod.sh
@@ -14,6 +14,16 @@ test_run()
if [ $? -ne 0 ]; then
rc=1
fi
+ else
+ # Use modprobe dry run to check for missing test_bpf module
+ if ! /sbin/modprobe -q -n test_bpf; then
+ echo "test_bpf: [SKIP]"
+ elif /sbin/modprobe -q test_bpf; then
+ echo "test_bpf: ok"
+ else
+ echo "test_bpf: [FAIL]"
+ rc=1
+ fi
fi
rmmod test_bpf 2> /dev/null
dmesg | grep FAIL
--
2.13.0

2017-09-07 09:17:03

by Daniel Borkmann

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] selftests: bpf: test_kmod.sh: check if module is present in the path before insert

On 09/07/2017 10:19 AM, [email protected] wrote:
> From: Naresh Kamboju <[email protected]>
>
> The test script works when kernel source and build module test_bpf.ko
> present on the machine. This patch will check if module is present in
> the path.
>
> Signed-off-by: Naresh Kamboju <[email protected]>

Looks good, what changed between v1 and v2? Didn't get the cover
letter in case there was one. ;)

Which tree are you targeting? There are usually a lot of changes
in BPF selftests going the usual route via net and net-next tree
as we often require to put test cases along the BPF patches. Given
the merge window now and given one can regard it as a fix, it's
net tree. I'm also ok if Shuah wants to pick it up this window as
test_kmod.sh hasn't been changed in quite a while, so no merge
conflicts expected.

Anyway, for the patch:

Acked-by: Daniel Borkmann <[email protected]>

Thanks!

> ---
> tools/testing/selftests/bpf/test_kmod.sh | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/test_kmod.sh b/tools/testing/selftests/bpf/test_kmod.sh
> index 6d58cca8e235..a53eb1cb54ef 100755
> --- a/tools/testing/selftests/bpf/test_kmod.sh
> +++ b/tools/testing/selftests/bpf/test_kmod.sh
> @@ -9,9 +9,11 @@ test_run()
>
> echo "[ JIT enabled:$1 hardened:$2 ]"
> dmesg -C
> - insmod $SRC_TREE/lib/test_bpf.ko 2> /dev/null
> - if [ $? -ne 0 ]; then
> - rc=1
> + if [ -f $SRC_TREE/lib/test_bpf.ko ]; then
> + insmod $SRC_TREE/lib/test_bpf.ko 2> /dev/null
> + if [ $? -ne 0 ]; then
> + rc=1
> + fi
> fi
> rmmod test_bpf 2> /dev/null
> dmesg | grep FAIL
>

2017-09-07 09:20:24

by Daniel Borkmann

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] selftests: bpf: test_kmod.sh: use modprobe on target device

On 09/07/2017 10:19 AM, [email protected] wrote:
> From: Naresh Kamboju <[email protected]>
>
> on ARM and ARM64 devices kernel source tree is not available so
> insmod "$SRC_TREE/lib/test_bpf.ko" is not working.
>
> on these target devices the test_bpf.ko is installed under
> /lib/modules/`uname -r`/kernel/lib/
> so use modprobe dry run to check for missing test_bpf.ko module and
> insert for testing.
>
> Signed-off-by: Naresh Kamboju <[email protected]>

Acked-by: Daniel Borkmann <[email protected]>

One really small nit that could probably be fixed up along the
way when applying:

> ---
> tools/testing/selftests/bpf/test_kmod.sh | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/test_kmod.sh b/tools/testing/selftests/bpf/test_kmod.sh
> index a53eb1cb54ef..eab9a970d742 100755
> --- a/tools/testing/selftests/bpf/test_kmod.sh
> +++ b/tools/testing/selftests/bpf/test_kmod.sh
> @@ -14,6 +14,16 @@ test_run()
> if [ $? -ne 0 ]; then
> rc=1
> fi
> + else

Looks like a whitespace slipped in right before the "else",
so should be removed to only habe the tab indent.

> + # Use modprobe dry run to check for missing test_bpf module
> + if ! /sbin/modprobe -q -n test_bpf; then
> + echo "test_bpf: [SKIP]"
> + elif /sbin/modprobe -q test_bpf; then
> + echo "test_bpf: ok"
> + else
> + echo "test_bpf: [FAIL]"
> + rc=1
> + fi
> fi
> rmmod test_bpf 2> /dev/null
> dmesg | grep FAIL
>

2017-11-06 15:17:39

by Naresh Kamboju

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] selftests: bpf: test_kmod.sh: use modprobe on target device

On 7 September 2017 at 14:50, Daniel Borkmann <[email protected]> wrote:
> On 09/07/2017 10:19 AM, [email protected] wrote:
>>
>> From: Naresh Kamboju <[email protected]>
>>
>> on ARM and ARM64 devices kernel source tree is not available so
>> insmod "$SRC_TREE/lib/test_bpf.ko" is not working.
>>
>> on these target devices the test_bpf.ko is installed under
>> /lib/modules/`uname -r`/kernel/lib/
>> so use modprobe dry run to check for missing test_bpf.ko module and
>> insert for testing.
>>
>> Signed-off-by: Naresh Kamboju <[email protected]>
>
>
> Acked-by: Daniel Borkmann <[email protected]>
>
> One really small nit that could probably be fixed up along the
> way when applying:
>

Sorry for that.

>> ---
>> tools/testing/selftests/bpf/test_kmod.sh | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>> diff --git a/tools/testing/selftests/bpf/test_kmod.sh
>> b/tools/testing/selftests/bpf/test_kmod.sh
>> index a53eb1cb54ef..eab9a970d742 100755
>> --- a/tools/testing/selftests/bpf/test_kmod.sh
>> +++ b/tools/testing/selftests/bpf/test_kmod.sh
>> @@ -14,6 +14,16 @@ test_run()
>> if [ $? -ne 0 ]; then
>> rc=1
>> fi
>> + else
>
>
> Looks like a whitespace slipped in right before the "else",
> so should be removed to only habe the tab indent.

Would it be a good idea to send v3 patch with right indent ?

- Naresh

>
>
>> + # Use modprobe dry run to check for missing test_bpf
>> module
>> + if ! /sbin/modprobe -q -n test_bpf; then
>> + echo "test_bpf: [SKIP]"
>> + elif /sbin/modprobe -q test_bpf; then
>> + echo "test_bpf: ok"
>> + else
>> + echo "test_bpf: [FAIL]"
>> + rc=1
>> + fi
>> fi
>> rmmod test_bpf 2> /dev/null
>> dmesg | grep FAIL
>>
>

From 1577872065601882047@xxx Thu Sep 07 09:21:03 +0000 2017
X-GM-THRID: 1577868221729956934
X-Gmail-Labels: Inbox,Category Forums

2017-11-06 17:09:47

by Shuah Khan

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] selftests: bpf: test_kmod.sh: check if module is present in the path before insert

On 11/06/2017 08:23 AM, Naresh Kamboju wrote:
> On 7 September 2017 at 14:46, Daniel Borkmann <[email protected]> wrote:
>> On 09/07/2017 10:19 AM, [email protected] wrote:
>>>
>>> From: Naresh Kamboju <[email protected]>
>>>
>>> The test script works when kernel source and build module test_bpf.ko
>>> present on the machine. This patch will check if module is present in
>>> the path.
>>>
>>> Signed-off-by: Naresh Kamboju <[email protected]>
>>
>>
>> Looks good, what changed between v1 and v2? Didn't get the cover
>> letter in case there was one. ;)
>
> No change for 1/2 to v2 1/2. The only change was to v2 2/2.
>
>>
>> Which tree are you targeting? There are usually a lot of changes
>> in BPF selftests going the usual route via net and net-next tree
>> as we often require to put test cases along the BPF patches. Given
>> the merge window now and given one can regard it as a fix, it's
>> net tree. I'm also ok if Shuah wants to pick it up this window as
>> test_kmod.sh hasn't been changed in quite a while, so no merge
>> conflicts expected.
>
> I did not see any conflict for this file (selftests/bpf/test_kmod.sh)
>
>>
>> Anyway, for the patch:
>>
>> Acked-by: Daniel Borkmann <[email protected]>
>>
>> Thanks!
>
> Thank you.
>
> - Naresh

Hi Naresh,

Please rebase and resend both patches with Daniel's Acks.

thanks,
-- Shuah

From 1583330729751332763@xxx Mon Nov 06 15:24:11 +0000 2017
X-GM-THRID: 1577868220909329273
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread

2017-11-06 15:24:11

by Naresh Kamboju

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] selftests: bpf: test_kmod.sh: check if module is present in the path before insert

On 7 September 2017 at 14:46, Daniel Borkmann <[email protected]> wrote:
> On 09/07/2017 10:19 AM, [email protected] wrote:
>>
>> From: Naresh Kamboju <[email protected]>
>>
>> The test script works when kernel source and build module test_bpf.ko
>> present on the machine. This patch will check if module is present in
>> the path.
>>
>> Signed-off-by: Naresh Kamboju <[email protected]>
>
>
> Looks good, what changed between v1 and v2? Didn't get the cover
> letter in case there was one. ;)

No change for 1/2 to v2 1/2. The only change was to v2 2/2.

>
> Which tree are you targeting? There are usually a lot of changes
> in BPF selftests going the usual route via net and net-next tree
> as we often require to put test cases along the BPF patches. Given
> the merge window now and given one can regard it as a fix, it's
> net tree. I'm also ok if Shuah wants to pick it up this window as
> test_kmod.sh hasn't been changed in quite a while, so no merge
> conflicts expected.

I did not see any conflict for this file (selftests/bpf/test_kmod.sh)

>
> Anyway, for the patch:
>
> Acked-by: Daniel Borkmann <[email protected]>
>
> Thanks!

Thank you.

- Naresh
>
>
>> ---
>> tools/testing/selftests/bpf/test_kmod.sh | 8 +++++---
>> 1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/testing/selftests/bpf/test_kmod.sh
>> b/tools/testing/selftests/bpf/test_kmod.sh
>> index 6d58cca8e235..a53eb1cb54ef 100755
>> --- a/tools/testing/selftests/bpf/test_kmod.sh
>> +++ b/tools/testing/selftests/bpf/test_kmod.sh
>> @@ -9,9 +9,11 @@ test_run()
>>
>> echo "[ JIT enabled:$1 hardened:$2 ]"
>> dmesg -C
>> - insmod $SRC_TREE/lib/test_bpf.ko 2> /dev/null
>> - if [ $? -ne 0 ]; then
>> - rc=1
>> + if [ -f $SRC_TREE/lib/test_bpf.ko ]; then
>> + insmod $SRC_TREE/lib/test_bpf.ko 2> /dev/null
>> + if [ $? -ne 0 ]; then
>> + rc=1
>> + fi
>> fi
>> rmmod test_bpf 2> /dev/null
>> dmesg | grep FAIL
>>
>

From 1577871854544574431@xxx Thu Sep 07 09:17:42 +0000 2017
X-GM-THRID: 1577868220909329273
X-Gmail-Labels: Inbox,Category Forums