diff mbox series

[bpf] selftests: bpf: check map in map pruning

Message ID 20211111161452.86864-1-lmb@cloudflare.com (mailing list archive)
State Accepted
Commit a583309d968b8763e6a780b42f7131f11e5c4a7c
Delegated to: BPF
Headers show
Series [bpf] selftests: bpf: check map in map pruning | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for bpf
netdev/fixes_present fail Series targets non-next tree, but doesn't contain any Fixes tags
netdev/subject_prefix success Link
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers warning 5 maintainers not CCed: songliubraving@fb.com john.fastabend@gmail.com yhs@fb.com kpsingh@kernel.org kafai@fb.com
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 39 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
bpf/vmtest-bpf-PR fail PR summary
bpf/vmtest-bpf fail VM_Test

Commit Message

Lorenz Bauer Nov. 11, 2021, 4:14 p.m. UTC
Ensure that two registers with a map_value loaded from a nested
map are considered equivalent for the purpose of state pruning
and don't cause the verifier to revisit a pruning point.

This uses a rather crude match on the number of insns visited by
the verifier, which might change in the future. I've therefore
tried to keep the code as "unpruneable" as possible by having
the code paths only converge on the second to last instruction.

Should you require to adjust the test in the future, reducing the
number of processed instructions should always be safe. Increasing
them could cause another regression, so proceed with caution.

Suggested-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Lorenz Bauer <lmb@cloudflare.com>
Link: https://lore.kernel.org/bpf/CACAyw99hVEJFoiBH_ZGyy=+oO-jyydoz6v1DeKPKs2HVsUH28w@mail.gmail.com/
---
 .../selftests/bpf/verifier/map_in_map.c       | 33 +++++++++++++++++++
 1 file changed, 33 insertions(+)

Comments

patchwork-bot+netdevbpf@kernel.org Nov. 12, 2021, 3:20 p.m. UTC | #1
Hello:

This patch was applied to bpf/bpf.git (master)
by Daniel Borkmann <daniel@iogearbox.net>:

On Thu, 11 Nov 2021 16:14:52 +0000 you wrote:
> Ensure that two registers with a map_value loaded from a nested
> map are considered equivalent for the purpose of state pruning
> and don't cause the verifier to revisit a pruning point.
> 
> This uses a rather crude match on the number of insns visited by
> the verifier, which might change in the future. I've therefore
> tried to keep the code as "unpruneable" as possible by having
> the code paths only converge on the second to last instruction.
> 
> [...]

Here is the summary with links:
  - [bpf] selftests: bpf: check map in map pruning
    https://git.kernel.org/bpf/bpf/c/a583309d968b

You are awesome, thank you!
Alexei Starovoitov Nov. 13, 2021, 1:27 a.m. UTC | #2
On Thu, Nov 11, 2021 at 8:16 AM Lorenz Bauer <lmb@cloudflare.com> wrote:
>
> Ensure that two registers with a map_value loaded from a nested
> map are considered equivalent for the purpose of state pruning
> and don't cause the verifier to revisit a pruning point.
>
> This uses a rather crude match on the number of insns visited by
> the verifier, which might change in the future. I've therefore
> tried to keep the code as "unpruneable" as possible by having
> the code paths only converge on the second to last instruction.
>
> Should you require to adjust the test in the future, reducing the
> number of processed instructions should always be safe. Increasing
> them could cause another regression, so proceed with caution.
>
> Suggested-by: Alexei Starovoitov <ast@kernel.org>
> Signed-off-by: Lorenz Bauer <lmb@cloudflare.com>
> Link: https://lore.kernel.org/bpf/CACAyw99hVEJFoiBH_ZGyy=+oO-jyydoz6v1DeKPKs2HVsUH28w@mail.gmail.com/
> ---
>  .../selftests/bpf/verifier/map_in_map.c       | 33 +++++++++++++++++++
>  1 file changed, 33 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/verifier/map_in_map.c b/tools/testing/selftests/bpf/verifier/map_in_map.c
> index 2798927ee9ff..f46c7121e216 100644
> --- a/tools/testing/selftests/bpf/verifier/map_in_map.c
> +++ b/tools/testing/selftests/bpf/verifier/map_in_map.c
> @@ -18,6 +18,39 @@
>         .fixup_map_in_map = { 3 },
>         .result = ACCEPT,
>  },
> +{
> +       "map in map state pruning",
> +       .insns = {
> +       BPF_ST_MEM(0, BPF_REG_10, -4, 0),
> +       BPF_MOV64_REG(BPF_REG_6, BPF_REG_10),
> +       BPF_ALU64_IMM(BPF_ADD, BPF_REG_6, -4),
> +       BPF_MOV64_REG(BPF_REG_2, BPF_REG_6),
> +       BPF_LD_MAP_FD(BPF_REG_1, 0),
> +       BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_lookup_elem),
> +       BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0, 1),
> +       BPF_EXIT_INSN(),
> +       BPF_MOV64_REG(BPF_REG_2, BPF_REG_6),
> +       BPF_MOV64_REG(BPF_REG_1, BPF_REG_0),
> +       BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_lookup_elem),
> +       BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0, 11),
> +       BPF_MOV64_REG(BPF_REG_2, BPF_REG_6),
> +       BPF_LD_MAP_FD(BPF_REG_1, 0),
> +       BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_lookup_elem),
> +       BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0, 1),
> +       BPF_EXIT_INSN(),
> +       BPF_MOV64_REG(BPF_REG_2, BPF_REG_6),
> +       BPF_MOV64_REG(BPF_REG_1, BPF_REG_0),
> +       BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_lookup_elem),
> +       BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0, 1),
> +       BPF_EXIT_INSN(),
> +       BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_0, 0),
> +       BPF_EXIT_INSN(),
> +       },
> +       .fixup_map_in_map = { 4, 14 },
> +       .flags = BPF_F_TEST_STATE_FREQ,
> +       .result = VERBOSE_ACCEPT,
> +       .errstr = "processed 25 insns",
> +},

Not sure how you've tested it, but it doesn't work in unpriv:
$ test_verifier 789
#789/u map in map state pruning FAIL
processed 26 insns (limit 1000000) max_states_per_insn 0 total_states
2 peak_states 2 mark_read 1
#789/p map in map state pruning OK

I've added
.prog_type = BPF_PROG_TYPE_XDP,
and force pushed.
Lorenz Bauer Nov. 17, 2021, 8:47 a.m. UTC | #3
On Sat, 13 Nov 2021 at 01:27, Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> Not sure how you've tested it, but it doesn't work in unpriv:
> $ test_verifier 789
> #789/u map in map state pruning FAIL
> processed 26 insns (limit 1000000) max_states_per_insn 0 total_states
> 2 peak_states 2 mark_read 1
> #789/p map in map state pruning OK

Strange, I have a script that I use for bisecting which uses a minimal
.config + virtue to run a vm, plus I was debugging in gdb at the same
time. I might have missed this, apologies.

I guess vmtest.sh is the canonical way to run tests now?
Alexei Starovoitov Nov. 18, 2021, 1 a.m. UTC | #4
On Wed, Nov 17, 2021 at 08:47:45AM +0000, Lorenz Bauer wrote:
> On Sat, 13 Nov 2021 at 01:27, Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > Not sure how you've tested it, but it doesn't work in unpriv:
> > $ test_verifier 789
> > #789/u map in map state pruning FAIL
> > processed 26 insns (limit 1000000) max_states_per_insn 0 total_states
> > 2 peak_states 2 mark_read 1
> > #789/p map in map state pruning OK
> 
> Strange, I have a script that I use for bisecting which uses a minimal
> .config + virtue to run a vm, plus I was debugging in gdb at the same
> time. I might have missed this, apologies.
> 
> I guess vmtest.sh is the canonical way to run tests now?

