Re: [ovs-dev] [PATCH] tunnel-push-pop.at: Mask source port in tunnel header.

2021-10-13 Thread Ilya Maximets
On 10/13/21 17:08, Timothy Redaelli wrote:
> On Tue, 12 Oct 2021 15:33:07 +0200
> Ilya Maximets  wrote:
> 
>> Source port is based on a packet hash and hash depends on a chosen
>> implementation.  Masking it to avoid test failures with '-msse4.2'.
>>
>> Fixes: 7e6b41ac8d9d ("dpif-netdev: Fix crash when PACKET_OUT is metered.")
>> Reported-by: Kumar Amber 
>> Signed-off-by: Ilya Maximets 
>> ---
>>  tests/tunnel-push-pop.at | 10 ++
>>  1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/tests/tunnel-push-pop.at b/tests/tunnel-push-pop.at
>> index 12fc1ef91..636465397 100644
>> --- a/tests/tunnel-push-pop.at
>> +++ b/tests/tunnel-push-pop.at
>> @@ -628,20 +628,22 @@ AT_CHECK([
>>  AT_CHECK([ovs-vsctl -- set Interface p0 options:tx_pcap=p0.pcap])
>>  
>>  packet=5054000a50540009123
>> -encap=f8bc124434b6aa55aa5508004532400040113406010102580101025c83a917c1001e65587b00
>> +dnl Source port is based on a packet hash, so it may differ depending on the
>> +dnl compiler flags and CPU type.  Masked with ''.
>> +encap=f8bc124434b6aa55aa5508004532400040113406010102580101025c17c1001e65587b00
>>  
>>  dnl Output to tunnel from a int-br internal port.
>>  dnl Checking that the packet arrived and it was correctly encapsulated.
>>  AT_CHECK([ovs-ofctl add-flow int-br 
>> "in_port=LOCAL,actions=debug_slow,output:2"])
>>  AT_CHECK([ovs-appctl netdev-dummy/receive int-br "${packet}4"])
>> -OVS_WAIT_UNTIL([test `ovs-pcap p0.pcap | grep "${encap}${packet}4" | wc -l` 
>> -ge 1])
>> +OVS_WAIT_UNTIL([test `ovs-pcap p0.pcap | egrep "${encap}${packet}4" | wc 
>> -l` -ge 1])
>>  dnl Sending again to exercise the non-miss upcall path.
>>  AT_CHECK([ovs-appctl netdev-dummy/receive int-br "${packet}4"])
>> -OVS_WAIT_UNTIL([test `ovs-pcap p0.pcap | grep "${encap}${packet}4" | wc -l` 
>> -ge 2])
>> +OVS_WAIT_UNTIL([test `ovs-pcap p0.pcap | egrep "${encap}${packet}4" | wc 
>> -l` -ge 2])
>>  
>>  dnl Output to tunnel from the controller.
>>  AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out int-br CONTROLLER 
>> "debug_slow,output:2" "${packet}5"])
>> -OVS_WAIT_UNTIL([test `ovs-pcap p0.pcap | grep "${encap}${packet}5" | wc -l` 
>> -ge 1])
>> +OVS_WAIT_UNTIL([test `ovs-pcap p0.pcap | egrep "${encap}${packet}5" | wc 
>> -l` -ge 1])
>>  
>>  dnl Datapath actions should not have tunnel push action.
>>  AT_CHECK([ovs-appctl dpctl/dump-flows | grep -q tnl_push], [1])
> 
> Hi,
> egrep should not be needed in this case, since . is part of basic
> regexp (see man 1 grep and [1]) and so plain grep is enough.
> 
> [1] 
> https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap09.html#tag_09_03_04
> 

Oh, cool!  Thanks for pointing that out.
This patch is already applied, but I'll keep that in mind for
the future changes.

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] tunnel-push-pop.at: Mask source port in tunnel header.

2021-10-13 Thread Timothy Redaelli
On Tue, 12 Oct 2021 15:33:07 +0200
Ilya Maximets  wrote:

> Source port is based on a packet hash and hash depends on a chosen
> implementation.  Masking it to avoid test failures with '-msse4.2'.
> 
> Fixes: 7e6b41ac8d9d ("dpif-netdev: Fix crash when PACKET_OUT is metered.")
> Reported-by: Kumar Amber 
> Signed-off-by: Ilya Maximets 
> ---
>  tests/tunnel-push-pop.at | 10 ++
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/tunnel-push-pop.at b/tests/tunnel-push-pop.at
> index 12fc1ef91..636465397 100644
> --- a/tests/tunnel-push-pop.at
> +++ b/tests/tunnel-push-pop.at
> @@ -628,20 +628,22 @@ AT_CHECK([
>  AT_CHECK([ovs-vsctl -- set Interface p0 options:tx_pcap=p0.pcap])
>  
>  packet=5054000a50540009123
> -encap=f8bc124434b6aa55aa5508004532400040113406010102580101025c83a917c1001e65587b00
> +dnl Source port is based on a packet hash, so it may differ depending on the
> +dnl compiler flags and CPU type.  Masked with ''.
> +encap=f8bc124434b6aa55aa5508004532400040113406010102580101025c17c1001e65587b00
>  
>  dnl Output to tunnel from a int-br internal port.
>  dnl Checking that the packet arrived and it was correctly encapsulated.
>  AT_CHECK([ovs-ofctl add-flow int-br 
> "in_port=LOCAL,actions=debug_slow,output:2"])
>  AT_CHECK([ovs-appctl netdev-dummy/receive int-br "${packet}4"])
> -OVS_WAIT_UNTIL([test `ovs-pcap p0.pcap | grep "${encap}${packet}4" | wc -l` 
> -ge 1])
> +OVS_WAIT_UNTIL([test `ovs-pcap p0.pcap | egrep "${encap}${packet}4" | wc -l` 
> -ge 1])
>  dnl Sending again to exercise the non-miss upcall path.
>  AT_CHECK([ovs-appctl netdev-dummy/receive int-br "${packet}4"])
> -OVS_WAIT_UNTIL([test `ovs-pcap p0.pcap | grep "${encap}${packet}4" | wc -l` 
> -ge 2])
> +OVS_WAIT_UNTIL([test `ovs-pcap p0.pcap | egrep "${encap}${packet}4" | wc -l` 
> -ge 2])
>  
>  dnl Output to tunnel from the controller.
>  AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out int-br CONTROLLER 
> "debug_slow,output:2" "${packet}5"])
> -OVS_WAIT_UNTIL([test `ovs-pcap p0.pcap | grep "${encap}${packet}5" | wc -l` 
> -ge 1])
> +OVS_WAIT_UNTIL([test `ovs-pcap p0.pcap | egrep "${encap}${packet}5" | wc -l` 
> -ge 1])
>  
>  dnl Datapath actions should not have tunnel push action.
>  AT_CHECK([ovs-appctl dpctl/dump-flows | grep -q tnl_push], [1])

Hi,
egrep should not be needed in this case, since . is part of basic
regexp (see man 1 grep and [1]) and so plain grep is enough.

[1] 
https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap09.html#tag_09_03_04

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] tunnel-push-pop.at: Mask source port in tunnel header.

2021-10-13 Thread Ilya Maximets
On 10/12/21 17:18, Alin-Gabriel Serdean wrote:
> Acked-by: Alin-Gabriel Serdean 

Thanks!  I applied this patch and backported down to 2.12.

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH] tunnel-push-pop.at: Mask source port in tunnel header.

2021-10-12 Thread Alin-Gabriel Serdean
Acked-by: Alin-Gabriel Serdean 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH] tunnel-push-pop.at: Mask source port in tunnel header.

2021-10-12 Thread Ilya Maximets
Source port is based on a packet hash and hash depends on a chosen
implementation.  Masking it to avoid test failures with '-msse4.2'.

Fixes: 7e6b41ac8d9d ("dpif-netdev: Fix crash when PACKET_OUT is metered.")
Reported-by: Kumar Amber 
Signed-off-by: Ilya Maximets 
---
 tests/tunnel-push-pop.at | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/tests/tunnel-push-pop.at b/tests/tunnel-push-pop.at
index 12fc1ef91..636465397 100644
--- a/tests/tunnel-push-pop.at
+++ b/tests/tunnel-push-pop.at
@@ -628,20 +628,22 @@ AT_CHECK([
 AT_CHECK([ovs-vsctl -- set Interface p0 options:tx_pcap=p0.pcap])
 
 packet=5054000a50540009123
-encap=f8bc124434b6aa55aa5508004532400040113406010102580101025c83a917c1001e65587b00
+dnl Source port is based on a packet hash, so it may differ depending on the
+dnl compiler flags and CPU type.  Masked with ''.
+encap=f8bc124434b6aa55aa5508004532400040113406010102580101025c17c1001e65587b00
 
 dnl Output to tunnel from a int-br internal port.
 dnl Checking that the packet arrived and it was correctly encapsulated.
 AT_CHECK([ovs-ofctl add-flow int-br 
"in_port=LOCAL,actions=debug_slow,output:2"])
 AT_CHECK([ovs-appctl netdev-dummy/receive int-br "${packet}4"])
-OVS_WAIT_UNTIL([test `ovs-pcap p0.pcap | grep "${encap}${packet}4" | wc -l` 
-ge 1])
+OVS_WAIT_UNTIL([test `ovs-pcap p0.pcap | egrep "${encap}${packet}4" | wc -l` 
-ge 1])
 dnl Sending again to exercise the non-miss upcall path.
 AT_CHECK([ovs-appctl netdev-dummy/receive int-br "${packet}4"])
-OVS_WAIT_UNTIL([test `ovs-pcap p0.pcap | grep "${encap}${packet}4" | wc -l` 
-ge 2])
+OVS_WAIT_UNTIL([test `ovs-pcap p0.pcap | egrep "${encap}${packet}4" | wc -l` 
-ge 2])
 
 dnl Output to tunnel from the controller.
 AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out int-br CONTROLLER 
"debug_slow,output:2" "${packet}5"])
-OVS_WAIT_UNTIL([test `ovs-pcap p0.pcap | grep "${encap}${packet}5" | wc -l` 
-ge 1])
+OVS_WAIT_UNTIL([test `ovs-pcap p0.pcap | egrep "${encap}${packet}5" | wc -l` 
-ge 1])
 
 dnl Datapath actions should not have tunnel push action.
 AT_CHECK([ovs-appctl dpctl/dump-flows | grep -q tnl_push], [1])
-- 
2.31.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev