Re: [ovs-dev] [PATCH] ovn-controller: Fix match crieria for dynamic mac binding flows

2016-09-08 Thread Russell Bryant 
On Fri, Sep 2, 2016 at 3:34 PM, Chandra S Vejendla 
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 
> Signed-off-by: Ryan Moats 
> Co-authored-by: Ryan Moats 
> ---
>  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.

-- 
Russell Bryant
___
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] ovn-controller: Fix match crieria for dynamic mac binding flows

2016-09-03 Thread Ryan Moats 


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: [email protected]
> 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 
> Signed-off-by: Ryan Moats 
> Co-authored-by: Ryan Moats 
> ---
>  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 

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? :)
___
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev