ovsdb-idl: Change interface to conditional monitoring.

Most users of OVSDB react to whatever is currently in their view of the
database, as opposed to keeping track of changes and reacting to those
changes individually.  The interface to conditional monitoring was
different, in that it expected the client to say what to add or remove from
monitoring instead of what to monitor.  This seemed reasonable at the time,
but in practice it turns out that the usual approach actually works better,
because the condition is generally a function of the data visible in the
database.  This commit changes the approach.

This commit also changes the meaning of an empty condition for a table.
Previously, an empty condition meant to replicate every row.  Now, an empty
condition means to replicate no rows.  This is more convenient for code
that gradually constructs conditions, because it does not need special
cases for replicating nothing.

This commit also changes the internal implementation of conditions from
linked lists to arrays.  I just couldn't see an advantage to using linked
lists.

Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Liran Schour <lirans@il.ibm.com>

9 files changed, 303 insertions, 495 deletions

diff --git a/lib/ovsdb-data.h b/lib/ovsdb-data.h
index ae2672e25..9fce380cf 100644
--- a/lib/ovsdb-data.h
+++ b/lib/ovsdb-data.h
@@ -127,6 +127,7 @@ struct ovsdb_datum {
     union ovsdb_atom *keys;     /* Each of the ovsdb_type's 'key_type'. */
     union ovsdb_atom *values;   /* Each of the ovsdb_type's 'value_type'. */
 };
+#define OVSDB_DATUM_INITIALIZER { 0, NULL, NULL }
 
 /* Basics. */
 void ovsdb_datum_init_empty(struct ovsdb_datum *);
diff --git a/lib/ovsdb-idl-provider.h b/lib/ovsdb-idl-provider.h
index 1f5a49eaa..8cfbb95aa 100644
--- a/lib/ovsdb-idl-provider.h
+++ b/lib/ovsdb-idl-provider.h
@@ -102,10 +102,6 @@ struct ovsdb_idl_table_class {
     void (*row_init)(struct ovsdb_idl_row *);
 };
 
-struct ovsdb_idl_condition {
-    struct ovs_list clauses;
-};
-
 struct ovsdb_idl_table {
     const struct ovsdb_idl_table_class *class;
     unsigned char *modes;    /* OVSDB_IDL_* bitmasks, indexed by column. */
diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index 3974eb669..1be1c6805 100644
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -24,6 +24,7 @@
 
 #include "bitmap.h"
 #include "coverage.h"
+#include "hash.h"
 #include "openvswitch/dynamic-string.h"
 #include "fatal-signal.h"
 #include "openvswitch/json.h"
@@ -219,7 +220,6 @@ ovsdb_idl_table_from_class(const struct ovsdb_idl *,
                            const struct ovsdb_idl_table_class *);
 static bool ovsdb_idl_track_is_set(struct ovsdb_idl_table *table);
 static void ovsdb_idl_send_cond_change(struct ovsdb_idl *idl);
-static void ovsdb_idl_condition_init(struct ovsdb_idl_condition *cnd);
 
 /* Creates and returns a connection to database 'remote', which should be in a
  * form acceptable to jsonrpc_session_open().  The connection will maintain an
@@ -279,6 +279,7 @@ ovsdb_idl_create(const char *remote, const struct ovsdb_idl_class *class,
             = table->change_seqno[OVSDB_IDL_CHANGE_DELETE] = 0;
         table->idl = idl;
         ovsdb_idl_condition_init(&table->condition);
+        ovsdb_idl_condition_add_clause_true(&table->condition);
         table->cond_changed = false;
     }
 
@@ -837,147 +838,210 @@ ovsdb_idl_add_table(struct ovsdb_idl *idl,
     OVS_NOT_REACHED();
 }
 
+/* A single clause within an ovsdb_idl_condition. */
 struct ovsdb_idl_clause {
-    struct ovs_list node;
-    enum ovsdb_function function;
-    const struct ovsdb_idl_column *column;
-    struct ovsdb_datum arg;
+    struct hmap_node hmap_node;   /* In struct ovsdb_idl_condition. */
+    enum ovsdb_function function; /* Never OVSDB_F_TRUE or OVSDB_F_FALSE. */
+    const struct ovsdb_idl_column *column; /* Must be nonnull. */
+    struct ovsdb_datum arg;       /* Has ovsdb_type ->column->type. */
 };
 
-static struct json *
-ovsdb_idl_clause_to_json(const struct ovsdb_idl_clause *clause)
+static uint32_t
+ovsdb_idl_clause_hash(const struct ovsdb_idl_clause *clause)
 {
-    if (clause->function != OVSDB_F_TRUE &&
-        clause->function != OVSDB_F_FALSE) {
-        const char *function = ovsdb_function_to_string(clause->function);
+    uint32_t hash = hash_pointer(clause->column, clause->function);
+    return ovsdb_datum_hash(&clause->arg, &clause->column->type, hash);
+}
 
-        return json_array_create_3(json_string_create(clause->column->name),
-                                   json_string_create(function),
-                                   ovsdb_datum_to_json(&clause->arg,
-                                                       &clause->column->type));
-    }
+static int
+ovsdb_idl_clause_equals(const struct ovsdb_idl_clause *a,
+                        const struct ovsdb_idl_clause *b)
+{
+    return (a->function == b->function
+            && a->column == b->column
+            && ovsdb_datum_equals(&a->arg, &b->arg, &a->column->type));
+}
 
-    return json_boolean_create(clause->function == OVSDB_F_TRUE ?
-                               true : false);
+static struct json *
+ovsdb_idl_clause_to_json(const struct ovsdb_idl_clause *clause)
+{
+    const char *function = ovsdb_function_to_string(clause->function);
+    return json_array_create_3(json_string_create(clause->column->name),
+                               json_string_create(function),
+                               ovsdb_datum_to_json(&clause->arg,
+                                                   &clause->column->type));
 }
 
 static void
-ovsdb_idl_clause_free(struct ovsdb_idl_clause *clause)
+ovsdb_idl_clause_destroy(struct ovsdb_idl_clause *clause)
 {
-    if (clause->function != OVSDB_F_TRUE &&
-        clause->function != OVSDB_F_FALSE) {
+    if (clause) {
         ovsdb_datum_destroy(&clause->arg, &clause->column->type);
+        free(clause);
     }
+}
+
+/* ovsdb_idl_condition. */
 
-    ovs_list_remove(&clause->node);
-    free(clause);
+void
+ovsdb_idl_condition_init(struct ovsdb_idl_condition *cnd)
+{
+    hmap_init(&cnd->clauses);
+    cnd->is_true = false;
 }
 
-/* Clears all of the conditional clauses from table 'tc', so that all of the
- * rows in the table will be replicated.  (This is the default, so this
- * function has an effect only if some clauses were added to 'tc' using
- * ovsdb_idl_condition_add_clause().) */
 void
-ovsdb_idl_condition_reset(struct ovsdb_idl *idl,
-                          const struct ovsdb_idl_table_class *tc)
+ovsdb_idl_condition_destroy(struct ovsdb_idl_condition *cond)
 {
-    struct ovsdb_idl_clause *c, *next;
-    struct ovsdb_idl_table *table = ovsdb_idl_table_from_class(idl, tc);
+    if (cond) {
+        ovsdb_idl_condition_clear(cond);
+        hmap_destroy(&cond->clauses);
+    }
+}
 
-    LIST_FOR_EACH_SAFE (c, next, node, &table->condition.clauses) {
-        ovsdb_idl_clause_free(c);
+void
+ovsdb_idl_condition_clear(struct ovsdb_idl_condition *cond)
+{
+    struct ovsdb_idl_clause *clause, *next;
+    HMAP_FOR_EACH_SAFE (clause, next, hmap_node, &cond->clauses) {
+        hmap_remove(&cond->clauses, &clause->hmap_node);
+        ovsdb_idl_clause_destroy(clause);
     }
-    idl->cond_changed = table->cond_changed = true;
+    cond->is_true = false;
 }
 
-static void
-ovsdb_idl_condition_init(struct ovsdb_idl_condition *cnd)
+bool
+ovsdb_idl_condition_is_true(const struct ovsdb_idl_condition *condition)
 {
-    ovs_list_init(&cnd->clauses);
+    return condition->is_true;
 }
 
 static struct ovsdb_idl_clause *
-ovsdb_idl_condition_find_clause(struct ovsdb_idl_table *table,
-                                enum ovsdb_function function,
-                                const struct ovsdb_idl_column *column,
-                                const struct ovsdb_datum *arg)
-{
-    struct ovsdb_idl_clause *c;
-    LIST_FOR_EACH (c, node, &table->condition.clauses) {
-        if (c->function == function &&
-            (!column || (c->column == column &&
-                         ovsdb_datum_equals(&c->arg,
-                                             arg, &column->type)))) {
-            return c;
+ovsdb_idl_condition_find_clause(const struct ovsdb_idl_condition *condition,
+                                const struct ovsdb_idl_clause *target,
+                                uint32_t hash)
+{
+    struct ovsdb_idl_clause *clause;
+    HMAP_FOR_EACH_WITH_HASH (clause, hmap_node, hash, &condition->clauses) {
+        if (ovsdb_idl_clause_equals(clause, target)) {
+            return clause;
         }
     }
     return NULL;
 }
 
+static void
+ovsdb_idl_condition_add_clause__(struct ovsdb_idl_condition *condition,
+                                 const struct ovsdb_idl_clause *src,
+                                 uint32_t hash)
+{
+    struct ovsdb_idl_clause *clause = xmalloc(sizeof *clause);
+    clause->function = src->function;
+    clause->column = src->column;
+    ovsdb_datum_clone(&clause->arg, &src->arg, &src->column->type);
+    hmap_insert(&condition->clauses, &clause->hmap_node, hash);
+}
+
 /* Adds a clause to the condition for replicating the table with class 'tc' in
  * 'idl'.
  *
- * By default, a table has no clauses, and in that case the IDL replicates all
- * its rows.  When a table has one or more clauses, the IDL replicates only
- * rows that satisfy at least one clause.
+ * The IDL replicates only rows in a table that satisfy at least one clause in
+ * the table's condition.  The default condition for a table has a single
+ * clause with function OVSDB_F_TRUE, so that the IDL replicates all rows in
+ * the table.  When the IDL client replaces the default condition by one of its
+ * own, the condition can have any number of clauses.  If it has no conditions,
+ * then no rows are replicated.
  *
- * Two distinct of clauses can be added:
+ * Two distinct of clauses can usefully be added:
  *
- *    - A 'function' of OVSDB_F_FALSE or OVSDB_F_TRUE adds a Boolean clause.  A
- *      "false" clause by itself prevents any rows from being replicated; in
- *      combination with other clauses it has no effect.  A "true" clause
- *      causes every row to be replicated, regardless of whether other clauses
- *      exist (thus, a condition that includes "true" is like a condition
- *      without any clauses at all).
+ *    - A 'function' of OVSDB_F_TRUE.  A "true" clause causes every row to be
+ *      replicated, regardless of whether other clauses exist.  'column' and
+ *      'arg' are ignored.
  *
- *      'column' should be NULL and 'arg' should be an empty datum (initialized
- *      with ovsdb_datum_init_empty()).
- *
- *    - Other 'functions' add a clause of the form "<column> <function> <arg>",
- *      e.g. "column == 5" or "column <= 10".  In this case, 'arg' must have a
- *      type that is compatible with 'column'.
+ *    - Binary 'functions' add a clause of the form "<column> <function>
+ *      <arg>", e.g. "column == 5" or "column <= 10".  In this case, 'arg' must
+ *      have a type that is compatible with 'column'.
  */
 void
-ovsdb_idl_condition_add_clause(struct ovsdb_idl *idl,
-                               const struct ovsdb_idl_table_class *tc,
+ovsdb_idl_condition_add_clause(struct ovsdb_idl_condition *condition,
                                enum ovsdb_function function,
                                const struct ovsdb_idl_column *column,
                                const struct ovsdb_datum *arg)
 {
-    struct ovsdb_idl_table *table = ovsdb_idl_table_from_class(idl, tc);
+    if (condition->is_true) {
+        /* Adding a clause to an always-true condition has no effect.  */
+    } else if (function == OVSDB_F_TRUE) {
+        ovsdb_idl_condition_add_clause_true(condition);
+    } else if (function == OVSDB_F_FALSE) {
+        /* Adding a "false" clause never has any effect. */
+    } else {
+        struct ovsdb_idl_clause clause = {
+            .function = function,
+            .column = column,
+            .arg = *arg,
+        };
+        uint32_t hash = ovsdb_idl_clause_hash(&clause);
+        if (!ovsdb_idl_condition_find_clause(condition, &clause, hash)) {
+            ovsdb_idl_condition_add_clause__(condition, &clause, hash);
+        }
+    }
+}
 
-    /* Return without doing anything, if this would be a duplicate clause. */
-    if (ovsdb_idl_condition_find_clause(table, function, column, arg)) {
-        return;
+void
+ovsdb_idl_condition_add_clause_true(struct ovsdb_idl_condition *condition)
+{
+    if (!condition->is_true) {
+        ovsdb_idl_condition_clear(condition);
+        condition->is_true = true;
     }
+}
 
-    struct ovsdb_idl_clause *clause = xzalloc(sizeof *clause);
-    ovs_list_init(&clause->node);
-    clause->function = function;
-    clause->column = column;
-    ovsdb_datum_clone(&clause->arg, arg,
-                      column ? &column->type : &ovsdb_type_boolean);
-    ovs_list_push_back(&table->condition.clauses, &clause->node);
-    idl->cond_changed = table->cond_changed = true;
-    poll_immediate_wake();
+static bool
+ovsdb_idl_condition_equals(const struct ovsdb_idl_condition *a,
+                           const struct ovsdb_idl_condition *b)
+{
+    if (hmap_count(&a->clauses) != hmap_count(&b->clauses)) {
+        return false;
+    }
+    if (a->is_true != b->is_true) {
+        return false;
+    }
+
+    const struct ovsdb_idl_clause *clause;
+    HMAP_FOR_EACH (clause, hmap_node, &a->clauses) {
+        if (!ovsdb_idl_condition_find_clause(b, clause,
+                                             clause->hmap_node.hash)) {
+            return false;
+        }
+    }
+    return true;
+}
+
+static void
+ovsdb_idl_condition_clone(struct ovsdb_idl_condition *dst,
+                          const struct ovsdb_idl_condition *src)
+{
+    ovsdb_idl_condition_init(dst);
+
+    dst->is_true = src->is_true;
+
+    const struct ovsdb_idl_clause *clause;
+    HMAP_FOR_EACH (clause, hmap_node, &src->clauses) {
+        ovsdb_idl_condition_add_clause__(dst, clause, clause->hmap_node.hash);
+    }
 }
 
-/* If a clause matching (function, column, arg) is included in the condition
- * for 'tc' within 'idl', removes it.  (If this was the last clause included in
- * the table's condition, then this means that the IDL will begin replicating
- * every row in the table.) */
+/* Sets the replication condition for 'tc' in 'idl' to 'condition' and arranges
+ * to send the new condition to the database server. */
 void
-ovsdb_idl_condition_remove_clause(struct ovsdb_idl *idl,
-                                  const struct ovsdb_idl_table_class *tc,
-                                  enum ovsdb_function function,
-                                  const struct ovsdb_idl_column *column,
-                                  const struct ovsdb_datum *arg)
+ovsdb_idl_set_condition(struct ovsdb_idl *idl,
+                        const struct ovsdb_idl_table_class *tc,
+                        const struct ovsdb_idl_condition *condition)
 {
     struct ovsdb_idl_table *table = ovsdb_idl_table_from_class(idl, tc);
-    struct ovsdb_idl_clause *c
-        = ovsdb_idl_condition_find_clause(table, function, column, arg);
-    if (c) {
-        ovsdb_idl_clause_free(c);
+    if (!ovsdb_idl_condition_equals(condition, &table->condition)) {
+        ovsdb_idl_condition_destroy(&table->condition);
+        ovsdb_idl_condition_clone(&table->condition, condition);
         idl->cond_changed = table->cond_changed = true;
         poll_immediate_wake();
     }
@@ -986,18 +1050,25 @@ ovsdb_idl_condition_remove_clause(struct ovsdb_idl *idl,
 static struct json *
 ovsdb_idl_condition_to_json(const struct ovsdb_idl_condition *cnd)
 {
-    struct json **clauses;
-    size_t i = 0, n_clauses = ovs_list_size(&cnd->clauses);
-    struct ovsdb_idl_clause *c;
+    if (cnd->is_true) {
+        return json_array_create_empty();
+    }
 
-    clauses = xmalloc(n_clauses * sizeof *clauses);
-    LIST_FOR_EACH (c, node, &cnd->clauses) {
-        clauses[i++] = ovsdb_idl_clause_to_json(c);
+    size_t n = hmap_count(&cnd->clauses);
+    if (!n) {
+        return json_array_create_1(json_boolean_create(false));
     }
 
-    return json_array_create(clauses, n_clauses);
+    struct json **clauses = xmalloc(n * sizeof *clauses);
+    const struct ovsdb_idl_clause *clause;
+    size_t i = 0;
+    HMAP_FOR_EACH (clause, hmap_node, &cnd->clauses) {
+        clauses[i++] = ovsdb_idl_clause_to_json(clause);
+    }
+    ovs_assert(i == n);
+    return json_array_create(clauses, n);
 }
-
+
 static struct json *
 ovsdb_idl_create_cond_change_req(struct ovsdb_idl_table *table)
 {
@@ -1398,8 +1469,8 @@ ovsdb_idl_send_monitor_request__(struct ovsdb_idl *idl,
 
             monitor_request = json_object_create();
             json_object_put(monitor_request, "columns", columns);
-            if (!strcmp(method, "monitor_cond") && table->cond_changed &&
-                ovs_list_size(&table->condition.clauses) > 0) {
+            if (!strcmp(method, "monitor_cond")
+                && !ovsdb_idl_condition_is_true(&table->condition)) {
                 where = ovsdb_idl_condition_to_json(&table->condition);
                 json_object_put(monitor_request, "where", where);
                 table->cond_changed = false;
diff --git a/lib/ovsdb-idl.h b/lib/ovsdb-idl.h
index 8786a418f..f0326b449 100644
--- a/lib/ovsdb-idl.h
+++ b/lib/ovsdb-idl.h
@@ -326,19 +326,31 @@ int ovsdb_idl_loop_commit_and_wait(struct ovsdb_idl_loop *);
  * replicates every row in the table.  These functions allow the client to
  * specify that only selected rows should be replicated, by constructing a
  * per-table condition that specifies the rows to replicate.
+ *
+ * A condition is a disjunction of clauses.  The condition is true, and thus a
+ * row is replicated, if any of the clauses evaluates to true for a given row.
+ * (Thus, a condition with no clauses is always false.)
  */
 
-void ovsdb_idl_condition_reset(struct ovsdb_idl *idl,
-                               const struct ovsdb_idl_table_class *tc);
-void ovsdb_idl_condition_add_clause(struct ovsdb_idl *idl,
-                                    const struct ovsdb_idl_table_class *tc,
+struct ovsdb_idl_condition {
+    struct hmap clauses;        /* Contains "struct ovsdb_idl_clause"s. */
+    bool is_true;               /* Is the condition unconditionally true? */
+};
+#define OVSDB_IDL_CONDITION_INIT(CONDITION) \
+    { HMAP_INITIALIZER(&(CONDITION)->clauses), false }
+
+void ovsdb_idl_condition_init(struct ovsdb_idl_condition *);
+void ovsdb_idl_condition_clear(struct ovsdb_idl_condition *);
+void ovsdb_idl_condition_destroy(struct ovsdb_idl_condition *);
+void ovsdb_idl_condition_add_clause(struct ovsdb_idl_condition *,
                                     enum ovsdb_function function,
                                     const struct ovsdb_idl_column *column,
                                     const struct ovsdb_datum *arg);
-void ovsdb_idl_condition_remove_clause(struct ovsdb_idl *idl,
-                                       const struct ovsdb_idl_table_class *tc,
-                                       enum ovsdb_function function,
-                                       const struct ovsdb_idl_column *column,
-                                       const struct ovsdb_datum *arg);
+void ovsdb_idl_condition_add_clause_true(struct ovsdb_idl_condition *);
+bool ovsdb_idl_condition_is_true(const struct ovsdb_idl_condition *);
+
+void ovsdb_idl_set_condition(struct ovsdb_idl *,
+                             const struct ovsdb_idl_table_class *,
+                             const struct ovsdb_idl_condition *);
 
 #endif /* ovsdb-idl.h */
diff --git a/ovsdb/ovsdb-idlc.in b/ovsdb/ovsdb-idlc.in
index cf6cd5845..f1c7a3598 100755
--- a/ovsdb/ovsdb-idlc.in
+++ b/ovsdb/ovsdb-idlc.in
@@ -243,7 +243,7 @@ bool %(s)s_is_updated(const struct %(s)s *, enum %(s)s_column_id);
                 print 'void %(s)s_update_%(c)s_delvalue(const struct %(s)s *, ' % {'s': structName, 'c': columnName},
                 print '%(valtype)s);' % {'valtype':column.type.key.to_const_c_type(prefix)}
 
-            print 'void %(s)s_add_clause_%(c)s(struct ovsdb_idl *idl, enum ovsdb_function function,' % {'s': structName, 'c': columnName},
+            print 'void %(s)s_add_clause_%(c)s(struct ovsdb_idl_condition *, enum ovsdb_function function,' % {'s': structName, 'c': columnName},
             if column.type.is_smap():
                 args = ['const struct smap *']
             else:
@@ -252,22 +252,7 @@ bool %(s)s_is_updated(const struct %(s)s *, enum %(s)s_column_id);
                 args = ['%(type)s%(name)s' % member for member in members]
             print '%s);' % ', '.join(args)
 
-        print 'void %s_add_clause_true(struct ovsdb_idl *idl);' % structName
-        print 'void %s_add_clause_false(struct ovsdb_idl *idl);' % structName
-
-        print
-        for columnName, column in sorted_columns(table):
-            print 'void %(s)s_remove_clause_%(c)s(struct ovsdb_idl *idl, enum ovsdb_function function,' % {'s': structName, 'c': columnName},
-            if column.type.is_smap():
-                args = ['const struct smap *']
-            else:
-                comment, members = cMembers(prefix, tableName, columnName,
-                                            column, True, refTable=False)
-                args = ['%(type)s%(name)s' % member for member in members]
-            print '%s);' % ', '.join(args)
-
-        print 'void %s_remove_clause_true(struct ovsdb_idl *idl);' % structName
-        print 'void %s_remove_clause_false(struct ovsdb_idl *idl);' % structName
+            print 'void %(s)s_set_condition(struct ovsdb_idl *, struct ovsdb_idl_condition *);' % {'s': structName},
 
         print
 
@@ -870,7 +855,7 @@ void
             if type.is_smap():
                 print comment
                 print """void
-%(s)s_add_clause_%(c)s(struct ovsdb_idl *idl, enum ovsdb_function function, const struct smap *%(c)s)
+%(s)s_add_clause_%(c)s(struct ovsdb_idl_condition *cond, enum ovsdb_function function, const struct smap *%(c)s)
 {
     struct ovsdb_datum datum;
 
@@ -880,8 +865,7 @@ void
         ovsdb_datum_init_empty(&datum);
     }
 
-    ovsdb_idl_condition_add_clause(idl,
-                                   &%(p)stable_%(tl)s,
+    ovsdb_idl_condition_add_clause(cond,
                                    function,
                                    &%(s)s_col_%(c)s,
                                    &datum);
@@ -911,7 +895,7 @@ void
 
             print comment
             print 'void'
-            print '%(s)s_add_clause_%(c)s(struct ovsdb_idl *idl, enum ovsdb_function function, %(args)s)' % \
+            print '%(s)s_add_clause_%(c)s(struct ovsdb_idl_condition *cond, enum ovsdb_function function, %(args)s)' % \
                 {'s': structName, 'c': columnName,
                  'args': ', '.join(['%(type)s%(name)s' % m for m in members])}
             print "{"
@@ -975,7 +959,7 @@ void
                 print "    ovsdb_datum_sort_unique(&datum, %s, %s);" % (
                     type.key.toAtomicType(), valueType)
 
-            print"""    ovsdb_idl_condition_add_clause(idl, &%(p)stable_%(tl)s,
+            print"""    ovsdb_idl_condition_add_clause(cond,
                           function,
                           &%(s)s_col_%(c)s,
                           &datum);\
@@ -990,177 +974,14 @@ void
                 print "    free(%s);" % var
             print "}"
 
-        print """\
-void
-%(s)s_add_clause_false(struct ovsdb_idl *idl)
-{
-    struct ovsdb_datum datum;
-
-    ovsdb_datum_init_empty(&datum);
-    ovsdb_idl_condition_add_clause(idl, &%(p)stable_%(tl)s, OVSDB_F_FALSE, NULL, &datum);
-}""" % {'s': structName,
-        'tl': tableName.lower(),
-        'p': prefix,
-        'P': prefix.upper()}
-
-        print """void
-%(s)s_add_clause_true(struct ovsdb_idl *idl)
-{
-    struct ovsdb_datum datum;
-
-    ovsdb_datum_init_empty(&datum);
-    ovsdb_idl_condition_add_clause(idl, &%(p)stable_%(tl)s, OVSDB_F_TRUE, NULL, &datum);
-}""" % {'s': structName,
-        'tl': tableName.lower(),
-        'p': prefix,
-        'P': prefix.upper()}
-
-        # Remove clause functions.
-        for columnName, column in sorted_columns(table):
-            type = column.type
-
-            comment, members = cMembers(prefix, tableName, columnName,
-                                        column, True, refTable=False)
-
-            if type.is_smap():
-                print comment
-                print """void
-%(s)s_remove_clause_%(c)s(struct ovsdb_idl *idl, enum ovsdb_function function, const struct smap *%(c)s)
-{
-    struct ovsdb_datum datum;
-
-    if (%(c)s) {
-        ovsdb_datum_from_smap(&datum, %(c)s);
-    } else {
-        ovsdb_datum_init_empty(&datum);
-    }
-
-    ovsdb_idl_condition_remove_clause(idl, &%(p)stable_%(tl)s,
-                                      function,
-                                      &%(s)s_col_%(c)s,
-                                      &datum);
-
-    ovsdb_datum_destroy(&datum, &%(s)s_col_%(c)s.type);
-}
-""" % {'tl': tableName.lower(),
-       'p': prefix,
-       'P': prefix.upper(),
-       's': structName,
-       'S': structName.upper(),
-       'c': columnName}
-                continue
-
-            keyVar = members[0]['name']
-            nVar = None
-            valueVar = None
-            if type.value:
-                valueVar = members[1]['name']
-                if len(members) > 2:
-                    nVar = members[2]['name']
-            else:
-                if len(members) > 1:
-                    nVar = members[1]['name']
-
-            print comment
-            print 'void'
-            print '%(s)s_remove_clause_%(c)s(struct ovsdb_idl *idl, enum ovsdb_function function, %(args)s)' % \
-                {'s': structName, 'c': columnName,
-                 'args': ', '.join(['%(type)s%(name)s' % m for m in members])}
-            print "{"
-            print "    struct ovsdb_datum datum;"
-            free = []
-            if type.n_min == 1 and type.n_max == 1:
-                print "    union ovsdb_atom key;"
-                if type.value:
-                    print "    union ovsdb_atom value;"
-                print
-                print "    datum.n = 1;"
-                print "    datum.keys = &key;"
-                print "    " + type.key.assign_c_value_casting_away_const("key.%s" % type.key.type.to_string(), keyVar, refTable=False)
-                if type.value:
-                    print "    datum.values = &value;"
-                    print "    "+ type.value.assign_c_value_casting_away_const("value.%s" % type.value.type.to_string(), valueVar, refTable=False)
-                else:
-                    print "    datum.values = NULL;"
-            elif type.is_optional_pointer():
-                print "    union ovsdb_atom key;"
-                print
-                print "    if (%s) {" % keyVar
-                print "        datum.n = 1;"
-                print "        datum.keys = &key;"
-                print "        " + type.key.assign_c_value_casting_away_const("key.%s" % type.key.type.to_string(), keyVar, refTable=False)
-                print "    } else {"
-                print "        datum.n = 0;"
-                print "        datum.keys = NULL;"
-                print "    }"
-                print "    datum.values = NULL;"
-            elif type.n_max == 1:
-                print "    union ovsdb_atom key;"
-                print
-                print "    if (%s) {" % nVar
-                print "        datum.n = 1;"
-                print "        datum.keys = &key;"
-                print "        " + type.key.assign_c_value_casting_away_const("key.%s" % type.key.type.to_string(), "*" + keyVar, refTable=False)
-                print "    } else {"
-                print "        datum.n = 0;"
-                print "        datum.keys = NULL;"
-                print "    }"
-                print "    datum.values = NULL;"
-            else:
-                print "    datum.n = %s;" % nVar
-                print "    datum.keys = %s ? xmalloc(%s * sizeof *datum.keys) : NULL;" % (nVar, nVar)
-                free += ['datum.keys']
-                if type.value:
-                    free += ['datum.values']
-                    print "    datum.values = xmalloc(%s * sizeof *datum.values);" % nVar
-                else:
-                    print "    datum.values = NULL;"
-                print "    for (size_t i = 0; i < %s; i++) {" % nVar
-                print "        " + type.key.assign_c_value_casting_away_const("datum.keys[i].%s" % type.key.type.to_string(), "%s[i]" % keyVar, refTable=False)
-                if type.value:
-                    print "        " + type.value.assign_c_value_casting_away_const("datum.values[i].%s" % type.value.type.to_string(), "%s[i]" % valueVar, refTable=False)
-                print "    }"
-                if type.value:
-                    valueType = type.value.toAtomicType()
-                else:
-                    valueType = "OVSDB_TYPE_VOID"
-                print "    ovsdb_datum_sort_unique(&datum, %s, %s);" % (
-                    type.key.toAtomicType(), valueType)
-
-            print"""    ovsdb_idl_condition_remove_clause(idl, &%(p)stable_%(tl)s,
-                          function,
-                          &%(s)s_col_%(c)s,
-                          &datum);\
-""" % {'tl': tableName.lower(),
-       'p': prefix,
-       'P': prefix.upper(),
-       's': structName,
-       'S': structName.upper(),
-       'c': columnName}
-            for var in free:
-                print "    free(%s);" % var
-            print "}"
-
-        print """void
-%(s)s_remove_clause_false(struct ovsdb_idl *idl)
-{
-    struct ovsdb_datum datum;
-
-    ovsdb_datum_init_empty(&datum);
-    ovsdb_idl_condition_remove_clause(idl, &%(p)stable_%(tl)s, OVSDB_F_FALSE, NULL, &datum);
-}
-
+        print """
 void
-%(s)s_remove_clause_true(struct ovsdb_idl *idl)
+%(s)s_set_condition(struct ovsdb_idl *idl, struct ovsdb_idl_condition *condition)
 {
-    struct ovsdb_datum datum;
-
-    ovsdb_datum_init_empty(&datum);
-    ovsdb_idl_condition_remove_clause(idl, &%(p)stable_%(tl)s, OVSDB_F_TRUE, NULL, &datum);
-}""" % {'s': structName,
-        'tl': tableName.lower(),
-        'p': prefix,
-        'P': prefix.upper(),}
+    ovsdb_idl_set_condition(idl, &%(p)stable_%(tl)s, condition);
+}""" % {'p': prefix,
+        's': structName,
+        'tl': tableName.lower()}
 
         # Table columns.
         for columnName, column in sorted_columns(table):
diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py
index 0178050d9..e43457c0b 100644
--- a/python/ovs/db/idl.py
+++ b/python/ovs/db/idl.py
@@ -1,4 +1,4 @@
-# Copyright (c) 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
+# Copyright (c) 2009, 2010, 2011, 2012, 2013, 2016 Nicira, Inc.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -144,7 +144,7 @@ class Idl(object):
             table.need_table = False
             table.rows = {}
             table.idl = self
-            table.condition = []
+            table.condition = [True]
             table.cond_changed = False
 
     def close(self):
@@ -265,23 +265,23 @@ class Idl(object):
                 self.__send_cond_change(table, table.condition)
                 table.cond_changed = False
 
-    def cond_change(self, table_name, add_cmd, cond):
-        """Change conditions for this IDL session. If session is not already
-        connected, add condtion to table and submit it on send_monitor_request.
-        Otherwise  send monitor_cond_change method with the requested
-        changes."""
+    def cond_change(self, table_name, cond):
+        """Sets the condition for 'table_name' to 'cond', which should be a
+        conditional expression suitable for use directly in the OVSDB
+        protocol, with the exception that the empty condition []
+        matches no rows (instead of matching every row).  That is, []
+        is equivalent to [False], not to [True].
+        """
 
         table = self.tables.get(table_name)
         if not table:
             raise error.Error('Unknown table "%s"' % table_name)
 
-        if add_cmd:
-            table.condition += cond
-        else:
-            for c in cond:
-                table.condition.remove(c)
-
-        table.cond_changed = True
+        if cond == []:
+            cond = [False]
+        if table.condition != cond:
+            table.condition = cond
+            table.cond_changed = True
 
     def wait(self, poller):
         """Arranges for poller.block() to wake up when self.run() has something
@@ -420,8 +420,7 @@ class Idl(object):
                         (column not in self.readonly[table.name])):
                     columns.append(column)
             monitor_requests[table.name] = {"columns": columns}
-            if method == "monitor_cond" and table.cond_changed and \
-                   table.condition:
+            if method == "monitor_cond" and table.condition != [True]:
                 monitor_requests[table.name]["where"] = table.condition
                 table.cond_change = False
 
diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at
index 9ed431bf9..d2c1ea6f3 100644
--- a/tests/ovsdb-idl.at
+++ b/tests/ovsdb-idl.at
@@ -393,8 +393,8 @@ OVSDB_CHECK_IDL([simple idl, conditional, false condition],
        "row": {"i": 1,
                "r": 2.0,
                "b": true}}]']],
-  [['condition add simple [false]' \
-    'condition remove simple [false]']],
+  [['condition simple []' \
+    'condition simple [true]']],
   [[000: change conditions
 001: empty
 002: change conditions
@@ -409,8 +409,8 @@ OVSDB_CHECK_IDL([simple idl, conditional, true condition],
        "row": {"i": 1,
                "r": 2.0,
                "b": true}}]']],
-  [['condition add simple [false]' \
-    'condition add simple [true]']],
+  [['condition simple []' \
+    'condition simple [true]']],
   [[000: change conditions
 001: empty
 002: change conditions
@@ -430,8 +430,8 @@ OVSDB_CHECK_IDL([simple idl, conditional, multiple clauses in condition],
        "row": {"i": 2,
                "r": 3.0,
                "b": true}}]']],
-  [['condition add simple [false]' \
-    'condition add simple [["i","==",1],["i","==",2]]']],
+  [['condition simple []' \
+    'condition simple [["i","==",1],["i","==",2]]']],
   [[000: change conditions
 001: empty
 002: change conditions
@@ -447,8 +447,8 @@ OVSDB_CHECK_IDL([simple idl, conditional, modify as insert due to condition],
        "row": {"i": 1,
                "r": 2.0,
                "b": true}}]']],
-  [['condition add simple [false]' \
-    'condition add simple [["i","==",1]]']],
+  [['condition simple []' \
+    'condition simple [["i","==",1]]']],
   [[000: change conditions
 001: empty
 002: change conditions
@@ -463,9 +463,9 @@ OVSDB_CHECK_IDL([simple idl, conditional, modify as delete due to condition],
        "row": {"i": 1,
                "r": 2.0,
                "b": true}}]']],
-  [['condition add simple [false]' \
-    'condition add simple [["i","==",1],["i","==",2]]' \
-    'condition remove simple [["i","==",1]]' \
+  [['condition simple []' \
+    'condition simple [["i","==",1],["i","==",2]]' \
+    'condition simple [["i","==",2]]' \
     '["idltest",
        {"op": "insert",
        "table": "simple",
@@ -498,10 +498,10 @@ OVSDB_CHECK_IDL([simple idl, conditional, multiple tables],
        "table": "link2",
        "row": {"i": 2},
        "uuid-name": "row0"}]']],
-  [['condition add simple [false];condition add link1 [false];condition add link2 [false]' \
-    'condition add simple [["i","==",1]]' \
-    'condition add link1 [["i","==",0]]' \
-    'condition add link2 [["i","==",3]]' \
+  [['condition simple [];link1 [];link2 []' \
+    'condition simple [["i","==",1]]' \
+    'condition link1 [["i","==",0]]' \
+    'condition link2 [["i","==",3]]' \
     '+["idltest",
        {"op": "insert",
        "table": "link2",
diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c
index 6a7d46753..968c1de22 100644
--- a/tests/test-ovsdb.c
+++ b/tests/test-ovsdb.c
@@ -2184,145 +2184,50 @@ find_table_class(const char *name)
 }
 
 static void
-parse_simple_json_clause(struct ovsdb_idl *idl, bool add_cmd,
-                         struct json *json)
-{
-    const char *c;
-    struct ovsdb_error *error;
-    enum ovsdb_function function;
-
-    if (json->type == JSON_TRUE) {
-        add_cmd ? idltest_simple_add_clause_true(idl) :
-            idltest_simple_remove_clause_true(idl);
-        return;
-    } else if (json->type == JSON_FALSE) {
-        add_cmd ? idltest_simple_add_clause_false(idl) :
-            idltest_simple_remove_clause_false(idl);
-        return;
-    }
-    if (json->type != JSON_ARRAY || json->u.array.n != 3) {
-        ovs_fatal(0, "Error parsing condition");
-    }
-
-    c = json_string(json->u.array.elems[0]);
-    error = ovsdb_function_from_string(json_string(json->u.array.elems[1]),
-                                       &function);
-    if (error) {
-        ovs_fatal(0, "Error parsing clause function %s",
-                  json_string(json->u.array.elems[1]));
-    }
-
-    /* add clause according to column */
-    if (!strcmp(c, "b")) {
-        add_cmd ? idltest_simple_add_clause_b(idl, function,
-                                     json_boolean(json->u.array.elems[2])) :
-            idltest_simple_remove_clause_b(idl, function,
-                                     json_boolean(json->u.array.elems[2]));
-    } else if (!strcmp(c, "i")) {
-        add_cmd ? idltest_simple_add_clause_i(idl, function,
-                                    json_integer(json->u.array.elems[2])) :
-            idltest_simple_remove_clause_i(idl, function,
-                                    json_integer(json->u.array.elems[2]));
-    } else if (!strcmp(c, "s")) {
-        add_cmd ? idltest_simple_add_clause_s(idl, function,
-                                    json_string(json->u.array.elems[2])) :
-            idltest_simple_remove_clause_s(idl, function,
-                                    json_string(json->u.array.elems[2]));
-    } else if (!strcmp(c, "u")) {
+parse_simple_json_clause(struct ovsdb_idl_condition *cond,
+                         enum ovsdb_function function,
+                         const char *column, const struct json *arg)
+{
+    if (!strcmp(column, "b")) {
+        idltest_simple_add_clause_b(cond, function, json_boolean(arg));
+    } else if (!strcmp(column, "i")) {
+         idltest_simple_add_clause_i(cond, function, json_integer(arg));
+    } else if (!strcmp(column, "s")) {
+        idltest_simple_add_clause_s(cond, function, json_string(arg));
+    } else if (!strcmp(column, "u")) {
         struct uuid uuid;
-        if (!uuid_from_string(&uuid,
-                              json_string(json->u.array.elems[2]))) {
-            ovs_fatal(0, "\"%s\" is not a valid UUID",
-                      json_string(json->u.array.elems[2]));
+        if (!uuid_from_string(&uuid, json_string(arg))) {
+            ovs_fatal(0, "\"%s\" is not a valid UUID", json_string(arg));
         }
-        add_cmd ? idltest_simple_add_clause_u(idl, function, uuid) :
-            idltest_simple_remove_clause_u(idl, function, uuid);
-    } else if (!strcmp(c, "r")) {
-        add_cmd ? idltest_simple_add_clause_r(idl, function,
-                                    json_real(json->u.array.elems[2])) :
-            idltest_simple_remove_clause_r(idl, function,
-                                    json_real(json->u.array.elems[2]));
+        idltest_simple_add_clause_u(cond, function, uuid);
+    } else if (!strcmp(column, "r")) {
+        idltest_simple_add_clause_r(cond, function, json_real(arg));
     } else {
-        ovs_fatal(0, "Unsupported columns name %s", c);
+        ovs_fatal(0, "Unsupported columns name %s", column);
     }
 }
 
 static void
-parse_link1_json_clause(struct ovsdb_idl *idl, bool add_cmd,
-                        struct json *json)
+parse_link1_json_clause(struct ovsdb_idl_condition *cond,
+                        enum ovsdb_function function,
+                        const char *column, const struct json *arg)
 {
-    const char *c;
-    struct ovsdb_error *error;
-    enum ovsdb_function function;
-
-    if (json->type == JSON_TRUE) {
-        add_cmd ? idltest_link1_add_clause_true(idl) :
-            idltest_link1_remove_clause_true(idl);
-        return;
-    } else if (json->type == JSON_FALSE) {
-        add_cmd ? idltest_link1_add_clause_false(idl) :
-            idltest_link1_remove_clause_false(idl);
-        return;
-    }
-    if (json->type != JSON_ARRAY || json->u.array.n != 3) {
-        ovs_fatal(0, "Error parsing condition");
-    }
-
-    c = json_string(json->u.array.elems[0]);
-    error = ovsdb_function_from_string(json_string(json->u.array.elems[1]),
-                                       &function);
-    if (error) {
-        ovs_fatal(0, "Error parsing clause function %s",
-                  json_string(json->u.array.elems[1]));
-    }
-
-    /* add clause according to column */
-    if (!strcmp(c, "i")) {
-        add_cmd ? idltest_link1_add_clause_i(idl, function,
-                                    json_integer(json->u.array.elems[2])) :
-            idltest_link1_remove_clause_i(idl, function,
-                                    json_integer(json->u.array.elems[2]));
+    if (!strcmp(column, "i")) {
+        idltest_link1_add_clause_i(cond, function, json_integer(arg));
     } else {
-        ovs_fatal(0, "Unsupported columns name %s", c);
+        ovs_fatal(0, "Unsupported columns name %s", column);
     }
 }
 
 static void
-parse_link2_json_clause(struct ovsdb_idl *idl, bool add_cmd, struct json *json)
+parse_link2_json_clause(struct ovsdb_idl_condition *cond,
+                        enum ovsdb_function function,
+                        const char *column, const struct json *arg)
 {
-    const char *c;
-    struct ovsdb_error *error;
-    enum ovsdb_function function;
-
-    if (json->type == JSON_TRUE) {
-        add_cmd ? idltest_link2_add_clause_true(idl) :
-            idltest_link2_remove_clause_true(idl);
-        return;
-    } else if (json->type == JSON_FALSE) {
-        add_cmd ? idltest_link2_add_clause_false(idl) :
-            idltest_link2_remove_clause_false(idl);
-        return;
-    }
-    if (json->type != JSON_ARRAY || json->u.array.n != 3) {
-        ovs_fatal(0, "Error parsing condition");
-    }
-
-    c = json_string(json->u.array.elems[0]);
-    error = ovsdb_function_from_string(json_string(json->u.array.elems[1]),
-                                       &function);
-    if (error) {
-        ovs_fatal(0, "Error parsing clause function %s",
-                  json_string(json->u.array.elems[1]));
-    }
-
-    /* add clause according to column */
-    if (!strcmp(c, "i")) {
-        add_cmd ? idltest_link2_add_clause_i(idl, function,
-                                    json_integer(json->u.array.elems[2])) :
-            idltest_link2_remove_clause_i(idl, function,
-                                    json_integer(json->u.array.elems[2]));
+    if (!strcmp(column, "i")) {
+        idltest_link2_add_clause_i(cond, function, json_integer(arg));
     } else {
-        ovs_fatal(0, "Unsupported columns name %s", c);
+        ovs_fatal(0, "Unsupported columns name %s", column);
     }
 }
 
@@ -2331,19 +2236,9 @@ update_conditions(struct ovsdb_idl *idl, char *commands)
 {
     char *cmd, *save_ptr1 = NULL;
     const struct ovsdb_idl_table_class *tc;
-    bool add_cmd = false;
 
     for (cmd = strtok_r(commands, ";", &save_ptr1); cmd;
          cmd = strtok_r(NULL, ";", &save_ptr1)) {
-        if (strstr(cmd, "condition add")) {
-            cmd += strlen("condition add ");
-            add_cmd = true;
-        } else if (strstr(cmd, "condition remove")) {
-            cmd += strlen("condition remove ");
-        } else {
-            ovs_fatal(0, "condition command should be add or remove");
-        }
-
         char *save_ptr2 = NULL;
         char *table_name = strtok_r(cmd, " ", &save_ptr2);
         struct json *json = parse_json(save_ptr2);
@@ -2358,17 +2253,37 @@ update_conditions(struct ovsdb_idl *idl, char *commands)
             ovs_fatal(0, "Table %s does not exist", table_name);
         }
 
-        //ovsdb_idl_condition_reset(idl, tc);
-
+        struct ovsdb_idl_condition cond = OVSDB_IDL_CONDITION_INIT(&cond);
         for (i = 0; i < json->u.array.n; i++) {
-            if (!strcmp(table_name, "simple")) {
-                parse_simple_json_clause(idl, add_cmd, json->u.array.elems[i]);
-            } else if (!strcmp(table_name, "link1")) {
-                parse_link1_json_clause(idl, add_cmd, json->u.array.elems[i]);
-            } else if (!strcmp(table_name, "link2")) {
-                parse_link2_json_clause(idl, add_cmd, json->u.array.elems[i]);
+            const struct json *clause = json->u.array.elems[i];
+            if (clause->type == JSON_TRUE) {
+                ovsdb_idl_condition_add_clause_true(&cond);
+            } else if (clause->type != JSON_ARRAY || clause->u.array.n != 3
+                       || clause->u.array.elems[0]->type != JSON_STRING
+                       || clause->u.array.elems[1]->type != JSON_STRING) {
+                ovs_fatal(0, "Error parsing condition");
+            } else {
+                enum ovsdb_function function;
+                const char *function_s = json_string(clause->u.array.elems[1]);
+                struct ovsdb_error *error = ovsdb_function_from_string(
+                    function_s, &function);
+                if (error) {
+                    ovs_fatal(0, "unknown clause function %s", function_s);
+                }
+
+                const char *column = json_string(clause->u.array.elems[0]);
+                const struct json *arg = clause->u.array.elems[2];
+                if (!strcmp(table_name, "simple")) {
+                    parse_simple_json_clause(&cond, function, column, arg);
+                } else if (!strcmp(table_name, "link1")) {
+                    parse_link1_json_clause(&cond, function, column, arg);
+                } else if (!strcmp(table_name, "link2")) {
+                    parse_link2_json_clause(&cond, function, column, arg);
+                }
             }
         }
+        ovsdb_idl_set_condition(idl, tc, &cond);
+        ovsdb_idl_condition_destroy(&cond);
         json_destroy(json);
     }
 }
@@ -2409,8 +2324,9 @@ do_idl(struct ovs_cmdl_context *ctx)
     setvbuf(stdout, NULL, _IONBF, 0);
 
     symtab = ovsdb_symbol_table_create();
-    if (ctx->argc > 2 && strstr(ctx->argv[2], "condition ")) {
-        update_conditions(idl, ctx->argv[2]);
+    const char cond_s[] = "condition ";
+    if (ctx->argc > 2 && strstr(ctx->argv[2], cond_s)) {
+        update_conditions(idl, ctx->argv[2] + strlen(cond_s));
         printf("%03d: change conditions\n", step++);
         i = 3;
     } else {
@@ -2451,8 +2367,8 @@ do_idl(struct ovs_cmdl_context *ctx)
         if (!strcmp(arg, "reconnect")) {
             printf("%03d: reconnect\n", step++);
             ovsdb_idl_force_reconnect(idl);
-        }  else if (strstr(arg, "condition ")) {
-            update_conditions(idl, arg);
+        }  else if (!strncmp(arg, cond_s, strlen(cond_s))) {
+            update_conditions(idl, arg + strlen(cond_s));
             printf("%03d: change conditions\n", step++);
         } else if (arg[0] != '[') {
             idl_set(idl, arg, step++);
diff --git a/tests/test-ovsdb.py b/tests/test-ovsdb.py
index 185215ae6..dced56b99 100644
--- a/tests/test-ovsdb.py
+++ b/tests/test-ovsdb.py
@@ -1,4 +1,4 @@
-# Copyright (c) 2009, 2010, 2011, 2012 Nicira, Inc.
+# Copyright (c) 2009, 2010, 2011, 2012, 2016 Nicira, Inc.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -531,25 +531,17 @@ def idl_set(idl, commands, step):
 
 
 def update_condition(idl, commands):
-    commands = commands.split(";")
+    commands = commands[len("condition "):].split(";")
     for command in commands:
-        command = command[len("condition "):]
-        if "add" in command:
-            add_cmd = True
-            command = command[len("add "):]
-        else:
-            add_cmd = False
-            command = command[len("remove "):]
-
         command = command.split(" ")
         if(len(command) != 2):
-            sys.stderr.write("Error parsong condition %s\n" % command)
+            sys.stderr.write("Error parsing condition %s\n" % command)
             sys.exit(1)
 
         table = command[0]
         cond = ovs.json.from_string(command[1])
 
-        idl.cond_change(table, add_cmd, cond)
+        idl.cond_change(table, cond)
 
 
 def do_idl(schema_file, remote, *commands):