[ovs-dev] [PATCH v2 1/2] ovn-controller: Tie OpenFlow and logical flows using OpenFlow cookie.

[Ben Pfaff]

On Mon, Dec 12, 2016 at 12:59:06PM -0800, Guru Shetty wrote:
> On 9 December 2016 at 22:56, Ben Pfaff <blp at ovn.org> wrote:
> 
> > This makes it easy to find the logical flow that generated a particular
> > OpenFlow flow, by running "ovn-sbctl dump-flows <cookie>".
> >
> > Later, this can be refined (and automated for "ofproto/trace"), but this
> > is still a significant advance.
> >
> > Signed-off-by: Ben Pfaff <blp at ovn.org>

...

> > diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
> > index 4e67365..5f49183 100644
> > --- a/ovn/controller/lflow.c
> > +++ b/ovn/controller/lflow.c
> > @@ -302,7 +302,7 @@ consider_logical_flow(const struct lport_index *lports,
> >          }
> >          if (!m->n) {
> >              ofctrl_add_flow(flow_table, ptable, lflow->priority,
> > &m->match,
> > -                            &ofpacts);
> > +                            lflow->header_.uuid.parts[0], &ofpacts);
> >          } else {
> >              uint64_t conj_stubs[64 / 8];
> >              struct ofpbuf conj;
> > @@ -318,7 +318,7 @@ consider_logical_flow(const struct lport_index *lports,
> >                  dst->n_clauses = src->n_clauses;
> >              }
> >              ofctrl_add_flow(flow_table, ptable, lflow->priority,
> > &m->match,
> > -                            &conj);
> > +                            0, &conj);
> >
> 
> If I understand it right, a logical flow of the form ip4.src == {a, b, c}
> after it is split into multiple openflow flows won't have a cookie?

Such a flow generates OpenFlow flows that look something like this, if
necessary (not actually in this case since there's only one field):

    ip,nw_src=a, actions=conjunction(123)
    ip,nw_src=b, actions=conjunction(123)
    ip,nw_src=c, actions=conjunction(123)
    conj_id=123, actions=...real actions...

In such a case, the flows with "conjunction" actions indeed will not get
a cookie, but really they can't because these might be shared by
multiple logical flows.  That is, if another OVN logical flow comes
along with ip4.src == {c,d,e} and other criteria, then you'd end up
with:

    ip,nw_src=a, actions=conjunction(123)
    ip,nw_src=b, actions=conjunction(123)
    ip,nw_src=c, actions=conjunction(123),conjunction(456)
    ip,nw_src=d, actions=conjunction(456)
    ip,nw_src=e, actions=conjunction(456)
    conj_id=123, actions=...real actions...
    conj_id=456, actions=...real actions...

and so in a case like that, which cookie should get used?

The flows with "conjunction" actions won't ever show up in a trace,
because they direct packet classification and never get executed like
normal actions.

The text added to the ovn-trace manpage in patch 2/2 describes this,
although it doesn't have an example like above.

> > +        /* Print the flow. */
> >          printf("  table=%-2" PRId64 "(%-19s), priority=%-5" PRId64
> >                 ", match=(%s), action=(%s)\n",
> >                 lflow->table_id,
> >                 smap_get_def(&lflow->external_ids, "stage-name", ""),
> >                 lflow->priority, lflow->match, lflow->actions);
> >
> 
> I wonder whether we should also print extenral_ids:source in the dump-flows
> output?

Maybe.  These lines are already pretty wide though.

> Acked-by: Gurucharan Shetty <guru at ovn.org>

Thanks!  I'm going to ask Justin whether he wants to review this before
I push it, because I know that he has opinions on how to provide this
extra information.