refactor(net): 重构TCP listening backlog为动态扩展 by Mico-rstar · Pull Request #1316 · DragonOS-Community/DragonOS

Mico-rstar · 2025-10-09T13:17:15Z

refactor(Listening): 使用队列维护inners
debug(Listening): fix inners没有正确扩展
refactor(poll): 多包一并notify改为单包notify
feat: 增加单元测试

- refactor(Listening): 使用队列维护inners - debug(Listening): fix inners没有正确扩展 - refactor(poll): 多包一并notify改为单包notify - feat: 增加单元测试

fslongjin · 2025-10-09T14:04:45Z

多包一并notify改为单包notify

为什么要这样做

Copilot

Pull Request Overview

This PR refactors the TCP listening backlog implementation to use dynamic expansion instead of pre-allocated sockets. The core changes involve moving from a fixed Vec of bound inners to a queue-based approach that dynamically creates new listening sockets as needed.

Refactored TCP listening to use a VecDeque with dynamic socket creation
Modified polling to process packets individually with immediate notification
Added unit test for backlog functionality

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File	Description
user/apps/c_unitest/test_backlogc.c	New unit test for TCP backlog functionality
kernel/src/net/socket/inet/stream/inner.rs	Refactored Listening struct to use dynamic socket management with VecDeque
kernel/src/driver/net/mod.rs	Changed polling strategy from batch to single-packet processing

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-10-09T14:05:37Z

user/apps/c_unitest/test_backlogc.c

+        }
+        printf("the %dth connection\n", ++cnt);
+        close(*client_sockfd);
+


Memory leak: allocated client_sockfd is never freed after closing the socket. Add free(client_sockfd); after line 80.

Suggested change

free(client_sockfd);

Copilot · 2025-10-09T14:05:38Z

kernel/src/net/socket/inet/stream/inner.rs

