On Tue, Feb 2, 2021 at 1:13 PM KP Singh <kpsingh@xxxxxxxxxx> wrote: > > [...] > > > > > > > > > > > > 2. Then something is re-downloaded every single time: > > > > > > > > % Total % Received % Xferd Average Speed Time Time Time Current > > > > Dload Upload Total Spent Left Speed > > > > 100 77713 100 77713 0 0 509k 0 --:--:-- --:--:-- --:--:-- 512k > > > > > > > > Unless it's to check if something newer appeared in S3, would be nice > > > > to skip that step. > > > > > > This is the kernel config. I wonder how we could check if there is something > > > new without downloading it, the file is called "latest.config". > > > > > > Maybe this is something we can add to the URL index as well in format similar > > > to the image. But since it's just a config file I am not sure > > > it's worth the extra effort. > > > > Curl supports the following option. Given we have a local cache in > > .bpf_selftests, check if it already has .config and pass it as -z > > '.bpf_selftests/.config'? Would be nice, if it works out. If not, I > > agree, config is small enough to not go to great lengths to avoid > > downloading it. > > > > -z/--time-cond <date expression> > > > > (HTTP/FTP) Request a file that has been modified later than the given > > time and date, or one that has been modified before that time. The > > date expression can be all sorts of date strings or if it doesn't > > match any internal ones, it tries to get the time from a given file > > name instead. > > This does not work with the github github raw URL so I had to do something like: > > diff --git a/tools/testing/selftests/bpf/run_in_vm.sh > b/tools/testing/selftests/bpf/run_in_vm.sh > index 46fbb0422e9e..132017981776 100755 > --- a/tools/testing/selftests/bpf/run_in_vm.sh > +++ b/tools/testing/selftests/bpf/run_in_vm.sh > @@ -14,6 +14,7 @@ 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"; > INDEX_URL="https://raw.githubusercontent.com/libbpf/libbpf/master/travis-ci/vmtest/configs/INDEX"; > NUM_COMPILE_JOBS="$(nproc)" > > @@ -236,6 +237,27 @@ is_rel_path() > [[ ${path:0:1} != "/" ]] > } > > +update_kconfig() > +{ > + local kconfig_file="$1" > + local update_command="curl -sLf ${KCONFIG_URL} -o ${kconfig_file}" > + # Github does not return the "last-modified" header when > retrieving the raw contents of the file. > + # Use the API call to get the last-modified time of the kernel > config and only update the config if > + # it has been updated after the previously cached config was > created. This avoids unnecessarily > + # compiling the kernel and selftests. > + if [[ -f "${kconfig_file}" ]]; then > + local last_modified_date="$(curl -sL -D - > "${KCONFIG_API_URL}" -o /dev/null | grep "last-modified" | awk -F ': ' > '{print $2}')" > + local remote_modified_timestamp="$(date -d > "${last_modified_date}" +"%s")" > + local local_creation_timestamp="$(stat -c %W "${kconfig_file}")" > + > + if [[ "${remote_modified_timestamp}" -gt > "${local_creation_timestamp}" ]]; then > + ${update_command} > + fi > + else > + ${update_command} > + fi > +} > + > main() > { > local script_dir="$(cd -P -- "$(dirname -- > "${BASH_SOURCE[0]}")" && pwd -P)" > @@ -314,7 +336,7 @@ main() > > mkdir -p "${OUTPUT_DIR}" > mkdir -p "${mount_dir}" > - curl -Lsf "${KCONFIG_URL}" -o "${kconfig_file}" > + update_kconfig "${kconfig_file}" > > if [[ "${kernel_recompile}" == "no" && ! -f "${kernel_bzimage}" ]]; then > echo "Kernel image not found in ${kernel_bzimage}, > kernel will be recompiled" > > > > > > > > > > > > > > > 3. Every single time I run the script it actually rebuilds kernel. > > > > Somehow Linux Makefile's logic to do nothing if nothing changed in > > > > Linux source code doesn't kick in, I wonder why? It's quite annoying > > > > and time-consuming for frequent selftest reruns. What's weird is that > > > > individual .o's are not re-built, but kernel is still re-linked and > > > > BTF is re-generated, which is the slow part :( > > > > > > I changed this from not compiling the kernel by default, to compiling it and you > > > can "keep your old kernel" with -k. This is because users may run the script, > > > not compile the kernel and run into issues with the image not being able to > > > mount as the kernel does not have the right config. > > > > > > The -k is for people who know what they are doing :) > > > > > > so you can always run > > > > > > ./bpf_presubmit.sh -k > > > > > > after you have the kernel built once. > > > > That's not what I'm saying. When running `make` to build Linux, if > > won't do much at all if nothing changed. That's a good property that > > saves tons of time. I'm saying your script somehow precludes that > > behavior and make does tons of unnecessary work. It might be because > > of always re-downloaded config, which might make the above (not > > redownloading it if it didn't change) more important. > > > > Sure -k might be used this way, but it's expected to happen > > automatically. I'm just pointing out that something is not wired > > optimally to allow make do its job properly. > > Ah, now I see what you are saying and yeah, it was indeed the downloading > of the config every time that was causing the kernel and selftest to be > recompiled. > > With the change I posted above this does not happen anymore. I guess, with > this we can simply remove the -k option? > Yeah, I think so. Very cool, with this behavior this script will probably become a go-to script for everyone doing even occasional BPF development. Thanks! > > > > > > > > > > > > > 4. Selftests are re-built from scratch every single time, even if > > > > nothing changed. Again, strange because they won't do it normally. And > > > > given there is a fixed re-usable .bpf_selftests "cache directory", we > > > > should be able to set everything up so that no extra compilation is > > > > performed, no? > > > > > > > > And this won't be solved with '-k' alone, probably? > > Yeah.. > > > > > > > 5. Before VM is started there is: > > > > > > > > > > > > #!/bin/bash > > [...]
