Hello Ilya, On Tue, Apr 4, 2023 at 8:01 PM Ilya Maximets <[email protected]> wrote: > > On 3/30/23 11:24, David Marchand wrote: > > Let's separate DPDK compilation from the rest of OVS build: > > - this avoids multiple jobs building DPDK in parallel, which especially > > affects builds in the dpdk-latest branch, > > - we separate concerns about DPDK build requirements from OVS build > > requirements, like python dependencies, > > - building DPDK does not depend on how we will link OVS against it, so we > > can use a single cache entry regardless of DPDK_SHARED option, > > Hi, David. Thanks for the patch! > > The change is not very helpful for the main CI due to caches > being already present. If anything, it might even increase > the toal time needed for CI by a small amount. However, it
I agree that, when the cache is already generated, this will slightly increase the overall build duration. if we check for cache hit as you suggested, the dpdk build job with an already generated cache comes down to less than 10s. I'll put this in v2. > may be very beneficial for cases where we can't use a cache, > like update to a new version of DPDK or the dpdk-latest branch. > So, I'm OK with the proposed solution. > > Not a full review, but a couple of somments inline. > > Best regards, Ilya Maximets. > > > > > Signed-off-by: David Marchand <[email protected]> > > --- > > .ci/dpdk-build.sh | 68 ++++++++++++++++++++++++++++ > > .ci/dpdk-prepare.sh | 11 +++++ > > .ci/linux-build.sh | 64 ++------------------------ > > .ci/linux-prepare.sh | 3 +- > > .github/workflows/build-and-test.yml | 66 +++++++++++++++++++++++++-- > > Makefile.am | 2 + > > 6 files changed, 150 insertions(+), 64 deletions(-) > > create mode 100755 .ci/dpdk-build.sh > > create mode 100755 .ci/dpdk-prepare.sh > > > > diff --git a/.ci/dpdk-build.sh b/.ci/dpdk-build.sh > > new file mode 100755 > > index 0000000000..71da4a2dfe > > --- /dev/null > > +++ b/.ci/dpdk-build.sh > > @@ -0,0 +1,68 @@ > > +#!/bin/bash > > + > > +set -o errexit > > +set -x > > + > > +function build_dpdk() > > +{ > > + local VERSION_FILE="dpdk-dir/cached-version" > > + local DPDK_VER=$1 > > + local DPDK_OPTS="" > > + > > + if [ "${DPDK_VER##refs/*/}" != "${DPDK_VER}" ]; then > > + # Avoid using cache for git tree build. > > + rm -rf dpdk-dir > > + > > + DPDK_GIT=${DPDK_GIT:-https://dpdk.org/git/dpdk} > > + git clone --single-branch $DPDK_GIT dpdk-dir -b > > "${DPDK_VER##refs/*/}" > > + pushd dpdk-dir > > + git log -1 --oneline > > + else > > + if [ -f "${VERSION_FILE}" ]; then > > + VER=$(cat ${VERSION_FILE}) > > + if [ "${VER}" = "${DPDK_VER}" ]; then > > + echo "Found already built DPDK ${VER} build in > > $(pwd)/dpdk-dir" > > + return > > + fi > > + fi > > + # No cache or version mismatch. > > + rm -rf dpdk-dir > > + wget https://fast.dpdk.org/rel/dpdk-$1.tar.xz > > + tar xvf dpdk-$1.tar.xz > /dev/null > > + DIR_NAME=$(tar -tf dpdk-$1.tar.xz | head -1 | cut -f1 -d"/") > > + mv ${DIR_NAME} dpdk-dir > > + pushd dpdk-dir > > + fi > > + > > + # Switching to 'default' machine to make dpdk-dir cache usable on > > + # different CPUs. We can't be sure that all CI machines are exactly > > same. > > + DPDK_OPTS="$DPDK_OPTS -Dmachine=default" > > + > > + # Disable building DPDK unit tests. Not needed for OVS build or tests. > > + DPDK_OPTS="$DPDK_OPTS -Dtests=false" > > + > > + # Disable DPDK developer mode, this results in less build checks and > > less > > + # meson verbose outputs. > > + DPDK_OPTS="$DPDK_OPTS -Ddeveloper_mode=disabled" > > + > > + # OVS compilation and "normal" unit tests (run in the CI) do not > > depend on > > + # any DPDK driver being present. > > + # We can disable all drivers to save compilation time. > > + DPDK_OPTS="$DPDK_OPTS -Ddisable_drivers=*/*" > > + > > + # Install DPDK using prefix. > > + DPDK_OPTS="$DPDK_OPTS --prefix=$(pwd)/build" > > + > > + meson $DPDK_OPTS build > > + ninja -C build > > + ninja -C build install > > + > > + echo "Installed DPDK source in $(pwd)" > > + popd > > + echo "${DPDK_VER}" > ${VERSION_FILE} > > +} > > + > > +if [ -z "$DPDK_VER" ]; then > > + DPDK_VER="22.11.1" > > +fi > > +build_dpdk $DPDK_VER > > diff --git a/.ci/dpdk-prepare.sh b/.ci/dpdk-prepare.sh > > new file mode 100755 > > index 0000000000..f7e6215dda > > --- /dev/null > > +++ b/.ci/dpdk-prepare.sh > > @@ -0,0 +1,11 @@ > > +#!/bin/bash > > + > > +set -ev > > + > > +# Installing wheel separately because it may be needed to build some > > +# of the packages during dependency backtracking and pip >= 22.0 will > > +# abort backtracking on build failures: > > +# https://github.com/pypa/pip/issues/10655 > > +pip3 install --disable-pip-version-check --user wheel > > +pip3 install --disable-pip-version-check --user pyelftools > > +pip3 install --user 'meson==0.53.2' > > diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh > > index 10021fddb2..99850a9434 100755 > > --- a/.ci/linux-build.sh > > +++ b/.ci/linux-build.sh > > @@ -9,9 +9,7 @@ EXTRA_OPTS="--enable-Werror" > > > > function install_dpdk() > > { > > - local DPDK_VER=$1 > > local VERSION_FILE="dpdk-dir/cached-version" > > - local DPDK_OPTS="" > > local DPDK_LIB=$(pwd)/dpdk-dir/build/lib/x86_64-linux-gnu > > > > if [ "$DPDK_SHARED" ]; then > > @@ -24,63 +22,14 @@ function install_dpdk() > > # Export the following path for pkg-config to find the .pc file. > > export PKG_CONFIG_PATH=$DPDK_LIB/pkgconfig/:$PKG_CONFIG_PATH > > > > - if [ "${DPDK_VER##refs/*/}" != "${DPDK_VER}" ]; then > > - # Avoid using cache for git tree build. > > - rm -rf dpdk-dir > > - > > - DPDK_GIT=${DPDK_GIT:-https://dpdk.org/git/dpdk} > > - git clone --single-branch $DPDK_GIT dpdk-dir -b > > "${DPDK_VER##refs/*/}" > > - pushd dpdk-dir > > - git log -1 --oneline > > - else > > - if [ -f "${VERSION_FILE}" ]; then > > - VER=$(cat ${VERSION_FILE}) > > - if [ "${VER}" = "${DPDK_VER}" ]; then > > - # Update the library paths. > > - sudo ldconfig > > - echo "Found cached DPDK ${VER} build in $(pwd)/dpdk-dir" > > - return > > - fi > > - fi > > - # No cache or version mismatch. > > - rm -rf dpdk-dir > > - wget https://fast.dpdk.org/rel/dpdk-$1.tar.xz > > - tar xvf dpdk-$1.tar.xz > /dev/null > > - DIR_NAME=$(tar -tf dpdk-$1.tar.xz | head -1 | cut -f1 -d"/") > > - mv ${DIR_NAME} dpdk-dir > > - pushd dpdk-dir > > + if [ ! -f "${VERSION_FILE}" ]; then > > + echo "Could not find DPDK in $(pwd)/dpdk-dir" > > + return 1 > > fi > > > > - # Switching to 'default' machine to make dpdk-dir cache usable on > > - # different CPUs. We can't be sure that all CI machines are exactly > > same. > > - DPDK_OPTS="$DPDK_OPTS -Dmachine=default" > > - > > - # Disable building DPDK unit tests. Not needed for OVS build or tests. > > - DPDK_OPTS="$DPDK_OPTS -Dtests=false" > > - > > - # Disable DPDK developer mode, this results in less build checks and > > less > > - # meson verbose outputs. > > - DPDK_OPTS="$DPDK_OPTS -Ddeveloper_mode=disabled" > > - > > - # OVS compilation and "normal" unit tests (run in the CI) do not > > depend on > > - # any DPDK driver being present. > > - # We can disable all drivers to save compilation time. > > - DPDK_OPTS="$DPDK_OPTS -Ddisable_drivers=*/*" > > - > > - # Install DPDK using prefix. > > - DPDK_OPTS="$DPDK_OPTS --prefix=$(pwd)/build" > > - > > - CC=gcc meson $DPDK_OPTS build > > - ninja -C build > > - ninja -C build install > > - > > # Update the library paths. > > sudo ldconfig > > - > > - > > - echo "Installed DPDK source in $(pwd)" > > - popd > > - echo "${DPDK_VER}" > ${VERSION_FILE} > > + echo "Found cached DPDK $(cat ${VERSION_FILE}) build in > > $(pwd)/dpdk-dir" > > } > > > > function configure_ovs() > > @@ -130,10 +79,7 @@ assert ovs.json.from_string('{\"a\": 42}') == {'a': 42}" > > fi > > > > if [ "$DPDK" ] || [ "$DPDK_SHARED" ]; then > > - if [ -z "$DPDK_VER" ]; then > > - DPDK_VER="22.11.1" > > - fi > > - install_dpdk $DPDK_VER > > + install_dpdk > > fi > > > > if [ "$CC" = "clang" ]; then > > diff --git a/.ci/linux-prepare.sh b/.ci/linux-prepare.sh > > index f414a879c7..c28b6819a3 100755 > > --- a/.ci/linux-prepare.sh > > +++ b/.ci/linux-prepare.sh > > @@ -23,8 +23,7 @@ cd .. > > # https://github.com/pypa/pip/issues/10655 > > pip3 install --disable-pip-version-check --user wheel > > pip3 install --disable-pip-version-check --user \ > > - flake8 'hacking>=3.0' netaddr pyparsing sphinx setuptools pyelftools > > -pip3 install --user 'meson==0.53.2' > > + flake8 'hacking>=3.0' netaddr pyparsing sphinx setuptools > > > > # Install python test dependencies > > pip3 install -r python/test_requirements.txt > > diff --git a/.github/workflows/build-and-test.yml > > b/.github/workflows/build-and-test.yml > > index 82675b9734..9536a4bab7 100644 > > --- a/.github/workflows/build-and-test.yml > > +++ b/.github/workflows/build-and-test.yml > > @@ -3,7 +3,68 @@ name: Build and Test > > on: [push, pull_request] > > > > jobs: > > + build-dpdk: > > + env: > > + dependencies: | > > + automake libtool gcc bc libjemalloc2 libjemalloc-dev \ > > + libssl-dev llvm-dev libelf-dev libnuma-dev libpcap-dev \ > > + ninja-build selinux-policy-dev libbpf-dev > > Are all these dependencies actually needed? I mean, auotmake? > selinux policy, libbpf and libpcap (we're not building any drivers). > jemalloc? Maybe some other stuff as well. You are right, it is unneeded, I'll clean this for v2. > > > + CC: ${{ matrix.compiler }} > > + > > + name: dpdk ${{ join(matrix.*, ' ') }} > > + runs-on: ubuntu-20.04 > > + timeout-minutes: 30 > > + > > + strategy: > > + fail-fast: false > > + matrix: > > + include: > > + - compiler: gcc > > Too many spaces. It was copy/pasted from the existing jobs. I'll clean this for v2. And I'll also remove the matrix part, as we only generate DPDK for a single compiler (this could be added back if the need arises in the future). > > > + > > + steps: > > + - name: checkout > > + uses: actions/checkout@v3 > > + > > + - name: update PATH > > + run: | > > + echo "$HOME/bin" >> $GITHUB_PATH > > + echo "$HOME/.local/bin" >> $GITHUB_PATH > > + > > + - name: set up python > > + uses: actions/setup-python@v4 > > + with: > > + python-version: '3.9' > > + > > + - name: create ci signature file for the dpdk cache key > > + # This will collect most of DPDK related lines, so hash will be > > different > > + # if something changed in a way we're building DPDK including > > DPDK_VER. > > + # This also allows us to use cache from any branch as long as version > > + # and a way we're building DPDK stays the same. > > + run: | > > + grep -irE 'RTE_|DPDK|meson|ninja' -r .ci/dpdk-* > dpdk-ci-signature > > + cat dpdk-ci-signature > > + > > + - name: cache > > + uses: actions/cache@v3 > > + env: > > + ci_key: ${{ hashFiles('dpdk-ci-signature') }} > > Too many spaces. Ack. > > > + with: > > + path: dpdk-dir > > + key: dpdk-${{ env.ci_key }} > > + > > + - name: update APT cache > > + run: sudo apt update || true > > + - name: install common dependencies > > + run: sudo apt install -y ${{ env.dependencies }} > > + > > + - name: prepare > > + run: ./.ci/dpdk-prepare.sh > > + > > + - name: build > > + run: ./.ci/dpdk-build.sh > > Is it possible to check for the cache before installing build dependencies > and configuring other stuff? It takes a noticeable amount of time to > install / configure things and then we check that the cache is up to date > and we don't need to do anything. Indeed, checking for cache hit saves quite some time. > > > + > > build-linux: > > + needs: build-dpdk > > env: > > dependencies: | > > automake libtool gcc bc libjemalloc2 libjemalloc-dev \ > > I would expect some extra dependencies like libelf to be removed from > this list or switched to non-dev versions of packages, i.e. we might > need libelf1, but do we need libelf-dev? I'll do an extra pass on other packages. But specifically, on libelf-dev, I don't see a strong relation to DPDK. libelf-dev is used by DPDK lib/bpf, which does not make sense with OVS. This could be a leftover from when we were compiling kmods in GHA, or from when we were still compiling libbpf/libxdp. I'll see to remove it in v2. -- David Marchand _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
