Re: [PATCH net-next v2 06/14] ice: bump to 256qs for VF

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] ice: virtchnl error path disables wrong queues when loop index != queue_id
--

commit a7e9d03ecfb2e4d593b7ee51e67a3a4e87ee7a85
Author: Przemek Kitszel <przemyslaw.kitszel@intel.com>

ice: bump to 256qs for VF

This commit adjusts limits, bitmaps, and arrays in the ice driver to support
up to 256 VF queues. It updates the requested and default queue limits while
retaining the legacy 16-queue limit for older virtchnl opcodes.

> diff --git a/drivers/net/ethernet/intel/ice/virt/queues.c b/drivers/net/ethernet/intel/ice/virt/queues.c
> index 2667634e3c8c3..61304493af34b 100644
> --- a/drivers/net/ethernet/intel/ice/virt/queues.c
> +++ b/drivers/net/ethernet/intel/ice/virt/queues.c
[ ... ]
> @@ -841,7 +841,7 @@ int ice_vc_cfg_qs_msg(struct ice_vf *vf, u8 *msg)
>  	if (!vsi)
>  		goto error_param;
>  
> -	if (qci->num_queue_pairs > ICE_MAX_QS_PER_VF_VCV1 ||
> +	if (qci->num_queue_pairs > ICE_MAX_QS_PER_VF ||
>  	    qci->num_queue_pairs > min_t(u16, vsi->alloc_txq, vsi->alloc_rxq)) {
>  		dev_err(ice_pf_to_dev(pf), "VF-%d requesting more than supported number of queues: %d\n",
>  			vf->vf_id, min_t(u16, vsi->alloc_txq, vsi->alloc_rxq));

[Severity: High]
With the queue limit raised to 256, does the error handling at the end of
ice_vc_cfg_qs_msg() disable the correct queues when a failure occurs?

Since 256 queue pairs exceed the 4096-byte admin queue buffer limit, the VF
driver must split queue configurations across multiple virtchnl messages. In
the second and subsequent messages, the array index i will not match the
actual queue ID being configured (qci->qpair[i].rxq.queue_id).

If an error occurs during these later messages, the cleanup path iterates
using the array index i:

error_param:
    /* disable whatever we can */
    for (; i >= 0; i--) {
        if (ice_vsi_ctrl_one_rx_ring(vsi, false, i, true))
            ...
        if (ice_vf_vsi_dis_single_txq(vf, vsi, i))
            ...
    }

Could this use of i instead of the actual queue ID inadvertently disable
unrelated queues, causing a denial of service to other active queue pairs,
while leaving the newly configured, partially-initialized queues active?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009121433.30347-1-przemyslaw.kitszel@intel.com?part=6