lib: mgmt: use SOMAXCONN for mgmtd socket listen backlog

[reinaldosaraiva]

Summary

The mgmtd frontend and backend UNIX sockets pass a compile-time constant of 32 to listen(2) as the accept-queue backlog (MGMTD_MAX_CONN in lib/mgmt_msg.h). Under fan-in from multiple concurrent clients (vtysh sessions, test harnesses, external controllers) the kernel accept queue saturates and new connect(2) attempts fail with EAGAIN before the msg_server handler ever runs.

This PR aligns mgmtd with the convention already used elsewhere in FRR — bgpd/bgp_network.c, bfdd/dplane.c, and pimd/pim_msdp_socket.c all pass SOMAXCONN to listen() — so the backlog defers to the platform default (on Linux, net.core.somaxconn, typically 4096 on modern kernels). The kernel remains the final arbiter of the effective queue length; operators who need a lower cap can still set net.core.somaxconn.

No API change: MGMTD_MAX_CONN keeps its name. An accompanying comment clarifies that it is a listen(2) backlog, not a cap on concurrent sessions (which can confuse readers given the name).

Reproduction

Stress test with ~1000 concurrent writer goroutines each opening its own msg_client connection to /var/run/frr/mgmtd_fe.sock and sending a small EDIT via the native frontend protocol. On an unpatched build:

backlog	connect successes	dial failures
32 (before)	246 / 1000 (24.6%)	754
4096 / SOMAXCONN (after)	1000 / 1000 (100%)	0

Kernel: Linux 5.15, net.core.somaxconn=4096. Observable via ss -xlp on the socket path (LISTEN 0 32 → LISTEN 0 4096).

Related Issue

None filed; happy to open one if preferred.

Components

mgmtd, lib

[greptile-apps]

Greptile Summary

This PR replaces the hard-coded listen(2) backlog constant 32 (MGMTD_MAX_CONN) in the mgmtd frontend/backend UNIX sockets with SOMAXCONN, deferring to the platform default and aligning with the pattern already used by bgpd, bfdd, and pimd. A clarifying comment is added to prevent MGMTD_MAX_CONN from being misread as a concurrent-session cap. The change is minimal, targeted, and correct.

Confidence Score: 5/5

Safe to merge — single-line change with no logic risk, correct include chain, and consistent with existing FRR conventions.

No P0 or P1 findings. MGMTD_MAX_CONN is used in exactly one place as a listen(2) backlog; SOMAXCONN is always defined before that call site via zebra.h → sys/socket.h. The change is idiomatic within the FRR codebase (bgpd, bfdd, pimd already use SOMAXCONN) and the added comment removes the naming ambiguity.

No files require special attention.

Important Files Changed

Filename	Overview
lib/mgmt_msg.h	Replaces #define MGMTD_MAX_CONN 32 with SOMAXCONN and adds a clarifying block comment; MGMTD_MAX_CONN is used in exactly one place (lib/mgmt_msg.c:876) as the listen(2) backlog, so no other callsites are affected. All translation units that include this header already pull in <sys/socket.h> (via <zebra.h>) before MGMTD_MAX_CONN is referenced, ensuring SOMAXCONN is defined at use-time.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["msg_server_init()"] --> B["socket(AF_UNIX)"]
    B --> C["bind(sopath)"]
    C --> D["listen(sock, MGMTD_MAX_CONN)"]
    D -->|"before: 32"| E["accept queue capped at 32\nconnect() → EAGAIN under load"]
    D -->|"after: SOMAXCONN"| F["accept queue defers to\nnet.core.somaxconn (e.g. 4096)"]
    F --> G["Client connects successfully\nunder high fan-in"]

Loading

_{Reviews (1): Last reviewed commit: "lib: mgmt: use SOMAXCONN for mgmtd socke..." | Re-trigger Greptile}

[ton31337]

LGTM

[donaldsharp]

ci:rerun

[donaldsharp]

this PR looks like it got caught up with the build breakage from yesterday. I've initiated a rerun but in the meantime a rebase + force push would work wonders too

[reinaldosaraiva]

Rebased onto upstream/master e66d35b2ed, no conflicts. Force-pushed as af7fd592ab. Thanks for the heads-up on the transient build breakage.

[reinaldosaraiva]

Friendly ping — @donaldsharp this PR is approved by @ton31337 and the rebase has all CI green. Ready for merge whenever you have a moment. Thanks!

[donaldsharp]

I asked @choppsv1 to look at this on tuesday. Waiting for him to take a gander at it. Frankly I'm not sure I agree with the need to have this at a value greater than 32, on the other hand why would we even care? I wanted someone who's more familiar with mgmtd design to look at this in any event.

[choppsv1]

32 is almost surely a valid value for any normal use of mgmtd -- probably much smaller actually. However, this larger value lets us pass some stress test and look better, and is a standard value ... ok.

We aren't using a TCP socket and we don't do authentication so this is file permission protected socket. If the kernel lets you initiate a connect() and get into the accept queue, then you're a-ok -- so sure, Cry 'Havoc!' ... :)

That said, MGMTD_MAX_CONN is very misleadingly named I think, since we're just going to use the standard suggested value, how about you just remove MGMTD_MAX_CONN entirely and put SOMAXCONN directly into the listen call.

[reinaldosaraiva]

yeah the named constant was mostly noise once it just equals SOMAXCONN. dropped the define, listen() takes SOMAXCONN directly, and the "this is the listen backlog, not a session cap" note moved down next to the call where someone might actually read it. pushed as fe1ec4abac.

[choppsv1]

yeah the named constant was mostly noise once it just equals SOMAXCONN. dropped the define, listen() takes SOMAXCONN directly, and the "this is the listen backlog, not a session cap" note moved down next to the call where someone might actually read it. pushed as fe1ec4abac.

Id remove the comment, we'd just be re-documenting the listen call. We should expect people to know what the args to the function do. It made more sense on the misnamed symbol defined away from use.

[reinaldosaraiva]

Thanks @choppsv1 — comment removed, force-pushed as a67d09f37f. Just the direct listen(sock, SOMAXCONN) call now, nothing else on the line.