Skip to content

fix(net): honor socket nonblocking mode and TCP accept readiness - #2317

Merged
fslongjin merged 2 commits into
DragonOS-Community:masterfrom
fslongjin:codex/fix-nginx-accept-nonblocking
Sep 22, 2026
Merged

fslongjin merged 2 commits into
DragonOS-Community:masterfrom
fslongjin:codex/fix-nginx-accept-nonblocking

Conversation

@fslongjin

Copy link
Copy Markdown
Member

Summary

Fix two distinct causes of incorrect TCP accept behavior and stalled nginx workers:

  • Treat only Established and CloseWait as acceptable ordinary passive opens, sharing the predicate between listener polling and accept. Capture both endpoints under the same SocketSet lock rather than looking up a potentially reset tuple later.
  • Synchronize socket nonblocking mode in the common File::set_flags path. FIONBIO previously changed the file flags but left the socket's cached mode unchanged, so nginx could block in accept4 after another worker consumed listener readiness.
  • Initialize the socket mode from its new open file description, preserving SOCK_NONBLOCK in socket/socketpair creation. This also keeps listener blocking behavior separate from the new socket flags supplied to accept4.
  • Remove fcntl's separate socket lookup/update and add required regression suites. No nginx workaround, request retry, extra queue, or network hot-path machinery is introduced.

Root-cause evidence

On the failing guest, a request had reached an accepted TCP connection (35 unread bytes, no recv calls), while its nginx worker was sleeping in accept4 on the listener. Updating FIONBIO's common mode propagation fixes this deterministic syscall contract violation instead of masking the HTTP timeout.

The packet-controlled handshake test independently demonstrates that an unanswered SYN must not make a listener readable or acceptable, while a completed handshake followed immediately by FIN must remain acceptable.

Validation

  • Linux: all 14 new tests pass.
  • Pre-fix guest: the handshake test exposes premature accept; nonblocking tests expose two blocking listener calls and four incorrect creation flag cases.
  • Fixed DragonOS guest: 235 focused regression tests pass, including the 14 new cases and existing TCP/UDP, dual-stack, UNIX socket, fcntl, epoll, readv, and inotify tests.
  • nginx 1.24.0, two workers, guest-local HTTP: 30,000/30,000 requests after regressions plus 10,000/10,000 after a fresh guest boot. Both use 16 concurrent clients, a five-second per-request deadline, response validation, and no retries.
  • make kernel, make fmt, FMT_CHECK=1 make fmt, and git diff --check pass.
  • Independent adversarial reviews of concurrency/security, correctness, and architecture/performance found no blocking issues.

This validates the reproduced guest-local failure; historical host-forwarding timeouts are not claimed to be fixed by this patch.

FIONBIO updated the open file description without updating the socket's
cached nonblocking mode. With competing nginx workers, a stale listener
readiness notification could therefore put a worker to sleep in accept4
instead of returning EAGAIN, leaving other accepted requests unread.

Synchronize socket mode in File::set_flags, under the existing update
lock, and remove fcntl's separate fd lookup and mode update. Initialize
socket mode from the new file description at open, including accepted
sockets, and preserve requested O_NONBLOCK in socket/socketpair file flags.
This keeps accept's listener mode separate from accept4's child flags.

Restrict ordinary passive-open readiness to Established and CloseWait.
SynReceived is not an acceptable connection even though smoltcp considers
it active. Share the predicate between poll and accept and capture both
endpoints under the same SocketSet lock, avoiding a later endpoint lookup
after a concurrent reset clears the tuple.

Add packet-controlled handshake tests and bounded nonblocking-mode tests
covering FIONBIO, dup, socket/socketpair creation, and accepted socket flags.
Register both suites in the required dunitest lists.

Validation: Linux passes all 14 added tests; DragonOS passes 235 focused
regressions, including all added tests. Kernel build and formatting checks
pass. The pre-fix guest reproduces premature accept and blocking FIONBIO
listener behavior with the same tests.

Signed-off-by: longjin <longjin@dragonos.org>
@github-actions github-actions Bot added the Bug fix A bug is fixed in this pull request label Sep 21, 2026
@fslongjin

Copy link
Copy Markdown
Member Author

@codex review

AF_PACKET ingress delivery lived in the local smoltcp RxToken, so bridge and routing consumers bypassed it. This made packet-controlled TCP handshake tests depend on incidental outgoing forwarding copies and fail when the peer route selected the other veth interface.

Deliver the ingress tap once before classification, following the Linux packet-tap ordering. Remove the old RxToken delivery and its unused driver reference. Preserve the existing ingress classification and admin-down quiescence locking.

Require PACKET_HOST replies in the handshake regression to prevent outgoing copies from masking this bug. The previous kernel fails all three strengthened cases; Linux passes them. The fixed guest passes 321 focused network regressions and 20 additional handshake rounds (60 cases) with the previously failing peer route. Kernel build and formatting checks pass.

Signed-off-by: longjin <longjin@dragonos.org>
@fslongjin

Copy link
Copy Markdown
Member Author

Fixed the CI failure in 82ea132. The three handshake failures exposed a veth ingress bug: AF_PACKET delivery occurred only for frames reaching the local smoltcp RxToken, after routing/bridge consumers could already discard them. The patch moves ingress packet taps before classification and removes the old delivery to avoid duplicates. The handshake tests now require actual PACKET_HOST replies instead of accepting incidental forwarded OUTGOING copies. Reproduced all three failures on the previous kernel; Linux passes the strengthened tests. The fixed guest passes 321 focused network regressions and 60 additional handshake cases under the previously failing peer route. Build and formatting checks pass. No tests were skipped and no timeouts were relaxed.

@fslongjin

Copy link
Copy Markdown
Member Author

@codex review

@fslongjin
fslongjin merged commit f5183d4 into DragonOS-Community:master Sep 22, 2026
15 checks passed
fslongjin added a commit that referenced this pull request Sep 22, 2026
Protect file-style socket reads used by TLS BIOs against divergence between FIONBIO file flags and the socket mode. The runtime synchronization fix is already present in #2317; no additional kernel or nginx workaround is needed.

Add eight parameterized cases covering TCP, UDP, Unix streams and Unix datagrams. Check empty-queue EAGAIN through a duplicated descriptor, successful reads followed by EAGAIN after draining, and restored blocking reads after FIONBIO is cleared. Reuse the existing bounded child watchdog and socket ownership helper. Send one byte to avoid assuming stream reads fill their buffers.

Validation: all 19 socket_nonblocking tests pass on Linux and DragonOS. All eight new cases detect erroneous blocking with the previous nonblocking implementation restored in a controlled guest. On current master, 75 focused guest tests and 200 TLS 1.2/1.3 HTTPS requests pass, including verified keepalive reuse. Kernel build, formatting checks, and independent plan/code review pass.

Signed-off-by: longjin <longjin@dragonos.org>
fslongjin added a commit that referenced this pull request Sep 22, 2026
…ness (#2319)

Add deterministic accept and accept4 regression coverage for readiness consumed through a duplicated listener. Enable listener nonblocking mode with FIONBIO, observe EPOLLIN, accept the sole connection through an alias, and require EAGAIN when using the stale readiness before processing a queued control message.

This protects the event-loop contract implicated in nginx graceful shutdown hangs. The runtime mode synchronization fix is already merged in #2317. Reuse the existing socket ownership helper and bounded child watchdog; no kernel changes, threads, sleeps, new dependencies, or nginx-specific behavior are introduced.

Validation: 21 socket_nonblocking cases pass on Linux and DragonOS; the two new cases detect blocking with only the previous FIONBIO mode divergence restored. 64 focused guest cases pass, as do ten default nginx start/quit cycles and graceful completion of an in-flight proxy request. Kernel build, formatting, and independent design/code review pass.

Additional validation limitation: all three existing tcp_accept_handshake cases fail to observe SYN-ACK on the unchanged master kernel, including a fresh guest before nginx or the new tests run. That separate baseline failure remains unresolved and is not counted as passing coverage.

Signed-off-by: longjin <longjin@dragonos.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug fix A bug is fixed in this pull request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant