[ovs-dev] [PATCH ovn] ovn-sbctl: Sort "dump-flows" output by actions if all else fails.

[Ben Pfaff]

On Tue, Mar 09, 2021 at 05:49:22PM -0800, Han Zhou wrote:
> On Fri, Mar 5, 2021 at 2:54 PM Ben Pfaff <blp at ovn.org> wrote:
> >
> > The Logical_Flow table can have rows that differ only in actions.  Such
> > rows would have identical matches and priorities and be in the same
> > datapath.  At first glance, any these rows would be a bug, since they'd
> > match the same packets and there'd be no way to choose between them.
> >
> > In practice, though, actions can have prerequisites.  If two different
> > flows have mutually exclusive prerequisites, the otherwise identical
> > flows are actually valid.
> >
> > This comes up in practice in DNS response flows.  There are two of
> > them, in the same table and pipeline and with the same match expression
> > "udp.dst == 53 && reg0[4]", but one of them includes the action
> > "ip4.src <-> ip4.dst;" and the other "ip6.src <-> ip6.dst;".  Because
> > the first has an IPv4 prerequisite and the latter an IPv6 prerequisite,
> > their matches don't really overlap.
> >
> > Anyway, regardless of whether they're valid, it's still good to sort
> > consistently, so that's all a digression.
> >
> > Signed-off-by: Ben Pfaff <blp at ovn.org>
> > ---
> >  utilities/ovn-sbctl.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/utilities/ovn-sbctl.c b/utilities/ovn-sbctl.c
> > index c38e8ec3bdbd..94e33d2bdfe6 100644
> > --- a/utilities/ovn-sbctl.c
> > +++ b/utilities/ovn-sbctl.c
> > @@ -754,13 +754,14 @@ sbctl_lflow_cmp(const void *a_, const void *b_)
> >
> >      int a_pipeline = pipeline_encode(a->pipeline);
> >      int b_pipeline = pipeline_encode(b->pipeline);
> > -    return (a_pipeline > b_pipeline ? 1
> > +    cmp = (a_pipeline > b_pipeline ? 1
> >              : a_pipeline < b_pipeline ? -1
> >              : a->table_id > b->table_id ? 1
> >              : a->table_id < b->table_id ? -1
> >              : a->priority > b->priority ? -1
> >              : a->priority < b->priority ? 1
> >              : strcmp(a->match, b->match));
> > +    return cmp ? cmp : strcmp(a->actions, b->actions);
> >  }
> >
> >  static char *
> > --
> > 2.29.2
> >
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> 
> Acked-by: Han Zhou <hzhou at ovn.org>

Thanks, Han.  I applied this to master.

I noticed that I forgot to update the indentation on the lines inside
the big expression, so I fixed that before applying.