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 <[email protected]> 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/[email protected]?part=6
