Re: [PATCH bpf-next v2 2/2] selftests/bpf: Add test for bpftool access to read-only protected maps

...

• make -C tools/bpf/bpftool -s -j"$ncpus" OUTPUT="$output_dir"/ >/dev/null
• echo ... finished building bpftool
• cd "$pwd"

+}

Given that you're reusing the BPF selftests infra, you shouldn't have to rebuild bpftool as part of the test. It's already built from the Makefile, and other tests just assume it's available already (see test_bpftool.py, test_bpftool.sh).

Agree, the build step will be removed for v3.

...

• # Test write access to the map
• if "$bpftool_path" map update name "$map_name" key $key value $value; then

• if [ "$write_should_succeed" = "true" ]; then
  

• 	echo "  Write access to $map_name succeeded as expected"
  

• else
  

• 	echo "  Write access to $map_name succeeded but should have failed"
  

• 	exit 1
  

• fi
  

• else

• if [ "$write_should_succeed" = "true" ]; then
  

• 	echo "  Write access to $map_name failed but should have succeeded"
  

• 	exit 1
  

• else
  

• 	echo "  Write access to $map_name failed as expected"
  

• fi
  

• fi

Can we try to delete an item as well, please?

I added an item deletion test to v3.

...

• # Pin the map to the BPF filesystem
• "$bpftool_path" map pin name "$map_name" "$pin_path"
• if [ -e "$pin_path" ]; then

• echo "  Successfully pinned $map_name to $pin_path"
  

• else

• echo "  Failed to pin $map_name"
  

• exit 1
  

• fi
• # Test read access to the pinned map
• if "$bpftool_path" map lookup pinned "$pin_path" key $key; then

• echo "  Read access to pinned $map_name succeeded"
  

• else

• echo "  Read access to pinned $map_name failed"
  

• exit 1
  

• fi
• # Test write access to the pinned map
• if "$bpftool_path" map update pinned "$pin_path" key $key value $value; then

• if [ "$write_should_succeed" = "true" ]; then
  

• 	echo "  Write access to pinned $map_name succeeded as expected"
  

• else
  

• 	echo "  Write access to pinned $map_name succeeded but should have failed"
  

• 	exit 1
  

• fi
  

• else

• if [ "$write_should_succeed" = "true" ]; then
  

• 	echo "  Write access to pinned $map_name failed but should have succeeded"
  

• 	exit 1
  

• else
  

• 	echo "  Write access to pinned $map_name failed as expected"
  

• fi
  

• fi

Maybe refactor lookup/update as a function that you can call before and after pinning the map? (I don't mind much.)

I changed it as suggested for v3.

...

+check_bpffs() {

• if [ -z "$BPF_FS" ]; then

• echo "Could not run test without bpffs mounted"
  

Why not? Bpftool will attempt to mount it for you if it's not available (create_and_mount_bpffs_dir()).

You could mount it manually to a specific location and unmount it during the clean-up phase, if you wanted to be sure that the test doesn't have any side effect on the filesystem.

I made changes as suggested for v3.

...

+# Load and attach the BPF programs to control maps access +"$BPFTOOL_PATH" prog loadall "$BPF_FILE_PATH" "$BPF_DIR"/prog autoattach

+# Test protected map (write should fail) +test_map_access "$PROTECTED_MAP_NAME" "false" "$BPFTOOL_PATH" "$BPF_DIR"

+# Test not protected map (write should succeed) +test_map_access "$NOT_PROTECTED_MAP_NAME" "true" "$BPFTOOL_PATH" "$BPF_DIR"

We could also test map creation here (possibly even with inner maps).

I added a test for map-of-maps creation for v3.