]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
MEDIUM: pattern: always consider gen_id for pat_ref lookup operations
authorAurelien DARRAGON <adarragon@haproxy.com>
Wed, 20 Nov 2024 18:03:00 +0000 (19:03 +0100)
committerAurelien DARRAGON <adarragon@haproxy.com>
Tue, 26 Nov 2024 15:12:31 +0000 (16:12 +0100)
Historically, pat_ref lookup operations were performed on the whole
pat_ref elements list. As such, set, find and delete operations on a given
key would cause any matching element in pat_ref to be considered.

When prepare/commit operations were added, gen_id was impelemnted in
order to be able to work on a subset from pat_ref without impacting
the current (live) version from pat_ref, until a new subset is committed
to replace the current one.

While the logic was good, there remained a design flaw from the historical
implementation: indeed, legacy functions such as pat_ref_set(),
pat_ref_delete() and pat_ref_find_elt() kept performing the lookups on the
whole set of elements instead of considering only elements from the current
subset. Because of this, mixing new prepare/commit operations with legacy
operations could yield unexpected results.

For instance, before this commit:

  echo "add map #0 key oldvalue" | socat /tmp/ha.sock -
  echo "prepare map #0" | socat /tmp/ha.sock -
  New version created: 1
  echo "add map @1 #0 key newvalue" | socat /tmp/ha.sock -
  echo "del map #0 key" | socat /tmp/ha.sock -
  echo "commit map @1 #0" | socat /tmp/ha.sock -

  -> the result would be that "key" entry doesn't exist anymore after the
  commit, while we would expect the new value to be there instead.

Thanks to the previous commits, we may finally fix this issue: for set,
find_elt and delete operations, the current generation id is considered.

With the above example, it means that the "del map #0 key" would only
target elements from the current subset, thus elements in "version 1" of
the map would be immune to the delete (as we would expect it to work).

src/pattern.c

index e342db10bb20f4e707c3b9f562489fcc0d23a301..2c1a4834243514902f3a6b40fb11beb7b7c6bcea 100644 (file)
@@ -1645,21 +1645,7 @@ int pat_ref_gen_delete(struct pat_ref *ref, unsigned int gen_id, const char *key
  */
 int pat_ref_delete(struct pat_ref *ref, const char *key)
 {
-       struct ebmb_node *node;
-       int found = 0;
-
-       /* delete pattern from reference */
-       node = ebst_lookup(&ref->ebmb_root, key);
-       while (node) {
-               struct pat_ref_elt *elt;
-
-               elt = ebmb_entry(node, struct pat_ref_elt, node);
-               node = ebmb_next_dup(node);
-               pat_ref_delete_by_ptr(ref, elt);
-               found = 1;
-       }
-
-       return found;
+       return pat_ref_gen_delete(ref, ref->curr_gen, key);
 }
 
 /*
@@ -1690,13 +1676,7 @@ struct pat_ref_elt *pat_ref_gen_find_elt(struct pat_ref *ref, unsigned int gen_i
  */
 struct pat_ref_elt *pat_ref_find_elt(struct pat_ref *ref, const char *key)
 {
-       struct ebmb_node *node;
-
-       node = ebst_lookup(&ref->ebmb_root, key);
-       if (node)
-               return ebmb_entry(node, struct pat_ref_elt, node);
-
-       return NULL;
+       return pat_ref_gen_find_elt(ref, ref->curr_gen, key);
 }
 
 
@@ -1870,11 +1850,7 @@ int pat_ref_gen_set(struct pat_ref *ref, unsigned int gen_id,
  */
 int pat_ref_set(struct pat_ref *ref, const char *key, const char *value, char **err)
 {
-       struct ebmb_node *node;
-
-       /* Look for pattern in the reference. */
-       node = ebst_lookup(&ref->ebmb_root, key);
-       return pat_ref_set_from_node(ref, node, value, err);
+       return pat_ref_gen_set(ref, ref->curr_gen, key, value, err);
 }
 
 /* helper function to create and initialize a generic pat_ref struct