Message ID | 20230413082331.1579334-1-david.marchand@redhat.com |
---|---|
State | Changes Requested |
Headers | show |
Series | [ovs-dev,v2] ci: Separate DPDK from OVS build. | expand |
Context | Check | Description |
---|---|---|
ovsrobot/apply-robot | success | apply and check: success |
ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
ovsrobot/intel-ovs-compilation | success | test: success |
On Thu, Apr 13, 2023 at 10:23:31AM +0200, 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, > > Signed-off-by: David Marchand <david.marchand@redhat.com> Reviewed-by: Simon Horman <simon.horman@corigine.com>
On 4/13/23 10:23, 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, > > Signed-off-by: David Marchand <david.marchand@redhat.com> > --- > Changes since v1: > - filtered dpdk build job dependencies: it only needs gcc, ninja and > libnuma-dev, > - removed matrix configuration for dpdk build single job, > - skipped most of the dpdk build job steps on cache hit, > - for dpdk-latest, DPDK_VER may refer to a branch. The cache key has > been updated to contain the git repository HEAD commit identifier, > and any reference to DPDK from the .github/ configuration files, > - removed libelf-dev and ninja from linux jobs dependencies, Thanks, David! See some comments inline. Best regards, Ilya Maximets. > > --- > .ci/dpdk-build.sh | 65 +++++++++++++++++++++++ > .ci/dpdk-prepare.sh | 11 ++++ > .ci/linux-build.sh | 64 ++-------------------- > .ci/linux-prepare.sh | 3 +- > .github/workflows/build-and-test.yml | 79 +++++++++++++++++++++++++--- > Makefile.am | 2 + > 6 files changed, 156 insertions(+), 68 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..64dec87247 > --- /dev/null > +++ b/.ci/dpdk-build.sh > @@ -0,0 +1,65 @@ > +#!/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. Do we need this branch since you added a comit hash to the cache signature? > + 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" Version file ligic is probably redundant as well, since the script will not be executed on a cache hit. > + 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} > +} > + > +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 39649c1b5c..2a5bd2b016 100644 > --- a/.github/workflows/build-and-test.yml > +++ b/.github/workflows/build-and-test.yml > @@ -3,17 +3,79 @@ name: Build and Test > on: [push, pull_request] > > jobs: > + build-dpdk: > + env: > + dependencies: gcc libnuma-dev ninja-build > + CC: gcc > + DPDK_VER: 22.11.1 It would be great to define this only in one place, but it looks like GHA doesn't really have an elegant native solution for this. So, it's OK as is. > + name: dpdk gcc > + runs-on: ubuntu-20.04 > + timeout-minutes: 30 > + > + steps: > + - name: checkout > + uses: actions/checkout@v3 > + > + - name: update PATH > + run: | > + echo "$HOME/bin" >> $GITHUB_PATH > + echo "$HOME/.local/bin" >> $GITHUB_PATH > + > + - 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' .ci/dpdk-* > dpdk-ci-signature > + grep -rwE 'DPDK_GIT|DPDK_VER' .github/ >> dpdk-ci-signature > + if [ "${DPDK_VER##refs/*/}" != "${DPDK_VER}" ]; then > + git ls-remote --heads $DPDK_GIT $DPDK_VER >> dpdk-ci-signature > + fi > + cat dpdk-ci-signature Should we move this into a separate small script file? It's duplicated in two places now and bacame a bit more complex. > + > + - name: cache > + id: dpdk_cache > + uses: actions/cache@v3 > + env: > + ci_key: ${{ hashFiles('dpdk-ci-signature') }} > + with: > + path: dpdk-dir > + key: dpdk-${{ env.ci_key }} > + > + - name: set up python > + if: steps.dpdk_cache.outputs.cache-hit != 'true' > + uses: actions/setup-python@v4 > + with: > + python-version: '3.9' > + > + - name: update APT cache > + if: steps.dpdk_cache.outputs.cache-hit != 'true' > + run: sudo apt update || true > + - name: install common dependencies > + if: steps.dpdk_cache.outputs.cache-hit != 'true' > + run: sudo apt install -y ${{ env.dependencies }} > + > + - name: prepare > + if: steps.dpdk_cache.outputs.cache-hit != 'true' > + run: ./.ci/dpdk-prepare.sh > + > + - name: build > + if: steps.dpdk_cache.outputs.cache-hit != 'true' > + run: ./.ci/dpdk-build.sh > + > build-linux: > + needs: 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 > + automake libtool gcc bc libjemalloc2 libjemalloc-dev libssl-dev \ > + llvm-dev libnuma-dev libpcap-dev selinux-policy-dev libbpf-dev > ASAN: ${{ matrix.asan }} > UBSAN: ${{ matrix.ubsan }} > CC: ${{ matrix.compiler }} > DPDK: ${{ matrix.dpdk }} > DPDK_SHARED: ${{ matrix.dpdk_shared }} > + DPDK_VER: 22.11.1 > LIBS: ${{ matrix.libs }} > M32: ${{ matrix.m32 }} > OPTS: ${{ matrix.opts }} > @@ -111,18 +173,21 @@ jobs: > # 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-ci-signature > + grep -irE 'RTE_|DPDK|meson|ninja' -r .ci/dpdk-* > dpdk-ci-signature > + grep -rwE 'DPDK_GIT|DPDK_VER' .github/ >> dpdk-ci-signature > + if [ "${DPDK_VER##refs/*/}" != "${DPDK_VER}" ]; then > + git ls-remote --heads $DPDK_GIT $DPDK_VER >> dpdk-ci-signature > + fi > cat dpdk-ci-signature > > - name: cache > if: matrix.dpdk != '' || matrix.dpdk_shared != '' > uses: actions/cache@v3 > env: > - matrix_key: ${{ matrix.dpdk }}${{ matrix.dpdk_shared }} > - ci_key: ${{ hashFiles('dpdk-ci-signature') }} > + ci_key: ${{ hashFiles('dpdk-ci-signature') }} > with: > path: dpdk-dir > - key: ${{ env.matrix_key }}-${{ env.ci_key }} > + key: dpdk-${{ env.ci_key }} > > - name: update APT cache > run: sudo apt update || true > diff --git a/Makefile.am b/Makefile.am > index e605187b81..df9c33dfe6 100644 > --- a/Makefile.am > +++ b/Makefile.am > @@ -75,6 +75,8 @@ EXTRA_DIST = \ > MAINTAINERS.rst \ > README.rst \ > NOTICE \ > + .ci/dpdk-build.sh \ > + .ci/dpdk-prepare.sh \ > .ci/linux-build.sh \ > .ci/linux-prepare.sh \ > .ci/osx-build.sh \
Hello, On Thu, Apr 27, 2023 at 2:45 PM Ilya Maximets <i.maximets@ovn.org> wrote: > > diff --git a/.ci/dpdk-build.sh b/.ci/dpdk-build.sh > > new file mode 100755 > > index 0000000000..64dec87247 > > --- /dev/null > > +++ b/.ci/dpdk-build.sh > > @@ -0,0 +1,65 @@ > > +#!/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. > > Do we need this branch since you added a comit hash to the cache signature? Getting the sources to build still depends on DPDK_VER. We need to distinguish the tarball case from a git "object" string. > > > + 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" > > Version file ligic is probably redundant as well, since the script will > not be executed on a cache hit. Yep. [snip] > > + name: dpdk gcc > > + runs-on: ubuntu-20.04 > > + timeout-minutes: 30 > > + > > + steps: > > + - name: checkout > > + uses: actions/checkout@v3 > > + > > + - name: update PATH > > + run: | > > + echo "$HOME/bin" >> $GITHUB_PATH > > + echo "$HOME/.local/bin" >> $GITHUB_PATH > > + > > + - 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' .ci/dpdk-* > dpdk-ci-signature > > + grep -rwE 'DPDK_GIT|DPDK_VER' .github/ >> dpdk-ci-signature > > + if [ "${DPDK_VER##refs/*/}" != "${DPDK_VER}" ]; then > > + git ls-remote --heads $DPDK_GIT $DPDK_VER >> dpdk-ci-signature > > + fi > > + cat dpdk-ci-signature > > Should we move this into a separate small script file? > It's duplicated in two places now and bacame a bit more complex. Good idea.
On 4/28/23 11:36, David Marchand wrote: > Hello, > > On Thu, Apr 27, 2023 at 2:45 PM Ilya Maximets <i.maximets@ovn.org> wrote: >>> diff --git a/.ci/dpdk-build.sh b/.ci/dpdk-build.sh >>> new file mode 100755 >>> index 0000000000..64dec87247 >>> --- /dev/null >>> +++ b/.ci/dpdk-build.sh >>> @@ -0,0 +1,65 @@ >>> +#!/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. >> >> Do we need this branch since you added a comit hash to the cache signature? > > Getting the sources to build still depends on DPDK_VER. > We need to distinguish the tarball case from a git "object" string. Yeah, cloning below is still necessary. I guess, the comment above should be changed, it misled me a little here. We're going to use cache for git tree builds, and this script is not going to be executed if we have a cache hit. > >> >>> + 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" >> >> Version file ligic is probably redundant as well, since the script will >> not be executed on a cache hit. > > Yep. > > [snip] > >>> + name: dpdk gcc >>> + runs-on: ubuntu-20.04 >>> + timeout-minutes: 30 >>> + >>> + steps: >>> + - name: checkout >>> + uses: actions/checkout@v3 >>> + >>> + - name: update PATH >>> + run: | >>> + echo "$HOME/bin" >> $GITHUB_PATH >>> + echo "$HOME/.local/bin" >> $GITHUB_PATH >>> + >>> + - 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' .ci/dpdk-* > dpdk-ci-signature >>> + grep -rwE 'DPDK_GIT|DPDK_VER' .github/ >> dpdk-ci-signature >>> + if [ "${DPDK_VER##refs/*/}" != "${DPDK_VER}" ]; then >>> + git ls-remote --heads $DPDK_GIT $DPDK_VER >> dpdk-ci-signature >>> + fi >>> + cat dpdk-ci-signature >> >> Should we move this into a separate small script file? >> It's duplicated in two places now and bacame a bit more complex. > > Good idea. > >
On Fri, Apr 28, 2023 at 11:36 AM David Marchand <david.marchand@redhat.com> wrote: > > > + - 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' .ci/dpdk-* > dpdk-ci-signature > > > + grep -rwE 'DPDK_GIT|DPDK_VER' .github/ >> dpdk-ci-signature > > > + if [ "${DPDK_VER##refs/*/}" != "${DPDK_VER}" ]; then > > > + git ls-remote --heads $DPDK_GIT $DPDK_VER >> dpdk-ci-signature > > > + fi > > > + cat dpdk-ci-signature > > > > Should we move this into a separate small script file? > > It's duplicated in two places now and bacame a bit more complex. > > Good idea. I'm going back on this. I had in my list of things to fix, the fact that querying the branch state is racy: if a dpdk commit happens while a OVS job is running (for dpdk-latest) we may generate two different keys => *boom*. So I think I should generate the DPDK key once and for all, expose it as an "output" in GHA, and have the build-ovs depending job call for this "output" for the cache key. As a bonus, this method will avoid setting the DPDK_VER in multiple places in GHA yml. Wdyt?
On Fri, Apr 28, 2023 at 2:26 PM David Marchand <david.marchand@redhat.com> wrote: > > On Fri, Apr 28, 2023 at 11:36 AM David Marchand > <david.marchand@redhat.com> wrote: > > > > + - 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' .ci/dpdk-* > dpdk-ci-signature > > > > + grep -rwE 'DPDK_GIT|DPDK_VER' .github/ >> dpdk-ci-signature > > > > + if [ "${DPDK_VER##refs/*/}" != "${DPDK_VER}" ]; then > > > > + git ls-remote --heads $DPDK_GIT $DPDK_VER >> dpdk-ci-signature > > > > + fi > > > > + cat dpdk-ci-signature > > > > > > Should we move this into a separate small script file? > > > It's duplicated in two places now and bacame a bit more complex. > > > > Good idea. > > I'm going back on this. > > I had in my list of things to fix, the fact that querying the branch > state is racy: if a dpdk commit happens while a OVS job is running > (for dpdk-latest) we may generate two different keys => *boom*. > > So I think I should generate the DPDK key once and for all, expose it > as an "output" in GHA, and have the build-ovs depending job call for > this "output" for the cache key. > As a bonus, this method will avoid setting the DPDK_VER in multiple > places in GHA yml. > It looks good, see v3.
On 4/28/23 14:55, David Marchand wrote: > On Fri, Apr 28, 2023 at 2:26 PM David Marchand > <david.marchand@redhat.com> wrote: >> >> On Fri, Apr 28, 2023 at 11:36 AM David Marchand >> <david.marchand@redhat.com> wrote: >>>>> + - 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' .ci/dpdk-* > dpdk-ci-signature >>>>> + grep -rwE 'DPDK_GIT|DPDK_VER' .github/ >> dpdk-ci-signature >>>>> + if [ "${DPDK_VER##refs/*/}" != "${DPDK_VER}" ]; then >>>>> + git ls-remote --heads $DPDK_GIT $DPDK_VER >> dpdk-ci-signature >>>>> + fi >>>>> + cat dpdk-ci-signature >>>> >>>> Should we move this into a separate small script file? >>>> It's duplicated in two places now and bacame a bit more complex. >>> >>> Good idea. >> >> I'm going back on this. >> >> I had in my list of things to fix, the fact that querying the branch >> state is racy: if a dpdk commit happens while a OVS job is running >> (for dpdk-latest) we may generate two different keys => *boom*. >> >> So I think I should generate the DPDK key once and for all, expose it >> as an "output" in GHA, and have the build-ovs depending job call for >> this "output" for the cache key. >> As a bonus, this method will avoid setting the DPDK_VER in multiple >> places in GHA yml. >> > > It looks good, see v3. Sounds interesting. Thanks!
diff --git a/.ci/dpdk-build.sh b/.ci/dpdk-build.sh new file mode 100755 index 0000000000..64dec87247 --- /dev/null +++ b/.ci/dpdk-build.sh @@ -0,0 +1,65 @@ +#!/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} +} + +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 39649c1b5c..2a5bd2b016 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -3,17 +3,79 @@ name: Build and Test on: [push, pull_request] jobs: + build-dpdk: + env: + dependencies: gcc libnuma-dev ninja-build + CC: gcc + DPDK_VER: 22.11.1 + name: dpdk gcc + runs-on: ubuntu-20.04 + timeout-minutes: 30 + + steps: + - name: checkout + uses: actions/checkout@v3 + + - name: update PATH + run: | + echo "$HOME/bin" >> $GITHUB_PATH + echo "$HOME/.local/bin" >> $GITHUB_PATH + + - 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' .ci/dpdk-* > dpdk-ci-signature + grep -rwE 'DPDK_GIT|DPDK_VER' .github/ >> dpdk-ci-signature + if [ "${DPDK_VER##refs/*/}" != "${DPDK_VER}" ]; then + git ls-remote --heads $DPDK_GIT $DPDK_VER >> dpdk-ci-signature + fi + cat dpdk-ci-signature + + - name: cache + id: dpdk_cache + uses: actions/cache@v3 + env: + ci_key: ${{ hashFiles('dpdk-ci-signature') }} + with: + path: dpdk-dir + key: dpdk-${{ env.ci_key }} + + - name: set up python + if: steps.dpdk_cache.outputs.cache-hit != 'true' + uses: actions/setup-python@v4 + with: + python-version: '3.9' + + - name: update APT cache + if: steps.dpdk_cache.outputs.cache-hit != 'true' + run: sudo apt update || true + - name: install common dependencies + if: steps.dpdk_cache.outputs.cache-hit != 'true' + run: sudo apt install -y ${{ env.dependencies }} + + - name: prepare + if: steps.dpdk_cache.outputs.cache-hit != 'true' + run: ./.ci/dpdk-prepare.sh + + - name: build + if: steps.dpdk_cache.outputs.cache-hit != 'true' + run: ./.ci/dpdk-build.sh + build-linux: + needs: 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 + automake libtool gcc bc libjemalloc2 libjemalloc-dev libssl-dev \ + llvm-dev libnuma-dev libpcap-dev selinux-policy-dev libbpf-dev ASAN: ${{ matrix.asan }} UBSAN: ${{ matrix.ubsan }} CC: ${{ matrix.compiler }} DPDK: ${{ matrix.dpdk }} DPDK_SHARED: ${{ matrix.dpdk_shared }} + DPDK_VER: 22.11.1 LIBS: ${{ matrix.libs }} M32: ${{ matrix.m32 }} OPTS: ${{ matrix.opts }} @@ -111,18 +173,21 @@ jobs: # 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-ci-signature + grep -irE 'RTE_|DPDK|meson|ninja' -r .ci/dpdk-* > dpdk-ci-signature + grep -rwE 'DPDK_GIT|DPDK_VER' .github/ >> dpdk-ci-signature + if [ "${DPDK_VER##refs/*/}" != "${DPDK_VER}" ]; then + git ls-remote --heads $DPDK_GIT $DPDK_VER >> dpdk-ci-signature + fi cat dpdk-ci-signature - name: cache if: matrix.dpdk != '' || matrix.dpdk_shared != '' uses: actions/cache@v3 env: - matrix_key: ${{ matrix.dpdk }}${{ matrix.dpdk_shared }} - ci_key: ${{ hashFiles('dpdk-ci-signature') }} + ci_key: ${{ hashFiles('dpdk-ci-signature') }} with: path: dpdk-dir - key: ${{ env.matrix_key }}-${{ env.ci_key }} + key: dpdk-${{ env.ci_key }} - name: update APT cache run: sudo apt update || true diff --git a/Makefile.am b/Makefile.am index e605187b81..df9c33dfe6 100644 --- a/Makefile.am +++ b/Makefile.am @@ -75,6 +75,8 @@ EXTRA_DIST = \ MAINTAINERS.rst \ README.rst \ NOTICE \ + .ci/dpdk-build.sh \ + .ci/dpdk-prepare.sh \ .ci/linux-build.sh \ .ci/linux-prepare.sh \ .ci/osx-build.sh \
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, Signed-off-by: David Marchand <david.marchand@redhat.com> --- Changes since v1: - filtered dpdk build job dependencies: it only needs gcc, ninja and libnuma-dev, - removed matrix configuration for dpdk build single job, - skipped most of the dpdk build job steps on cache hit, - for dpdk-latest, DPDK_VER may refer to a branch. The cache key has been updated to contain the git repository HEAD commit identifier, and any reference to DPDK from the .github/ configuration files, - removed libelf-dev and ninja from linux jobs dependencies, --- .ci/dpdk-build.sh | 65 +++++++++++++++++++++++ .ci/dpdk-prepare.sh | 11 ++++ .ci/linux-build.sh | 64 ++-------------------- .ci/linux-prepare.sh | 3 +- .github/workflows/build-and-test.yml | 79 +++++++++++++++++++++++++--- Makefile.am | 2 + 6 files changed, 156 insertions(+), 68 deletions(-) create mode 100755 .ci/dpdk-build.sh create mode 100755 .ci/dpdk-prepare.sh