diff mbox series

[ovs-dev,PATCHv2] tests: Skip tests using tcp nc under check-afxdp.

Message ID 1572650860-73811-1-git-send-email-u9012063@gmail.com
State Rejected
Headers show
Series [ovs-dev,PATCHv2] tests: Skip tests using tcp nc under check-afxdp. | expand

Commit Message

William Tu Nov. 1, 2019, 11:27 p.m. UTC
AF_XDP veth does not support TCP with namespaces.
This patch skips two cases that use it.
  118: conntrack - floating IP
  119: conntrack - negative test for recirculation optimization

Signed-off-by: William Tu <u9012063@gmail.com>
---
v2:
  - skip only 118 and 119.
v1:
  - Tested-at: https://travis-ci.org/williamtu/ovs-travis/builds/606194550
  - make check-afxdp all pass on my machine, but cirrus, it is still unstable,
    observe 1 failed at:
    https://cirrus-ci.com/task/6597038589870080
---
 tests/system-afxdp-macros.at     | 6 ++++++
 tests/system-kmod-macros.at      | 6 ++++++
 tests/system-traffic.at          | 2 ++
 tests/system-userspace-macros.at | 6 ++++++
 4 files changed, 20 insertions(+)

Comments

Eelco Chaudron Nov. 4, 2019, 10:56 a.m. UTC | #1
William,

I was wondering if you could add some more details to the documentation 
why it’s not working/supported.
Currently, all that is in the documentation is this:

“Due to limitations of current upstream kernel, TCP and various 
offloading (vlan, cvlan) is not working over virtual interfaces (i.e. 
veth pair).”

Cheers,

Eelco

On 2 Nov 2019, at 0:27, William Tu wrote:

> AF_XDP veth does not support TCP with namespaces.
> This patch skips two cases that use it.
>   118: conntrack - floating IP
>   119: conntrack - negative test for recirculation optimization
>
> Signed-off-by: William Tu <u9012063@gmail.com>
> ---
> v2:
>   - skip only 118 and 119.
> v1:
>   - Tested-at: 
> https://travis-ci.org/williamtu/ovs-travis/builds/606194550
>   - make check-afxdp all pass on my machine, but cirrus, it is still 
> unstable,
>     observe 1 failed at:
>     https://cirrus-ci.com/task/6597038589870080
> ---
>  tests/system-afxdp-macros.at     | 6 ++++++
>  tests/system-kmod-macros.at      | 6 ++++++
>  tests/system-traffic.at          | 2 ++
>  tests/system-userspace-macros.at | 6 ++++++
>  4 files changed, 20 insertions(+)
>
> diff --git a/tests/system-afxdp-macros.at 
> b/tests/system-afxdp-macros.at
> index f0683c0a901b..3392c7e5ada3 100644
> --- a/tests/system-afxdp-macros.at
> +++ b/tests/system-afxdp-macros.at
> @@ -37,3 +37,9 @@ m4_define([CONFIGURE_VETH_OFFLOADS],
>  #
>  m4_define([OVS_START_L7],
>     [AT_SKIP_IF([:])])
> +
> +# OVS_SKIP_AFXDP()
> +#
> +# Skip when check-afxdp.
> +m4_define([OVS_SKIP_AFXDP],
> +    [AT_SKIP_IF([:])])
> diff --git a/tests/system-kmod-macros.at b/tests/system-kmod-macros.at
> index 9e89aec43734..cfcd63ab0bab 100644
> --- a/tests/system-kmod-macros.at
> +++ b/tests/system-kmod-macros.at
> @@ -211,3 +211,9 @@ m4_define([VSCTL_ADD_DATAPATH_TABLE],
>  # or necessary for the userspace datapath as it is checking for a 
> kernel
>  # specific regression.
>  m4_define([CHECK_L3L4_CONNTRACK_REASM])
> +
> +# OVS_SKIP_AFXDP()
> +#
> +# Skip when check-afxdp.
> +m4_define([OVS_SKIP_AFXDP],
> +    [AT_SKIP_IF([false])])
> diff --git a/tests/system-traffic.at b/tests/system-traffic.at
> index 870a05efe04c..32af74c9953b 100644
> --- a/tests/system-traffic.at
> +++ b/tests/system-traffic.at
> @@ -5660,6 +5660,7 @@ AT_CLEANUP
>
>  AT_SETUP([conntrack - floating IP])
>  AT_SKIP_IF([test $HAVE_NC = no])
> +OVS_SKIP_AFXDP()
>  CHECK_CONNTRACK()
>  OVS_TRAFFIC_VSWITCHD_START()
>  OVS_CHECK_CT_CLEAR()
> @@ -5735,6 +5736,7 @@ AT_SETUP([conntrack - negative test for 
> recirculation optimization])
>  dnl This test will fail if 'conn' caching is being used, because the 
> tuple
>  dnl has been changed outside of conntrack.
>  AT_SKIP_IF([test $HAVE_NC = no])
> +OVS_SKIP_AFXDP()
>  CHECK_CONNTRACK()
>  OVS_TRAFFIC_VSWITCHD_START()
>  OVS_CHECK_CT_CLEAR()
> diff --git a/tests/system-userspace-macros.at 
> b/tests/system-userspace-macros.at
> index a419f30c1563..6cf2d70ea046 100644
> --- a/tests/system-userspace-macros.at
> +++ b/tests/system-userspace-macros.at
> @@ -323,3 +323,9 @@ m4_define([CHECK_L3L4_CONNTRACK_REASM],
>  [
>      AT_SKIP_IF([:])
>  ])
> +
> +# OVS_SKIP_AFXDP()
> +#
> +# Skip when check-afxdp.
> +m4_define([OVS_SKIP_AFXDP],
> +    [AT_SKIP_IF([false])])
> -- 
> 2.7.4
William Tu Nov. 4, 2019, 4:17 p.m. UTC | #2
On Mon, Nov 4, 2019 at 2:56 AM Eelco Chaudron <echaudro@redhat.com> wrote:
>
> William,
>
> I was wondering if you could add some more details to the documentation
> why it’s not working/supported.
> Currently, all that is in the documentation is this:
>
> “Due to limitations of current upstream kernel, TCP and various
> offloading (vlan, cvlan) is not working over virtual interfaces (i.e.
> veth pair).”
>
> Cheers,
>

Sure. Thanks.
I should add it in the commit message and also in afxdp.rst.

TCP doesn't work with XDP over veth interfaces. This is a known kernel
issue that could be work arounded by the following patch to a kernel:
    https://github.com/cilium/cilium/issues/3077#issuecomment-430801467 .
Until proper solution implemented in kernel we probably should skip all the
tests that involves TCP.

William
Ilya Maximets Nov. 4, 2019, 4:58 p.m. UTC | #3
On 04.11.2019 17:17, William Tu wrote:
> On Mon, Nov 4, 2019 at 2:56 AM Eelco Chaudron <echaudro@redhat.com> wrote:
>>
>> William,
>>
>> I was wondering if you could add some more details to the documentation
>> why it’s not working/supported.
>> Currently, all that is in the documentation is this:
>>
>> “Due to limitations of current upstream kernel, TCP and various
>> offloading (vlan, cvlan) is not working over virtual interfaces (i.e.
>> veth pair).”
>>
>> Cheers,
>>
> 
> Sure. Thanks.
> I should add it in the commit message and also in afxdp.rst.
> 
> TCP doesn't work with XDP over veth interfaces. This is a known kernel
> issue that could be work arounded by the following patch to a kernel:
>      https://github.com/cilium/cilium/issues/3077#issuecomment-430801467 .
> Until proper solution implemented in kernel we probably should skip all the
> tests that involves TCP.

BTW, as I told previously, I'm working on best-effort xdp-mode configuration.
And it will allow us to unskip all the tcp tests because native XDP for veth
works just fine. Native means "drv" without zerocopy, which is impossible
configuration for now in OVS. I'm testing it in my local setup and http tests
works in this mode.

Best regards, Ilya Maximets.
Eelco Chaudron Nov. 4, 2019, 5:05 p.m. UTC | #4
On 4 Nov 2019, at 17:58, Ilya Maximets wrote:

> On 04.11.2019 17:17, William Tu wrote:
>> On Mon, Nov 4, 2019 at 2:56 AM Eelco Chaudron <echaudro@redhat.com> 
>> wrote:
>>>
>>> William,
>>>
>>> I was wondering if you could add some more details to the 
>>> documentation
>>> why it’s not working/supported.
>>> Currently, all that is in the documentation is this:
>>>
>>> “Due to limitations of current upstream kernel, TCP and various
>>> offloading (vlan, cvlan) is not working over virtual interfaces 
>>> (i.e.
>>> veth pair).”
>>>
>>> Cheers,
>>>
>>
>> Sure. Thanks.
>> I should add it in the commit message and also in afxdp.rst.
>>
>> TCP doesn't work with XDP over veth interfaces. This is a known 
>> kernel
>> issue that could be work arounded by the following patch to a kernel:
>>      https://github.com/cilium/cilium/issues/3077#issuecomment-430801467 
>> .
>> Until proper solution implemented in kernel we probably should skip 
>> all the
>> tests that involves TCP.
>
> BTW, as I told previously, I'm working on best-effort xdp-mode 
> configuration.
> And it will allow us to unskip all the tcp tests because native XDP 
> for veth
> works just fine. Native means "drv" without zerocopy, which is 
> impossible
> configuration for now in OVS. I'm testing it in my local setup and 
> http tests
> works in this mode.

All good info to add to the doc… Thanks!
William Tu Nov. 4, 2019, 7:08 p.m. UTC | #5
On Mon, Nov 4, 2019 at 9:05 AM Eelco Chaudron <echaudro@redhat.com> wrote:
>
>
>
> On 4 Nov 2019, at 17:58, Ilya Maximets wrote:
>
> > On 04.11.2019 17:17, William Tu wrote:
> >> On Mon, Nov 4, 2019 at 2:56 AM Eelco Chaudron <echaudro@redhat.com>
> >> wrote:
> >>>
> >>> William,
> >>>
> >>> I was wondering if you could add some more details to the
> >>> documentation
> >>> why it’s not working/supported.
> >>> Currently, all that is in the documentation is this:
> >>>
> >>> “Due to limitations of current upstream kernel, TCP and various
> >>> offloading (vlan, cvlan) is not working over virtual interfaces
> >>> (i.e.
> >>> veth pair).”
> >>>
> >>> Cheers,
> >>>
> >>
> >> Sure. Thanks.
> >> I should add it in the commit message and also in afxdp.rst.
> >>
> >> TCP doesn't work with XDP over veth interfaces. This is a known
> >> kernel
> >> issue that could be work arounded by the following patch to a kernel:
> >>      https://github.com/cilium/cilium/issues/3077#issuecomment-430801467
> >> .
> >> Until proper solution implemented in kernel we probably should skip
> >> all the
> >> tests that involves TCP.
> >
> > BTW, as I told previously, I'm working on best-effort xdp-mode
> > configuration.
> > And it will allow us to unskip all the tcp tests because native XDP
> > for veth
> > works just fine. Native means "drv" without zerocopy, which is
> > impossible
> > configuration for now in OVS. I'm testing it in my local setup and
> > http tests
> > works in this mode.

Hi Ilya,

Then so should I wait for your patch?
Once your drv-copy-mode (drv without zerocopy) is supported, then TCP
with veth + drv-copy-mode should work.
Then we don't need to disable this NC TCP case.

Regards,
William
Ilya Maximets Nov. 4, 2019, 7:41 p.m. UTC | #6
On 04.11.2019 20:08, William Tu wrote:
> On Mon, Nov 4, 2019 at 9:05 AM Eelco Chaudron <echaudro@redhat.com> wrote:
>>
>>
>>
>> On 4 Nov 2019, at 17:58, Ilya Maximets wrote:
>>
>>> On 04.11.2019 17:17, William Tu wrote:
>>>> On Mon, Nov 4, 2019 at 2:56 AM Eelco Chaudron <echaudro@redhat.com>
>>>> wrote:
>>>>>
>>>>> William,
>>>>>
>>>>> I was wondering if you could add some more details to the
>>>>> documentation
>>>>> why it’s not working/supported.
>>>>> Currently, all that is in the documentation is this:
>>>>>
>>>>> “Due to limitations of current upstream kernel, TCP and various
>>>>> offloading (vlan, cvlan) is not working over virtual interfaces
>>>>> (i.e.
>>>>> veth pair).”
>>>>>
>>>>> Cheers,
>>>>>
>>>>
>>>> Sure. Thanks.
>>>> I should add it in the commit message and also in afxdp.rst.
>>>>
>>>> TCP doesn't work with XDP over veth interfaces. This is a known
>>>> kernel
>>>> issue that could be work arounded by the following patch to a kernel:
>>>>       https://github.com/cilium/cilium/issues/3077#issuecomment-430801467
>>>> .
>>>> Until proper solution implemented in kernel we probably should skip
>>>> all the
>>>> tests that involves TCP.
>>>
>>> BTW, as I told previously, I'm working on best-effort xdp-mode
>>> configuration.
>>> And it will allow us to unskip all the tcp tests because native XDP
>>> for veth
>>> works just fine. Native means "drv" without zerocopy, which is
>>> impossible
>>> configuration for now in OVS. I'm testing it in my local setup and
>>> http tests
>>> works in this mode.
> 
> Hi Ilya,
> 
> Then so should I wait for your patch?
> Once your drv-copy-mode (drv without zerocopy) is supported, then TCP
> with veth + drv-copy-mode should work.
> Then we don't need to disable this NC TCP case.

This might make sense to wait.
Basically "drv-copy-mode" on veth interfaces should be available starting
4.19 kernel, but we're suggesting kernel 5.0+ for running unit tests, so
we should not have issues with running tcp tests on those kernels.
I didn't test exact kernel versions though.

I hope to finish the patch until the end of this week.

Best regards, Ilya Maximets.
diff mbox series

Patch

diff --git a/tests/system-afxdp-macros.at b/tests/system-afxdp-macros.at
index f0683c0a901b..3392c7e5ada3 100644
--- a/tests/system-afxdp-macros.at
+++ b/tests/system-afxdp-macros.at
@@ -37,3 +37,9 @@  m4_define([CONFIGURE_VETH_OFFLOADS],
 #
 m4_define([OVS_START_L7],
    [AT_SKIP_IF([:])])
+
+# OVS_SKIP_AFXDP()
+#
+# Skip when check-afxdp.
+m4_define([OVS_SKIP_AFXDP],
+    [AT_SKIP_IF([:])])
diff --git a/tests/system-kmod-macros.at b/tests/system-kmod-macros.at
index 9e89aec43734..cfcd63ab0bab 100644
--- a/tests/system-kmod-macros.at
+++ b/tests/system-kmod-macros.at
@@ -211,3 +211,9 @@  m4_define([VSCTL_ADD_DATAPATH_TABLE],
 # or necessary for the userspace datapath as it is checking for a kernel
 # specific regression.
 m4_define([CHECK_L3L4_CONNTRACK_REASM])
+
+# OVS_SKIP_AFXDP()
+#
+# Skip when check-afxdp.
+m4_define([OVS_SKIP_AFXDP],
+    [AT_SKIP_IF([false])])
diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index 870a05efe04c..32af74c9953b 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -5660,6 +5660,7 @@  AT_CLEANUP
 
 AT_SETUP([conntrack - floating IP])
 AT_SKIP_IF([test $HAVE_NC = no])
+OVS_SKIP_AFXDP()
 CHECK_CONNTRACK()
 OVS_TRAFFIC_VSWITCHD_START()
 OVS_CHECK_CT_CLEAR()
@@ -5735,6 +5736,7 @@  AT_SETUP([conntrack - negative test for recirculation optimization])
 dnl This test will fail if 'conn' caching is being used, because the tuple
 dnl has been changed outside of conntrack.
 AT_SKIP_IF([test $HAVE_NC = no])
+OVS_SKIP_AFXDP()
 CHECK_CONNTRACK()
 OVS_TRAFFIC_VSWITCHD_START()
 OVS_CHECK_CT_CLEAR()
diff --git a/tests/system-userspace-macros.at b/tests/system-userspace-macros.at
index a419f30c1563..6cf2d70ea046 100644
--- a/tests/system-userspace-macros.at
+++ b/tests/system-userspace-macros.at
@@ -323,3 +323,9 @@  m4_define([CHECK_L3L4_CONNTRACK_REASM],
 [
     AT_SKIP_IF([:])
 ])
+
+# OVS_SKIP_AFXDP()
+#
+# Skip when check-afxdp.
+m4_define([OVS_SKIP_AFXDP],
+    [AT_SKIP_IF([false])])