Hi Jose,
Apologies for the late reply, I haven't been feeling well for the past
few days.
> This patch adds the vmtest-tool subdirectory under contrib which tests
> BPF programs under a live kernel using a QEMU VM. It automatically
> builds the specified kernel version with eBPF support enabled
> and stores it under "~/.vmtest-tool", which is reused for future
> invocations.
I wonder, would it be a good idea to have "bpf" as part of the name of
the directory. Something like bpf-vmtest-tool?
I think adding the "bpf" prefix is a good idea. Should we also rename
the directory under contrib to include the bpf prefix for consistency?
> +To run a BPF source file in the VM:
> +
> + python main.py --kernel-image 6.15 --bpf-src fail.c
> +
Wouldn't --kernel-image expect the path to a kernel image? Typo?
Thanks for catching that. I’ll fix it in the revision.
> +DEVELOPMENT
> +===========
> +
> +This tool uses `uv` (https://github.com/astral-sh/uv) for virtual environment
> +and dependency management.
> +
> +To install development dependencies:
> +
> + uv sync
> +
> +To run the test suite:
> +
> + uv run pytest
> +
> +A `.pre-commit-config.yaml` is provided to assist with development.
> +Pre-commit hooks will auto-generate `requirements-dev.txt` and lint python
> +files.
> +
> +To enable pre-commit hooks:
> +
> + uv run pre-commit install
Having uv installed would only be necessary for vmtest-tool development
purposes, right? Not to run the script. I see that all the
dependencies in vmtest-tool/pyproject.toml are related to testing and
python linting.
Yes, uv is only needed for the development setup. The main script only
uses the standard library. Since all metadata is in the pyproject.toml,
any standard Python package manager (like pip, poetry, etc.) can be
used; there's no strict requirement to use uv. Only the pre-commit
script depends on uv
Is the 3.10 minimum Python version requirement associated with any of
these dependencies? If so, we could maybe relax the minimum version
requirement for users of the script? My Debian-like system has python
3.9.2, for example.
I chose Python 3.10 as the minimum because 3.9 was nearing EOL in a few
months. The script only uses the standard library, so it works with 3.9
by making the following change, since the | type union isn't supported
in 3.9. I'll update the next revision to support 3.9
diff --git a/contrib/vmtest-tool/bpf.py b/contrib/vmtest-tool/bpf.py
index 291e251e64c..91bfc0c5ae1 100644
--- a/contrib/vmtest-tool/bpf.py
+++ b/contrib/vmtest-tool/bpf.py
@@ -3,6 +3,7 @@ import subprocess
import logging
from pathlib import Path
import tempfile
+from typing import Optional
import utils
import config
@@ -25,8 +26,8 @@ class BPFProgram:
def __init__(
self,
- source_path: Path | None = None,
- bpf_bytecode_path: Path | None = None,
+ source_path: Optional[Path] = None,
+ bpf_bytecode_path: Optional[Path] = None,
use_temp_dir: bool = False,
):
path = source_path or bpf_bytecode_path
> +
> + def _compile_bpf(self):
> + """Compile the eBPF program using gcc"""
> + logger.info(f"Compiling eBPF source: {self.bpf_src}")
> + cmd = [
> + "bpf-unknown-none-gcc",
> + "-g",
> + "-O2",
> + "-std=gnu17",
> + f"-D__TARGET_ARCH_{config.ARCH}",
> + "-gbtf",
> + "-Wno-error=attributes",
> + "-Wno-error=address-of-packed-member",
> + "-Wno-compare-distinct-pointer-types",
> + *BPF_INCLUDES,
> + "-c",
> + str(self.bpf_src),
> + "-o",
> + str(self.bpf_obj),
> + ]
It shall definitely be possible to specify the set of compilation flags
using some environment variables like BPF_CFLAGS and BPF_CPPFLAGS, or
arguments to the script. The GCC testsuite will want to do torture-like
testing of the bpf programs by compiling and running them using
different set of optimization options (-O0, -Os, -O2, other options...)
any of which may break verifiability.
Thanks for the suggestion. I'll add support for this in the next
revision. One can also pass a precompiled BPF object with the desired
optimization options to the script to check it with the verifier
> +
> + // STEP 3: Load the program (this will trigger verifier log
output)
> + err = {self.name}__load(skel);
> + fprintf(
> + stderr,
> + "--- Verifier log start ---\\n"
> + "%s\\n"
> + "--- Verifier log end ---\\n",
> + log_buf
Eventually, the output of the loader when the verifier rejects a program
ought to be suitable for our dejagnu glue code to interpret it as a pass
or a fail.
Yes, for now this is meant to print to stdout for the user. This will
change with DejaGNU integration.
Thanks!
