Message ID | 20221216155054.986464-7-simon.horman@corigine.com |
---|---|
State | Changes Requested |
Headers | show |
Series | Add support for DPDK meter HW offload | 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 | fail | test: fail |
On Fri, Dec 16, 2022 at 4:52 PM Simon Horman <simon.horman@corigine.com> wrote: > > From: Peng Zhang <peng.zhang@corigine.com> > > This commit adds support for OVS-DPDK with -DALLOW_EXPERIMENTAL_API. > > Tunnel offloads and Meter offloads are experimental APIs in DPDK. To > enable these features, compile need add -DALLOW_EXPERIMENTAL_API. So > in workflow, we also need need the new test with > -DALLOW_EXPERIMENTAL_API. > > Signed-off-by: Peng Zhang <peng.zhang@corigine.com> We have a similar patch in the dpdk-latest branch. https://github.com/openvswitch/ovs/commit/a8f6be98801f0c43d52173843d649df2af5e1c0d Is something wrong with it?
> -----邮件原件----- > 发件人: David Marchand <david.marchand@redhat.com> > 发送时间: 2022年12月17日 4:02 > 收件人: Simon Horman <simon.horman@corigine.com> > 抄送: dev@openvswitch.org; Eli Britstein <elibr@nvidia.com>; Chaoyong He > <chaoyong.he@corigine.com>; oss-drivers <oss-drivers@corigine.com>; Ilya > Maximets <i.maximets@ovn.org>; Nole Zhang <peng.zhang@corigine.com> > 主题: Re: [ovs-dev] [PATCH 6/6] ci: add the opts about > ALLOW_EXPERIMENTAL_API > > [You don't often get email from david.marchand@redhat.com. Learn why this > is important at https://aka.ms/LearnAboutSenderIdentification ] > > On Fri, Dec 16, 2022 at 4:52 PM Simon Horman <simon.horman@corigine.com> > wrote: > > > > From: Peng Zhang <peng.zhang@corigine.com> > > > > This commit adds support for OVS-DPDK with > -DALLOW_EXPERIMENTAL_API. > > > > Tunnel offloads and Meter offloads are experimental APIs in DPDK. To > > enable these features, compile need add -DALLOW_EXPERIMENTAL_API. So > > in workflow, we also need need the new test with > > -DALLOW_EXPERIMENTAL_API. > > > > Signed-off-by: Peng Zhang <peng.zhang@corigine.com> > > We have a similar patch in the dpdk-latest branch. > https://github.com/openvswitch/ovs/commit/a8f6be98801f0c43d52173843d > 649df2af5e1c0d > Is something wrong with it? The patch is good for me, I just didn't notice it,thanks for your notice. > > > -- > David Marchand
On 12/17/22 07:15, Nole Zhang wrote: > > >> -----邮件原件----- >> 发件人: David Marchand <david.marchand@redhat.com> >> 发送时间: 2022年12月17日 4:02 >> 收件人: Simon Horman <simon.horman@corigine.com> >> 抄送: dev@openvswitch.org; Eli Britstein <elibr@nvidia.com>; Chaoyong He >> <chaoyong.he@corigine.com>; oss-drivers <oss-drivers@corigine.com>; Ilya >> Maximets <i.maximets@ovn.org>; Nole Zhang <peng.zhang@corigine.com> >> 主题: Re: [ovs-dev] [PATCH 6/6] ci: add the opts about >> ALLOW_EXPERIMENTAL_API >> >> [You don't often get email from david.marchand@redhat.com. Learn why this >> is important at https://aka.ms/LearnAboutSenderIdentification ] >> >> On Fri, Dec 16, 2022 at 4:52 PM Simon Horman <simon.horman@corigine.com> >> wrote: >>> >>> From: Peng Zhang <peng.zhang@corigine.com> >>> >>> This commit adds support for OVS-DPDK with >> -DALLOW_EXPERIMENTAL_API. >>> >>> Tunnel offloads and Meter offloads are experimental APIs in DPDK. To >>> enable these features, compile need add -DALLOW_EXPERIMENTAL_API. So >>> in workflow, we also need need the new test with >>> -DALLOW_EXPERIMENTAL_API. >>> >>> Signed-off-by: Peng Zhang <peng.zhang@corigine.com> >> >> We have a similar patch in the dpdk-latest branch. >> https://github.com/openvswitch/ovs/commit/a8f6be98801f0c43d52173843d >> 649df2af5e1c0d >> Is something wrong with it? > > The patch is good for me, I just didn't notice it,thanks for your notice. I think, the main thing is that this patch set needs to be posted against dpdk-latest branch, i.e. has the '[PATCH dpdk-latest]' subject prefix. Changes that are using experimental DPDK features are supposed to be developed and can be accepted in that branch. We did an exception in the past and accepted experimental tunnel offloading support because it required extensive changes in many generic parts of OVS and it would be a burden trying to maintain it separately. But the time showed that it wasn't a good decision. I'm actually considering a possibility of removing that support because current DPDK API for tunnel offloading is not usable in most cases [1]. It requires changes, but not going anywhere AFAIK. [1] https://inbox.dpdk.org/dev/5248c2ca-f2a6-3fb0-38b8-7f659bfa40de@ovn.org/ Best regards, Ilya Maximets.
On Mon, Dec 19, 2022 at 03:39:50PM +0100, Ilya Maximets wrote: > On 12/17/22 07:15, Nole Zhang wrote: > > > > > >> -----邮件原件----- > >> 发件人: David Marchand <david.marchand@redhat.com> > >> 发送时间: 2022年12月17日 4:02 > >> 收件人: Simon Horman <simon.horman@corigine.com> > >> 抄送: dev@openvswitch.org; Eli Britstein <elibr@nvidia.com>; Chaoyong He > >> <chaoyong.he@corigine.com>; oss-drivers <oss-drivers@corigine.com>; Ilya > >> Maximets <i.maximets@ovn.org>; Nole Zhang <peng.zhang@corigine.com> > >> 主题: Re: [ovs-dev] [PATCH 6/6] ci: add the opts about > >> ALLOW_EXPERIMENTAL_API > >> > >> [You don't often get email from david.marchand@redhat.com. Learn why this > >> is important at https://aka.ms/LearnAboutSenderIdentification ] > >> > >> On Fri, Dec 16, 2022 at 4:52 PM Simon Horman <simon.horman@corigine.com> > >> wrote: > >>> > >>> From: Peng Zhang <peng.zhang@corigine.com> > >>> > >>> This commit adds support for OVS-DPDK with > >> -DALLOW_EXPERIMENTAL_API. > >>> > >>> Tunnel offloads and Meter offloads are experimental APIs in DPDK. To > >>> enable these features, compile need add -DALLOW_EXPERIMENTAL_API. So > >>> in workflow, we also need need the new test with > >>> -DALLOW_EXPERIMENTAL_API. > >>> > >>> Signed-off-by: Peng Zhang <peng.zhang@corigine.com> > >> > >> We have a similar patch in the dpdk-latest branch. > >> https://github.com/openvswitch/ovs/commit/a8f6be98801f0c43d52173843d > >> 649df2af5e1c0d > >> Is something wrong with it? > > > > The patch is good for me, I just didn't notice it,thanks for your notice. > > I think, the main thing is that this patch set needs to be posted > against dpdk-latest branch, i.e. has the '[PATCH dpdk-latest]' > subject prefix. Changes that are using experimental DPDK features > are supposed to be developed and can be accepted in that branch. Thanks Ilya, got it. Will do so with v2. > We did an exception in the past and accepted experimental tunnel > offloading support because it required extensive changes in many > generic parts of OVS and it would be a burden trying to maintain it > separately. But the time showed that it wasn't a good decision. > I'm actually considering a possibility of removing that support > because current DPDK API for tunnel offloading is not usable in > most cases [1]. It requires changes, but not going anywhere AFAIK. > > [1] https://inbox.dpdk.org/dev/5248c2ca-f2a6-3fb0-38b8-7f659bfa40de@ovn.org/ > > Best regards, Ilya Maximets.
diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh index 485109672388..1dc6414a162d 100755 --- a/.ci/linux-build.sh +++ b/.ci/linux-build.sh @@ -227,6 +227,10 @@ assert ovs.json.from_string('{\"a\": 42}') == {'a': 42}" exit 0 fi +if [ "$CFG_OPTS" ]; then + CFLAGS_FOR_OVS="${CFLAGS_FOR_OVS} ${CFG_OPTS}" +fi + if [ "$KERNEL" ]; then install_kernel $KERNEL fi diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index e08d7b1bac1e..d42b61acfb7e 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -20,6 +20,7 @@ jobs: M32: ${{ matrix.m32 }} OPTS: ${{ matrix.opts }} TESTSUITE: ${{ matrix.testsuite }} + CFG_OPTS: ${{ matrix.cfg_opts }} name: linux ${{ join(matrix.*, ' ') }} runs-on: ubuntu-20.04 @@ -53,9 +54,17 @@ jobs: - compiler: gcc testsuite: test dpdk: dpdk + - compiler: gcc + testsuite: test + dpdk: dpdk + cfg_opts: -DALLOW_EXPERIMENTAL_API + - compiler: clang + testsuite: test + dpdk: dpdk - compiler: clang testsuite: test dpdk: dpdk + cfg_opts: -DALLOW_EXPERIMENTAL_API - compiler: gcc testsuite: test @@ -74,21 +83,43 @@ jobs: - compiler: gcc dpdk: dpdk opts: --enable-shared + - compiler: gcc + dpdk: dpdk + opts: --enable-shared + cfg_opts: -DALLOW_EXPERIMENTAL_API + - compiler: clang + dpdk: dpdk + opts: --enable-shared - compiler: clang dpdk: dpdk opts: --enable-shared + cfg_opts: -DALLOW_EXPERIMENTAL_API - compiler: gcc dpdk_shared: dpdk-shared + - compiler: gcc + dpdk_shared: dpdk-shared + cfg_opts: -DALLOW_EXPERIMENTAL_API - compiler: clang dpdk_shared: dpdk-shared + - compiler: clang + dpdk_shared: dpdk-shared + cfg_opts: -DALLOW_EXPERIMENTAL_API - compiler: gcc dpdk_shared: dpdk-shared opts: --enable-shared + - compiler: gcc + dpdk_shared: dpdk-shared + opts: --enable-shared + cfg_opts: -DALLOW_EXPERIMENTAL_API + - compiler: clang + dpdk_shared: dpdk-shared + opts: --enable-shared - compiler: clang dpdk_shared: dpdk-shared opts: --enable-shared + cfg_opts: -DALLOW_EXPERIMENTAL_API - compiler: gcc m32: m32