A select group is currently identified by a hash of its whole content, so
any change to its member list yields a different group: a new group_id is
allocated, the group is installed from scratch, every logical flow that
referenced the old one is rewritten, and the old group is deleted.

Give the "select" action an optional "group_key" modifier.  When it is
present, the group is identified by that key plus the parts of the group
that do not depend on its members, so its group_id no longer changes as
members come and go.  The members are tracked individually as buckets,
keyed by the value the bucket loads into the result field, and each bucket
keeps a stable bucket_id derived from that key.

Signed-off-by: Alexandra Rukomoinikova <[email protected]>
---
 controller/ofctrl.c   | 176 ++++++++++++++++++++++++++-
 include/ovn/actions.h |   4 +-
 lib/actions.c         | 129 ++++++++++++++++----
 lib/extend-table.c    | 275 +++++++++++++++++++++++++++++++++++++++++-
 lib/extend-table.h    |  39 ++++++
 5 files changed, 592 insertions(+), 31 deletions(-)

diff --git a/controller/ofctrl.c b/controller/ofctrl.c
index f3ca6613d..c2f71a0d3 100644
--- a/controller/ofctrl.c
+++ b/controller/ofctrl.c
@@ -2158,6 +2158,67 @@ add_group_mod(struct ofputil_group_mod *gm,
     ofputil_uninit_group_mod(&split);
 }
 
+/* Parses 'group_string' as a 'command' group_mod and queues it up in 'msgs'.
+ * 'descr' names the operation in the log message if parsing fails. */
+static void
+add_group_mod_str(uint16_t command, const char *group_string,
+                  const char *descr, struct ofputil_bundle_ctrl_msg *bc,
+                  struct ovs_list *msgs)
+{
+    struct ofputil_group_mod gm;
+    enum ofputil_protocol usable_protocols;
+    char *error = parse_ofp_group_mod_str(&gm, command, group_string, NULL,
+                                          NULL, &usable_protocols);
+    if (!error) {
+        add_group_mod(&gm, bc, msgs);
+    } else {
+        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
+        VLOG_ERR_RL(&rl, "%s %s %s", descr, error, group_string);
+        free(error);
+    }
+    ofputil_uninit_group_mod(&gm);
+}
+
+static int
+bucket_id_cmp(const void *a_, const void *b_)
+{
+    const struct ovn_extend_table_bucket *const *a = a_;
+    const struct ovn_extend_table_bucket *const *b = b_;
+
+    return ((*a)->bucket_id > (*b)->bucket_id)
+           - ((*a)->bucket_id < (*b)->bucket_id);
+}
+
+static struct ovn_extend_table_bucket **
+sorted_buckets(struct hmap *buckets, size_t *n_bucketsp)
+{
+    *n_bucketsp = hmap_count(buckets);
+    if (!*n_bucketsp) {
+        return NULL;
+    }
+
+    struct ovn_extend_table_bucket **bs = xmalloc(*n_bucketsp * sizeof *bs);
+    struct ovn_extend_table_bucket *b;
+    size_t i = 0;
+    HMAP_FOR_EACH (b, hmap_node, buckets) {
+        bs[i++] = b;
+    }
+    qsort(bs, *n_bucketsp, sizeof *bs, bucket_id_cmp);
+
+    return bs;
+}
+
+/* Returns true if 'b' is already installed in 'installed' exactly as it is
+ * wanted, so that it needs no group_mod at all. */
+static bool
+bucket_is_installed(struct hmap *installed,
+                    const struct ovn_extend_table_bucket *b)
+{
+    const struct ovn_extend_table_bucket *e =
+        ovn_extend_table_bucket_find(installed, b->key);
+
+    return e && !strcmp(e->content, b->content);
+}
 
 static struct ofpbuf *
 encode_meter_mod(const struct ofputil_meter_mod *mm)
@@ -2912,9 +2973,26 @@ ofctrl_put(struct ovn_desired_flow_table *lflow_table,
         /* Create and install new group. */
         struct ofputil_group_mod gm;
         enum ofputil_protocol usable_protocols;
-        char *group_string = xasprintf("group_id=%"PRIu32",%s",
-                                       desired->table_id,
-                                       desired->name);
+        char *group_string;
+        if (desired->group_header) {
+            struct ds group_ds = DS_EMPTY_INITIALIZER;
+            ds_put_format(&group_ds, "group_id=%"PRIu32",%s",
+                          desired->table_id, desired->group_header);
+            size_t n_buckets;
+            struct ovn_extend_table_bucket **bs =
+                sorted_buckets(&desired->desired_buckets, &n_buckets);
+            for (size_t i = 0; i < n_buckets; i++) {
+                ds_put_format(&group_ds, ",bucket=bucket_id=%"PRIu32",%s",
+                              bs[i]->bucket_id, bs[i]->content);
+            }
+            free(bs);
+            group_string = ds_steal_cstr(&group_ds);
+            ds_destroy(&group_ds);
+        } else {
+            group_string = xasprintf("group_id=%"PRIu32",%s",
+                                     desired->table_id,
+                                     desired->name);
+        }
         char *error = parse_ofp_group_mod_str(&gm, OFPGC15_ADD, group_string,
                                               NULL, NULL, &usable_protocols);
         if (!error) {
@@ -2928,6 +3006,98 @@ ofctrl_put(struct ovn_desired_flow_table *lflow_table,
         ofputil_uninit_group_mod(&gm);
     }
 
+    /* Iterate through desired groups that already have a matching group_id
+     * installed (so they were skipped by the loop above) but managed
+     * incrementally (without deleting the group) and may need incremental
+     * bucket updates. */
+    HMAP_FOR_EACH (desired, hmap_node, &groups->desired) {
+        if (!desired->group_header) {
+            continue;
+        }
+        struct ovn_extend_table_info *existing =
+            ovn_extend_table_lookup(&groups->existing, desired);
+        if (!existing) {
+            continue;
+        }
+
+        /* Remove old buckets */
+        struct ovn_extend_table_bucket *e;
+        HMAP_FOR_EACH (e, hmap_node, &existing->existing_buckets) {
+            const struct ovn_extend_table_bucket *d =
+                ovn_extend_table_bucket_find(&desired->desired_buckets,
+                                             e->key);
+            if (d && !strcmp(d->content, e->content)) {
+                 continue;
+            }
+
+            char *group_string = xasprintf(
+                "group_id=%"PRIu32",command_bucket_id=%"PRIu32,
+                desired->table_id, e->bucket_id);
+            add_group_mod_str(OFPGC15_REMOVE_BUCKET, group_string,
+                              "remove bucket", &bc, &msgs);
+            free(group_string);
+        }
+
+        size_t n_buckets;
+        struct ovn_extend_table_bucket **bs =
+            sorted_buckets(&desired->desired_buckets, &n_buckets);
+        struct ds insert_ds = DS_EMPTY_INITIALIZER;
+        struct ds anchor = DS_EMPTY_INITIALIZER;
+        ds_put_cstr(&anchor, "first");
+
+        /* After removing the old buckets, insert the new, desired ones
+         * into the group.
+         * Install the buckets that are not on the switch yet, keeping the
+         * group ordered by bucket id (see sorted_buckets()).
+         *
+         * OpenFlow 1.5 cannot insert a bucket *before* another one: an
+         * INSERT_BUCKET places its buckets right after 'command_bucket_id',
+         * or at the head of the group for "first". A position is therefore
+         * expressed through the bucket that has to precede it, so adjacent
+         * buckets to install are collected into a run and the run is sent
+         * anchored on the last bucket before it that is already in place
+         * ('anchor').  With the group ordered by bucket id, that is exactly
+         * where the run belongs.
+         *
+         * Only a bucket that stays untouched may anchor a run: the removals
+         * above skip it, so it is still in the group by the time the switch
+         * processes the insert, and the bundle is ordered.
+         *
+         * The extra iteration past the end of 'bs' carries no bucket. It
+         * stands in for the "next bucket already in place" that flushes a
+         * run, for the case where the run reaches the end of the list. */
+        for (size_t i = 0; i <= n_buckets; i++) {
+            struct ovn_extend_table_bucket *d = i < n_buckets ? bs[i] : NULL;
+
+            if (d && !bucket_is_installed(&existing->existing_buckets, d)) {
+                VLOG_DBG("group %"PRIu32": bucket %s (id %"PRIu32", position "
+                         "%"PRIuSIZE" of %"PRIuSIZE") queued to go after %s.",
+                         desired->table_id, d->key, d->bucket_id, i, n_buckets,
+                         ds_cstr(&anchor));
+                ds_put_format(&insert_ds, ",bucket=bucket_id=%"PRIu32",%s",
+                              d->bucket_id, d->content);
+                continue;
+            }
+
+            if (insert_ds.length) {
+                char *group_string = xasprintf(
+                    "group_id=%"PRIu32",command_bucket_id=%s%s",
+                    desired->table_id, ds_cstr(&anchor), ds_cstr(&insert_ds));
+                add_group_mod_str(OFPGC15_INSERT_BUCKET, group_string,
+                                  "insert bucket", &bc, &msgs);
+                free(group_string);
+                ds_clear(&insert_ds);
+            }
+            if (d) {
+                ds_clear(&anchor);
+                ds_put_format(&anchor, "%"PRIu32, d->bucket_id);
+            }
+        }
+        ds_destroy(&anchor);
+        ds_destroy(&insert_ds);
+        free(bs);
+    }
+
     /* If skipped last time, then process the flow table
      * (tracked) flows even if lflows_changed is not set.
      * Same for pflows_changed. */
diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index 2b81f3f05..194840f8f 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -344,7 +344,8 @@ struct ovnact_ct_lb {
 };
 
 struct ovnact_select_dst {
-    uint16_t id;
+    uint32_t id;
+    bool ipv4;
     uint16_t weight;
 };
 
@@ -355,6 +356,7 @@ struct ovnact_select {
     size_t n_dsts;
     uint8_t ltable;             /* Logical table ID of next table. */
     char *hash_fields;
+    char *group_key;
     struct expr_field res_field;
 };
 
diff --git a/lib/actions.c b/lib/actions.c
index 4b5fda856..9454dc143 100644
--- a/lib/actions.c
+++ b/lib/actions.c
@@ -1599,8 +1599,9 @@ parse_select_action(struct action_context *ctx, struct 
expr_field *res_field)
     }
 
     struct vector dsts = VECTOR_EMPTY_INITIALIZER(struct ovnact_select_dst);
-    bool requires_hash_fields = false;
+    bool values_form = false;
     char *hash_fields = NULL;
+    char *group_key = NULL;
 
     lexer_get(ctx->lexer); /* Skip "select". */
     lexer_get(ctx->lexer); /* Skip '('. */
@@ -1608,12 +1609,36 @@ parse_select_action(struct action_context *ctx, struct 
expr_field *res_field)
     if (lexer_match_id(ctx->lexer, "values")) {
         lexer_force_match(ctx->lexer, LEX_T_EQUALS);
         lexer_force_match(ctx->lexer, LEX_T_LPAREN);
-        requires_hash_fields = true;
+        values_form = true;
     }
 
+    bool has_modifiers = values_form;
     while (!lexer_match(ctx->lexer, LEX_T_RPAREN)) {
+        if (!values_form && ctx->lexer->token.type == LEX_T_SEMICOLON) {
+            has_modifiers = true;
+            break;
+        }
+
         struct ovnact_select_dst dst;
-        if (!action_parse_uint16(ctx, &dst.id, "id")) {
+        dst.ipv4 = false;
+        if (lexer_is_int(ctx->lexer)
+            && ntohll(ctx->lexer->token.value.integer) <= UINT32_MAX) {
+            dst.id = ntohll(ctx->lexer->token.value.integer);
+            lexer_get(ctx->lexer);
+        } else if (ctx->lexer->token.type == LEX_T_INTEGER
+                   && ctx->lexer->token.format == LEX_F_IPV4) {
+            dst.id = ntohl(ctx->lexer->token.value.ipv4);
+            dst.ipv4 = true;
+            lexer_get(ctx->lexer);
+        } else {
+            lexer_syntax_error(ctx->lexer, "expecting id");
+            vector_destroy(&dsts);
+            return;
+        }
+        if (res_field->n_bits < 32
+            && dst.id >= (UINT32_C(1) << res_field->n_bits)) {
+            lexer_error(ctx->lexer, "value %"PRIu32" does not fit in the "
+                        "%d-bit result field.", dst.id, res_field->n_bits);
             vector_destroy(&dsts);
             return;
         }
@@ -1643,22 +1668,40 @@ parse_select_action(struct action_context *ctx, struct 
expr_field *res_field)
         return;
     }
 
-    if (requires_hash_fields) {
+    if (has_modifiers) {
         lexer_force_match(ctx->lexer, LEX_T_SEMICOLON);
-        if (!lexer_match_id(ctx->lexer, "hash_fields")) {
-            lexer_syntax_error(ctx->lexer, "expecting hash_fields");
-            vector_destroy(&dsts);
-            return;
+
+        bool expect_hash_fields = true;
+        if (lexer_match_id(ctx->lexer, "group_key")) {
+            if (!lexer_match(ctx->lexer, LEX_T_EQUALS) ||
+                ctx->lexer->token.type != LEX_T_STRING) {
+                lexer_syntax_error(ctx->lexer, "invalid group_key");
+                vector_destroy(&dsts);
+                return;
+            }
+            group_key = xstrdup(ctx->lexer->token.s);
+            lexer_get(ctx->lexer);
+            expect_hash_fields = lexer_match(ctx->lexer, LEX_T_SEMICOLON);
         }
-        if (!lexer_match(ctx->lexer, LEX_T_EQUALS) ||
-            ctx->lexer->token.type != LEX_T_STRING ||
-            lexer_lookahead(ctx->lexer) != LEX_T_RPAREN) {
-            lexer_syntax_error(ctx->lexer, "invalid hash_fields");
-            vector_destroy(&dsts);
-            return;
+
+        if (expect_hash_fields) {
+            if (!lexer_match_id(ctx->lexer, "hash_fields")) {
+                lexer_syntax_error(ctx->lexer, "expecting hash_fields");
+                vector_destroy(&dsts);
+                free(group_key);
+                return;
+            }
+            if (!lexer_match(ctx->lexer, LEX_T_EQUALS) ||
+                ctx->lexer->token.type != LEX_T_STRING ||
+                lexer_lookahead(ctx->lexer) != LEX_T_RPAREN) {
+                lexer_syntax_error(ctx->lexer, "invalid hash_fields");
+                vector_destroy(&dsts);
+                free(group_key);
+                return;
+            }
+            hash_fields = xstrdup(ctx->lexer->token.s);
+            lexer_get(ctx->lexer);
         }
-        hash_fields = xstrdup(ctx->lexer->token.s);
-        lexer_get(ctx->lexer);
         lexer_force_match(ctx->lexer, LEX_T_RPAREN);
     }
 
@@ -1668,6 +1711,7 @@ parse_select_action(struct action_context *ctx, struct 
expr_field *res_field)
     select->dsts = vector_steal_array(&dsts);
     select->res_field = *res_field;
     select->hash_fields = hash_fields;
+    select->group_key = group_key;
 }
 
 static void
@@ -1677,7 +1721,7 @@ format_SELECT(const struct ovnact_select *select, struct 
ds *s)
     ds_put_cstr(s, " = ");
     ds_put_cstr(s, "select");
     ds_put_char(s, '(');
-    if (select->hash_fields) {
+    if (select->hash_fields || select->group_key) {
         ds_put_format(s, "values=(");
     }
     for (size_t i = 0; i < select->n_dsts; i++) {
@@ -1686,12 +1730,22 @@ format_SELECT(const struct ovnact_select *select, 
struct ds *s)
         }
 
         const struct ovnact_select_dst *dst = &select->dsts[i];
-        ds_put_format(s, "%"PRIu16, dst->id);
+        if (dst->ipv4) {
+            ds_put_format(s, IP_FMT, IP_ARGS(htonl(dst->id)));
+        } else {
+            ds_put_format(s, "%"PRIu32, dst->id);
+        }
         ds_put_format(s, "=%"PRIu16, dst->weight);
     }
     ds_put_char(s, ')');
+    if (select->group_key) {
+        ds_put_format(s, "; group_key=\"%s\"", select->group_key);
+    }
     if (select->hash_fields) {
-      ds_put_format(s, "; hash_fields=\"%s\")", select->hash_fields);
+        ds_put_format(s, "; hash_fields=\"%s\"", select->hash_fields);
+    }
+    if (select->hash_fields || select->group_key) {
+        ds_put_char(s, ')');
     }
     ds_put_char(s, ';');
 }
@@ -1722,8 +1776,28 @@ encode_SELECT(const struct ovnact_select *select,
 
     struct mf_subfield sf = expr_resolve_field(&select->res_field);
 
+    bool incremental = select->group_key;
+
+    struct ds group_key = DS_EMPTY_INITIALIZER;
+    struct ovn_extend_table_bucket_spec *bucket_specs = NULL;
+    if (incremental) {
+        ds_put_format(&group_key, "%s|%s|table=%d|%s[%u..%u]",
+                      select->group_key, ds_cstr(&ds), resubmit_table,
+                      sf.field->name, sf.ofs, sf.ofs + sf.n_bits - 1);
+        bucket_specs = xmalloc(select->n_dsts * sizeof *bucket_specs);
+    }
+
     for (size_t bucket_id = 0; bucket_id < select->n_dsts; bucket_id++) {
         const struct ovnact_select_dst *dst = &select->dsts[bucket_id];
+        if (incremental) {
+            bucket_specs[bucket_id].key = xasprintf("%"PRIu32, dst->id);
+            bucket_specs[bucket_id].content =
+                xasprintf("weight:%"PRIu16",actions=load:%u->%s[%u..%u],"
+                          "resubmit(,%d)", dst->weight, dst->id,
+                          sf.field->name, sf.ofs, sf.ofs + sf.n_bits - 1,
+                          resubmit_table);
+            continue;
+        }
         ds_put_format(&ds, ",bucket=bucket_id=%"PRIuSIZE",weight:%"PRIu16
                       ",actions=", bucket_id, dst->weight);
         ds_put_format(&ds, "load:%u->%s[%u..%u],", dst->id, sf.field->name,
@@ -1731,8 +1805,20 @@ encode_SELECT(const struct ovnact_select *select,
         ds_put_format(&ds, "resubmit(,%d)", resubmit_table);
     }
 
-    table_id = ovn_extend_table_assign_id(ep->group_table, ds_cstr(&ds),
-                                          ep->lflow_uuid);
+    if (incremental) {
+        table_id = ovn_extend_table_assign_group_id(
+            ep->group_table, ds_cstr(&group_key), ds_cstr(&ds), bucket_specs,
+            select->n_dsts, ep->lflow_uuid);
+        for (size_t i = 0; i < select->n_dsts; i++) {
+            free(CONST_CAST(char *, bucket_specs[i].key));
+            free(CONST_CAST(char *, bucket_specs[i].content));
+        }
+        free(bucket_specs);
+    } else {
+        table_id = ovn_extend_table_assign_id(ep->group_table, ds_cstr(&ds),
+                                              ep->lflow_uuid);
+    }
+    ds_destroy(&group_key);
     ds_destroy(&ds);
     if (table_id == EXT_TABLE_ID_INVALID) {
         return;
@@ -1748,6 +1834,7 @@ ovnact_select_free(struct ovnact_select *select)
 {
     free(select->dsts);
     free(select->hash_fields);
+    free(select->group_key);
 }
 
 static void
diff --git a/lib/extend-table.c b/lib/extend-table.c
index 03bec516e..c7abe6f2f 100644
--- a/lib/extend-table.c
+++ b/lib/extend-table.c
@@ -78,9 +78,46 @@ ovn_extend_table_info_alloc(const char *name, uint32_t id,
     }
     e->hmap_node.hash = hash;
     hmap_init(&e->references);
+    e->group_header = NULL;
+    hmap_init(&e->desired_buckets);
+    hmap_init(&e->existing_buckets);
     return e;
 }
 
+static void
+ovn_extend_table_buckets_clear(struct hmap *buckets)
+{
+    struct ovn_extend_table_bucket *b;
+    HMAP_FOR_EACH_SAFE (b, hmap_node, buckets) {
+        hmap_remove(buckets, &b->hmap_node);
+        if (b->peer) {
+            b->peer->peer = NULL;
+        }
+        free(b->key);
+        free(b->content);
+        free(b);
+    }
+}
+
+static void
+ovn_extend_table_desired_buckets_replace(struct ovn_extend_table_info *info,
+                                         struct hmap *new_desired)
+{
+    hmap_swap(&info->desired_buckets, new_desired);
+
+    struct ovn_extend_table_bucket *old;
+    HMAP_FOR_EACH_SAFE (old, hmap_node, new_desired) {
+        hmap_remove(new_desired, &old->hmap_node);
+        if (old->peer && old->peer->peer == old) {
+            old->peer->peer = NULL;
+        }
+        free(old->key);
+        free(old->content);
+        free(old);
+    }
+    hmap_destroy(new_desired);
+}
+
 static void
 ovn_extend_table_info_destroy(struct ovn_extend_table_info *e)
 {
@@ -92,6 +129,13 @@ ovn_extend_table_info_destroy(struct ovn_extend_table_info 
*e)
         free(r);
     }
     hmap_destroy(&e->references);
+
+    free(e->group_header);
+    ovn_extend_table_buckets_clear(&e->desired_buckets);
+    hmap_destroy(&e->desired_buckets);
+    ovn_extend_table_buckets_clear(&e->existing_buckets);
+    hmap_destroy(&e->existing_buckets);
+
     free(e);
 }
 
@@ -292,6 +336,85 @@ ovn_extend_table_remove_desired(struct ovn_extend_table 
*table,
     ovn_extend_table_delete_desired(table, l);
 }
 
+static struct ovn_extend_table_bucket *
+ovn_extend_table_bucket_lookup(struct hmap *buckets, const char *key,
+                               uint32_t hash)
+{
+    struct ovn_extend_table_bucket *b;
+    HMAP_FOR_EACH_WITH_HASH (b, hmap_node, hash, buckets) {
+        if (!strcmp(b->key, key)) {
+            return b;
+        }
+    }
+    return NULL;
+}
+
+struct ovn_extend_table_bucket *
+ovn_extend_table_bucket_find(struct hmap *buckets, const char *key)
+{
+    return ovn_extend_table_bucket_lookup(buckets, key, hash_string(key, 0));
+}
+
+static struct ovn_extend_table_bucket *
+ovn_extend_table_bucket_alloc(const char *key, uint32_t id,
+                              const char *content,
+                              struct ovn_extend_table_bucket *peer,
+                              uint32_t hash)
+{
+    struct ovn_extend_table_bucket *b = xmalloc(sizeof *b);
+    b->key = xstrdup(key);
+    b->bucket_id = id;
+    b->content = xstrdup(content);
+    b->peer = peer;
+    if (peer) {
+        peer->peer = b;
+    }
+    b->hmap_node.hash = hash;
+    return b;
+}
+
+static void
+ovn_extend_table_sync_buckets(struct ovn_extend_table_info *desired,
+                              struct ovn_extend_table_info *existing)
+{
+    /* Drop existing buckets that are no longer desired. */
+    struct ovn_extend_table_bucket *b;
+    HMAP_FOR_EACH_SAFE (b, hmap_node, &existing->existing_buckets) {
+        uint32_t hash = hash_string(b->key, 0);
+        if (!ovn_extend_table_bucket_lookup(&desired->desired_buckets,
+                                            b->key, hash)) {
+            hmap_remove(&existing->existing_buckets, &b->hmap_node);
+            if (b->peer) {
+                b->peer->peer = NULL;
+            }
+            free(b->key);
+            free(b->content);
+            free(b);
+        }
+    }
+
+    /* Add or refresh existing buckets to match what's desired. */
+    struct ovn_extend_table_bucket *d;
+    HMAP_FOR_EACH (d, hmap_node, &desired->desired_buckets) {
+        uint32_t hash = hash_string(d->key, 0);
+        struct ovn_extend_table_bucket *e =
+            ovn_extend_table_bucket_lookup(&existing->existing_buckets,
+                                           d->key, hash);
+        if (e) {
+            /* Refresh content in case e.g. weight/health status changed. */
+            free(e->content);
+            e->content = xstrdup(d->content);
+            e->peer = d;
+            d->peer = e;
+        } else {
+            struct ovn_extend_table_bucket *new_e =
+                ovn_extend_table_bucket_alloc(d->key, d->bucket_id,
+                                              d->content, d, hash);
+            hmap_insert(&existing->existing_buckets, &new_e->hmap_node, hash);
+        }
+    }
+}
+
 void
 ovn_extend_table_sync(struct ovn_extend_table *table)
 {
@@ -299,15 +422,22 @@ ovn_extend_table_sync(struct ovn_extend_table *table)
 
     /* Copy the contents of desired to existing. */
     HMAP_FOR_EACH_SAFE (desired, hmap_node, &table->desired) {
-        if (!ovn_extend_table_lookup(&table->existing, desired)) {
-            struct ovn_extend_table_info *existing =
-                ovn_extend_table_info_alloc(desired->name,
-                                            desired->table_id,
-                                            desired,
-                                            desired->hmap_node.hash);
+        struct ovn_extend_table_info *existing =
+            ovn_extend_table_lookup(&table->existing, desired);
+        if (!existing) {
+            existing = ovn_extend_table_info_alloc(desired->name,
+                                                    desired->table_id,
+                                                    desired,
+                                                    desired->hmap_node.hash);
             hmap_insert(&table->existing, &existing->hmap_node,
                         existing->hmap_node.hash);
         }
+
+        if (desired->group_header) {
+            free(existing->group_header);
+            existing->group_header = xstrdup(desired->group_header);
+            ovn_extend_table_sync_buckets(desired, existing);
+        }
     }
 }
 
@@ -366,6 +496,139 @@ ovn_extend_table_assign_id(struct ovn_extend_table 
*table, const char *name,
     return table_id;
 }
 
+struct ovn_extend_table_bucket_id_node {
+    struct hmap_node hmap_node;
+    uint32_t bucket_id;
+};
+
+static bool
+ovn_extend_table_bucket_id_claim(struct hmap *used, uint32_t id)
+{
+    uint32_t hash = hash_int(id, 0);
+    struct ovn_extend_table_bucket_id_node *n;
+
+    HMAP_FOR_EACH_WITH_HASH (n, hmap_node, hash, used) {
+        if (n->bucket_id == id) {
+            return false;
+        }
+    }
+
+    n = xmalloc(sizeof *n);
+    n->bucket_id = id;
+    hmap_insert(used, &n->hmap_node, hash);
+    return true;
+}
+
+static void
+ovn_extend_table_bucket_ids_destroy(struct hmap *used)
+{
+    struct ovn_extend_table_bucket_id_node *n;
+    HMAP_FOR_EACH_POP (n, hmap_node, used) {
+        free(n);
+    }
+    hmap_destroy(used);
+}
+
+/* OpenFlow 1.5 only allows bucket ids up to OFPG15_BUCKET_MAX (0xffffff00);
+ * everything above that is reserved (OFPG15_BUCKET_FIRST/LAST/ALL).  Masking
+ * to 31 bits stays well clear of the reserved range. */
+#define OVN_BUCKET_ID_MASK 0x7fffffff
+
+/* One desired bucket, while ovn_extend_table_assign_group_id() works out the
+ * bucket ids for the whole set. */
+struct ovn_extend_table_pending_bucket {
+    struct ovn_extend_table_bucket *peer;
+    uint32_t hash;
+    uint32_t bucket_id;
+    bool has_id;
+};
+
+uint32_t
+ovn_extend_table_assign_group_id(
+    struct ovn_extend_table *table,
+    const char *group_key, const char *group_header,
+    const struct ovn_extend_table_bucket_spec *buckets,
+    size_t n_buckets, struct uuid lflow_uuid)
+{
+    uint32_t table_id = ovn_extend_table_assign_id(table, group_key,
+                                                    lflow_uuid);
+    if (table_id == EXT_TABLE_ID_INVALID) {
+        return table_id;
+    }
+
+    struct ovn_extend_table_info *info =
+        ovn_extend_table_desired_lookup_by_name(table, group_key);
+    ovs_assert(info);
+
+    free(info->group_header);
+    info->group_header = xstrdup(group_header);
+
+    struct hmap *existing_buckets =
+        info->peer ? &info->peer->existing_buckets : NULL;
+
+    struct hmap used_ids = HMAP_INITIALIZER(&used_ids);
+    struct ovn_extend_table_pending_bucket *pending =
+        n_buckets ? xmalloc(n_buckets * sizeof *pending) : NULL;
+
+    for (size_t i = 0; i < n_buckets; i++) {
+        const struct ovn_extend_table_bucket_spec *spec = &buckets[i];
+        uint32_t hash = hash_string(spec->key, 0);
+        struct ovn_extend_table_bucket *old =
+            ovn_extend_table_bucket_lookup(&info->desired_buckets,
+                                           spec->key, hash);
+        struct ovn_extend_table_bucket *existing =
+            (!old && existing_buckets)
+            ? ovn_extend_table_bucket_lookup(existing_buckets,
+                                             spec->key, hash)
+            : NULL;
+
+        pending[i].peer = old ? old->peer : existing;
+        pending[i].hash = hash;
+        pending[i].has_id = false;
+
+        struct ovn_extend_table_bucket *known = old ? old : existing;
+        if (known) {
+            pending[i].bucket_id = known->bucket_id;
+            pending[i].has_id =
+                ovn_extend_table_bucket_id_claim(&used_ids, known->bucket_id);
+        }
+    }
+
+    if (existing_buckets) {
+        struct ovn_extend_table_bucket *b;
+        HMAP_FOR_EACH (b, hmap_node, existing_buckets) {
+            ovn_extend_table_bucket_id_claim(&used_ids, b->bucket_id);
+        }
+    }
+
+    for (size_t i = 0; i < n_buckets; i++) {
+        if (pending[i].has_id) {
+            continue;
+        }
+        uint32_t bucket_id = pending[i].hash & OVN_BUCKET_ID_MASK;
+        while (!ovn_extend_table_bucket_id_claim(&used_ids, bucket_id)) {
+            bucket_id = (bucket_id + 1) & OVN_BUCKET_ID_MASK;
+        }
+        pending[i].bucket_id = bucket_id;
+        pending[i].has_id = true;
+    }
+
+    struct hmap new_desired = HMAP_INITIALIZER(&new_desired);
+    for (size_t i = 0; i < n_buckets; i++) {
+        struct ovn_extend_table_bucket *b = ovn_extend_table_bucket_alloc(
+            buckets[i].key, pending[i].bucket_id, buckets[i].content,
+            pending[i].peer, pending[i].hash);
+        hmap_insert(&new_desired, &b->hmap_node, pending[i].hash);
+    }
+
+    ovn_extend_table_desired_buckets_replace(info, &new_desired);
+
+    ovn_extend_table_bucket_ids_destroy(&used_ids);
+    free(pending);
+
+    return table_id;
+}
+
 struct ovn_extend_table_info *
 ovn_extend_table_desired_lookup_by_name(struct ovn_extend_table * table,
                                         const char *name)
diff --git a/lib/extend-table.h b/lib/extend-table.h
index 90e6e470d..c7b269b74 100644
--- a/lib/extend-table.h
+++ b/lib/extend-table.h
@@ -65,6 +65,28 @@ struct ovn_extend_table_info {
     struct hmap references; /* The lflows that are using this item, with
                              * ovn_extend_table_lflow_ref nodes. Only useful
                              * for items in ovn_extend_table.desired. */
+
+    /* The fields below are only populated for group entries created via
+     * ovn_extend_table_assign_group_id(), to support incremental
+     * INSERT_BUCKET/REMOVE_BUCKET updates instead of whole-group replace.
+     * 'group_header' is NULL for entries created via the regular
+     * ovn_extend_table_assign_id() (whole-content-hash) path. */
+    char *group_header;
+    struct hmap desired_buckets;
+    struct hmap existing_buckets;
+};
+
+struct ovn_extend_table_bucket {
+    struct hmap_node hmap_node;
+    char *key;
+    uint32_t bucket_id;
+    char *content;
+    struct ovn_extend_table_bucket *peer;
+};
+
+struct ovn_extend_table_bucket_spec {
+    const char *key;
+    const char *content;
 };
 
 /* Maintains the link between a lflow and an ovn_extend_table_info item in
@@ -108,10 +130,27 @@ uint32_t ovn_extend_table_assign_id(struct 
ovn_extend_table *,
                                     const char *name,
                                     struct uuid lflow_uuid);
 
+/* Like ovn_extend_table_assign_id(), but keyed by a caller-provided stable
+ * 'group_key' (independent of bucket content, e.g. an LB's stage-hint)
+ * instead of the full group content. The group's table_id therefore stays
+ * stable when 'buckets' changes across calls with the same 'group_key'.
+ *
+ * Replaces the info's desired bucket set with 'buckets' on every call, so
+ * that ofctrl.c can diff it against what's already installed
+ * (existing_buckets) and emit incremental INSERT_BUCKET/REMOVE_BUCKET
+ * instead of a whole-group replace. */
+uint32_t ovn_extend_table_assign_group_id(
+    struct ovn_extend_table *table, const char *group_key,
+    const char *group_header, const struct ovn_extend_table_bucket_spec *,
+    size_t n_buckets, struct uuid lflow_uuid);
+
 struct ovn_extend_table_info *
 ovn_extend_table_desired_lookup_by_name(struct ovn_extend_table * table,
                                         const char *name);
 
+struct ovn_extend_table_bucket *
+ovn_extend_table_bucket_find(struct hmap *buckets, const char *key);
+
 /* Iterates 'DESIRED' through all of the 'ovn_extend_table_info's in
  * 'TABLE'->desired that are not in 'TABLE'->existing.  (The loop body
  * presumably adds them.) */
-- 
2.48.1

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

Reply via email to