diff mbox series

[ovs-dev,RFC,ovn,2/6] tests: Fix get_arp/get_nd tests mac-binding table id.

Message ID 1591815613-49682-3-git-send-email-hzhou@ovn.org
State Superseded
Headers show
Series Avoid ARP flow explosion. | expand

Commit Message

Han Zhou June 10, 2020, 7 p.m. UTC
The table id used in test is not the same as the one used in
real implementation. Although it doesn't affect correctness, it
may cause confusion when people are studying test cases.

Signed-off-by: Han Zhou <hzhou@ovn.org>
---
 tests/ovn.at     | 8 ++++----
 tests/test-ovn.c | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

Comments

Numan Siddique June 11, 2020, 4:38 p.m. UTC | #1
On Thu, Jun 11, 2020 at 12:30 AM Han Zhou <hzhou@ovn.org> wrote:

> The table id used in test is not the same as the one used in
> real implementation. Although it doesn't affect correctness, it
> may cause confusion when people are studying test cases.
>
> Signed-off-by: Han Zhou <hzhou@ovn.org>
> ---
>  tests/ovn.at     | 8 ++++----
>  tests/test-ovn.c | 2 +-
>  2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 15b40ca..b7976c6 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -1149,10 +1149,10 @@ arp { };
>
>  # get_arp
>  get_arp(outport, ip4.dst);
> -    encodes as
> push:NXM_NX_REG0[],push:NXM_OF_IP_DST[],pop:NXM_NX_REG0[],set_field:00:00:00:00:00:00->eth_dst,resubmit(,65),pop:NXM_NX_REG0[]
> +    encodes as
> push:NXM_NX_REG0[],push:NXM_OF_IP_DST[],pop:NXM_NX_REG0[],set_field:00:00:00:00:00:00->eth_dst,resubmit(,66),pop:NXM_NX_REG0[]
>      has prereqs eth.type == 0x800
>  get_arp(inport, reg0);
> -    encodes as
> push:NXM_NX_REG15[],push:NXM_NX_REG0[],push:NXM_NX_XXREG0[96..127],push:NXM_NX_REG14[],pop:NXM_NX_REG15[],pop:NXM_NX_REG0[],set_field:00:00:00:00:00:00->eth_dst,resubmit(,65),pop:NXM_NX_REG0[],pop:NXM_NX_REG15[]
> +    encodes as
> push:NXM_NX_REG15[],push:NXM_NX_REG0[],push:NXM_NX_XXREG0[96..127],push:NXM_NX_REG14[],pop:NXM_NX_REG15[],pop:NXM_NX_REG0[],set_field:00:00:00:00:00:00->eth_dst,resubmit(,66),pop:NXM_NX_REG0[],pop:NXM_NX_REG15[]
>
>  get_arp;
>      Syntax error at `;' expecting `('.
> @@ -1253,10 +1253,10 @@ nd_na_router { eth.src = 12:34:56:78:9a:bc; nd.tll
> = 12:34:56:78:9a:bc; outport
>
>  # get_nd
>  get_nd(outport, ip6.dst);
> -    encodes as
> push:NXM_NX_XXREG0[],push:NXM_NX_IPV6_DST[],pop:NXM_NX_XXREG0[],set_field:00:00:00:00:00:00->eth_dst,resubmit(,65),pop:NXM_NX_XXREG0[]
> +    encodes as
> push:NXM_NX_XXREG0[],push:NXM_NX_IPV6_DST[],pop:NXM_NX_XXREG0[],set_field:00:00:00:00:00:00->eth_dst,resubmit(,66),pop:NXM_NX_XXREG0[]
>      has prereqs eth.type == 0x86dd
>  get_nd(inport, xxreg0);
> -    encodes as
> push:NXM_NX_REG15[],push:NXM_NX_REG14[],pop:NXM_NX_REG15[],set_field:00:00:00:00:00:00->eth_dst,resubmit(,65),pop:NXM_NX_REG15[]
> +    encodes as
> push:NXM_NX_REG15[],push:NXM_NX_REG14[],pop:NXM_NX_REG15[],set_field:00:00:00:00:00:00->eth_dst,resubmit(,66),pop:NXM_NX_REG15[]
>  get_nd;
>      Syntax error at `;' expecting `('.
>  get_nd();
> diff --git a/tests/test-ovn.c b/tests/test-ovn.c
> index a77d2f1..72b2985 100644
> --- a/tests/test-ovn.c
> +++ b/tests/test-ovn.c
> @@ -1335,7 +1335,7 @@ test_parse_actions(struct ovs_cmdl_context *ctx
> OVS_UNUSED)
>                  .ingress_ptable = 8,
>                  .egress_ptable = 40,
>                  .output_ptable = 64,
> -                .mac_bind_ptable = 65,
> +                .mac_bind_ptable = 66,
>                  .mac_lookup_ptable = 67,
>              };
>

Hi Han,

controller/lflow.h has macros defined for these tables.
Maybe we should include controller/flow.h here and use these macros instead
?
We can also move the macros to include/ovn/ too.
With this, we will not see mismatches again. wdyt ?

Thanks
Numan

             struct ofpbuf ofpacts;
> --
> 2.1.0
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
Han Zhou July 19, 2020, 10:28 p.m. UTC | #2
On Thu, Jun 11, 2020 at 9:38 AM Numan Siddique <numans@ovn.org> wrote:
>
>
>
> On Thu, Jun 11, 2020 at 12:30 AM Han Zhou <hzhou@ovn.org> wrote:
>>
>> The table id used in test is not the same as the one used in
>> real implementation. Although it doesn't affect correctness, it
>> may cause confusion when people are studying test cases.
>>
>> Signed-off-by: Han Zhou <hzhou@ovn.org>
>> ---
>>  tests/ovn.at     | 8 ++++----
>>  tests/test-ovn.c | 2 +-
>>  2 files changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/tests/ovn.at b/tests/ovn.at
>> index 15b40ca..b7976c6 100644
>> --- a/tests/ovn.at
>> +++ b/tests/ovn.at
>> @@ -1149,10 +1149,10 @@ arp { };
>>
>>  # get_arp
>>  get_arp(outport, ip4.dst);
>> -    encodes as
push:NXM_NX_REG0[],push:NXM_OF_IP_DST[],pop:NXM_NX_REG0[],set_field:00:00:00:00:00:00->eth_dst,resubmit(,65),pop:NXM_NX_REG0[]
>> +    encodes as
push:NXM_NX_REG0[],push:NXM_OF_IP_DST[],pop:NXM_NX_REG0[],set_field:00:00:00:00:00:00->eth_dst,resubmit(,66),pop:NXM_NX_REG0[]
>>      has prereqs eth.type == 0x800
>>  get_arp(inport, reg0);
>> -    encodes as
push:NXM_NX_REG15[],push:NXM_NX_REG0[],push:NXM_NX_XXREG0[96..127],push:NXM_NX_REG14[],pop:NXM_NX_REG15[],pop:NXM_NX_REG0[],set_field:00:00:00:00:00:00->eth_dst,resubmit(,65),pop:NXM_NX_REG0[],pop:NXM_NX_REG15[]
>> +    encodes as
push:NXM_NX_REG15[],push:NXM_NX_REG0[],push:NXM_NX_XXREG0[96..127],push:NXM_NX_REG14[],pop:NXM_NX_REG15[],pop:NXM_NX_REG0[],set_field:00:00:00:00:00:00->eth_dst,resubmit(,66),pop:NXM_NX_REG0[],pop:NXM_NX_REG15[]
>>
>>  get_arp;
>>      Syntax error at `;' expecting `('.
>> @@ -1253,10 +1253,10 @@ nd_na_router { eth.src = 12:34:56:78:9a:bc;
nd.tll = 12:34:56:78:9a:bc; outport
>>
>>  # get_nd
>>  get_nd(outport, ip6.dst);
>> -    encodes as
push:NXM_NX_XXREG0[],push:NXM_NX_IPV6_DST[],pop:NXM_NX_XXREG0[],set_field:00:00:00:00:00:00->eth_dst,resubmit(,65),pop:NXM_NX_XXREG0[]
>> +    encodes as
push:NXM_NX_XXREG0[],push:NXM_NX_IPV6_DST[],pop:NXM_NX_XXREG0[],set_field:00:00:00:00:00:00->eth_dst,resubmit(,66),pop:NXM_NX_XXREG0[]
>>      has prereqs eth.type == 0x86dd
>>  get_nd(inport, xxreg0);
>> -    encodes as
push:NXM_NX_REG15[],push:NXM_NX_REG14[],pop:NXM_NX_REG15[],set_field:00:00:00:00:00:00->eth_dst,resubmit(,65),pop:NXM_NX_REG15[]
>> +    encodes as
push:NXM_NX_REG15[],push:NXM_NX_REG14[],pop:NXM_NX_REG15[],set_field:00:00:00:00:00:00->eth_dst,resubmit(,66),pop:NXM_NX_REG15[]
>>  get_nd;
>>      Syntax error at `;' expecting `('.
>>  get_nd();
>> diff --git a/tests/test-ovn.c b/tests/test-ovn.c
>> index a77d2f1..72b2985 100644
>> --- a/tests/test-ovn.c
>> +++ b/tests/test-ovn.c
>> @@ -1335,7 +1335,7 @@ test_parse_actions(struct ovs_cmdl_context *ctx
OVS_UNUSED)
>>                  .ingress_ptable = 8,
>>                  .egress_ptable = 40,
>>                  .output_ptable = 64,
>> -                .mac_bind_ptable = 65,
>> +                .mac_bind_ptable = 66,
>>                  .mac_lookup_ptable = 67,
>>              };
>
>
> Hi Han,
>
> controller/lflow.h has macros defined for these tables.
> Maybe we should include controller/flow.h here and use these macros
instead ?
> We can also move the macros to include/ovn/ too.
> With this, we will not see mismatches again. wdyt ?

Thanks for the suggestion. I will use macros.
>
> Thanks
> Numan
>
>>              struct ofpbuf ofpacts;
>> --
>> 2.1.0
>>
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
diff mbox series

Patch

diff --git a/tests/ovn.at b/tests/ovn.at
index 15b40ca..b7976c6 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -1149,10 +1149,10 @@  arp { };
 
 # get_arp
 get_arp(outport, ip4.dst);
-    encodes as push:NXM_NX_REG0[],push:NXM_OF_IP_DST[],pop:NXM_NX_REG0[],set_field:00:00:00:00:00:00->eth_dst,resubmit(,65),pop:NXM_NX_REG0[]
+    encodes as push:NXM_NX_REG0[],push:NXM_OF_IP_DST[],pop:NXM_NX_REG0[],set_field:00:00:00:00:00:00->eth_dst,resubmit(,66),pop:NXM_NX_REG0[]
     has prereqs eth.type == 0x800
 get_arp(inport, reg0);
-    encodes as push:NXM_NX_REG15[],push:NXM_NX_REG0[],push:NXM_NX_XXREG0[96..127],push:NXM_NX_REG14[],pop:NXM_NX_REG15[],pop:NXM_NX_REG0[],set_field:00:00:00:00:00:00->eth_dst,resubmit(,65),pop:NXM_NX_REG0[],pop:NXM_NX_REG15[]
+    encodes as push:NXM_NX_REG15[],push:NXM_NX_REG0[],push:NXM_NX_XXREG0[96..127],push:NXM_NX_REG14[],pop:NXM_NX_REG15[],pop:NXM_NX_REG0[],set_field:00:00:00:00:00:00->eth_dst,resubmit(,66),pop:NXM_NX_REG0[],pop:NXM_NX_REG15[]
 
 get_arp;
     Syntax error at `;' expecting `('.
@@ -1253,10 +1253,10 @@  nd_na_router { eth.src = 12:34:56:78:9a:bc; nd.tll = 12:34:56:78:9a:bc; outport
 
 # get_nd
 get_nd(outport, ip6.dst);
-    encodes as push:NXM_NX_XXREG0[],push:NXM_NX_IPV6_DST[],pop:NXM_NX_XXREG0[],set_field:00:00:00:00:00:00->eth_dst,resubmit(,65),pop:NXM_NX_XXREG0[]
+    encodes as push:NXM_NX_XXREG0[],push:NXM_NX_IPV6_DST[],pop:NXM_NX_XXREG0[],set_field:00:00:00:00:00:00->eth_dst,resubmit(,66),pop:NXM_NX_XXREG0[]
     has prereqs eth.type == 0x86dd
 get_nd(inport, xxreg0);
-    encodes as push:NXM_NX_REG15[],push:NXM_NX_REG14[],pop:NXM_NX_REG15[],set_field:00:00:00:00:00:00->eth_dst,resubmit(,65),pop:NXM_NX_REG15[]
+    encodes as push:NXM_NX_REG15[],push:NXM_NX_REG14[],pop:NXM_NX_REG15[],set_field:00:00:00:00:00:00->eth_dst,resubmit(,66),pop:NXM_NX_REG15[]
 get_nd;
     Syntax error at `;' expecting `('.
 get_nd();
diff --git a/tests/test-ovn.c b/tests/test-ovn.c
index a77d2f1..72b2985 100644
--- a/tests/test-ovn.c
+++ b/tests/test-ovn.c
@@ -1335,7 +1335,7 @@  test_parse_actions(struct ovs_cmdl_context *ctx OVS_UNUSED)
                 .ingress_ptable = 8,
                 .egress_ptable = 40,
                 .output_ptable = 64,
-                .mac_bind_ptable = 65,
+                .mac_bind_ptable = 66,
                 .mac_lookup_ptable = 67,
             };
             struct ofpbuf ofpacts;