[ovs-dev] [RFC PATCH 1/1]: Enhance conjunctive match support in OVN

[Ben Pfaff]

On Fri, Feb 02, 2018 at 09:06:39PM +0530, nusiddiq at redhat.com wrote:
> From: Numan Siddique <nusiddiq at redhat.com>
> 
> Presently, if a logical flow has possible conjunctive matches, OVN expression
> parser has support for that. But certain fields like ip4.src, ip4.dst are not
> considered as dimensions in the conjunctive matches.
> 
> In order to support all the possible fields as dimensions, this patch has added
> a new expression type 'EXPR_T_CONJ'. After a expression is simplified by
> expr_simplify(), before calling expr_normalize(), a new function
> expr_eval_conj() is called, which evaluates for possible dimensions for
> conjunctive matches.
> 
> For example if the match expression is
> "ip4 && ip4.src == {10.0.0.4, 10.0.0.5, 10.0.0.6} &&
>  ip4.dst == {20.0.0.4, 20.0.0.5, 20.0.0.6}"
> 
> expr_simplify() would have generated the expression as -
> 
> AND(CMP(IP4),
>     OR((CMP(ip4.src == 10.0.0.4), CMP(ip4.src == 10.0.0.5),
>         CMP(ip4.src == 10.0.0.6)),
>     OR((CMP(ip4.dst == 20.0.0.4), CMP(ip4.src == 20.0.0.5),
>         CMP(ip4.src == 20.0.0.6))).
> 
> expr_eval_conj() would return a new expression something like
> 
> CONJ(AND(CMP(IP4),
>          OR((CMP(ip4.src == 10.0.0.4), CMP(ip4.src == 10.0.0.5),
>              CMP(ip4.src == 10.0.0.6))),
>      AND(CMP(IP4),
>          OR((CMP(ip4.dst == 20.0.0.4), CMP(ip4.dst == 20.0.0.5),
>              CMP(ip4.dst == 20.0.0.6))))
> 
> expr_normalize() would normalize each individual 'AND' clause in the CONJ and
> expr_to_matches() would add the necessary conjunctive matches.
> 
> TODO: If the proposed approach is reasonable, then test cases and necessary
> code comments needs to be added.

I think I like this approach, but I also think that it's worthwhile
trying to figure out whether it's possible to do it without adding the
extra EXPR_T_CONJ type and the extra processing step.  I started playing
with that idea yesterday.  I think it's possible, but I didn't actually
finish implementing it.

I did come up with a few independent patches to improve the expr code,
though.  Would you take a look?  They are at:
        https://patchwork.ozlabs.org/project/openvswitch/list/?series=27221

This patch causes one warning from Clang (and I think it's right that
this code is fishy):

../ovn/lib/expr.c:1470:16: error: implicit conversion from enumeration type 'enum expr_type' to different enumeration type 'enum expr_level' [-Werror,-Wenum-conversion]

Thanks,

Ben.