Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Timeout handling releases msg_id for immediate reuse without flushing the hardware queue or reinitializing completions, leading to data corruption and race conditions from late hardware completions. -- commit fa131293a7bcd61d0524a5c94944a9ade71d8bda Author: Manish Awasthi <[email protected]> net: mana: Use the HWC destination queues reported by the hardware This commit simplifies the MANA driver by unconditionally using the hardware- reported destination queues for HWC requests. It removes the PF-specific checks since the hardware already supplies values that are valid for the given function. > diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c > b/drivers/net/ethernet/microsoft/mana/hw_channel.c > index 263e7c4e29341..5d75c2478a8a8 100644 > --- a/drivers/net/ethernet/microsoft/mana/hw_channel.c > +++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c [ ... ] > @@ -890,10 +889,13 @@ int mana_hwc_send_request(struct hw_channel_context > *hwc, u32 req_len, > tx_wr->msg_size = req_len; > command = req_msg->req.msg_type; > > - if (gc->is_pf) { > - dest_vrq = hwc->pf_dest_vrq_id; > - dest_vrcq = hwc->pf_dest_vrcq_id; > - } > + /* The hardware reports the HWC destination queues through > + * HWC_INIT_DATA_PF_DEST_RQ_ID and HWC_INIT_DATA_PF_DEST_CQ_ID, and > + * always supplies values that are valid for this function, so no > + * PF-specific handling is needed here. > + */ > + dest_vrq = hwc->pf_dest_vrq_id; > + dest_vrcq = hwc->pf_dest_vrcq_id; > > err = mana_hwc_post_tx_wqe(txq, tx_wr, dest_vrq, dest_vrcq, false); > if (err) { [Severity: High] This isn't a bug introduced by this patch, but I noticed a pre-existing issue in drivers/net/ethernet/microsoft/mana/hw_channel.c within mana_hwc_send_request() and mana_hwc_handle_resp(). If mana_hwc_send_request() times out waiting for the hardware: if (!wait_for_completion_timeout(&ctx->comp_event, (msecs_to_jiffies(hwc->hwc_timeout)))) { ... err = -ETIMEDOUT; goto out; } It proceeds to release the msg_id for immediate reuse without flushing the hardware queue: out: mana_hwc_put_msg_index(hwc, msg_id); return err; If a concurrent or subsequent thread calls mana_hwc_send_request() and reuses this msg_id, could a late hardware response for the original timed-out request cause memory corruption? When the late response arrives, mana_hwc_handle_resp() would see that the msg_id is currently active, copy the stale response into the newly allocated request's buffer, and prematurely signal its completion event: if (!test_bit(msg_id, hwc->inflight_msg_res.map)) { ... } ctx = hwc->caller_ctx + msg_id; ... memcpy(ctx->output_buf, resp_msg, resp_len); out: ... complete(&ctx->comp_event); Because reinit_completion() is not called upon reuse, the completion event would also be left in an incorrect signaled state. Does this race need to be addressed to prevent data corruption? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
