feat(igc): enable multi-queue tx/rx#693
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR extends the Intel IGC (I225/I226) driver to support multiple TX/RX queues, aligning the driver with the kernel networking stack’s multi-queue model and MSI-X queue interrupt layout.
Changes:
- Added a per-queue service routine and updated the interrupt/tick paths to service RX and TX reclaim per queue.
- Removed single-queue constraints in TX/RX paths and updated TX reclaim (
igc_txeof) call sites to be queue-aware. - Introduced queue-count selection based on available MSI-X vectors and CPU count (capped to 4), and enabled MSI-X masks for all queue vectors plus the event/link vector.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
| if let Some(que_id) = irq_queue { | ||
| Self::service_queue(&inner, que_id)?; | ||
| } | ||
|
|
There was a problem hiding this comment.
Using ? here short-circuits before the IGC_IMS / IGC_EIMS re-arm writes below. A transient error from igc_rx_recv or igc_txeof will leave queue/link interrupts masked and the NIC silent. Capture the first error in a local, continue servicing the remaining queues, and return it only after the EIMS/IMS writes have executed — matching the prior que_result semantics.
| @@ -925,10 +943,6 @@ impl IgcInner { | |||
| que_id: usize, | |||
There was a problem hiding this comment.
The previous if que_id != 0 { return Err(Param) } guard is removed without a replacement bounds check, so self.queue_info.que[que_id] will panic for any out-of-range caller. Add if que_id >= self.queue_info.que.len() { return Err(IgcDriverErr::Param); } at the top of igc_send, and consider the same for igc_rx_recv (which also lost its debug_assert_eq! guard).
| for que_id in 0..inner.queue_info.que.len() { | ||
| let mut node = MCSNode::new(); | ||
| let mut tx = inner.queue_info.que[que_id].tx.lock(&mut node); | ||
| if inner.igc_txeof(que_id, &mut tx).is_ok() && tx.igc_desc_unused() > 0 { |
There was a problem hiding this comment.
igc_txeof performs a membar_sync() plus a descriptor walk; calling it for every queue on the can_send probe path serializes TX locks and contends with the ISR-side service_queue. Since the descriptor reclaim is already driven by the interrupt/poll path, tx.igc_desc_unused() > 0 alone is enough here — drop the igc_txeof call from this hot path.
| let cpu_count = core::cmp::max(1, awkernel_lib::cpu::num_cpu()); | ||
| let available = core::cmp::min(core::cmp::min(available_vectors, cpu_count), 4); | ||
|
|
||
| if available >= 4 { |
There was a problem hiding this comment.
After clamping by min(available_vectors, cpu_count, 4), the result is snapped to {1, 2, 4}, so 3 available vectors collapse to 2 queues and waste a vector/CPU. The EIMS mask and igc_initialize_rss_mapping both handle arbitrary nqueues, so the power-of-two stepping is unnecessary — return the clamped value directly. If a power-of-two is required by some RSS constraint, please add a comment stating that constraint.
| !(addr[0] & 1 != 0 || addr.iter().all(|&x| x == 0)) | ||
| } | ||
|
|
||
| fn igc_select_num_queues(available_vectors: usize) -> usize { |
There was a problem hiding this comment.
This function encodes a non-trivial policy (MSI-X vector count, CPU count, hard cap of 4, power-of-two snap) but has no doc comment. The 4 in particular is the I225 RSS queue limit — without a /// doc it reads as a magic number once the PR description is gone. Please add a doc comment explaining the three constraints and the rationale for the cap.
| } | ||
|
|
||
| fn igc_txeof(&self, tx: &mut Tx) -> Result<(), IgcDriverErr> { | ||
| fn igc_txeof(&self, _que_id: usize, tx: &mut Tx) -> Result<(), IgcDriverErr> { |
There was a problem hiding this comment.
The reason _que_id is unused (DD-bit reclaim does not need the hardware head register, parameter kept for call-site uniformity / future per-queue stats) lives only in the PR body. A future reader seeing the underscore will wonder whether this is an oversight. Please move that rationale into a short comment above the function.
Description
This PR adds multi-queue TX/RX support to the IGC driver. It is the fifth step in the incremental IGC bring-up stack; PR4 (#691) must be merged before this one.
Related links
How was this PR tested?
Notes for reviewers