diff mbox series

[ovs-dev,6/6] ci: add the opts about ALLOW_EXPERIMENTAL_API

Message ID 20221216155054.986464-7-simon.horman@corigine.com
State Changes Requested
Headers show
Series Add support for DPDK meter HW offload | 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 fail test: fail

Commit Message

Simon Horman Dec. 16, 2022, 3:50 p.m. UTC 
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>
---
 .ci/linux-build.sh                   |  4 ++++
 .github/workflows/build-and-test.yml | 31 ++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+)

Comments

David Marchand Dec. 16, 2022, 8:01 p.m. UTC | #1 
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?
Nole Zhang Dec. 17, 2022, 6:15 a.m. UTC | #2 
> -----邮件原件-----
> 发件人: 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
Ilya Maximets Dec. 19, 2022, 2:39 p.m. UTC | #3 
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.
Simon Horman Dec. 20, 2022, 10:17 a.m. UTC | #4 
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 mbox series

Patch

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