+        let mut inners_guard = self.inners.write();
+        let inner = match inners_guard.back() {
+            Some(inner) => inner,
+            None => return debug!("the tcp socket inners is empty"),


Using debug! macro in a return statement is unconventional. Consider using a separate debug! call followed by return, or return an appropriate value/error.

Suggested change

None => return debug!("the tcp socket inners is empty"),

None => {

debug!("the tcp socket inners is empty");

return;

}

Copilot · 2025-10-09T14:05:38Z

kernel/src/net/socket/inet/stream/inner.rs

+                        )),
+                ) {
+                    Ok(inner) => inner,
+                    Err(e) => return debug!("bind err: {:#?}", e),


Using debug! macro in a return statement is unconventional. Consider using a separate debug! call followed by return, or return an appropriate value/error.

kernel/src/net/socket/inet/stream/inner.rs

fslongjin · 2025-10-09T14:09:48Z

and,这个PR是为了解决什么问题？有无测试用例/问题复现案例？

Mico-rstar · 2025-10-09T14:59:31Z

and,这个PR是为了解决什么问题？有无测试用例/问题复现案例？

当前tcp listening backlog的实现会在执行系统调用sys_listen后创建backlog个socket等待连接，这样即便处于空闲状态下依旧占用固定内存，且占用大小取决于用户程序指定的backlog大小。这个PR用队列来维护socket，将完成tcp三次握手的socket push到队列，调用accept从队列pop出socket，从而让队列大小随着并发连接数和客户端处理能力的变化而变化

Mico-rstar · 2025-10-09T15:05:34Z

多包一并notify改为单包notify

为什么要这样做

smoltcp没有提供listen接口，不能在连接到来时自动创建新的socket，所以现在的实现是总是绑定连接数+1个smoltcp socket，单包处理就可以在update_io_event中bind新的socket供下一次连接

Mico-rstar · 2025-10-09T15:29:35Z

and,这个PR是为了解决什么问题？有无测试用例/问题复现案例？

dragonos运行test_backlogc
使用外部tcp连接脚本，并发数：5，连接数：20

fslongjin · 2025-10-09T15:37:51Z

这个实现我初看了一下，怎么感觉容易在poll的地方卡很久？如果一直有包到来的话。

@sparkzky @Samuka007 麻烦帮忙review一下？

sparkzky · 2025-10-12T14:20:04Z

kernel/src/driver/net/mod.rs

+            let mut sockets = self.sockets.lock_irqsave();
+            let result = interface.poll_ingress_single(timestamp, device, &mut sockets);
+            drop(sockets);
+            match result {
+                smoltcp::iface::PollIngressSingleResult::None => break,
+                smoltcp::iface::PollIngressSingleResult::PacketProcessed => {},
+                smoltcp::iface::PollIngressSingleResult::SocketStateChanged => {
+                    self.bounds.read_irqsave().iter().for_each(|bound_socket| {
+                        bound_socket.notify();
+                        let _woke = bound_socket
+                            .wait_queue()
+                            .wakeup(Some(ProcessState::Blocked(true)));
+                    });
+                }
+            }


能说说你这里为什么要逐包处理吗
之前的写法可以一次性处理多个包，然后唤醒bound_sockets，然后可以同时唤醒多个在等待数据的socket
但是现在你这样的写法，每次有一个包的到来，都会去唤醒整个bound_sockets，但是最终只有一个socket真正收到数据，会不会有点不太必要

之前的写法能够一次处理多个包，关键在于提前bind backlog个数量的socket，这个pr希望实现类似于linux动态扩展wait_queue的机制。考虑过两种解决方案：

逐包处理，将处理粒度变小，让上层TCP_Socket可以及时扩充队列大小以供新的连接

在smoltcp层面，提供一个listen接口，并维护一个wait_queue来根据连接数量动态扩展
方案2相对优雅，且可以对smoltcp的分发机制进行优化，例如用hashmap来维护sockets set，但需要对smoltcp进行修改。
是否有更好的方案，或者是否有准备对smoltcp进行优化

sparkzky · 2025-10-12T14:28:40Z

kernel/src/net/socket/inet/stream/inner.rs

+            Inner::Listening(listen) => Some(listen.inners.read().front().unwrap().iface().clone()),
+            Inner::Established(est) => Some(est.inner.iface().clone()),


这里直接unwarp吗，加个判断是否为空吧，为空的话就返回None

refactor(net): 重构TCP listening backlog为动态扩展

4b9f744

- refactor(Listening): 使用队列维护inners - debug(Listening): fix inners没有正确扩展 - refactor(poll): 多包一并notify改为单包notify - feat: 增加单元测试

github-actions bot added the enhancement New feature or request label Oct 9, 2025

fslongjin requested review from Samuka007 and Copilot October 9, 2025 14:04

Copilot AI reviewed Oct 9, 2025

View reviewed changes

fslongjin requested a review from sparkzky October 9, 2025 14:08

sparkzky reviewed Oct 12, 2025

View reviewed changes

Samuka007 removed their request for review February 7, 2026 08:29

-            None => return debug!("the tcp socket inners is empty"),
+            None => {
+                debug!("the tcp socket inners is empty");
+                return;
+            }

		Inner::Listening(listen) => Some(listen.inners.read().front().unwrap().iface().clone()),
		Inner::Established(est) => Some(est.inner.iface().clone()),

Uh oh!

Conversation

Mico-rstar commented Oct 9, 2025

Uh oh!

fslongjin commented Oct 9, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fslongjin commented Oct 9, 2025

Uh oh!

Mico-rstar commented Oct 9, 2025

Uh oh!

Mico-rstar commented Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Mico-rstar commented Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fslongjin commented Oct 9, 2025

Uh oh!

sparkzky Oct 12, 2025

Choose a reason for hiding this comment

Uh oh!

Mico-rstar Oct 12, 2025

Choose a reason for hiding this comment

Uh oh!

sparkzky Oct 12, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Mico-rstar commented Oct 9, 2025 •

edited

Loading

Mico-rstar commented Oct 9, 2025 •

edited

Loading