Re: [PATCH net-next v2] net: mana: Use the HWC destination queues reported by the hardware

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] When mana_hwc_send_request() times out, it releases the msg_id back to the pool immediately without waiting for late hardware responses or clearing the completion state. This allows delayed hardware responses to corrupt memory of new requests reusing the msg_id, or falsely trigger stale completions.
--

commit 57ce74bdb654c3d07799a1543f2838a5e3407388
Author: Manish Awasthi <mawasthi@linux.microsoft.com>

net: mana: Use the HWC destination queues reported by the hardware

The hardware provides valid HWC destination vRQ and vRCQ IDs for all PFs
and VFs. Use these IDs unconditionally in mana_hwc_send_request(), as
the PF-specific gate is not required.

Link: https://lore.kernel.org/netdev/20260903184104.3194819-1-mawasthi@linux.microsoft.com/

> diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
> index 263e7c4e29341..3bca4b683134b 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_DEST_RQ_ID and HWC_INIT_DATA_DEST_CQ_ID, and
> +	 * always supplies values that are valid for this function, so no
> +	 * PF-specific handling is needed here.
> +	 */
> +	dest_vrq = hwc->dest_vrq_id;
> +	dest_vrcq = hwc->dest_vrcq_id;
>  
>  	err = mana_hwc_post_tx_wqe(txq, tx_wr, dest_vrq, dest_vrcq, false);

[Severity: High]
This is a pre-existing issue, but should there be a call to
reinit_completion(&ctx->comp_event) here before posting the WQE to the
hardware to ensure stale completions are cleared?

When mana_hwc_send_request() times out, it appears to jump to the out label
and immediately release the msg_id back to the pool via
mana_hwc_put_msg_index(). If a delayed hardware response arrives later,
mana_hwc_handle_resp() will process it:

mana_hwc_handle_resp() {
    ...
    ctx->status_code = resp_msg->status;
    memcpy(ctx->output_buf, resp_msg, resp_len);
out:
    ctx->error = err;
    ...
    complete(&ctx->comp_event);
}

If another thread calls mana_hwc_send_request() and reuses the same msg_id,
could this late response write to a stale ctx->output_buf and cause stack
corruption or overwrite the new request's data?

Furthermore, without clearing the completion state, wouldn't the delayed
complete() call leave the completion marked as done, causing the new
request's wait_for_completion_timeout() to falsely complete immediately?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260910014427.54272-1-mawasthi@linux.microsoft.com?part=1