diff --git a/Makefile b/Makefile index 743f428..f497c67 100644 --- a/Makefile +++ b/Makefile @@ -23,7 +23,7 @@ BUILD_DIR := $(CURDIR)/build # the package version from there directly. The C code picks up VERSION # via the generated src/version.h (written by the version-header target # below and depended on by the module build). -VERSION := 0.7.0 +VERSION := 0.7.1 NGINX_SRC ?= diff --git a/debian/changelog b/debian/changelog index 4fbcbec..2fe8c9e 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,16 @@ +nginx-ipng-stats-plugin (0.7.1-1) unstable; urgency=medium + + * Pre-release v0.7.1. + - Listen wrapper now deduplicates on (server block, sockaddr) + across both plain and device-tagged listens. A plain + `listen 80;` and `listen 80 device=eth0 ...;` in the same + server block now coexist, matching the runtime reality under + the IP_PKTINFO attribution model (all same-sockaddr listens + collapse to one wildcard kernel socket). Docs updated to + drop the "must be device-tagged" restriction. + + -- Pim van Pelt Sun, 19 Apr 2026 10:00:00 +0200 + nginx-ipng-stats-plugin (0.7.0-1) unstable; urgency=medium * Pre-release v0.7.0. diff --git a/docs/design.md b/docs/design.md index fa31507..ba75d0b 100644 --- a/docs/design.md +++ b/docs/design.md @@ -84,10 +84,12 @@ Each requirement carries a unique identifier (`FR-X.Y` or `NFR-X.Y`) so that lat - **FR-1.3** A listening socket with neither `device=` nor `ipng_source_tag=` MUST be tagged with the configured default source string (see `ipng_stats_default_source`, FR-5.3). The default default is the literal string `direct`. - **FR-1.4** A listening socket with `device=X` but no `ipng_source_tag=` MUST be tagged with the interface name `X`. -- **FR-1.5** Two `listen` directives that share `address:port` but differ in `device=` MUST coexist. Since no `SO_BINDTODEVICE` - is applied, the kernel delivers all matching SYNs to a single wildcard listening socket and the module distinguishes them by - reading `ifindex` from the per-connection cmsg — so "multiple device-tagged listens at the same port" at the config level - collapses to one kernel socket at runtime without any userspace contortions. +- **FR-1.5** Two or more `listen` directives sharing `address:port` MUST coexist regardless of whether each carries `device=`. + Since no `SO_BINDTODEVICE` is applied, the kernel delivers all matching SYNs to a single wildcard listening socket and the + module distinguishes them by reading `ifindex` from the per-connection cmsg. The listen wrapper therefore deduplicates on + `(server block, sockaddr)` across both plain and device-tagged listens: the first occurrence registers the kernel socket, + and subsequent same-sockaddr siblings (plain or device-tagged) are accepted without tripping nginx's duplicate-listen check. + Device-tagged siblings additionally register an entry in the attribution table. - **FR-1.6** A `listen` directive that uses a wildcard address (`80`, `[::]:80`) together with `device=` MUST attribute every connection whose ingress interface is `` — regardless of which local address the client addressed — to that listen's source tag. Traffic on other interfaces MUST fall back to the configured default source (see FR-1.3). diff --git a/docs/user-guide.md b/docs/user-guide.md index 4e71420..d2ceb55 100644 --- a/docs/user-guide.md +++ b/docs/user-guide.md @@ -112,8 +112,9 @@ http { ipng_stats_flush_interval 1s; ipng_stats_default_source direct; - # Attributed vhost. Every listen on this port must be device-tagged — - # see "All listens on a shared port must be device-tagged" below. + # Attributed vhost. Wildcard listens below register one binding + # per (device, family); all collapse to a single kernel socket + # under the IP_PKTINFO attribution model. server { include /etc/nginx/ipng-stats/listens.conf; @@ -176,22 +177,14 @@ You do not need to enumerate VIPs in `listen`. A wildcard `listen 80 device=gre- served through the `gre-mg1` interface, and nginx routes per-request to the right vhost by `server_name` / `Host:` header. Adding a new VIP is a `server_name` change; adding a new interface is an append to `listens.conf`. -### All listens on a shared port must be device-tagged - -If you use multiple `listen` directives on the same port (e.g. port 80), **every one of them must carry `device=`**. Mixing -a device-tagged listen with a plain `listen 80;` or with an address-specific `listen 192.0.2.1:80;` on the same port is **not -supported** — nginx's config-level dedup rejects same-sockaddr listens within a server block, and the module's wrapper only -exempts directives that carry `device=`. - -For "direct" traffic — clients hitting the host on a non-attributed interface — use a **separate port** on the direct interface -(e.g. `listen 198.51.100.1:8081;`). That listen then has no `device=`, so it falls back to the tag set by -`ipng_stats_default_source` (`direct` by default). - ### Sharing a single port across address families and devices -Within the device-tagged set, you're free to share port numbers freely across devices and address families: as long as each listen -has a distinct `device=`, the kernel keeps them apart, and within one device you can either reuse a single tag or split by family. -For example: +Under the `IP_PKTINFO` attribution model, all listens at a given sockaddr collapse to a single wildcard kernel socket at runtime — +the kernel stamps every accepted connection with its ingress ifindex, and the module looks that up in the table of `device=` +bindings registered by the listen wrapper. Multiple device-tagged wildcard listens on port 80 are therefore not "multiple +sockets"; they're one wildcard socket plus N entries in the attribution table. + +A device can reuse one tag across address families or split into per-family tags — whichever reads better in the scrape output: ```nginx listen 80 device=gre-mg1 ipng_source_tag=mg1; @@ -200,6 +193,12 @@ listen 80 device=gre-mg2 ipng_source_tag=mg2-v4; listen [::]:80 device=gre-mg2 ipng_source_tag=mg2-v6; # per-family tags ``` +A plain `listen 80;` can sit alongside device-tagged listens in the same server block; the wrapper treats the first occurrence +at a given `(server, sockaddr)` pair as the one that registers the kernel socket and lets subsequent device-tagged siblings +register bindings without tripping nginx's duplicate-listen check. Traffic arriving on an interface that has no binding falls +back to `ipng_stats_default_source` (`direct` by default). Keeping "direct" traffic on its own port — e.g. +`listen 198.51.100.1:8081;` — remains a fine pattern when you want a hard split, but it's no longer required. + ## 4. Verify with curl Generate some traffic (or wait for real traffic), then scrape the endpoint locally: diff --git a/src/ngx_http_ipng_stats_module.c b/src/ngx_http_ipng_stats_module.c index 71434c5..751659d 100644 --- a/src/ngx_http_ipng_stats_module.c +++ b/src/ngx_http_ipng_stats_module.c @@ -214,11 +214,16 @@ typedef struct { /* Per-(cscf, sockaddr) tracking used only during config parse. The * listen wrapper uses it to avoid invoking the core `listen` handler * twice for the same (server block, sockaddr) pair — a valid config - * with N device-tagged listens on the same port in the same server - * block generates exactly one core-handler call and one cscf - * attachment, with the remaining listens handled by init_module - * cloning. Across server blocks the same sockaddr legitimately recurs, - * and each distinct cscf gets its own core-handler call. */ + * with N listens on the same port in the same server block generates + * exactly one core-handler call and one cscf attachment, regardless + * of whether those listens are plain or device-tagged. Under the + * IP_PKTINFO attribution model all listens at a given sockaddr + * collapse to a single wildcard kernel socket at runtime, so mixing + * a plain `listen 80;` with `listen 80 device=eth0 ...;` in one + * server block is legitimate and the wrapper must not let nginx's + * own same-sockaddr dedup reject it. Across server blocks the same + * sockaddr legitimately recurs, and each distinct cscf gets its own + * core-handler call. */ typedef struct { void *cscf; /* ngx_http_core_srv_conf_t * */ ngx_sockaddr_t sockaddr; @@ -544,15 +549,10 @@ ngx_http_ipng_stats_listen_wrapper(ngx_conf_t *cf, ngx_command_t *cmd, i++; } - if (device.len == 0 && source.len == 0) { - /* Plain listen with no module-specific parameters — let nginx - * handle it end-to-end. */ - return ngx_http_core_listen_orig(cf, cmd, conf); - } - - /* Parse the listen address ourselves so we can dedup per (cscf, - * sockaddr) — nginx's core handler rejects the same sockaddr - * appearing twice in the same server block. */ + /* Parse the listen address so we can dedup per (cscf, sockaddr) + * across both plain and device-tagged listens. If the address + * doesn't parse, fall back to nginx's handler and let it report + * the error. */ ngx_memzero(&u, sizeof(ngx_url_t)); u.url = value[1]; u.listen = 1; @@ -564,13 +564,6 @@ ngx_http_ipng_stats_listen_wrapper(ngx_conf_t *cf, ngx_command_t *cmd, imcf = ngx_http_conf_get_module_main_conf(cf, ngx_http_ipng_stats_module); - if (imcf->bindings == NULL) { - imcf->bindings = ngx_array_create(cf->pool, 8, - sizeof(ngx_http_ipng_stats_binding_t)); - if (imcf->bindings == NULL) { - return NGX_CONF_ERROR; - } - } if (imcf->listens_seen == NULL) { imcf->listens_seen = ngx_array_create(cf->pool, 16, sizeof(ngx_http_ipng_stats_seen_t)); @@ -580,11 +573,14 @@ ngx_http_ipng_stats_listen_wrapper(ngx_conf_t *cf, ngx_command_t *cmd, } /* Skip the core handler when this (cscf, sockaddr) pair was - * already processed — matches nginx's own "duplicate listen" - * check and lets a server block carry multiple device-tagged - * listens at the same port. Across different server blocks the - * same sockaddr re-appears and nginx merges the cscf via - * ngx_http_add_server. */ + * already processed — all listens at a given sockaddr (plain or + * device-tagged) collapse to one kernel socket under the + * IP_PKTINFO model, so a plain `listen 80;` and `listen 80 + * device=eth0 ...;` in the same server block are compatible: + * nginx's handler runs once (creating the socket), subsequent + * same-sockaddr listens just register additional device + * bindings. Across different server blocks the same sockaddr + * re-appears and nginx merges the cscf via ngx_http_add_server. */ void *cscf = ngx_http_conf_get_module_srv_conf(cf, ngx_http_core_module); ngx_http_ipng_stats_seen_t *seen = imcf->listens_seen->elts; ngx_uint_t same_cscf_sockaddr = 0; @@ -614,6 +610,22 @@ ngx_http_ipng_stats_listen_wrapper(ngx_conf_t *cf, ngx_command_t *cmd, ngx_memcpy(&s->sockaddr, u.addrs[0].sockaddr, u.addrs[0].socklen); } + /* Plain listens contribute no attribution binding — they only + * populate listens_seen so later device-tagged siblings on the + * same sockaddr aren't rejected by nginx's duplicate-listen + * check. */ + if (device.len == 0 && source.len == 0) { + return NGX_CONF_OK; + } + + if (imcf->bindings == NULL) { + imcf->bindings = ngx_array_create(cf->pool, 8, + sizeof(ngx_http_ipng_stats_binding_t)); + if (imcf->bindings == NULL) { + return NGX_CONF_ERROR; + } + } + /* Dedup bindings on (device, family) pair. Same device appearing * under multiple server blocks (because they share a listen * include) collapses to a single binding per family. The same diff --git a/tests/01-module/lab/server/nginx.conf b/tests/01-module/lab/server/nginx.conf index a41b82c..2ff5cff 100644 --- a/tests/01-module/lab/server/nginx.conf +++ b/tests/01-module/lab/server/nginx.conf @@ -8,10 +8,10 @@ # can verify that the module can either combine or distinguish # families per device. # -# Mgmt/direct traffic hits a separate server block on port 9180. -# Mixing a naked `listen 8080;` or a specific-address `listen -# 172.20.40.2:8080;` with device-tagged wildcards on the same port -# is not supported — see docs/user-guide.md. +# Mgmt/direct traffic hits a separate server block on port 9180 — +# a clean port split rather than a technical requirement; plain and +# device-tagged listens may share a port under the IP_PKTINFO model +# (see docs/user-guide.md). load_module /usr/lib/nginx/modules/ngx_http_ipng_stats_module.so;