diff mbox series

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

Message ID 20230330092404.728239-1-david.marchand@redhat.com
State Changes Requested
Headers show
Series [ovs-dev] 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 March 30, 2023, 9:24 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>
---
 .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

Comments

David Marchand March 31, 2023, 2:38 p.m. UTC | #1
On Thu, Mar 30, 2023 at 11:24 AM David Marchand
<david.marchand@redhat.com> 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,

Err, this patch actually breaks dpdk-latest since the hash does not
depend on DPDK_VER and GHA does not allow overwritting an existing
cache entry.
I'll wait a bit before trying to solve this and see if others have an
opinion on the approach at least.


> - 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>
Ilya Maximets April 4, 2023, 6:01 p.m. UTC | #2
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
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 <david.marchand@redhat.com>
> ---
>  .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.

> +      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.

> +
> +    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.

> +      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.

> +
>    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?


> @@ -111,18 +172,17 @@ 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
>          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') }}
>        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 12, 2023, 8:21 a.m. UTC | #3
Hello Ilya,

On Tue, Apr 4, 2023 at 8:01 PM Ilya Maximets <i.maximets@ovn.org> 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 <david.marchand@redhat.com>
> > ---
> >  .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.
diff mbox series

Patch

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
+      CC:          ${{ matrix.compiler }}
+
+    name: dpdk ${{ join(matrix.*, ' ') }}
+    runs-on: ubuntu-20.04
+    timeout-minutes: 30
+
+    strategy:
+      fail-fast: false
+      matrix:
+        include:
+          - compiler:     gcc
+
+    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') }}
+      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
+
   build-linux:
+    needs: build-dpdk
     env:
       dependencies: |
         automake libtool gcc bc libjemalloc2 libjemalloc-dev    \
@@ -111,18 +172,17 @@  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
         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') }}
       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 \