Re: [PATCH net-next v12 05/12] enic: add admin CQ service with MSI-X interrupt and workqueue polling
From: Jakub Kicinski <kuba@kernel.org>
Date: 2026-07-28 00:41:45
Also in:
lkml
This is an AI-generated review of your patch. The human sending this email has considered the AI review valid, or at least plausible. ---
--- Patch [5]: [PATCH net-next v12 05/12] enic: add admin CQ service with MSI-X interrupt and workqueue polling ---commit 91aacafcb0c0e3c10237617f2d69726711e41d5c Author: Satish Kharat [off-list ref] enic: add admin CQ service with MSI-X interrupt and workqueue polling This commit adds a completion queue service for the admin channel work queue and receive queue. It sets up an MSI-X interrupt that schedules a workqueue poller to drain completions and dispatch received messages.
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/cisco/enic/enic_admin.c b/drivers/net/ethernet/cisco/enic/enic_admin.c index b2be420921064..9358165263d98 100644 --- a/drivers/net/ethernet/cisco/enic/enic_admin.c +++ b/drivers/net/ethernet/cisco/enic/enic_admin.c
[ ... ]
+unsigned int enic_admin_rq_cq_service(struct enic *enic)
+{[ ... ]
+ enic_admin_msg_enqueue(enic, buf->os_buf, bytes_written); + +next_desc: + enic_admin_rq_buf_clean(rq, rq->to_clean); + rq->to_clean = rq->to_clean->next; + rq->ring.desc_avail++; + + vnic_cq_inc_to_clean(cq); + work++; + desc = vnic_cq_to_clean(cq); + } + + if (enic_admin_rq_fill(enic, GFP_KERNEL) && net_ratelimit()) + netdev_warn(enic->netdev, + "admin RQ refill failed\n");
[Severity: High] What happens if enic_admin_rq_fill() fails to allocate memory here? If it fails under memory pressure and returns without posting new buffers to the hardware, does the admin channel permanently stall? It looks like there is no retry timer or fallback mechanism to refill the queue later. Without available receive buffers, the hardware won't be able to post further receive completions, meaning the MSI-X interrupt won't fire again to recover the channel.
+ + return work; +}
[ ... ]
quoted hunk ↗ jump to hunk
@@ -220,6 +500,19 @@ int enic_admin_channel_open(struct enic *enic) return err; } + spin_lock_init(&enic->admin_msg_lock); + INIT_LIST_HEAD(&enic->admin_msg_list); + INIT_WORK(&enic->admin_msg_work, enic_admin_msg_work_handler); + INIT_WORK(&enic->admin_poll_work, enic_admin_poll_work_handler); + + err = enic_admin_setup_intr(enic); + if (err) { + netdev_err(enic->netdev, + "Admin channel requires MSI-X, SR-IOV unavailable: %d\n", + err); + goto free_resources; + } + + enic_admin_init_resources(enic);
[Severity: High] Since enic_admin_setup_intr() calls request_irq(), is it possible for an interrupt to fire before enic_admin_init_resources() initializes the hardware queues? If a spurious interrupt arrives immediately after request_irq(), it would schedule admin_poll_work which then calls enic_admin_rq_cq_service(). Because the ring pointers (such as rq->to_use) haven't been initialized by enic_admin_init_resources() yet, could this lead to a NULL pointer dereference or lockless data corruption?
quoted hunk ↗ jump to hunk
vnic_wq_enable(&enic->admin_wq);@@ -239,17 +532,31 @@ int enic_admin_channel_open(struct enic *enic)