RepoMirrors
/
haproxy
mirror of http://git.haproxy.org/git/haproxy.git/
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

BUG/MINOR: map: list-based matching potential ordering regression 

An unexpected side-effect was introduced by 5fea597 ("MEDIUM: map/acl:
Accelerate several functions using pat_ref_elt struct ->head list")

The above commit tried to use eb tree API to manipulate elements as much
as possible in the hope to accelerate some functions.

Prior to 5fea597, pattern_read_from_file() used to iterate over all
elements from the map file in the same order they were seen in the file
(using list_for_each_entry) to push them in the pattern expression.

Now, since eb api is used to iterate over elements, the ordering is lost
very early.

This is known to cause behavior changes with existing setups (same conf
and map file) when compared with previous versions for some list-based
matching methods as described in GH #2400. For instance, the map_dom()
converter may return a different matching key from the one that was
returned by older haproxy versions.

For IP or STR matching, matching is based on tree lookups for better
efficiency, so in this case the ordering is lost at the name of
performance. The order in which they are loaded doesn't matter because
tree ordering is based on the content, it is not positional.

But with some other types, matching is based on list lookups (e.g.: dom),
and the order in which elements are pushed into the list can affect the
matching element that will be returned (in case of multiple matches, since
only the first matching element in the list will be returned).

Despite the documentation not officially stating that the file ordering
should be preserved for list-based matching methods, it's probably best
to be conservative here and stick to historical behavior. Moreover, there
was no performance benefit from using the eb tree api to iterate over
elements in pattern_read_from_file() since all elements are visited
anyway.

This should be backported to 2.9.
This commit is contained in:
Aurelien DARRAGON 2024-01-03 11:54:03 +01:00
parent 37d5a26cc5
commit b546bb6d67
1 changed files with 5 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
src/pattern.c
View File

@ -2419,7 +2419,6 @@ int pattern_read_from_file(struct pattern_head *head, unsigned int refflags,
{
struct pat_ref *ref;
struct pattern_expr *expr;
struct ebmb_node *node;
struct pat_ref_elt *elt;
int reuse = 0;
@ -2511,11 +2510,11 @@ int pattern_read_from_file(struct pattern_head *head, unsigned int refflags,
if (reuse)
return 1;
/* Load reference content in the pattern expression. */
node = ebmb_first(&ref->ebmb_root);
while (node) {
elt = ebmb_entry(node, struct pat_ref_elt, node);
node = ebmb_next(node);
/* Load reference content in the pattern expression.
* We need to load elements in the same order they were seen in the
* file as list-based matching types may rely on it.
*/
list_for_each_entry(elt, &ref->head, list) {
if (!pat_ref_push(elt, expr, patflags, err)) {
if (elt->line > 0)
memprintf(err, "%s at line %d of file '%s'",