Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: deliver UDP packets from same connection in receiving order #1540

Merged
merged 1 commit into from
Sep 25, 2024

Conversation

HamsterReserved
Copy link

当前 UDP 转发的实现是,将 UDP 报文收到一个 channel 内,然后启动多个 worker 一并 poll 该 channel. 这样一来,同一条 UDP 连接的包可能被不同的 worker 收走,worker 内又没有保序机制,由于 worker 的异步特性,最终导致发出的 UDP 包乱序。

虽然说 UDP 不提供任何顺序保证,但乱序还是会不可避免地导致上层感受到的延迟经常跳动,还是会影响使用体验。因此,能够避免的乱序还是最好要避免。特别是,即便专门采用了能够保证底层有序的传输通道,这里的问题会导致发出的包仍然乱序,这样也是比较难接受的。

本提交参考网卡的 RSS 机制,将 queue 分为每个 worker 一个,将传入的包的 IP 地址、端口、协议进行哈希(实际因为都是 UDP,所以不再计算协议),然后根据哈希结果将包分配到一个 queue、一个 worker 去处理,这样对于同一条连接的包就可以保证都在一个 worker 中处理,借此解决乱序问题。

性能方面我预计是没有明显影响的,因为以前 natTable 里面每条连接也有锁,导致以前的单连接转发性能也有单 worker 瓶颈。多连接性能至少是不受影响,且因为锁的争抢减少乃至消失,甚至还可能有提升。新引入的哈希代码会有一些开销,但是相比后面的加解密部分,这里的开销应当可以忽略不计。


这基本上算是我第一次接触 Go 语言,如有问题,请不吝赐教。谢谢!

@wwqgtxx
Copy link
Collaborator

wwqgtxx commented Sep 24, 2024

暂时不打算接受这个修改,作为udp应用应该有正常处理乱序数据包的能力,不应该依赖接收顺序


从代码上说,Metadata中的RawSrcAddrRawDstAddr不应该被依赖,很多listener并没有正常设置这些值,应该使用SourceAddress()RemoteAddress()替代,此外也没必要公开暴漏HashFiveTupleToQueue这个函数,直接放在HandleUDPPacket内部即可

@wwqgtxx wwqgtxx closed this Sep 24, 2024
@HamsterReserved
Copy link
Author

HamsterReserved commented Sep 24, 2024

暂时不打算接受这个修改,作为udp应用应该有正常处理乱序数据包的能力,不应该依赖接收顺序

从概念上讲,确实是这样。但:

  1. 从实际效果来说,“正常处理”仅仅是应用业务不至于出现严重的中断,这不意味着“体验与不乱序的场景完全一致”。乱序至少会使得对端进入 OOO 处理逻辑而无法以最快速度将数据交给上层,是否有其他影响则需要视具体应用而定。无论包间延迟多小,一旦乱序,所造成的影响都是客观存在的。

    我在我的局域网内布置了一个 Wireguard 客户端经 mihomo TPROXY 转 Shadowsocks (none encryption) 连接到 Wireguard 服务器的拓扑。Wireguard 客户端、mihomo、[Shadowsocks 服务器 + Wireguard 服务器] 是三台独立设备。所有设备之间以万兆连接。

在从 Wireguard 客户端经 iperf3 单线程 TCP 上行跑流到 Wireguard 服务端的测试中,在 59a2b24 提交处编译的 mihomo 能够跑到 880Mbps 的性能,在前述提交的基础上带本 PR 以后,则是跑到 1.02Gbps. 每个 mihomo 版本测试至少三次,并来回切换 mihomo 版本,均可稳定复现此数据。

我认为 Wireguard 已经是比较规范的 UDP 应用,该数据能够证明乱序问题有切实的影响。

  1. 从需求合理性来说,
    a. 在业界实践中,下至 PC 网卡,上至商用路由器,几乎所有的“传输”设备都采用了类似 RSS 的分流机制。虽然保序不一定是其根本目标,但它们确实都实现了本 PR 这个程度的保序。我认为对于同样是转发业务的 mihomo 而言,保序至少也是 nice to have 的 feature,并非过度设计。
    b. 有一些应用客观上更容易受到乱序影响,例如游戏,要么引入 buffer 增加延迟,要么需要回撤用户已经看到的动作,都是很明显有感知的。对于这些应用,乱序情况每减少一点,都能够看到收益。我非常清楚只解决 mihomo 的乱序问题无法完全消除乱序这一现象的客观存在,但这总归是一个改善的步骤。

代码的问题我会按前述意见修改。关于是否接纳本提交的问题,还希望能得到重新评估。

All UDP packets are queued into a single channel, and multiple
workers are launched to poll the channel in current design.

This introduces a problem where UDP packets from a single connection
are delivered to different workers, thus forwarded in a random order
if workers are on different CPU cores. Though UDP peers normally
have their own logic to handle out-of-order packets, this behavior will
inevitably cause significant variance in delay and harm connection quality.
Furthermore, this out-of-order behavior is noticeable even if the underlying
transport could provide guaranteed orderly delivery -  this is unacceptable.

This commit takes the idea of RSS in terms of NICs: it creates a distinct
queue for each worker, hashes incoming packets, and distribute the packet
to a worker by hash result. The tuple (SrcIP, SrcPort, DstIP, DstPort, Proto)
is used for hashing (Proto is always UDP so it's dropped from final
implementation), thus packets from the same connection can be sent to
the same worker, keeping the receiving order. Different connections can be
hashed to different workers to maintain performance.

Performance for single UDP connection is not affected, as there is already
a lock in natTable that prevents multiple packets being processed in different
workers, limiting single connection forwarding performance to 1 worker.
The only performance penalty is the hashing code, which should be neglectable
given the footprint of en/decryption work.
@wwqgtxx wwqgtxx merged commit 3922b17 into MetaCubeX:Alpha Sep 25, 2024
55 checks passed
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.

3 participants