vmtest.sh runs test_progs only. That's the minimum bar that
developers have to pass before sending patches.
BPF CI runs test_progs, test_progs-no_alu32, test_verifier and test_maps.
If in doubt run them all.
Andrii Nakryiko Nov. 18, 2021, 1:29 a.m. UTC | #5
On Wed, Nov 17, 2021 at 5:01 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Wed, Nov 17, 2021 at 08:47:45AM +0000, Lorenz Bauer wrote:
> > On Sat, 13 Nov 2021 at 01:27, Alexei Starovoitov
> > <alexei.starovoitov@gmail.com> wrote:
> > >
> > > Not sure how you've tested it, but it doesn't work in unpriv:
> > > $ test_verifier 789
> > > #789/u map in map state pruning FAIL
> > > processed 26 insns (limit 1000000) max_states_per_insn 0 total_states
> > > 2 peak_states 2 mark_read 1
> > > #789/p map in map state pruning OK
> >
> > Strange, I have a script that I use for bisecting which uses a minimal
> > .config + virtue to run a vm, plus I was debugging in gdb at the same
> > time. I might have missed this, apologies.
> >
> > I guess vmtest.sh is the canonical way to run tests now?
>
> vmtest.sh runs test_progs only. That's the minimum bar that

It runs test_progs by default, unless something else is requested. You
can run anything inside it, e.g.:

./vmtest.sh -- ./test_maps

BTW, we recently moved configs around in libbpf repo on Github, so
this script broke. I'm sending a fix in a few minutes, hopefully.

> developers have to pass before sending patches.
> BPF CI runs test_progs, test_progs-no_alu32, test_verifier and test_maps.
> If in doubt run them all.
Andrii Nakryiko Nov. 18, 2021, 1:38 a.m. UTC | #6
On Wed, Nov 17, 2021 at 5:29 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Wed, Nov 17, 2021 at 5:01 PM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Wed, Nov 17, 2021 at 08:47:45AM +0000, Lorenz Bauer wrote:
> > > On Sat, 13 Nov 2021 at 01:27, Alexei Starovoitov
> > > <alexei.starovoitov@gmail.com> wrote:
> > > >
> > > > Not sure how you've tested it, but it doesn't work in unpriv:
> > > > $ test_verifier 789
> > > > #789/u map in map state pruning FAIL
> > > > processed 26 insns (limit 1000000) max_states_per_insn 0 total_states
> > > > 2 peak_states 2 mark_read 1
> > > > #789/p map in map state pruning OK
> > >
> > > Strange, I have a script that I use for bisecting which uses a minimal
> > > .config + virtue to run a vm, plus I was debugging in gdb at the same
> > > time. I might have missed this, apologies.
> > >
> > > I guess vmtest.sh is the canonical way to run tests now?
> >
> > vmtest.sh runs test_progs only. That's the minimum bar that
>
> It runs test_progs by default, unless something else is requested. You
> can run anything inside it, e.g.:
>
> ./vmtest.sh -- ./test_maps
>
> BTW, we recently moved configs around in libbpf repo on Github, so
> this script broke. I'm sending a fix in a few minutes, hopefully.

... and of course it's not that simple. [0] recently changed how we
build qemu image and vmtest.sh had some assumptions. Some trivial
things I fixed, but I'm not too familiar with the init scripts stuff.
Adding Ilya and KP to hopefully help with this. Ilya, KP, can you
please help restore vmtest.sh functionality?

After fixing few paths:

diff --git a/tools/testing/selftests/bpf/vmtest.sh
b/tools/testing/selftests/bpf/vmtest.sh
index 027198768fad..7ea40108b85d 100755
--- a/tools/testing/selftests/bpf/vmtest.sh
+++ b/tools/testing/selftests/bpf/vmtest.sh
@@ -13,8 +13,8 @@ DEFAULT_COMMAND="./test_progs"
 MOUNT_DIR="mnt"
 ROOTFS_IMAGE="root.img"
 OUTPUT_DIR="$HOME/.bpf_selftests"
-KCONFIG_URL="https://raw.githubusercontent.com/libbpf/libbpf/master/travis-ci/vmtest/configs/latest.config"
-KCONFIG_API_URL="https://api.github.com/repos/libbpf/libbpf/contents/travis-ci/vmtest/configs/latest.config"
+KCONFIG_URL="https://raw.githubusercontent.com/libbpf/libbpf/master/travis-ci/vmtest/configs/config-latest.x86_64"
+KCONFIG_API_URL="https://api.github.com/repos/libbpf/libbpf/contents/travis-ci/vmtest/configs/config-latest.x86_64"
 INDEX_URL="https://raw.githubusercontent.com/libbpf/libbpf/master/travis-ci/vmtest/configs/INDEX"
 NUM_COMPILE_JOBS="$(nproc)"
 LOG_FILE_BASE="$(date +"bpf_selftests.%Y-%m-%d_%H-%M-%S")"
@@ -85,7 +85,7 @@ newest_rootfs_version()
 {
        {
        for file in "${!URLS[@]}"; do
-               if [[ $file =~ ^libbpf-vmtest-rootfs-(.*)\.tar\.zst$ ]]; then
+               if [[ $file =~
^x86_64/libbpf-vmtest-rootfs-(.*)\.tar\.zst$ ]]; then
                        echo "${BASH_REMATCH[1]}"
                fi
        done

... the next problem is more severe. Script complains about missing
/etc/rcS.d, if I just force-created it, when kernel boots we get:


[    1.050803] ---[ end Kernel panic - not syncing: No working init
found.  Try passing init= option to kernel. See Linux
Documentation/admin-guide/init.rst for guidance. ]---


Please help.

  [0] https://github.com/libbpf/libbpf/pull/204

>
> > developers have to pass before sending patches.
> > BPF CI runs test_progs, test_progs-no_alu32, test_verifier and test_maps.
> > If in doubt run them all.
Ilya Leoshkevich Nov. 18, 2021, 11:05 a.m. UTC | #7
On Wed, 2021-11-17 at 17:38 -0800, Andrii Nakryiko wrote:
> On Wed, Nov 17, 2021 at 5:29 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> > 
> > On Wed, Nov 17, 2021 at 5:01 PM Alexei Starovoitov
> > <alexei.starovoitov@gmail.com> wrote:
> > > 
> > > On Wed, Nov 17, 2021 at 08:47:45AM +0000, Lorenz Bauer wrote:
> > > > On Sat, 13 Nov 2021 at 01:27, Alexei Starovoitov
> > > > <alexei.starovoitov@gmail.com> wrote:
> > > > > 
> > > > > Not sure how you've tested it, but it doesn't work in unpriv:
> > > > > $ test_verifier 789
> > > > > #789/u map in map state pruning FAIL
> > > > > processed 26 insns (limit 1000000) max_states_per_insn 0
> > > > > total_states
> > > > > 2 peak_states 2 mark_read 1
> > > > > #789/p map in map state pruning OK
> > > > 
> > > > Strange, I have a script that I use for bisecting which uses a
> > > > minimal
> > > > .config + virtue to run a vm, plus I was debugging in gdb at
> > > > the same
> > > > time. I might have missed this, apologies.
> > > > 
> > > > I guess vmtest.sh is the canonical way to run tests now?
> > > 
> > > vmtest.sh runs test_progs only. That's the minimum bar that
> > 
> > It runs test_progs by default, unless something else is requested.
> > You
> > can run anything inside it, e.g.:
> > 
> > ./vmtest.sh -- ./test_maps
> > 
> > BTW, we recently moved configs around in libbpf repo on Github, so
> > this script broke. I'm sending a fix in a few minutes, hopefully.
> 
> ... and of course it's not that simple. [0] recently changed how we
> build qemu image and vmtest.sh had some assumptions. Some trivial
> things I fixed, but I'm not too familiar with the init scripts stuff.
> Adding Ilya and KP to hopefully help with this. Ilya, KP, can you
> please help restore vmtest.sh functionality?
> 
> After fixing few paths:
> 
> diff --git a/tools/testing/selftests/bpf/vmtest.sh
> b/tools/testing/selftests/bpf/vmtest.sh
> index 027198768fad..7ea40108b85d 100755
> --- a/tools/testing/selftests/bpf/vmtest.sh
> +++ b/tools/testing/selftests/bpf/vmtest.sh
> @@ -13,8 +13,8 @@ DEFAULT_COMMAND="./test_progs"
>  MOUNT_DIR="mnt"
>  ROOTFS_IMAGE="root.img"
>  OUTPUT_DIR="$HOME/.bpf_selftests"
> -
> KCONFIG_URL="https://raw.githubusercontent.com/libbpf/libbpf/master/travis-ci/vmtest/configs/latest.config
> "
> -
> KCONFIG_API_URL="https://api.github.com/repos/libbpf/libbpf/contents/travis-ci/vmtest/configs/latest.config
> "
> +KCONFIG_URL="https://raw.githubusercontent.com/libbpf/libbpf/master/travis-ci/vmtest/configs/config-latest.x86_64
> "
> +KCONFIG_API_URL="https://api.github.com/repos/libbpf/libbpf/contents/travis-ci/vmtest/configs/config-latest.x86_64
> "
>  INDEX_URL="https://raw.githubusercontent.com/libbpf/libbpf/master/travis-ci/vmtest/configs/INDEX
> "
>  NUM_COMPILE_JOBS="$(nproc)"
>  LOG_FILE_BASE="$(date +"bpf_selftests.%Y-%m-%d_%H-%M-%S")"
> @@ -85,7 +85,7 @@ newest_rootfs_version()
>  {
>         {
>         for file in "${!URLS[@]}"; do
> -               if [[ $file =~ ^libbpf-vmtest-rootfs-(.*)\.tar\.zst$
> ]]; then
> +               if [[ $file =~
> ^x86_64/libbpf-vmtest-rootfs-(.*)\.tar\.zst$ ]]; then
>                         echo "${BASH_REMATCH[1]}"
>                 fi
>         done
> 
> ... the next problem is more severe. Script complains about missing
> /etc/rcS.d, if I just force-created it, when kernel boots we get:
> 
> 
> [    1.050803] ---[ end Kernel panic - not syncing: No working init
> found.  Try passing init= option to kernel. See Linux
> Documentation/admin-guide/init.rst for guidance. ]---
> 
> 
> Please help.

I'll have a look.

Do you think whether it would make sense to reference a fixed
libbpf commitid instead of master in vmtest.sh? So that updates like
this could be done in a controlled manner.

Best regards,
Ilya
Ilya Leoshkevich Nov. 18, 2021, 11:56 a.m. UTC | #8
On Wed, 2021-11-17 at 17:38 -0800, Andrii Nakryiko wrote:
> On Wed, Nov 17, 2021 at 5:29 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> > 
> > On Wed, Nov 17, 2021 at 5:01 PM Alexei Starovoitov
> > <alexei.starovoitov@gmail.com> wrote:
> > > 
> > > On Wed, Nov 17, 2021 at 08:47:45AM +0000, Lorenz Bauer wrote:
> > > > On Sat, 13 Nov 2021 at 01:27, Alexei Starovoitov
> > > > <alexei.starovoitov@gmail.com> wrote:
> > > > > 
> > > > > Not sure how you've tested it, but it doesn't work in unpriv:
> > > > > $ test_verifier 789
> > > > > #789/u map in map state pruning FAIL
> > > > > processed 26 insns (limit 1000000) max_states_per_insn 0
> > > > > total_states
> > > > > 2 peak_states 2 mark_read 1
> > > > > #789/p map in map state pruning OK
> > > > 
> > > > Strange, I have a script that I use for bisecting which uses a
> > > > minimal
> > > > .config + virtue to run a vm, plus I was debugging in gdb at the
> > > > same
> > > > time. I might have missed this, apologies.
> > > > 
> > > > I guess vmtest.sh is the canonical way to run tests now?
> > > 
> > > vmtest.sh runs test_progs only. That's the minimum bar that
> > 
> > It runs test_progs by default, unless something else is requested.
> > You
> > can run anything inside it, e.g.:
> > 
> > ./vmtest.sh -- ./test_maps
> > 
> > BTW, we recently moved configs around in libbpf repo on Github, so
> > this script broke. I'm sending a fix in a few minutes, hopefully.
> 
> ... and of course it's not that simple. [0] recently changed how we
> build qemu image and vmtest.sh had some assumptions. Some trivial
> things I fixed, but I'm not too familiar with the init scripts stuff.
> Adding Ilya and KP to hopefully help with this. Ilya, KP, can you
> please help restore vmtest.sh functionality?
> 
> After fixing few paths:
> 
> diff --git a/tools/testing/selftests/bpf/vmtest.sh
> b/tools/testing/selftests/bpf/vmtest.sh
> index 027198768fad..7ea40108b85d 100755
> --- a/tools/testing/selftests/bpf/vmtest.sh
> +++ b/tools/testing/selftests/bpf/vmtest.sh
> @@ -13,8 +13,8 @@ DEFAULT_COMMAND="./test_progs"
>  MOUNT_DIR="mnt"
>  ROOTFS_IMAGE="root.img"
>  OUTPUT_DIR="$HOME/.bpf_selftests"
> -
> KCONFIG_URL="https://raw.githubusercontent.com/libbpf/libbpf/master/travis-ci/vmtest/configs/latest.config
> "
> -
> KCONFIG_API_URL="https://api.github.com/repos/libbpf/libbpf/contents/travis-ci/vmtest/configs/latest.config
> "
> +KCONFIG_URL="https://raw.githubusercontent.com/libbpf/libbpf/master/travis-ci/vmtest/configs/config-latest.x86_64
> "
> +KCONFIG_API_URL="https://api.github.com/repos/libbpf/libbpf/contents/travis-ci/vmtest/configs/config-latest.x86_64
> "
>  INDEX_URL="https://raw.githubusercontent.com/libbpf/libbpf/master/travis-ci/vmtest/configs/INDEX
> "
>  NUM_COMPILE_JOBS="$(nproc)"
>  LOG_FILE_BASE="$(date +"bpf_selftests.%Y-%m-%d_%H-%M-%S")"
> @@ -85,7 +85,7 @@ newest_rootfs_version()
>  {
>         {
>         for file in "${!URLS[@]}"; do
> -               if [[ $file =~ ^libbpf-vmtest-rootfs-(.*)\.tar\.zst$
> ]]; then
> +               if [[ $file =~
> ^x86_64/libbpf-vmtest-rootfs-(.*)\.tar\.zst$ ]]; then
>                         echo "${BASH_REMATCH[1]}"
>                 fi
>         done
> 
> ... the next problem is more severe. Script complains about missing
> /etc/rcS.d, if I just force-created it, when kernel boots we get:
> 
> 
> [    1.050803] ---[ end Kernel panic - not syncing: No working init
> found.  Try passing init= option to kernel. See Linux
> Documentation/admin-guide/init.rst for guidance. ]---
> 
> 
> Please help.
> 
>   [0] https://github.com/libbpf/libbpf/pull/204

I've posted a fix, please give it a try:

https://lore.kernel.org/bpf/20211118115225.1349726-1-iii@linux.ibm.com/

Missing was the ${ARCH} prefix when downloading the image, so it ended
up being empty. Now your ~/.bpf_selftests is poisoned with it, so
you'll need to run vmtest.sh with -i switch once in order to remove the
bad image.

Best regards,
Ilya
KP Singh Nov. 18, 2021, 3:20 p.m. UTC | #9
On Thu, Nov 18, 2021 at 12:56 PM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>
> On Wed, 2021-11-17 at 17:38 -0800, Andrii Nakryiko wrote:
> > On Wed, Nov 17, 2021 at 5:29 PM Andrii Nakryiko
> > <andrii.nakryiko@gmail.com> wrote:
> > >
> > > On Wed, Nov 17, 2021 at 5:01 PM Alexei Starovoitov
> > > <alexei.starovoitov@gmail.com> wrote:
> > > >
> > > > On Wed, Nov 17, 2021 at 08:47:45AM +0000, Lorenz Bauer wrote:
> > > > > On Sat, 13 Nov 2021 at 01:27, Alexei Starovoitov
> > > > > <alexei.starovoitov@gmail.com> wrote:
> > > > > >
> > > > > > Not sure how you've tested it, but it doesn't work in unpriv:
> > > > > > $ test_verifier 789
> > > > > > #789/u map in map state pruning FAIL
> > > > > > processed 26 insns (limit 1000000) max_states_per_insn 0
> > > > > > total_states
> > > > > > 2 peak_states 2 mark_read 1
> > > > > > #789/p map in map state pruning OK
> > > > >
> > > > > Strange, I have a script that I use for bisecting which uses a
> > > > > minimal
> > > > > .config + virtue to run a vm, plus I was debugging in gdb at the
> > > > > same
> > > > > time. I might have missed this, apologies.
> > > > >
> > > > > I guess vmtest.sh is the canonical way to run tests now?
> > > >
> > > > vmtest.sh runs test_progs only. That's the minimum bar that
> > >
> > > It runs test_progs by default, unless something else is requested.
> > > You
> > > can run anything inside it, e.g.:
> > >
> > > ./vmtest.sh -- ./test_maps
> > >
> > > BTW, we recently moved configs around in libbpf repo on Github, so
> > > this script broke. I'm sending a fix in a few minutes, hopefully.
> >
> > ... and of course it's not that simple. [0] recently changed how we
> > build qemu image and vmtest.sh had some assumptions. Some trivial
> > things I fixed, but I'm not too familiar with the init scripts stuff.
> > Adding Ilya and KP to hopefully help with this. Ilya, KP, can you
> > please help restore vmtest.sh functionality?
> >
> > After fixing few paths:
> >
> > diff --git a/tools/testing/selftests/bpf/vmtest.sh
> > b/tools/testing/selftests/bpf/vmtest.sh
> > index 027198768fad..7ea40108b85d 100755
> > --- a/tools/testing/selftests/bpf/vmtest.sh
> > +++ b/tools/testing/selftests/bpf/vmtest.sh
> > @@ -13,8 +13,8 @@ DEFAULT_COMMAND="./test_progs"
> >  MOUNT_DIR="mnt"
> >  ROOTFS_IMAGE="root.img"
> >  OUTPUT_DIR="$HOME/.bpf_selftests"
> > -
> > KCONFIG_URL="https://raw.githubusercontent.com/libbpf/libbpf/master/travis-ci/vmtest/configs/latest.config
> > "
> > -
> > KCONFIG_API_URL="https://api.github.com/repos/libbpf/libbpf/contents/travis-ci/vmtest/configs/latest.config
> > "
> > +KCONFIG_URL="https://raw.githubusercontent.com/libbpf/libbpf/master/travis-ci/vmtest/configs/config-latest.x86_64
> > "
> > +KCONFIG_API_URL="https://api.github.com/repos/libbpf/libbpf/contents/travis-ci/vmtest/configs/config-latest.x86_64
> > "
> >  INDEX_URL="https://raw.githubusercontent.com/libbpf/libbpf/master/travis-ci/vmtest/configs/INDEX
> > "
> >  NUM_COMPILE_JOBS="$(nproc)"
> >  LOG_FILE_BASE="$(date +"bpf_selftests.%Y-%m-%d_%H-%M-%S")"
> > @@ -85,7 +85,7 @@ newest_rootfs_version()
> >  {
> >         {
> >         for file in "${!URLS[@]}"; do
> > -               if [[ $file =~ ^libbpf-vmtest-rootfs-(.*)\.tar\.zst$
> > ]]; then
> > +               if [[ $file =~
> > ^x86_64/libbpf-vmtest-rootfs-(.*)\.tar\.zst$ ]]; then
> >                         echo "${BASH_REMATCH[1]}"
> >                 fi
> >         done
> >
> > ... the next problem is more severe. Script complains about missing
> > /etc/rcS.d, if I just force-created it, when kernel boots we get:
> >
> >
> > [    1.050803] ---[ end Kernel panic - not syncing: No working init
> > found.  Try passing init= option to kernel. See Linux
> > Documentation/admin-guide/init.rst for guidance. ]---
> >
> >
> > Please help.
> >
> >   [0] https://github.com/libbpf/libbpf/pull/204
>
> I've posted a fix, please give it a try:
>
> https://lore.kernel.org/bpf/20211118115225.1349726-1-iii@linux.ibm.com/
>
> Missing was the ${ARCH} prefix when downloading the image, so it ended
> up being empty. Now your ~/.bpf_selftests is poisoned with it, so
> you'll need to run vmtest.sh with -i switch once in order to remove the
> bad image.

Thanks for taking a look and sending a fix.
diff mbox series

Patch

diff --git a/tools/testing/selftests/bpf/verifier/map_in_map.c b/tools/testing/selftests/bpf/verifier/map_in_map.c
index 2798927ee9ff..f46c7121e216 100644
--- a/tools/testing/selftests/bpf/verifier/map_in_map.c
+++ b/tools/testing/selftests/bpf/verifier/map_in_map.c
@@ -18,6 +18,39 @@ 
 	.fixup_map_in_map = { 3 },
 	.result = ACCEPT,
 },
+{
+	"map in map state pruning",
+	.insns = {
+	BPF_ST_MEM(0, BPF_REG_10, -4, 0),
+	BPF_MOV64_REG(BPF_REG_6, BPF_REG_10),
+	BPF_ALU64_IMM(BPF_ADD, BPF_REG_6, -4),
+	BPF_MOV64_REG(BPF_REG_2, BPF_REG_6),
+	BPF_LD_MAP_FD(BPF_REG_1, 0),
+	BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_lookup_elem),
+	BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0, 1),
+	BPF_EXIT_INSN(),
+	BPF_MOV64_REG(BPF_REG_2, BPF_REG_6),
+	BPF_MOV64_REG(BPF_REG_1, BPF_REG_0),
+	BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_lookup_elem),
+	BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0, 11),
+	BPF_MOV64_REG(BPF_REG_2, BPF_REG_6),
+	BPF_LD_MAP_FD(BPF_REG_1, 0),
+	BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_lookup_elem),
+	BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0, 1),
+	BPF_EXIT_INSN(),
+	BPF_MOV64_REG(BPF_REG_2, BPF_REG_6),
+	BPF_MOV64_REG(BPF_REG_1, BPF_REG_0),
+	BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_lookup_elem),
+	BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0, 1),
+	BPF_EXIT_INSN(),
+	BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_0, 0),
+	BPF_EXIT_INSN(),
+	},
+	.fixup_map_in_map = { 4, 14 },
+	.flags = BPF_F_TEST_STATE_FREQ,
+	.result = VERBOSE_ACCEPT,
+	.errstr = "processed 25 insns",
+},
 {
 	"invalid inner map pointer",
 	.insns = {