On Fri, Nov 29, 2019 at 1:08 AM <[email protected]> wrote: > > From: Numan Siddique <[email protected]> > > If ofctrl_check_and_add_flow(F') is called where flow F' has match-actions (M, A2) > and if there already exists a flow F with match-actions (M, A1) in the desired flow > table, then this function doesn't update the existing flow F with new actions > actions A2. > > This patch fixes it. Presently we don't see any issues because of this behaviour. > But it's better to fix it. >
Hi Numan, could you explain why do you think the F' should override the F? For my understanding, this is a problem of duplicated logical flows generated by ovn-northd and can't be solved in ovn-controller. The desired flows have conflict and there is no way to judge which one should be applied. > Signed-off-by: Numan Siddique <[email protected]> > --- > controller/ofctrl.c | 23 ++++++++++++++++------- > 1 file changed, 16 insertions(+), 7 deletions(-) > > diff --git a/controller/ofctrl.c b/controller/ofctrl.c > index 10edd84fb..5a174da48 100644 > --- a/controller/ofctrl.c > +++ b/controller/ofctrl.c > @@ -667,14 +667,23 @@ ofctrl_check_and_add_flow(struct ovn_desired_flow_table *flow_table, > > ovn_flow_log(f, "ofctrl_add_flow"); > > - if (ovn_flow_lookup(&flow_table->match_flow_table, f, true)) { > - if (log_duplicate_flow) { > - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5); > - if (!VLOG_DROP_DBG(&rl)) { > - char *s = ovn_flow_to_string(f); > - VLOG_DBG("dropping duplicate flow: %s", s); > - free(s); > + struct ovn_flow *existing_f = > + ovn_flow_lookup(&flow_table->match_flow_table, f, true); > + if (existing_f) { > + if (ofpacts_equal(f->ofpacts, f->ofpacts_len, > + existing_f->ofpacts, existing_f->ofpacts_len)) { > + if (log_duplicate_flow) { > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5); > + if (!VLOG_DROP_DBG(&rl)) { > + char *s = ovn_flow_to_string(f); > + VLOG_DBG("dropping duplicate flow: %s", s); > + free(s); > + } > } > + } else { > + free(existing_f->ofpacts); > + existing_f->ofpacts = xmemdup(f->ofpacts, f->ofpacts_len); > + existing_f->ofpacts_len = f->ofpacts_len; > } > ovn_flow_destroy(f); > return; > -- > 2.23.0 > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
