net: implement TCP accept syscall - #454
Conversation
940ba67 to
37cb678
Compare
|
build_prs |
|
Job is started, see https://github.com/vivoblueos/kernel/actions/runs/32344128512. |
|
✅ All jobs completed successfully, see https://github.com/vivoblueos/kernel/actions/runs/32344128512. |
|
build_prs |
|
Job is started, see https://github.com/vivoblueos/kernel/actions/runs/32356837034. |
|
✅ All jobs completed successfully, see https://github.com/vivoblueos/kernel/actions/runs/32356837034. |
SummaryThe PR implements The implementation is structurally sound and the test additions are appropriate. However, there are several real bugs, the most severe of which is a deadlock between blocking Findings are ranked most-severe first. Correctness findings outrank cleanup/altitude. P0 — Critical1.
|
|
Thanks for the detailed review. We agree that the shared The actual ordering is:
Therefore, in this scenario, Thread B never reaches The underlying problem is broader than A correct fix requires a network-layer refactor, such as per-operation reply channels together with cancellation/generation handling for pending operations. Reordering the shutdown waker or forcibly resetting the shared futex in this PR would be incomplete and could introduce stale operations or deliver the wrong reply to a caller. For this PR, the current supported usage should remain one in-flight operation per fd. In particular, applications should not concurrently operate on the same fd from multiple threads, especially The accepted-fd leak is still a consequence of the blocked unsupported-concurrency scenario, but it should be addressed as part of that broader cancellation/ownership redesign rather than as an accept-specific P0 fix in this PR. |
a87eff0 to
698501c
Compare
698501c to
c7ed444
Compare
|
Thanks for the detailed review. I rechecked all eight findings against the current implementation and the actual operation/FD ownership transitions:
The branch now contains the fixes in |
|
build_prs |
|
Job is started, see https://github.com/vivoblueos/kernel/actions/runs/32557384017. |
|
❌ Job failed. Failed jobs: build_and_check_boards (failure), see https://github.com/vivoblueos/kernel/actions/runs/32557384017. |
|
build_prs |
|
Job is started, see https://github.com/vivoblueos/kernel/actions/runs/32557723574. |
|
❌ Job failed. Failed jobs: build_and_check_boards (failure), see https://github.com/vivoblueos/kernel/actions/runs/32557723574. |
|
build_prs |
|
Job is started, see https://github.com/vivoblueos/kernel/actions/runs/32559206486. |
|
❌ Job failed. Failed jobs: build_and_check_boards (failure), see https://github.com/vivoblueos/kernel/actions/runs/32559206486. |
|
build_prs |
|
Job is started, see https://github.com/vivoblueos/kernel/actions/runs/32561174321. |
|
✅ All jobs completed successfully, see https://github.com/vivoblueos/kernel/actions/runs/32561174321. |
Summary
accept()end to end across the syscall, IPC, network manager, and smoltcp TCP layers.SO_REUSEADDR, whichstd::net::TcpListener::bind()sets before binding a listener.Design
NetworkManagerafter successful shutdown, and RefCell borrow scopes are explicit around that cleanup.option_nameselects one option, and constants can share bits; for example,SO_REUSEADDR (0x0004) & SO_RCVTIMEO (0x1006)is non-zero.SO_REUSEADDRstate is stored onConnection, exposed throughsetsockopt()/getsockopt(), and inherited by accepted connections. The current port allocator remains strict because BlueOS does not retain port leases for TCPTIME_WAITsockets.Testing
rustfmt --checkon all changed Rust filesgit diff --checkninja -C out/qemu_mps2_an385.debug obj/kernel/kernel/blueos/libblueos.rlibninja -C out/seeed_xiao_esp32c3.debug tcp_server_examplekernel_unittestreaches final linking but is currently blocked by the repository/toolchainlog -> stdversus kernelpanic_implduplicate lang-item error.Related