diff mbox

[ovs-dev] test: Make test independent of the recirc_id

Message ID 1446156560-30090-1-git-send-email-azhou@nicira.com
State Accepted
Headers show

Commit Message

Andy Zhou Oct. 29, 2015, 10:09 p.m. UTC
Commit 8ae8176fd0d8ed919e3301cc961dcf02b65ff49d (tests: Make test
independent of the hash function) improves the test "ofprot-dpif
- balance-tcp bonding, different recirc flow" to not dependent on
the values of dp-hash, but it still depends on the value of recirc_id,
which can be a different value based on runs, specifically, it depends
which one of the two bonds allocates recirc id first.

Since both dp_hash and recirc_id values are runtime dependent,
consolidate the masking scripts into ofctl_strip.

Bug-report: http://openvswitch.org/pipermail/discuss/2015-October/019269.html
Reported-by: Gerhrd Stenzel <gstenzel@linux.vnet.ibm.com>
Signed-off-by: Andy Zhou <azhou@nicira.com>
---
 tests/ofproto-dpif.at   | 5 ++---
 tests/ofproto-macros.at | 2 ++
 2 files changed, 4 insertions(+), 3 deletions(-)

Comments

Jarno Rajahalme Oct. 29, 2015, 10:25 p.m. UTC | #1
LGTM,

Acked-by: Jarno Rajahalme <jrajahalme@nicira.com>

> On Oct 29, 2015, at 3:09 PM, Andy Zhou <azhou@nicira.com> wrote:
> 
> Commit 8ae8176fd0d8ed919e3301cc961dcf02b65ff49d (tests: Make test
> independent of the hash function) improves the test "ofprot-dpif
> - balance-tcp bonding, different recirc flow" to not dependent on
> the values of dp-hash, but it still depends on the value of recirc_id,
> which can be a different value based on runs, specifically, it depends
> which one of the two bonds allocates recirc id first.
> 
> Since both dp_hash and recirc_id values are runtime dependent,
> consolidate the masking scripts into ofctl_strip.
> 
> Bug-report: http://openvswitch.org/pipermail/discuss/2015-October/019269.html
> Reported-by: Gerhrd Stenzel <gstenzel@linux.vnet.ibm.com>
> Signed-off-by: Andy Zhou <azhou@nicira.com>
> ---
> tests/ofproto-dpif.at   | 5 ++---
> tests/ofproto-macros.at | 2 ++
> 2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
> index bc2daf1..f1c7cb6 100644
> --- a/tests/ofproto-dpif.at
> +++ b/tests/ofproto-dpif.at
> @@ -186,7 +186,6 @@ table=0 priority=2 in_port=5 dl_vlan=1 actions=drop
> AT_CHECK([ovs-ofctl add-flows br-int flows.txt])
> 
> # Sends a packet to trigger recirculation.
> -# Should generate recirc_id(0x2),dp_hash(0xc1261ba2/0xff).
> AT_CHECK([ovs-appctl netdev-dummy/receive p5 "in_port(5),eth(src=50:54:00:00:00:05,dst=50:54:00:00:01:00),eth_type(0x0800),ipv4(src=10.0.0.2,dst=10.0.0.1)"])
> 
> # Collects flow stats.
> @@ -194,8 +193,8 @@ AT_CHECK([ovs-appctl revalidator/purge], [0])
> 
> # Checks the flow stats in br1, should only be one flow with non-zero
> # 'n_packets' from internal table.
> -AT_CHECK([ovs-appctl bridge/dump-flows br1 | ofctl_strip | grep -- "n_packets" | grep -- "table_id" | sed -e 's/dp_hash=0x[[0-9a-f]][[0-9a-f]]*/dp_hash=0x0/' -e 's/output:[[0-9]][[0-9]]*/output/'], [0], [dnl
> -table_id=254, n_packets=1, n_bytes=64, priority=20,recirc_id=0x2,dp_hash=0x0/0xff,actions=output
> +AT_CHECK([ovs-appctl bridge/dump-flows br1 | ofctl_strip | grep -- "n_packets" | grep -- "table_id" | sed -e 's/output:[[0-9]][[0-9]]*/output/'] , [0], [dnl
> +table_id=254, n_packets=1, n_bytes=64, priority=20,recirc_id=0x0,dp_hash=0x0/0xff,actions=output
> ])
> 
> # Checks the flow stats in br-int, should be only one match.
> diff --git a/tests/ofproto-macros.at b/tests/ofproto-macros.at
> index fe99186..fcd36fb 100644
> --- a/tests/ofproto-macros.at
> +++ b/tests/ofproto-macros.at
> @@ -12,6 +12,8 @@ s/ n_packets=0,//
> s/ n_bytes=0,//
> s/ idle_age=[0-9]*,//
> s/ hard_age=[0-9]*,//
> +s/dp_hash=0x[0-9a-f]*\//dp_hash=0x0\//
> +s/recirc_id=0x[0-9a-f]*,/recirc_id=0x0,/
> '
> }
> 
> -- 
> 1.9.1
> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
Andy Zhou Oct. 30, 2015, 6:05 p.m. UTC | #2
On Thu, Oct 29, 2015 at 3:25 PM, Jarno Rajahalme <jrajahalme@nicira.com> wrote:
> LGTM,
>
> Acked-by: Jarno Rajahalme <jrajahalme@nicira.com>
>

Thanks for the review Jarno! I will push it to master in a few
minutes, and back port this fix to branch 2.4 and branch 2.3.
diff mbox

Patch

diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index bc2daf1..f1c7cb6 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -186,7 +186,6 @@  table=0 priority=2 in_port=5 dl_vlan=1 actions=drop
 AT_CHECK([ovs-ofctl add-flows br-int flows.txt])
 
 # Sends a packet to trigger recirculation.
-# Should generate recirc_id(0x2),dp_hash(0xc1261ba2/0xff).
 AT_CHECK([ovs-appctl netdev-dummy/receive p5 "in_port(5),eth(src=50:54:00:00:00:05,dst=50:54:00:00:01:00),eth_type(0x0800),ipv4(src=10.0.0.2,dst=10.0.0.1)"])
 
 # Collects flow stats.
@@ -194,8 +193,8 @@  AT_CHECK([ovs-appctl revalidator/purge], [0])
 
 # Checks the flow stats in br1, should only be one flow with non-zero
 # 'n_packets' from internal table.
-AT_CHECK([ovs-appctl bridge/dump-flows br1 | ofctl_strip | grep -- "n_packets" | grep -- "table_id" | sed -e 's/dp_hash=0x[[0-9a-f]][[0-9a-f]]*/dp_hash=0x0/' -e 's/output:[[0-9]][[0-9]]*/output/'], [0], [dnl
-table_id=254, n_packets=1, n_bytes=64, priority=20,recirc_id=0x2,dp_hash=0x0/0xff,actions=output
+AT_CHECK([ovs-appctl bridge/dump-flows br1 | ofctl_strip | grep -- "n_packets" | grep -- "table_id" | sed -e 's/output:[[0-9]][[0-9]]*/output/'] , [0], [dnl
+table_id=254, n_packets=1, n_bytes=64, priority=20,recirc_id=0x0,dp_hash=0x0/0xff,actions=output
 ])
 
 # Checks the flow stats in br-int, should be only one match.
diff --git a/tests/ofproto-macros.at b/tests/ofproto-macros.at
index fe99186..fcd36fb 100644
--- a/tests/ofproto-macros.at
+++ b/tests/ofproto-macros.at
@@ -12,6 +12,8 @@  s/ n_packets=0,//
 s/ n_bytes=0,//
 s/ idle_age=[0-9]*,//
 s/ hard_age=[0-9]*,//
+s/dp_hash=0x[0-9a-f]*\//dp_hash=0x0\//
+s/recirc_id=0x[0-9a-f]*,/recirc_id=0x0,/
 '
 }