Hi,
I'd like to get your opinion on an approach before investing more in it.
The problem
-----------
The map_* converters take a single key, so one map file can only be shared
between several independent lists when the key itself can carry a
discriminating prefix. That works for the string and regex match methods --
we use keys like "<type>:<site>:<value>" -- but it does not work for map_ip,
whose keys must be parsable IP addresses or networks. There is simply
nowhere to put the site.
As a result, per-site address lists force one map file and one configuration
rule per site:
http-request set-var(txn.ip_grp)
src,map_ip(/etc/acl/shop.example.com/ip_list.map,ip=0) if { var(txn.site) -m
str shop.example.com }
http-request set-var(txn.ip_grp)
src,map_ip(/etc/acl/news.example.com/ip_list.map,ip=0) if { var(txn.site) -m
str news.example.com }
... one more line per site ...
What we are actually doing
--------------------------
Some context on why these are maps and not ACLs in the configuration, since
it explains the constraints below.
We run a WAF in front of many sites. A request is first classified along
several dimensions -- source address, host, user agent, URL, TLS version and
so on. Each dimension is looked up in its own list, and the list does not say
"deny", it says which group of that list the request fell into:
# address list # url list
10.20.10.0/23 ip=1 /admin/.* url=11
203.0.113.7 ip=99 /api/.* url=33
Those group identifiers are then concatenated into one string, and a second
map holds the actual rules, one per line, each matching a combination:
# <site>:<address group>:<url group> <action>
shop.example.com:ip=99:url=11 deny
shop.example.com:ip=11:url=33 allow
Both layers are edited through the runtime API while traffic flows. Blocking
one more address is a single "add map" on the address list; the rule line
that references ip=99 is not touched, and neither is the configuration. That
separation is what lets one of our lists hold about 80,000 addresses that
change all day long, and it is why the membership has to live in a map.
Why it hurts at our scale
-------------------------
More than 100 sites per node, each with its own address / string / regex
lists.
We started with one map file per site for every list type. Reload became very
slow -- hundreds of pattern files to open and index. We fixed that by merging
the string and regex lists into one file each, using the prefixed-key trick
above. Reload got fast again.
That left us with two problems we could not solve:
- The address lists could not be merged at all, so the per-site files and
the generated per-site config block are still there. With 100+ sites that
block is 100+ rules, and since "set-var" is not a terminal action every
one of those conditions is evaluated on every request.
- Merging cost us lookup locality. map_reg walks a list, so once all sites
share one file, a lookup for one site runs the regexes of all the others
too. They fail fast on the literal prefix, but it is still one pcre2 call
per entry per lookup.
So we could have a fast reload or a fast lookup, but not both.
What the patch does
-------------------
It adds a "conditional map": the file holds one extra leading key, and only
the entries whose leading key exactly matches take part in the lookup. Every
site's address list then fits in one file, and the same network can mean
different things on different sites:
# <condition> <pattern> <value>
shop.example.com 10.20.10.0/23 ip=1
shop.example.com 203.0.113.7 ip=99
news.example.com 10.20.10.0/23 ip=7
http-request set-var(txn.site) req.hdr(host),lower
http-request set-var(txn.ip_grp)
src,cmap_ip(/etc/acl/ip_list.map,txn.site,ip=0)
The 100+ rules above collapse into that one line, and the lookup only walks
the group belonging to that site. One file, fast reload, and the locality we
had back when the files were split.
Implementation notes:
- Entries are spread over per-condition groups (struct pattern_cond_grp)
indexed on the condition key in the pattern_tree of the top-level
expression. Each group owns a regular sub-expression, filled and looked up
with the stock parse/index/match functions of the underlying match method.
The second key is therefore parsed, indexed and matched exactly like in a
plain map, tree lookups included.
- Only pat_ref_push() (which picks the group a pattern is indexed in) and
pattern_exec_match() (which selects the group to look up) had to become
condition-aware. No new match function was needed. Groups are created on
demand and released with their last entry.
- The condition argument is a variable name, or a log-format expression when
it is not one. It may be at most 255 characters.
- The two keys are stored space-separated as a single pat_ref key, so the
runtime API keeps working: "show map" reports the three fields like the
file does, and "prepare / add @ver / commit" still replaces a map
atomically. On the CLI the condition is passed as its own argument, so
add/set/del/get spell an entry out exactly like the file does. "get map"
also reports whether the map holds any entry for the condition at all
('cond="shop.example.com", cond_found=yes'), which tells an unknown
condition apart from a condition whose entries simply did not match.
- A file used by a conditional map cannot also be used by a plain map or by
an ACL; both are rejected at config parse time.
- The whole cmap_<match_type>[_<output_type>] family mirrors map_* 1:1.
What I paid attention to
------------------------
Existing map_* and ACL behaviour is untouched. A reviewer here compared the
per-function disassembly of a pristine build against a patched one: of the 15
functions those paths use, 13 are identical in instruction sequence and
relocation targets; pat_delete_gen differs only in the line number baked into
a BUG_ON string, and cli_parse_show_map only in three relocation targets
shifted by section layout. The conditional lookup lives in a separate
pattern_exec_match_cond(), and the handful of shared functions are all behind
a "flags & PAT_REF_COND" test that a plain map never takes.
The patch is against 3.2.21 and includes reg-tests/converter/cmap.vtc, which
covers both tree- and list-indexed match methods, IPv6, condition isolation,
the runtime API, and the release of a group with its last entry.
What is missing
---------------
There is no doc/configuration.txt yet, and the change is one commit rather
than a logical series. I held both back on purpose: if the approach is not
something you would want upstream, documentation hunks in a file that changes
often are a rebase cost we would carry for nothing. If the direction looks
reasonable I will gladly write the documentation and split the series.
The cmap_* name is also not something I am attached to. "Conditional" already
means ACL conditions in haproxy, so something like gmap_* ("grouped map") may
read better. Happy to rename before any merge.
Does this look like a direction worth pursuing, or is there an existing
mechanism I have missed for sharing one map file between several scopes?
Jinho Kong (1):
MINOR: map: add the cmap_* conditional map converters
include/haproxy/map-t.h | 3 +
include/haproxy/map.h | 2 +
include/haproxy/pattern-t.h | 24 ++
include/haproxy/pattern.h | 37 +++
reg-tests/converter/cmap.vtc | 134 +++++++++
src/map.c | 565 ++++++++++++++++++++++++++++++++++-
src/pattern.c | 323 +++++++++++++++++++-
7 files changed, 1069 insertions(+), 19 deletions(-)
create mode 100644 reg-tests/converter/cmap.vtc
--
2.50.1 (Apple Git-155)
