Re: [ovs-dev] [PATCH 1/4] expr: Simplify "x == 0/0" into 1.

[Andy Zhou]

On Thu, Oct 6, 2016 at 8:30 PM, Ben Pfaff  wrote:

> An expression like "x == 0/0" does not test any actual bits in field x,
> so it resolves to true, but expr_simplify() was not smart enough to see
> this.
>
> This goes beyond an optimization, to become a bug fix, because
> expr_normalize() will assert-fail for expressions that become trivial
> when this simplification is omitted.  For example:
>
> $ echo 'eth.dst == 0/0 && eth.dst == 0/0' | tests/ovstest test-ovn
> normalize-expr
> test-ovn: ../include/openvswitch/list.h:245: assertion
> !ovs_list_is_empty(list) failed in ovs_list_front()
> Aborted (core dumped)
>
> This commit fixes this and related problems.
>
> The test added by this commit is very specific to the particular problem,
> whereas a more general test would be better.  A later commit adds the
> general test.
>
> Signed-off-by: Ben Pfaff 

Acked-by: Andy Zhou 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH 1/4] expr: Simplify "x == 0/0" into 1.

[Ben Pfaff]

An expression like "x == 0/0" does not test any actual bits in field x,
so it resolves to true, but expr_simplify() was not smart enough to see
this.

This goes beyond an optimization, to become a bug fix, because
expr_normalize() will assert-fail for expressions that become trivial
when this simplification is omitted.  For example:

$ echo 'eth.dst == 0/0 && eth.dst == 0/0' | tests/ovstest test-ovn 
normalize-expr
test-ovn: ../include/openvswitch/list.h:245: assertion 
!ovs_list_is_empty(list) failed in ovs_list_front()
Aborted (core dumped)

This commit fixes this and related problems.

The test added by this commit is very specific to the particular problem,
whereas a more general test would be better.  A later commit adds the
general test.

Signed-off-by: Ben Pfaff 
---
 ovn/lib/expr.c | 15 ++-
 tests/ovn.at   |  8 
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/ovn/lib/expr.c b/ovn/lib/expr.c
index 4ae6b0b..a5dbcfa 100644
--- a/ovn/lib/expr.c
+++ b/ovn/lib/expr.c
@@ -1669,6 +1669,18 @@ expr_annotate(struct expr *expr, const struct shash 
*symtab, char **errorp)
 }
 
 static struct expr *
+expr_simplify_eq(struct expr *expr)
+{
+const union mf_subvalue *mask = >cmp.mask;
+if (is_all_zeros(mask, sizeof *mask)) {
+/* Simplify "ip4.dst == 0/0" to just "1" (plus a prerequisite). */
+expr_destroy(expr);
+return expr_create_boolean(true);
+}
+return expr;
+}
+
+static struct expr *
 expr_simplify_ne(struct expr *expr)
 {
 struct expr *new = NULL;
@@ -1742,7 +1754,8 @@ expr_simplify(struct expr *expr)
 
 switch (expr->type) {
 case EXPR_T_CMP:
-return (expr->cmp.relop == EXPR_R_EQ || !expr->cmp.symbol->width ? expr
+return (!expr->cmp.symbol->width ? expr
+: expr->cmp.relop == EXPR_R_EQ ? expr_simplify_eq(expr)
 : expr->cmp.relop == EXPR_R_NE ? expr_simplify_ne(expr)
 : expr_simplify_relational(expr));
 
diff --git a/tests/ovn.at b/tests/ovn.at
index 8be774e..b2808e2 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -425,6 +425,14 @@ AT_CHECK([ovstest test-ovn exhaustive --operation=simplify 
--nvars=1 --svars=1 3
 ])
 AT_CLEANUP
 
+AT_SETUP([ovn -- simplification special cases])
+simplify() {
+echo "$1" | ovstest test-ovn simplify-expr
+}
+AT_CHECK([simplify 'eth.dst == 0/0'], [0], [1
+])
+AT_CLEANUP
+
 AT_SETUP([ovn -- 4-term numeric expression normalization])
 AT_CHECK([ovstest test-ovn exhaustive --operation=normalize --nvars=3 
--svars=0 --bits=1 4], [0],
   [Tested normalizing 1207162 expressions of 4 terminals with 3 numeric vars 
(each 1 bits) in terms of operators == != < <= > >=.
-- 
2.1.3

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev