diff mbox series

[ovs-dev,v2,9/9] ci: Exclude tests that show random failures through GitHub actions.

Message ID 170108887661.394857.13069626770525543995.stgit@ebuild
State Changes Requested
Headers show
Series ci: Add remaining check tests to GitHub actions. | 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

Eelco Chaudron Nov. 27, 2023, 12:41 p.m. UTC
I ran 80 series of full tests, and the following tests showed failures:

 802.1ad - vlan_limit
   +2023-11-20T10:32:11.245Z|00001|dpif_netdev(revalidator5)|ERR|internal
     error parsing flow key recirc_id(0),dp_hash(0),skb_priority(0),
     in_port(2),skb_mark(0),ct_state(0),ct_zone(0),ct_mark(0),ct_label(0),
     packet_type(ns=0,id=0),eth(src=42:7e:4b:46:68:1b,dst=33:33:ff:46:68:1b),
     eth_type(0x88a8),vlan(vid=4094,pcp=0),encap(eth_type(0x8100),
     vlan(vid=100,pcp=0),encap(eth_type(0x86dd),ipv6(
     src=::,dst=ff02::1:ff46:681b,label=0,proto=58,tclass=0,hlimit=255,
     frag=no),icmpv6(type=135,code=0),nd(target=fe80::407e:4bff:fe46:681b,
     sll=00:00:00:00:00:00,tll=00:00:00:00:00:00)))
   +2023-11-20T10:32:11.245Z|00002|dpif(revalidator5)|WARN|netdev@ovs-netdev:
     failed to put[modify] (Invalid argument)
     ufid:ef1ca90c-dbd0-4ca7-9869-411bdffd1ece recirc_id(0),dp_hash(0/0),
     skb_priority(0/0),in_port(2),skb_mark(0/0),ct_state(0/0),ct_zone(0/0),
     ct_mark(0/0),ct_label(0/0),packet_type(ns=0,id=0),
     eth(src=42:7e:4b:46:68:1b,dst=33:33:ff:46:68:1b),eth_type(0x88a8),
     vlan(vid=4094,pcp=0/0x0),encap(eth_type(0x8100),
     vlan(vid=100/0x0,pcp=0/0x0),encap(eth_type(0x86dd),
     ipv6(src=::/::,dst=ff02::1:ff46:681b/::,label=0/0,proto=58/0,
     tclass=0/0,hlimit=255/0,frag=no),icmpv6(type=135/0,code=0/0),
     nd(target=fe80::407e:4bff:fe46:681b/::,
     sll=00:00:00:00:00:00/00:00:00:00:00:00,
     tll=00:00:00:00:00:00/00:00:00:00:00:00))), actions:drop

  conntrack - zones from other field, more tests
    +2023-11-20T10:45:43.015Z|00001|dpif(handler5)|WARN|system@ovs-system:
      execute ct(commit),3 failed (Invalid argument) on packet tcp,
      vlan_tci=0x0000,dl_src=42:7e:4b:46:68:1b,dl_dst=ba:72:4c:a5:31:6b,
      nw_src=10.1.1.1,nw_dst=10.1.1.2,nw_tos=0,nw_ecn=0,nw_ttl=64,
      nw_frag=no,tp_src=53738,tp_dst=80,tcp_flags=psh|ack tcp_csum:e4a

  conntrack - limit by zone
    ./system-traffic.at:5154: ovs-appctl dpctl/ct-get-limits zone=0,1,2,3,4,5
    --- -	2023-11-20 10:51:09.965375141 +0000
    +++ /home/runner/work/ovs/ovs/tests/system-kmod-testsuite.dir/at-groups/
      114/stdout	2023-11-20 10:51:09.956723756 +0000
    @@ -1,5 +1,5 @@
     default limit=10
    -zone=0,limit=5,count=5
    +zone=0,limit=5,count=6

  conntrack - Multiple ICMP traverse
    ./system-traffic.at:7571: ovs-appctl dpctl/dump-conntrack | grep "dst=10.1.1" | sed -e 's/port=[0-9]*/port=<cleared>/g' -e 's/id=[0-9]*/id=<cleared>/g'
      -e 's/state=[0-9_A-Z]*/state=<cleared>/g' | sort | uniq
    --- -	2023-11-20 15:36:02.591051192 +0000
    +++ /home/runner/work/ovs/ovs/tests/system-kmod-testsuite.dir/at-groups/156/stdout	2023-11-20 15:36:02.585722099 +0000
    @@ -1,2 +1,9 @@
    +tcp,orig=(src=10.1.1.7,dst=13.107.43.16,sport=<cleared>,
      dport=<cleared>),reply=(src=13.107.43.16,dst=10.1.1.7,sport=<cleared>,
      dport=<cleared>),protoinfo=(state=<cleared>)
    +tcp,orig=(src=10.1.1.7,dst=168.63.129.16,sport=<cleared>,
      dport=<cleared>),reply=(src=168.63.129.16,dst=10.1.1.7,sport=<cleared>,
      dport=<cleared>),protoinfo=(state=<cleared>)
    ...
    +tcp,orig=(src=20.22.98.201,dst=10.1.1.7,sport=<cleared>,dport=<cleared>),
      reply=(src=10.1.1.7,dst=20.22.98.201,sport=<cleared>,dport=<cleared>),
      protoinfo=(state=<cleared>)

  conntrack - ct flush
    +++ /home/runner/work/ovs/ovs/tests/system-kmod-testsuite.dir/at-groups/57/stdout	2023-11-22 13:51:04.234496131 +0000
    @@ -1,3 +1,5 @@
    +tcp,orig=(src=10.1.1.154,dst=13.107.42.16,sport=45300,dport=443),
      reply=(src=13.107.42.16,dst=10.1.1.154,sport=443,dport=45300),
      protoinfo=(state=ESTABLISHED)
    +tcp,orig=(src=10.1.1.154,dst=20.72.125.48,sport=45572,dport=443),
      reply=(src=20.72.125.48,dst=10.1.1.154,sport=443,dport=45572),
      protoinfo=(state=ESTABLISHED)

As I do not see those failures when running these stand alone on the
same Ubuntu distribution, I've disabled first three tests for now.

For the other tests we narrowed the result to not include any of the local
IP addresses in the test results.

Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
---
 tests/system-traffic.at |   45 ++++++++++++++++++++++++---------------------
 1 file changed, 24 insertions(+), 21 deletions(-)

Comments

Ilya Maximets Nov. 27, 2023, 6:16 p.m. UTC | #1
On 11/27/23 13:41, Eelco Chaudron wrote:
> I ran 80 series of full tests, and the following tests showed failures:

Hi, Eelco.

It looks like this patch should go before enabling the actual testing.
Some of the previous patches in the sat failed CI because of these issues.

> 
>  802.1ad - vlan_limit
>    +2023-11-20T10:32:11.245Z|00001|dpif_netdev(revalidator5)|ERR|internal
>      error parsing flow key recirc_id(0),dp_hash(0),skb_priority(0),
>      in_port(2),skb_mark(0),ct_state(0),ct_zone(0),ct_mark(0),ct_label(0),
>      packet_type(ns=0,id=0),eth(src=42:7e:4b:46:68:1b,dst=33:33:ff:46:68:1b),
>      eth_type(0x88a8),vlan(vid=4094,pcp=0),encap(eth_type(0x8100),
>      vlan(vid=100,pcp=0),encap(eth_type(0x86dd),ipv6(
>      src=::,dst=ff02::1:ff46:681b,label=0,proto=58,tclass=0,hlimit=255,
>      frag=no),icmpv6(type=135,code=0),nd(target=fe80::407e:4bff:fe46:681b,
>      sll=00:00:00:00:00:00,tll=00:00:00:00:00:00)))
>    +2023-11-20T10:32:11.245Z|00002|dpif(revalidator5)|WARN|netdev@ovs-netdev:
>      failed to put[modify] (Invalid argument)
>      ufid:ef1ca90c-dbd0-4ca7-9869-411bdffd1ece recirc_id(0),dp_hash(0/0),
>      skb_priority(0/0),in_port(2),skb_mark(0/0),ct_state(0/0),ct_zone(0/0),
>      ct_mark(0/0),ct_label(0/0),packet_type(ns=0,id=0),
>      eth(src=42:7e:4b:46:68:1b,dst=33:33:ff:46:68:1b),eth_type(0x88a8),
>      vlan(vid=4094,pcp=0/0x0),encap(eth_type(0x8100),
>      vlan(vid=100/0x0,pcp=0/0x0),encap(eth_type(0x86dd),
>      ipv6(src=::/::,dst=ff02::1:ff46:681b/::,label=0/0,proto=58/0,
>      tclass=0/0,hlimit=255/0,frag=no),icmpv6(type=135/0,code=0/0),
>      nd(target=fe80::407e:4bff:fe46:681b/::,
>      sll=00:00:00:00:00:00/00:00:00:00:00:00,
>      tll=00:00:00:00:00:00/00:00:00:00:00:00))), actions:drop

BTW, we should be able to fix this test now, since David fixed the
revalidator pause/resume logic.

> 
>   conntrack - zones from other field, more tests
>     +2023-11-20T10:45:43.015Z|00001|dpif(handler5)|WARN|system@ovs-system:
>       execute ct(commit),3 failed (Invalid argument) on packet tcp,
>       vlan_tci=0x0000,dl_src=42:7e:4b:46:68:1b,dl_dst=ba:72:4c:a5:31:6b,
>       nw_src=10.1.1.1,nw_dst=10.1.1.2,nw_tos=0,nw_ecn=0,nw_ttl=64,
>       nw_frag=no,tp_src=53738,tp_dst=80,tcp_flags=psh|ack tcp_csum:e4a
> 
>   conntrack - limit by zone
>     ./system-traffic.at:5154: ovs-appctl dpctl/ct-get-limits zone=0,1,2,3,4,5
>     --- -	2023-11-20 10:51:09.965375141 +0000
>     +++ /home/runner/work/ovs/ovs/tests/system-kmod-testsuite.dir/at-groups/
>       114/stdout	2023-11-20 10:51:09.956723756 +0000
>     @@ -1,5 +1,5 @@
>      default limit=10
>     -zone=0,limit=5,count=5
>     +zone=0,limit=5,count=6
> 
>   conntrack - Multiple ICMP traverse
>     ./system-traffic.at:7571: ovs-appctl dpctl/dump-conntrack | grep "dst=10.1.1" | sed -e 's/port=[0-9]*/port=<cleared>/g' -e 's/id=[0-9]*/id=<cleared>/g'
>       -e 's/state=[0-9_A-Z]*/state=<cleared>/g' | sort | uniq
>     --- -	2023-11-20 15:36:02.591051192 +0000
>     +++ /home/runner/work/ovs/ovs/tests/system-kmod-testsuite.dir/at-groups/156/stdout	2023-11-20 15:36:02.585722099 +0000

Please, try to avoid such a long lines in the commit message.
Some can be split, some can be trimmed as most of the info in
them is not actually useful.

I didn't fully review the actual code change here.

Best regards, Ilya Maximets.
Eelco Chaudron Nov. 28, 2023, 7:52 a.m. UTC | #2
On 27 Nov 2023, at 19:16, Ilya Maximets wrote:

> On 11/27/23 13:41, Eelco Chaudron wrote:
>> I ran 80 series of full tests, and the following tests showed failures:
>
> Hi, Eelco.
>
> It looks like this patch should go before enabling the actual testing.
> Some of the previous patches in the sat failed CI because of these issues.

ACK I noticed too, will move this be one of the first patches.
>>
>>  802.1ad - vlan_limit
>>    +2023-11-20T10:32:11.245Z|00001|dpif_netdev(revalidator5)|ERR|internal
>>      error parsing flow key recirc_id(0),dp_hash(0),skb_priority(0),
>>      in_port(2),skb_mark(0),ct_state(0),ct_zone(0),ct_mark(0),ct_label(0),
>>      packet_type(ns=0,id=0),eth(src=42:7e:4b:46:68:1b,dst=33:33:ff:46:68:1b),
>>      eth_type(0x88a8),vlan(vid=4094,pcp=0),encap(eth_type(0x8100),
>>      vlan(vid=100,pcp=0),encap(eth_type(0x86dd),ipv6(
>>      src=::,dst=ff02::1:ff46:681b,label=0,proto=58,tclass=0,hlimit=255,
>>      frag=no),icmpv6(type=135,code=0),nd(target=fe80::407e:4bff:fe46:681b,
>>      sll=00:00:00:00:00:00,tll=00:00:00:00:00:00)))
>>    +2023-11-20T10:32:11.245Z|00002|dpif(revalidator5)|WARN|netdev@ovs-netdev:
>>      failed to put[modify] (Invalid argument)
>>      ufid:ef1ca90c-dbd0-4ca7-9869-411bdffd1ece recirc_id(0),dp_hash(0/0),
>>      skb_priority(0/0),in_port(2),skb_mark(0/0),ct_state(0/0),ct_zone(0/0),
>>      ct_mark(0/0),ct_label(0/0),packet_type(ns=0,id=0),
>>      eth(src=42:7e:4b:46:68:1b,dst=33:33:ff:46:68:1b),eth_type(0x88a8),
>>      vlan(vid=4094,pcp=0/0x0),encap(eth_type(0x8100),
>>      vlan(vid=100/0x0,pcp=0/0x0),encap(eth_type(0x86dd),
>>      ipv6(src=::/::,dst=ff02::1:ff46:681b/::,label=0/0,proto=58/0,
>>      tclass=0/0,hlimit=255/0,frag=no),icmpv6(type=135/0,code=0/0),
>>      nd(target=fe80::407e:4bff:fe46:681b/::,
>>      sll=00:00:00:00:00:00/00:00:00:00:00:00,
>>      tll=00:00:00:00:00:00/00:00:00:00:00:00))), actions:drop
>
> BTW, we should be able to fix this test now, since David fixed the
> revalidator pause/resume logic.

Let me take a look at the test. I am not sure how to verify as I can not make it fail easily.

>>
>>   conntrack - zones from other field, more tests
>>     +2023-11-20T10:45:43.015Z|00001|dpif(handler5)|WARN|system@ovs-system:
>>       execute ct(commit),3 failed (Invalid argument) on packet tcp,
>>       vlan_tci=0x0000,dl_src=42:7e:4b:46:68:1b,dl_dst=ba:72:4c:a5:31:6b,
>>       nw_src=10.1.1.1,nw_dst=10.1.1.2,nw_tos=0,nw_ecn=0,nw_ttl=64,
>>       nw_frag=no,tp_src=53738,tp_dst=80,tcp_flags=psh|ack tcp_csum:e4a
>>
>>   conntrack - limit by zone
>>     ./system-traffic.at:5154: ovs-appctl dpctl/ct-get-limits zone=0,1,2,3,4,5
>>     --- -	2023-11-20 10:51:09.965375141 +0000
>>     +++ /home/runner/work/ovs/ovs/tests/system-kmod-testsuite.dir/at-groups/
>>       114/stdout	2023-11-20 10:51:09.956723756 +0000
>>     @@ -1,5 +1,5 @@
>>      default limit=10
>>     -zone=0,limit=5,count=5
>>     +zone=0,limit=5,count=6
>>
>>   conntrack - Multiple ICMP traverse
>>     ./system-traffic.at:7571: ovs-appctl dpctl/dump-conntrack | grep "dst=10.1.1" | sed -e 's/port=[0-9]*/port=<cleared>/g' -e 's/id=[0-9]*/id=<cleared>/g'
>>       -e 's/state=[0-9_A-Z]*/state=<cleared>/g' | sort | uniq
>>     --- -	2023-11-20 15:36:02.591051192 +0000
>>     +++ /home/runner/work/ovs/ovs/tests/system-kmod-testsuite.dir/at-groups/156/stdout	2023-11-20 15:36:02.585722099 +0000
>
> Please, try to avoid such a long lines in the commit message.
> Some can be split, some can be trimmed as most of the info in
> them is not actually useful.

ACK.

> I didn't fully review the actual code change here.

I’ll wait for some comments on the previous patch replies and send a v3.

Thanks for reviewing the series!

//Eelco
Eelco Chaudron Dec. 5, 2023, 2:38 p.m. UTC | #3
On 28 Nov 2023, at 8:52, Eelco Chaudron wrote:

> On 27 Nov 2023, at 19:16, Ilya Maximets wrote:
>
>> On 11/27/23 13:41, Eelco Chaudron wrote:
>>> I ran 80 series of full tests, and the following tests showed failures:
>>
>> Hi, Eelco.
>>
>> It looks like this patch should go before enabling the actual testing.
>> Some of the previous patches in the sat failed CI because of these issues.
>
> ACK I noticed too, will move this be one of the first patches.
>>>
>>>  802.1ad - vlan_limit
>>>    +2023-11-20T10:32:11.245Z|00001|dpif_netdev(revalidator5)|ERR|internal
>>>      error parsing flow key recirc_id(0),dp_hash(0),skb_priority(0),
>>>      in_port(2),skb_mark(0),ct_state(0),ct_zone(0),ct_mark(0),ct_label(0),
>>>      packet_type(ns=0,id=0),eth(src=42:7e:4b:46:68:1b,dst=33:33:ff:46:68:1b),
>>>      eth_type(0x88a8),vlan(vid=4094,pcp=0),encap(eth_type(0x8100),
>>>      vlan(vid=100,pcp=0),encap(eth_type(0x86dd),ipv6(
>>>      src=::,dst=ff02::1:ff46:681b,label=0,proto=58,tclass=0,hlimit=255,
>>>      frag=no),icmpv6(type=135,code=0),nd(target=fe80::407e:4bff:fe46:681b,
>>>      sll=00:00:00:00:00:00,tll=00:00:00:00:00:00)))
>>>    +2023-11-20T10:32:11.245Z|00002|dpif(revalidator5)|WARN|netdev@ovs-netdev:
>>>      failed to put[modify] (Invalid argument)
>>>      ufid:ef1ca90c-dbd0-4ca7-9869-411bdffd1ece recirc_id(0),dp_hash(0/0),
>>>      skb_priority(0/0),in_port(2),skb_mark(0/0),ct_state(0/0),ct_zone(0/0),
>>>      ct_mark(0/0),ct_label(0/0),packet_type(ns=0,id=0),
>>>      eth(src=42:7e:4b:46:68:1b,dst=33:33:ff:46:68:1b),eth_type(0x88a8),
>>>      vlan(vid=4094,pcp=0/0x0),encap(eth_type(0x8100),
>>>      vlan(vid=100/0x0,pcp=0/0x0),encap(eth_type(0x86dd),
>>>      ipv6(src=::/::,dst=ff02::1:ff46:681b/::,label=0/0,proto=58/0,
>>>      tclass=0/0,hlimit=255/0,frag=no),icmpv6(type=135/0,code=0/0),
>>>      nd(target=fe80::407e:4bff:fe46:681b/::,
>>>      sll=00:00:00:00:00:00/00:00:00:00:00:00,
>>>      tll=00:00:00:00:00:00/00:00:00:00:00:00))), actions:drop
>>
>> BTW, we should be able to fix this test now, since David fixed the
>> revalidator pause/resume logic.
>
> Let me take a look at the test. I am not sure how to verify as I can not make it fail easily.
>

Looked at this, but it’s not revalidator related, it’s formatting done by the userspace part.

Tried to replicate this but was not successful outside of GitHub actions. Will add this to my TODO, and revisit it later. For now will keep it excluded in this patch set.


//Eelco

<SNIP>
Ilya Maximets Dec. 5, 2023, 2:56 p.m. UTC | #4
On 12/5/23 15:38, Eelco Chaudron wrote:
> 
> 
> On 28 Nov 2023, at 8:52, Eelco Chaudron wrote:
> 
>> On 27 Nov 2023, at 19:16, Ilya Maximets wrote:
>>
>>> On 11/27/23 13:41, Eelco Chaudron wrote:
>>>> I ran 80 series of full tests, and the following tests showed failures:
>>>
>>> Hi, Eelco.
>>>
>>> It looks like this patch should go before enabling the actual testing.
>>> Some of the previous patches in the sat failed CI because of these issues.
>>
>> ACK I noticed too, will move this be one of the first patches.
>>>>
>>>>  802.1ad - vlan_limit
>>>>    +2023-11-20T10:32:11.245Z|00001|dpif_netdev(revalidator5)|ERR|internal
>>>>      error parsing flow key recirc_id(0),dp_hash(0),skb_priority(0),
>>>>      in_port(2),skb_mark(0),ct_state(0),ct_zone(0),ct_mark(0),ct_label(0),
>>>>      packet_type(ns=0,id=0),eth(src=42:7e:4b:46:68:1b,dst=33:33:ff:46:68:1b),
>>>>      eth_type(0x88a8),vlan(vid=4094,pcp=0),encap(eth_type(0x8100),
>>>>      vlan(vid=100,pcp=0),encap(eth_type(0x86dd),ipv6(
>>>>      src=::,dst=ff02::1:ff46:681b,label=0,proto=58,tclass=0,hlimit=255,
>>>>      frag=no),icmpv6(type=135,code=0),nd(target=fe80::407e:4bff:fe46:681b,
>>>>      sll=00:00:00:00:00:00,tll=00:00:00:00:00:00)))
>>>>    +2023-11-20T10:32:11.245Z|00002|dpif(revalidator5)|WARN|netdev@ovs-netdev:
>>>>      failed to put[modify] (Invalid argument)
>>>>      ufid:ef1ca90c-dbd0-4ca7-9869-411bdffd1ece recirc_id(0),dp_hash(0/0),
>>>>      skb_priority(0/0),in_port(2),skb_mark(0/0),ct_state(0/0),ct_zone(0/0),
>>>>      ct_mark(0/0),ct_label(0/0),packet_type(ns=0,id=0),
>>>>      eth(src=42:7e:4b:46:68:1b,dst=33:33:ff:46:68:1b),eth_type(0x88a8),
>>>>      vlan(vid=4094,pcp=0/0x0),encap(eth_type(0x8100),
>>>>      vlan(vid=100/0x0,pcp=0/0x0),encap(eth_type(0x86dd),
>>>>      ipv6(src=::/::,dst=ff02::1:ff46:681b/::,label=0/0,proto=58/0,
>>>>      tclass=0/0,hlimit=255/0,frag=no),icmpv6(type=135/0,code=0/0),
>>>>      nd(target=fe80::407e:4bff:fe46:681b/::,
>>>>      sll=00:00:00:00:00:00/00:00:00:00:00:00,
>>>>      tll=00:00:00:00:00:00/00:00:00:00:00:00))), actions:drop
>>>
>>> BTW, we should be able to fix this test now, since David fixed the
>>> revalidator pause/resume logic.
>>
>> Let me take a look at the test. I am not sure how to verify as I can not make it fail easily.
>>
> 
> Looked at this, but it’s not revalidator related, it’s formatting done by the userspace part.

Well, yes, but formatting is done by the revalidator and this
formatting/parsing depends on the vlan_limit configuration.
We just need to avoid the race between revalidation and the
flow removal from the datapath.

> 
> Tried to replicate this but was not successful outside of GitHub actions. Will add this to my TODO, and revisit it later. For now will keep it excluded in this patch set.
> 
> 
> //Eelco
> 
> <SNIP>
>
Eelco Chaudron Dec. 5, 2023, 3:06 p.m. UTC | #5
On 5 Dec 2023, at 15:56, Ilya Maximets wrote:

> On 12/5/23 15:38, Eelco Chaudron wrote:
>>
>>
>> On 28 Nov 2023, at 8:52, Eelco Chaudron wrote:
>>
>>> On 27 Nov 2023, at 19:16, Ilya Maximets wrote:
>>>
>>>> On 11/27/23 13:41, Eelco Chaudron wrote:
>>>>> I ran 80 series of full tests, and the following tests showed failures:
>>>>
>>>> Hi, Eelco.
>>>>
>>>> It looks like this patch should go before enabling the actual testing.
>>>> Some of the previous patches in the sat failed CI because of these issues.
>>>
>>> ACK I noticed too, will move this be one of the first patches.
>>>>>
>>>>>  802.1ad - vlan_limit
>>>>>    +2023-11-20T10:32:11.245Z|00001|dpif_netdev(revalidator5)|ERR|internal
>>>>>      error parsing flow key recirc_id(0),dp_hash(0),skb_priority(0),
>>>>>      in_port(2),skb_mark(0),ct_state(0),ct_zone(0),ct_mark(0),ct_label(0),
>>>>>      packet_type(ns=0,id=0),eth(src=42:7e:4b:46:68:1b,dst=33:33:ff:46:68:1b),
>>>>>      eth_type(0x88a8),vlan(vid=4094,pcp=0),encap(eth_type(0x8100),
>>>>>      vlan(vid=100,pcp=0),encap(eth_type(0x86dd),ipv6(
>>>>>      src=::,dst=ff02::1:ff46:681b,label=0,proto=58,tclass=0,hlimit=255,
>>>>>      frag=no),icmpv6(type=135,code=0),nd(target=fe80::407e:4bff:fe46:681b,
>>>>>      sll=00:00:00:00:00:00,tll=00:00:00:00:00:00)))
>>>>>    +2023-11-20T10:32:11.245Z|00002|dpif(revalidator5)|WARN|netdev@ovs-netdev:
>>>>>      failed to put[modify] (Invalid argument)
>>>>>      ufid:ef1ca90c-dbd0-4ca7-9869-411bdffd1ece recirc_id(0),dp_hash(0/0),
>>>>>      skb_priority(0/0),in_port(2),skb_mark(0/0),ct_state(0/0),ct_zone(0/0),
>>>>>      ct_mark(0/0),ct_label(0/0),packet_type(ns=0,id=0),
>>>>>      eth(src=42:7e:4b:46:68:1b,dst=33:33:ff:46:68:1b),eth_type(0x88a8),
>>>>>      vlan(vid=4094,pcp=0/0x0),encap(eth_type(0x8100),
>>>>>      vlan(vid=100/0x0,pcp=0/0x0),encap(eth_type(0x86dd),
>>>>>      ipv6(src=::/::,dst=ff02::1:ff46:681b/::,label=0/0,proto=58/0,
>>>>>      tclass=0/0,hlimit=255/0,frag=no),icmpv6(type=135/0,code=0/0),
>>>>>      nd(target=fe80::407e:4bff:fe46:681b/::,
>>>>>      sll=00:00:00:00:00:00/00:00:00:00:00:00,
>>>>>      tll=00:00:00:00:00:00/00:00:00:00:00:00))), actions:drop
>>>>
>>>> BTW, we should be able to fix this test now, since David fixed the
>>>> revalidator pause/resume logic.
>>>
>>> Let me take a look at the test. I am not sure how to verify as I can not make it fail easily.
>>>
>>
>> Looked at this, but it’s not revalidator related, it’s formatting done by the userspace part.
>
> Well, yes, but formatting is done by the revalidator and this
> formatting/parsing depends on the vlan_limit configuration.
> We just need to avoid the race between revalidation and the
> flow removal from the datapath.

That was my initial thought also so I made some changes to the netdev-specific function dpif_netdev_mask_from_nlattrs/dpif_netdev_flow_from_nlattrs (in line with what was done for tc). But it did not seem to solve the problem.

I’ll work on this outside of this patchset, and exclude it for now.

>>
>> Tried to replicate this but was not successful outside of GitHub actions. Will add this to my TODO, and revisit it later. For now will keep it excluded in this patch set.
>>
>>
>> //Eelco
>>
>> <SNIP>
>>
diff mbox series

Patch

diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index a7d4ed83b..459e6e07b 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -2584,133 +2584,133 @@  AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 packet=50540000000a5
 AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=2 packet=50540000000a50540000000908004500001c000000000011a4cd0a0101020a0101010002000100080000 actions=resubmit(,0)"])
 
 
-AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1" | sort], [0], [dnl
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1," | sort], [0], [dnl
 udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2),reply=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1)
 udp,orig=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1),reply=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2),zone=5
 ])
 
 AT_CHECK([FLUSH_CMD 'ct_nw_proto=17,ct_tp_src=1'])
 
-AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1"], [0], [dnl
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1,"], [0], [dnl
 udp,orig=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1),reply=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2),zone=5
 ])
 
 AT_CHECK([FLUSH_CMD 'ct_nw_proto=17,ct_tp_src=2'])
 
-AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1"], [1])
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1,"], [1])
 
 dnl Test UDP from port 1 and 2, partial flush by dst port
 AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 packet=50540000000a50540000000908004500001c000000000011a4cd0a0101010a0101020001000200080000 actions=resubmit(,0)"])
 AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=2 packet=50540000000a50540000000908004500001c000000000011a4cd0a0101020a0101010002000100080000 actions=resubmit(,0)"])
 
 
-AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1" | sort], [0], [dnl
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1," | sort], [0], [dnl
 udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2),reply=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1)
 udp,orig=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1),reply=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2),zone=5
 ])
 
 AT_CHECK([FLUSH_CMD 'ct_nw_proto=17,ct_tp_dst=2'])
 
-AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1"], [0], [dnl
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1,"], [0], [dnl
 udp,orig=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1),reply=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2),zone=5
 ])
 
 AT_CHECK([FLUSH_CMD 'ct_nw_proto=17,ct_tp_dst=1'])
 
-AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1"], [1])
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1,"], [1])
 
 dnl Test UDP from port 1 and 2, partial flush by src address
 AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 packet=50540000000a50540000000908004500001c000000000011a4cd0a0101010a0101020001000200080000 actions=resubmit(,0)"])
 AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=2 packet=50540000000a50540000000908004500001c000000000011a4cd0a0101020a0101010002000100080000 actions=resubmit(,0)"])
 
 
-AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1" | sort], [0], [dnl
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1," | sort], [0], [dnl
 udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2),reply=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1)
 udp,orig=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1),reply=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2),zone=5
 ])
 
 AT_CHECK([FLUSH_CMD 'ct_nw_src=10.1.1.1'])
 
-AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1"], [0], [dnl
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1,"], [0], [dnl
 udp,orig=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1),reply=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2),zone=5
 ])
 
 AT_CHECK([FLUSH_CMD 'ct_nw_src=10.1.1.2'])
 
-AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1"], [1])
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1,"], [1])
 
 dnl Test UDP from port 1 and 2, partial flush by dst address
 AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 packet=50540000000a50540000000908004500001c000000000011a4cd0a0101010a0101020001000200080000 actions=resubmit(,0)"])
 AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=2 packet=50540000000a50540000000908004500001c000000000011a4cd0a0101020a0101010002000100080000 actions=resubmit(,0)"])
 
 
-AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1" | sort], [0], [dnl
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1," | sort], [0], [dnl
 udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2),reply=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1)
 udp,orig=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1),reply=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2),zone=5
 ])
 
 AT_CHECK([FLUSH_CMD 'ct_nw_dst=10.1.1.2'])
 
-AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1"], [0], [dnl
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1,"], [0], [dnl
 udp,orig=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1),reply=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2),zone=5
 ])
 
 AT_CHECK([FLUSH_CMD 'ct_nw_dst=10.1.1.1'])
 
-AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1"], [1])
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1,"], [1])
 
 dnl Test UDP from port 1 and 2, partial flush by src address in reply direction
 AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 packet=50540000000a50540000000908004500001c000000000011a4cd0a0101010a0101020001000200080000 actions=resubmit(,0)"])
 AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=2 packet=50540000000a50540000000908004500001c000000000011a4cd0a0101020a0101010002000100080000 actions=resubmit(,0)"])
 
 
-AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1" | sort], [0], [dnl
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1," | sort], [0], [dnl
 udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2),reply=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1)
 udp,orig=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1),reply=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2),zone=5
 ])
 
 AT_CHECK([FLUSH_CMD '' 'ct_nw_src=10.1.1.2'])
 
-AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1"], [0], [dnl
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1,"], [0], [dnl
 udp,orig=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1),reply=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2),zone=5
 ])
 
 AT_CHECK([FLUSH_CMD zone=5 '' 'ct_nw_src=10.1.1.1'])
 
-AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1"], [1])
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1,"], [1])
 
 dnl Test UDP from port 1 and 2, flush without arguments
 AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 packet=50540000000a50540000000908004500001c000000000011a4cd0a0101010a0101020001000200080000 actions=resubmit(,0)"])
 AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=2 packet=50540000000a50540000000908004500001c000000000011a4cd0a0101020a0101010002000100080000 actions=resubmit(,0)"])
 
 
-AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1" | sort], [0], [dnl
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1," | sort], [0], [dnl
 udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2),reply=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1)
 udp,orig=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1),reply=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2),zone=5
 ])
 
 AT_CHECK([FLUSH_CMD])
 
-AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1"], [1])
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1,"], [1])
 
 dnl Test SCTP flush based on port.
 AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 packet=50540000000a50540000000908004500003400010000408464410a0101010a01010200010002000000009178f7d30100001470e18ccc00000000000a000a00000000 actions=resubmit(,0)"])
 AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=2 packet=50540000000950540000000a08004500003400010000408464410a0101020a010101000200010000000098f29e470100001470e18ccc00000000000a000a00000000 actions=resubmit(,0)"])
 
-AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1" | sed "s/,protoinfo=.*$//" | sort], [0], [dnl
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1," | sed "s/,protoinfo=.*$//" | sort], [0], [dnl
 sctp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2),reply=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1)
 sctp,orig=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1),reply=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2),zone=5
 ])
 
 AT_CHECK([FLUSH_CMD 'ct_nw_src=10.1.1.1,ct_nw_proto=132,ct_tp_src=1,ct_tp_dst=2'])
 
-AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1" | sed "s/,protoinfo=.*$//" | sort], [0], [dnl
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1," | sed "s/,protoinfo=.*$//" | sort], [0], [dnl
 sctp,orig=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1),reply=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2),zone=5
 ])
 
 AT_CHECK([FLUSH_CMD 'ct_nw_src=10.1.1.2,ct_nw_proto=132,ct_tp_src=2,ct_tp_dst=1'])
 
-AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1"], [1])
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1,"], [1])
 ])
 
 dnl Test flush with invalid arguments
@@ -3155,6 +3155,7 @@  AT_CLEANUP
 
 AT_SETUP([conntrack - zones from other field, more tests])
 CHECK_CONNTRACK()
+OVS_CHECK_GITHUB_ACTION()
 OVS_TRAFFIC_VSWITCHD_START()
 
 ADD_NAMESPACES(at_ns0, at_ns1)
@@ -5114,6 +5115,7 @@  AT_CLEANUP
 
 AT_SETUP([conntrack - limit by zone])
 CHECK_CONNTRACK()
+OVS_CHECK_GITHUB_ACTION()
 OVS_TRAFFIC_VSWITCHD_START()
 
 ADD_NAMESPACES(at_ns0, at_ns1)
@@ -7566,7 +7568,7 @@  NS_CHECK_EXEC([at_ns0], [$PYTHON3 $srcdir/sendpkt.py p0 f0 00 00 01 01 02 f0 00
 sleep 1
 
 dnl ensure CT picked up the packet
-AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1)], [0], [dnl
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2)], [0], [dnl
 icmp,orig=(src=10.1.1.1,dst=10.1.1.2,id=<cleared>,type=8,code=0),reply=(src=10.1.1.2,dst=10.1.1.1,id=<cleared>,type=0,code=0)
 ])
 
@@ -7815,6 +7817,7 @@  AT_CLEANUP
 AT_BANNER([802.1ad])
 
 AT_SETUP([802.1ad - vlan_limit])
+OVS_CHECK_GITHUB_ACTION()
 OVS_TRAFFIC_VSWITCHD_START([set Open_vSwitch . other_config:vlan-limit=0])
 OVS_CHECK_8021AD()