[ovs-dev] [PATCH v7 2/7] ovn: add is_chassis_resident match expression component

[Mickey Spiegel]

Thanks for the review.

On Fri, Jan 6, 2017 at 3:14 PM, Ben Pfaff <blp at ovn.org> wrote:

> On Fri, Jan 06, 2017 at 12:00:29PM -0800, Mickey Spiegel wrote:
> > This patch introduces a new match expression component
> > is_chassis_resident().  Unlike match expression comparisons,
> > is_chassis_resident is not pushed down to OpenFlow.  It is a
> > conditional that is evaluated in the controller during expr_simplify(),
> > when it is replaced by a boolean expression.  The is_chassis_resident
> > conditional evaluates to "true" when the specified string identifies a
> > port name that is resident on this controller chassis, i.e., the
> > corresponding southbound database Port_Binding has a chassis column
> > that matches this chassis.  Otherwise it evaluates to "false".
> >
> > This allows higher level features to specify flows that are only
> > installed on some chassis rather than on all chassis with the
> > corresponding datapath.
> >
> > Suggested-by: Ben Pfaff <blp at ovn.org>
> > Signed-off-by: Mickey Spiegel <mickeys.dev at gmail.com>
>
> Here's an incremental I suggest folding in.  Some of the differences are
> style, but I also added documentation and changed the details of the
> expr_simplify_condition() interface.
>

Done

>
> I suggest calling these "functions" instead of "conditions", but I
> didn't make that change, except in the documentation.
>

I was primarily distinguishing match criteria passed on to openflow,
versus match criteria evaluated in expr_simplify(). A question for you:

Suppose hypothetically that we solve my table 32 issue with
something other than "chassisredirect" ports (though I still
think that is the right way to go). In that case I would need to add
another "condition" to only install certain flows on certain chassis.

Should the syntax for that be:
chassis == "chassis_name"
or should it be:
is_chassis("chassis_name")
?
Should all criteria evaluated in expr_simplify() take on the syntax
of functions?

If the answer is yes / is_chassis, then I will change the name from
"conditions" to "functions".

Mickey