| Message ID | 1472844875-79722-1-git-send-email-csvejend@us.ibm.com |
|---|---|
| State | Superseded |
| Headers | show |
Chandra Sekhar Vejendla/San Jose/IBM@IBMUS wrote on 09/02/2016 02:34:35 PM: > From: Chandra Sekhar Vejendla/San Jose/IBM@IBMUS > To: dev@openvswitch.org > Cc: Chandra Sekhar Vejendla/San Jose/IBM@IBMUS, Ryan Moats/Omaha/IBM@IBMUS > Date: 09/02/2016 02:35 PM > Subject: [PATCH] ovn-controller: Fix match crieria for dynamic mac > binding flows > > match struct is not initialized before adding flows for each entry in > mac_bindings table. This results in incorrect match criteria. > > Signed-off-by: Chandra Sekhar Vejendla <csvejend@us.ibm.com> > Signed-off-by: Ryan Moats <rmoats@us.ibm.com> > Co-authored-by: Ryan Moats <rmoats@us.ibm.com> > --- > ovn/controller/lflow.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c > index efac5b3..fa639ae 100644 > --- a/ovn/controller/lflow.c > +++ b/ovn/controller/lflow.c > @@ -399,12 +399,12 @@ add_neighbor_flows(struct controller_ctx *ctx, > struct hmap *flow_table) > { > struct ofpbuf ofpacts; > - struct match match; > - match_init_catchall(&match); > ofpbuf_init(&ofpacts, 0); > > const struct sbrec_mac_binding *b; > SBREC_MAC_BINDING_FOR_EACH (b, ctx->ovnsb_idl) { > + struct match match; > + match_init_catchall(&match); > consider_neighbor_flow(lports, b, &ofpacts, &match, flow_table); > } > > -- This looks obvious to me, so ... Acked-by: Ryan Moats <rmoats@us.ibm.com> Side question: I'm not seeing a unit test case around flows and mac bindings that would have caught this. Is this actually missing or am I just blind today? :)
On Fri, Sep 2, 2016 at 3:34 PM, Chandra S Vejendla <csvejend@us.ibm.com> wrote: > match struct is not initialized before adding flows for each entry in > mac_bindings table. This results in incorrect match criteria. > > Signed-off-by: Chandra Sekhar Vejendla <csvejend@us.ibm.com> > Signed-off-by: Ryan Moats <rmoats@us.ibm.com> > Co-authored-by: Ryan Moats <rmoats@us.ibm.com> > --- > ovn/controller/lflow.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c > index efac5b3..fa639ae 100644 > --- a/ovn/controller/lflow.c > +++ b/ovn/controller/lflow.c > @@ -399,12 +399,12 @@ add_neighbor_flows(struct controller_ctx *ctx, > struct hmap *flow_table) > { > struct ofpbuf ofpacts; > - struct match match; > - match_init_catchall(&match); > ofpbuf_init(&ofpacts, 0); > > const struct sbrec_mac_binding *b; > SBREC_MAC_BINDING_FOR_EACH (b, ctx->ovnsb_idl) { > + struct match match; > + match_init_catchall(&match); > consider_neighbor_flow(lports, b, &ofpacts, &match, flow_table); > } > If this is the fix, this appears to be the only place consider_neighbor_flow() is used, so it's not clear there is any value for having match passed in as an argument at all.
diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c index efac5b3..fa639ae 100644 --- a/ovn/controller/lflow.c +++ b/ovn/controller/lflow.c @@ -399,12 +399,12 @@ add_neighbor_flows(struct controller_ctx *ctx, struct hmap *flow_table) { struct ofpbuf ofpacts; - struct match match; - match_init_catchall(&match); ofpbuf_init(&ofpacts, 0); const struct sbrec_mac_binding *b; SBREC_MAC_BINDING_FOR_EACH (b, ctx->ovnsb_idl) { + struct match match; + match_init_catchall(&match); consider_neighbor_flow(lports, b, &ofpacts, &match, flow_table); }