diff mbox series

[ovs-dev,v2] ci: Separate DPDK from OVS build.

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

Checks

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

Commit Message

David Marchand April 13, 2023, 8:23 a.m. UTC 
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

Comments

Simon Horman April 20, 2023, 2:01 p.m. UTC | #1 
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>
Ilya Maximets April 27, 2023, 12:45 p.m. UTC | #2 
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 \
David Marchand April 28, 2023, 9:36 a.m. UTC | #3 
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.
Ilya Maximets April 28, 2023, 11:05 a.m. UTC | #4 
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.
> 
>
David Marchand April 28, 2023, 12:26 p.m. UTC | #5 
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?
David Marchand April 28, 2023, 12:55 p.m. UTC | #6 
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.
Ilya Maximets April 28, 2023, 1:22 p.m. UTC | #7 
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 mbox series

Patch

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 \