On 4/28/23 14:54, 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,
> 
> Reviewed-by: Simon Horman <[email protected]>
> Signed-off-by: David Marchand <[email protected]>
> ---
> Changes since v2:
> - removed unneeded checks when the cache is hit,
> - exposed the cache key computed in the first stage of the build
>   pipeline, and reused it for the second stage,

Thanks, David!  This looks nice.

I tried it out on master and also on the dpdk-latest and it seems
to work fine, except for a couple minor issues inline.

Best regards, Ilya Maximets.

> 
> 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                    | 54 +++++++++++++++++
>  .ci/dpdk-prepare.sh                  | 11 ++++
>  .ci/linux-build.sh                   | 64 ++------------------
>  .ci/linux-prepare.sh                 |  3 +-
>  .github/workflows/build-and-test.yml | 88 ++++++++++++++++++++++------
>  Makefile.am                          |  2 +
>  6 files changed, 144 insertions(+), 78 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..0930ff448a
> --- /dev/null
> +++ b/.ci/dpdk-build.sh
> @@ -0,0 +1,54 @@
> +#!/bin/bash
> +
> +set -o errexit
> +set -x
> +
> +function build_dpdk()
> +{
> +    local VERSION_FILE="dpdk-dir/cached-version"
> +    local DPDK_VER=$1
> +    local DPDK_OPTS=""
> +
> +    rm -rf dpdk-dir
> +
> +    if [ "${DPDK_VER##refs/*/}" != "${DPDK_VER}" ]; then
> +        git clone --single-branch $DPDK_GIT dpdk-dir -b 
> "${DPDK_VER##refs/*/}"
> +        git log -1 --oneline

This git log call should be after we enter dpdk-dir, otherwise
it prints the latest OVS commit, which wasn't the intention.

> +    else
> +        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
> +    fi
> +
> +    pushd dpdk-dir
> +
> +    # 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 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..45bebefd51 100644
> --- a/.github/workflows/build-and-test.yml
> +++ b/.github/workflows/build-and-test.yml
> @@ -3,12 +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
> +    outputs:
> +      dpdk_key: ${{ steps.gen_dpdk_key.outputs.key }}
> +    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

DPDK_GIT is not defined anywhere.  This only affects the dpdk-latest branch,
so it can be argued that it should be defined only there.  But since this
is part of the code, and someone can technically change the DPDK_VER for
some local branch for testing purposes, I think, we need to define it on
the master branch.  What do you think?

Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to