feat(net): implement PACKET_FANOUT for AF_PACKET load balancing#2119
Conversation
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f87abf8575
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let group: Arc<FanoutGroup> = if unique { | ||
| // UNIQUEID always creates a fresh group with a kernel-assigned id; | ||
| // it cannot attach to an existing group. | ||
| let new_id = writer.id_alloc.alloc().ok_or(SystemError::ENOMEM)? as u16; | ||
| let group = FanoutGroup::new(new_id, params); |
There was a problem hiding this comment.
使用 PACKET_FANOUT_FLAG_UNIQUEID 创建组后，该组无法再加入第二个 socket：后续调用若继续携带 UNIQUEID，会在此分支忽略返回的组 ID 并新建另一组；若清除 UNIQUEID 后用 getsockopt 返回的 ID 加入，又会因为现有组持久保存了 UNIQUEID、与新请求的 flags 不同而返回 EINVAL。因此这个公开支持的 flag 只能产生单成员组，需在分配 ID 后从组的持久 flags 中清除 UNIQUEID。
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Thanks, this was correct for the previous revision. Fixed in 9750841: UNIQUEID is now treated only as an ID allocation request and is stripped before the group flags and socket membership value are persisted. getsockopt(PACKET_FANOUT) therefore returns the assigned ID without UNIQUEID, and another socket can join with that returned value. This is covered by AfPacketSockopt.FanoutUniqueIdIsAllocationOnly.
| FanoutMode::Lb | FanoutMode::Rollover => { | ||
| self.rr_counter.fetch_add(1, Ordering::Relaxed) as usize % live_count | ||
| } |
There was a problem hiding this comment.
PACKET_FANOUT_ROLLOVER 在当前成员仍有空间时应持续投递给该成员，仅在其拥塞后才移动到后继成员；这里却与 LB 共用每包递增的计数器，所以两个空闲成员也会交替收包，ROLLOVER 实际退化成 round-robin。任何依赖该模式在拥塞前保持流量局部性的程序都会观察到与 Linux 6.6 不同的分发结果。
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Thanks, fixed in 9750841. ROLLOVER no longer shares the LB round-robin counter. Each rollover-capable primary owns a cursor; pure ROLLOVER starts at member 0, keeps selecting the current member while it has room, and advances the cursor only after selecting a fallback. This follows the stateful behavior of Linux fanout_demux_rollover().
| let sock_type = self.sock_type; | ||
| let (bound_ifindex, bound_protocol) = self.binding.load(); | ||
| self.netns.fanout_group_join( |
There was a problem hiding this comment.
组配置只在这里快照一次 (ifindex, protocol)，但 binding.rs::bind_to 仍允许已入组 socket 后续修改 binding，且这里采样时也未持有 bind_lock。重新绑定或与 join 并发绑定后，组会按旧 profile 选择该成员，而 deliver_from_iface 按新 binding 拒绝帧且不会改投其他成员，造成间歇性丢包；应按统一锁序列化 join/bind，并像 Linux 一样拒绝 fanout 成员重新 bind。
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Thanks, fixed in 9750841. bind and fanout join are now serialized by the same bind_lock, with the global order bind_lock -> packet_sockets_writer. bind returns EINVAL once the socket belongs to a fanout group, while join snapshots the binding under bind_lock. This removes both the post-join rebind case and the join/bind race. The behavior is covered by AfPacketSockopt.FanoutControlSemanticsMatchLinux.
| if !rollover || socket.rx_has_room() { | ||
| socket.deliver(ingress_ifindex, frame, pkt_type); | ||
| return stale; |
There was a problem hiding this comment.
在多核同时向同一目标投递且接收缓冲区接近上限时，rx_has_room() 返回 true 后可能被另一 CPU 抢先占满；随后 deliver() 的原子容量预留失败并丢包，但这里仍直接返回，不再尝试组内其他空闲成员。启用 ROLLOVER 的背压场景因此会在存在可用成员时丢包，投递函数需要反馈“队列已满”，让该循环继续扫描。
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Thanks for raising this race. I did not add a post-enqueue retry because Linux 6.6 packet_rcv_fanout() selects one index through fanout_demux_rollover() and then invokes that socket's prot_hook.func exactly once. packet_rcv_has_room() is an advisory selection check; Linux does not select a second member if the subsequent enqueue loses a race. Retrying here would change Linux-visible drop/filter accounting and would require a transactional delivery API to rule out duplicate delivery. The current implementation therefore performs one selection and one final delivery, recording a single drop when that queue cannot accept the frame.
| for offset in 0..live_count { | ||
| let target = (base + offset) % live_count; | ||
| let Some(socket) = nth_live_member(&snapshot.members, target) else { |
There was a problem hiding this comment.
启用 ROLLOVER 且连续多个成员队列已满时，每轮都会调用从表头扫描的 nth_live_member()，使一次收包分发变为 O(n²)；该循环位于 NAPI 热路径，大组在持续背压和高入站流量下会消耗大量 CPU。应一次定位起点后直接环形遍历快照，使每个成员每包最多访问一次。
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Thanks, fixed in 9750841. The immutable group snapshot now stores members in directly indexable order, and rollover performs one bounded circular pass over that array. Each member is inspected at most once, so the fallback path is O(n) rather than repeatedly scanning from the beginning.
| // Publish the new member list first, then set the socket's field, so a | ||
| // concurrent deliver that already sees the member also skips it in the | ||
| group.add_member(socket.self_ref().clone()); | ||
| socket.set_fanout_group(group.clone()); |
There was a problem hiding this comment.
追加成员前没有执行 Linux 6.6 的 PACKET_FANOUT_MAX（256）限制，因此第 257 个及后续 socket 仍会成功加入，而预期应返回 ENOSPC。这不仅破坏接口兼容性，也让成员快照复制和每包扫描成本可被持有 CAP_NET_RAW 的进程无界放大。
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Thanks for checking the bound. The numeric premise here is not correct for Linux 6.6: net/packet/internal.h defines PACKET_FANOUT_MAX as (1 << 16). The value 256 is only the legacy default when the 4-byte ABI is used or max_num_members is zero. The current revision accepts the exact 4-byte and 8-byte ABIs, defaults legacy groups to 256, permits an explicit capacity up to 65536, returns ENOSPC at the configured group capacity, and rejects values above 65536 with EINVAL. AfPacketSockopt.FanoutArgsControlsGroupCapacity covers these cases.
| let sock_type = self.sock_type; | ||
| let (bound_ifindex, bound_protocol) = self.binding.load(); | ||
| self.netns.fanout_group_join( |
There was a problem hiding this comment.
当 AF_PACKET socket 以 protocol=0 创建且尚未绑定协议时，这里仍允许其创建或加入 fanout 组，但 deliver_from_iface 会因 bound_protocol == 0 拒绝所有帧；若该成员被选中，帧也不会改投其他成员。Linux 要求 socket 已处于 running 状态，因而这种调用应返回 EINVAL。
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Thanks, fixed in 9750841. join_fanout now rejects bound_protocol == 0 with EINVAL before publishing any group membership, matching the Linux PACKET_SOCK_RUNNING requirement. AfPacketSockopt.FanoutRejectsInactiveAndDualRollover covers this case.
| if socket.has_fanout_group() { | ||
| return Err(SystemError::EBUSY); |
There was a problem hiding this comment.
同一 socket 第二次调用 setsockopt(PACKET_FANOUT) 时，Linux 6.6 的 fanout_add 返回 EALREADY，这里却返回 EBUSY；依赖 errno 区分“已经加入”与资源忙的兼容程序会进入错误处理分支。应使用 SystemError::EALREADY。
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Thanks, fixed in 9750841. Both the socket-side check under bind_lock and the netns writer-side defensive check now return EALREADY when the socket already belongs to a fanout group. AfPacketSockopt.FanoutControlSemanticsMatchLinux verifies the repeated setsockopt errno.
f87abf8 to
9750841
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9750841ef5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| self.packet_sockets_need_cleanup | ||
| .store(true, Ordering::Release); |
There was a problem hiding this comment.
当关闭 AF_PACKET socket 时拓扑重建因 ENOMEM 失败，这里只设置清理标志，但 cleanup_packet_sockets() 仅在 NetnsPoller::polling() 被唤醒后运行；若该命名空间没有接口定时器或后续网络事件，poller 会以无限超时休眠，导致失效 fanout 成员、by_id 条目及已分配的 group ID 永远得不到回收。该失败分支应在设置标志后调用 wakeup_poll_thread()，确保注释所述的异步重试实际发生。
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in the rebased PR #2119 head, commit 38846e2. The finding is valid: merely re-arming the cleanup flag could leave the poller asleep indefinitely. The fix now posts a dedicated cleanup-only wake reason to the netns poller instead of calling the public network wake path, because that path also schedules every interface NAPI. Failed topology rebuilds are retried with a bounded 100 ms–5 s exponential backoff and an absolute deadline, while inactive plain and fanout members are skipped immediately so closed sockets cannot receive during deferred cleanup. The rebased PACKET_MR_PROMISC + PACKET_FANOUT close path is covered by a new regression test. QEMU results: af_packet_sockopt_test 38/38, af_packet_e2e_test 14/14, and af_packet_mcast_test 14/14.
Implement PACKET_FANOUT (setsockopt SOL_PACKET option 18) for AF_PACKET sockets, distributing ingress frames across group members instead of broadcasting a full copy to every socket. Supported fanout modes: HASH (flow hash), LB (round-robin), CPU, ROLLOVER, RND. Supported flags: FLAG_ROLLOVER (backup fallback), FLAG_UNIQUEID (kernel-assigned group id). Unsupported modes (QM/CBPF/EBPF) and flags (DEFRAG/IGNORE_OUTGOING) return EINVAL. Architecture: FanoutGroup registry in NetNamespace using RCU copy-on-write (RcuArcSlot), mirroring the existing packet_sockets broadcast registry. deliver_to_packet_sockets skips fanout members in the broadcast loop and dispatches one copy per group via group.deliver(). join/leave TOCTOU closed under fanout_groups_writer lock. Per-packet deliver path is zero-allocation (two-pass member selection). AtomicBool fanout_active mirror avoids RwSem on the broadcast hot path. Closes DragonOS-Community#2032 Signed-off-by: sparkzky <sparkhhhhhhhhhh@outlook.com>
…_TIMESTAMP Fix three P1 issues found in adversarial review: 1. ABBA deadlock: join acquired fanout_groups_writer → fanout.write() while leave acquired fanout.write() → fanout_groups_writer. Unified lock order to always fanout_groups_writer → fanout.write() by moving the socket's fanout field clear into the netns leave path under the writer lock. 2. TOCTOU race in join_fanout: the EBUSY check (fanout.read().is_some()) ran outside the writer lock, allowing concurrent setsockopt on the same socket to bypass it and leak the socket into two groups simultaneously. Moved the check inside fanout_group_join after acquiring the writer lock. 3. PACKET_TIMESTAMP constant (value 17) was inadvertently removed from the packet_option module. Restored. Signed-off-by: sparkzky <sparkhhhhhhhhhh@outlook.com>
Bundle the 7 fanout join parameters (id_req, unique, mode, flags, sock_type, bound_ifindex, bound_protocol) into a FanoutJoinParams struct, reducing fanout_group_join's argument count from 9 to 3 and clearing the clippy::too_many_arguments error under #![deny(clippy::all)]. Signed-off-by: sparkzky <sparkhhhhhhhhhh@outlook.com>
Reduce FanoutGroup::new from 6 positional args to (id, params) by reusing the existing FanoutJoinParams bundle (a Copy struct), eliminating the duplicated argument lists at both join call sites. Extract publish_fanout_groups() helper for the registry Vec rebuild + RCU snapshot publish pattern that was triplicated across join, leave, and cleanup. Net -20 lines; behavior unchanged. Signed-off-by: sparkzky <sparkhhhhhhhhhh@outlook.com>
Rework fanout registration and delivery around one immutable per-netns RCU topology so plain sockets and fanout groups cannot observe split registry state. Make join, leave, close, and orphan cleanup construct replacement snapshots before publication and preserve allocator and membership invariants on allocation failure. Align the data path with Linux semantics for HASH, LB, CPU, RND, and ROLLOVER modes, including per-primary rollover state, fragment-safe flow hashing, nested VLAN protocol discovery, PACKET_FANOUT_FLAG_UNIQUEID, IGNORE_OUTGOING, and the 8-byte fanout_args max_num_members ABI. Keep protocol and flow parsing lazy and frame-local, preserve the existing cBPF/VLAN ingress path, and avoid allocations or locks in packet delivery. Add control-plane and end-to-end coverage for option validation, group capacity, lifecycle rules, and exactly-once balanced delivery. The new LB regression sends 16 frames and requires a total of 16 deliveries split 8/8 across two sockets. Validation: make kernel; af_packet_sockopt_test (38/38); af_packet_e2e_test (14/14). Signed-off-by: longjin <longjin@dragonos.org>
Wake the network-namespace poller when a fallible AF_PACKET topology unregister leaves stale entries behind. Keep cleanup and network wake reasons separate so cleanup-only work does not spuriously schedule interface NAPI. Retry failed immutable-topology rebuilds with a bounded exponential delay and an absolute deadline. Treat inactive plain and fanout members as stale immediately so closed sockets cannot receive frames while deferred cleanup is pending. Add a regression test covering the rebased PACKET_MR_PROMISC and PACKET_FANOUT close path, including restoration of the interface promiscuity reference count. Tested with make kernel and DragonOS QEMU dunitest suites: af_packet_sockopt_test (38/38), af_packet_e2e_test (14/14), and af_packet_mcast_test (14/14). Signed-off-by: longjin <longjin@dragonos.org>
9750841 to
38846e2
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Keep the pull request focused on the implementation and regression coverage by removing the internal development plan from the published change set. Signed-off-by: longjin <longjin@dragonos.org>
2 participants
Summary
Implement Linux-compatible
PACKET_FANOUTsupport for AF_PACKET sockets. Sockets in the same fanout group receive one selected copy of each matching frame instead of every socket receiving a broadcast copy.Closes #2032.
Supported behavior
fanout_argsgroup-capacity ABI andPACKET_FANOUT_DATAvalidation;PACKET_ADD_MEMBERSHIP/PACKET_DROP_MEMBERSHIPfrom currentmaster.QM, CBPF, EBPF, and DEFRAG remain unsupported because their required RSS, BPF, or defragmentation infrastructure is not available; invalid or unsupported combinations return
EINVAL.Architecture and lifecycle
Validation
make all -j 8make kernelaf_packet_sockopt_test— 38/38 passedaf_packet_e2e_test— 14/14 passedaf_packet_mcast_test— 14/14 passedThe final rebased diff was independently reviewed for Linux semantics, concurrency and lifetime safety, architecture, performance, security, over-design, and regression risk.