ipng/nginx-ipng-stats-plugin
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Allow plain and device-tagged listens to share a sockaddr (v0.7.1)

Browse Source 
The previous wrapper skipped nginx's duplicate-listen check only
for listens that carried device=, so a `listen 80;` next to a
`listen 80 device=eth0 ...;` in the same server block was
rejected at config time. Under SO_BINDTODEVICE that restriction
tracked a real kernel constraint (device-tagged listens created
separate sockets, a bare listen alongside them was genuinely
ambiguous). Under the IP_PKTINFO model introduced in 450391a
the constraint no longer exists — all same-sockaddr listens
collapse to one wildcard kernel socket and attribution is a
per-connection cmsg readback — but the wrapper kept enforcing
the old rule by accident.

Extend the (cscf, sockaddr) dedup in the listen wrapper to
cover plain listens too: the first occurrence at a given
(server, sockaddr) pair calls nginx's handler and registers the
kernel socket, and every subsequent sibling — plain or
device-tagged — is accepted without tripping nginx's
duplicate-listen check. Device-tagged siblings additionally
push a binding into the attribution table as before; plain
siblings contribute only the seen-list entry. No code path
exercised by the existing 22 e2e tests changes behavior.

Update FR-1.5, the user-guide "shared port" section, the
module's top-of-function comments, and the test nginx.conf
comment to describe the relaxed rule. Bump VERSION and add a
debian/changelog entry for 0.7.1.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Pim van Pelt
2026-04-19 09:17:44 +02:00
parent 8e0b1cdde9
commit badb684431
6 changed files with 77 additions and 51 deletions
Download Patch File Download Diff File Expand all files Collapse all files

64
src/ngx_http_ipng_stats_module.c
View File

@@ -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