[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]>

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to