On Thu, Aug 31, 2017 at 09:48:14AM +0300, Liran Schour wrote:
> [email protected] wrote on 30/08/2017 07:33:13 PM:
> > The current implementation of ovsdb-server caches only non-conditional
> > monitors, that is, monitors for every table row, not those that monitor
> > only rows that match some condition.  To figure out which monitors are
> > conditional, the code track the number of tables that have conditions 
> that
> > are uniformly true (cond->n_true_cnd) and compares that against the 
> number
> > of tables in the condition (shash_count(&cond->tables)).  If they are 
> the
> > same, then every table has (effectively) no condition, and so
> > cond->conditional is set to false.
> > 
> > However, the implementation was buggy.  The function that adds a new
> > table condition, ovsdb_monitor_table_condition_create(), only updated
> > cond->conditional if the table condition being added was true.  This is
> > wrong; only adding a non-true condition can actually change
> > cond->conditional.  This commit fixes the problem by always 
> recalculating
> > cond->conditional.
> > 
> > The most visible side effect of cond->conditional being true when it
> > should be false, as caused by this bug, was that conditional monitors 
> were
> > being mixed with unconditional monitors for the purpose of caching. This
> > meant that, if a client requested a conditional monitor that was the
> > same as an unconditional one, except for the condition, then the client
> > would receive the cached data previously sent for the unconditional one.
> > This commit fixes the problem.
> > 
> > Signed-off-by: Ben Pfaff <[email protected]>
> > ---
> >  ovsdb/monitor.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/ovsdb/monitor.c b/ovsdb/monitor.c
> > index b98100703091..7a5c2f905560 100644
> > --- a/ovsdb/monitor.c
> > +++ b/ovsdb/monitor.c
> > @@ -651,8 +651,8 @@ ovsdb_monitor_table_condition_create(
> >      ovsdb_condition_clone(&mtc->new_condition, &mtc->old_condition);
> >      if (ovsdb_condition_is_true(&mtc->old_condition)) {
> >          condition->n_true_cnd++;
> > -        ovsdb_monitor_session_condition_set_mode(condition);
> >      }
> > +    ovsdb_monitor_session_condition_set_mode(condition);
> > 
> >      return NULL;
> >  }
> > -- 
> > 2.10.2
> > 
> 
> Yes. It is a bug. Thanks.
> 
> Acked-by: Liran Schour <[email protected]>
> 

Thanks Andy and Liran, I applied this to master and as far back as
branch-2.6.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to