> Some suggestions for rebasing now that GHA are in the master branch.
> 

Thanks for the feedback David, re-working a v2 now and just waiting on the GHA 
build to complete before submitting the v2.

Minor comments inline.

> A small note: $HOME is world writable in GHA envs, this triggers a
> dpdk init failure when loading a plugin from a path under $HOME (EAL
> has a runtime check on o-w permissions for loaded binaries).
> My workaround with GHA for dpdk to chmod o-w $HOME, but I did not see
> any issue for OVS, since no plugin is loaded in the CI.
> 

Just to clarify here David, are you saying that there isn't currently an issue 
(i.e. it could potentially happen) but you haven’t seen it?

> 
> On Wed, Nov 25, 2020 at 11:42 PM Ian Stokes <[email protected]> wrote:
> > diff --git a/.travis.yml b/.travis.yml
> > index 9fd8bbe01..78411bd83 100644
> > --- a/.travis.yml
> > +++ b/.travis.yml
> > @@ -27,10 +27,14 @@ addons:
> >        - selinux-policy-dev
> >        - libunbound-dev
> >        - libunwind-dev
> > +      - python3-setuptools
> > +      - python3-wheel
> > +      - ninja-build
> 
> Ah ok, python3-wheel was indeed missing in Travis too.
> I missed the change from Sunil who added it in dpdk-latest when I was
> testing Ilya GHA support in dpdk-latest.
> 
> So we need an updated packages list in GHA.
> And meson is not found for a user-only install unless updating PATH.

Thanks for flagging, and providing the patch below, I've rolled this into the 
v2.


> 
> --- a/.github/workflows/build-and-test.yml
> +++ b/.github/workflows/build-and-test.yml
> @@ -8,7 +8,8 @@ jobs:
>        dependencies: |
>          automake libtool gcc bc libjemalloc1 libjemalloc-dev    \
>          libssl-dev llvm-dev libelf-dev libnuma-dev libpcap-dev  \
> -        python3-openssl python3-pip python3-sphinx              \
> +        ninja-build python3-openssl python3-pip                 \
> +        python3-setuptools python3-sphinx python3-wheel         \
>          selinux-policy-dev
>        deb_dependencies: |
>          linux-headers-$(uname -r) build-essential fakeroot devscripts equivs
> @@ -146,7 +147,7 @@ jobs:
>        run:  ./.ci/linux-prepare.sh
> 
>      - name: build
> -      run:  PATH="$PATH:$HOME/bin" ./.ci/linux-build.sh
> +      run:  PATH="$PATH:$HOME/bin:$HOME/.local/bin" ./.ci/linux-build.sh

Out of interest, is this the workaround you’ve 'referenced for using $HOME 
earlier in your response?

> 
>      - name: upload deb packages
>        if:   matrix.deb_package != ''
> 
> >
> >  before_install: ./.travis/${TRAVIS_OS_NAME}-prepare.sh
> >
> > -before_script: export PATH=$PATH:$HOME/bin
> > +before_script:
> > +  - export PATH=$PATH:$HOME/bin
> 
> Nit: no need for this change.

Agreed, I think this was a reformat from when we made dpdk-latest point to the 
main branch in DPDPK, will remove.
> 
> 
> >
> >  env:
> >    - OPTS="--disable-ssl"
> 
> [snip]
> 
> > diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh
> > index 60d8931f3..750b6dd0c 100755
> > --- a/.travis/linux-build.sh
> > +++ b/.travis/linux-build.sh
> > @@ -124,19 +137,19 @@ function install_dpdk()
> >          pushd dpdk-dir
> >      fi
> >
> 
> Here we must switch to "default" for compilation.
> 
>     # 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"
> 

Sure will do.

> 
> > -    make config CC=gcc T=$TARGET
> > +    # Disable building DPDK unit tests. Not needed for OVS build or tests.
> > +    DPDK_OPTS="$DPDK_OPTS -Dtests=false"
> >
> > -    if [ "$DPDK_SHARED" ]; then
> > -        sed -i '/CONFIG_RTE_BUILD_SHARED_LIB=n/s/=n/=y/' build/.config
> > -        export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:$(pwd)/$TARGET/lib
> > -    fi
> > +    # Install DPDK using prefix.
> > +    DPDK_OPTS="$DPDK_OPTS --prefix=$(pwd)/build"
> > +
> > +    CC=gcc meson $DPDK_OPTS build
> > +    ninja -C build
> > +    ninja -C build install
> 
> [snip]
> 
> > diff --git a/Documentation/intro/install/dpdk.rst
> b/Documentation/intro/install/dpdk.rst
> > index fe11571d2..a4484b917 100644
> > --- a/Documentation/intro/install/dpdk.rst
> > +++ b/Documentation/intro/install/dpdk.rst
> 
> [snip]
> 
> >  .. _DPDK sources: http://dpdk.org/rel
> > +.. _DPDK documentation:
> > +   https://doc.dpdk.org/guides-20.08/linux_gsg/build_dpdk.html
> 
> 20.11*
> 
> Please use "versioned" links to 20.11.
> They will be valid for later stable LTS 20.11 releases and it will
> avoid pointing to main branch documentation.
> 
> This might need some double checking, but
> s/guides-19.11/guides-20.11/g should do the job.

Fully agree, at the time of the RFC I couldn't find the versioned links but my 
intention was to update them in the v2 once 20.11 was released.

Will do this for the v2.

Thanks for the review and feedback, much appreciated.

Thanks
Ian
> 
> 
> --
> David Marchand

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to