Skip to content

edge, supernode: allow -t 0 to disable the management UDP port#1242

Open
larsborn wants to merge 5 commits into
ntop:devfrom
larsborn:dev
Open

edge, supernode: allow -t 0 to disable the management UDP port#1242
larsborn wants to merge 5 commits into
ntop:devfrom
larsborn:dev

Conversation

@larsborn

@larsborn larsborn commented Jun 10, 2026

Copy link
Copy Markdown

Summary

Passing -t 0 to edge (default mgmt port 5644) or supernode (default 5645) now skips opening the management UDP socket entirely. The socket field stays at its -1 init value and the select()-loop FD_SET/FD_ISSET calls are guarded with >= 0.

Motivation: operators who don't use the JSON-over-UDP management API and want one fewer loopback socket bound (e.g. for hardening) had no way to suppress it. The mgmt port also exposes a small UDP parser and request dispatcher that some deployments would rather not have running.

Changes

  • src/supernode.c, src/sn_utils.c — gate the mgmt-socket open and the loop's FD_SET/FD_ISSET calls; remove the misleading bad management port format, defaulting to %u warning that fired on -t 0 (it never actually restored the default — the // default is made sure in sn_init() comment was wrong; sn_init runs before CLI parsing).
  • src/edge.c, src/edge_utils.c — same gating for the edge.
  • supernode.1, edge.8 — one-line note under the -t entry: "A value of 0 disables the management API entirely (no UDP socket is bound)."
  • Help text for -t in both binaries appended `; 0 disables the management API`.

The existing `MAX(...)` expressions in both `select()` loops absorb a `-1` correctly because the macro is a signed comparison, so they're left untouched.

No tests added. The existing `tests-*` unit suite doesn't cover socket lifecycle. The shell-based integration harness exercises the management API on default ports; extending it to cover the disabled case would require spinning a second daemon instance. Happy to add either on request.

@lucaderi

Copy link
Copy Markdown
Member

Many tests are failing, please fix them first

@larsborn

Copy link
Copy Markdown
Author

Happy to, but if I'm reading this correctly, those tests would fail even without my changes:

Error: This request has been automatically failed because it uses a deprecated version of `actions/upload-artifact: v3`. Learn more: https://github.blog/changelog/2024-04-16-deprecation-notice-v3-of-the-artifact-actions/

Should I just push the fix to this MR here or would you rather have a separate one and then we rebase this one?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants