Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] ice: u8 overflow of num_ring_rx and num_ring_tx in ice_vc_map_q_vector_msg() --
commit cafaf07076ec22c624fc3bc56c70b0e708de5ea9 Author: Brett Creeley <[email protected]> ice: introduce handling of virtchnl LARGE VF opcodes This commit adds virtchnl message handlers for VIRTCHNL_OP_ENABLE_QUEUES_V2, VIRTCHNL_OP_DISABLE_QUEUES_V2, and VIRTCHNL_OP_MAP_QUEUE_VECTOR to support VFs with more than 16 queue pairs, and allowlists the VIRTCHNL_VF_LARGE_NUM_QPAIRS opcodes. > diff --git a/drivers/net/ethernet/intel/ice/virt/queues.c > b/drivers/net/ethernet/intel/ice/virt/queues.c > index 7f2b459dbc836..f87ccf5d370bb 100644 > --- a/drivers/net/ethernet/intel/ice/virt/queues.c > +++ b/drivers/net/ethernet/intel/ice/virt/queues.c > @@ -1046,3 +1046,342 @@ int ice_vc_request_qs_msg(struct ice_vf *vf, u8 *msg) [ ... ] > +int ice_vc_map_q_vector_msg(struct ice_vf *vf, u8 *msg, u16 msglen) > +{ > + enum virtchnl_status_code v_ret = VIRTCHNL_STATUS_ERR_PARAM; > + struct virtchnl_queue_vector_maps *qv_maps; > + struct ice_vsi *vsi; > + > + qv_maps = (struct virtchnl_queue_vector_maps *)msg; > + > + if (!ice_vc_validate_qv_maps(vf, qv_maps, msglen)) > + goto error_param; > + > + vsi = ice_get_vf_vsi(vf); > + if (!vsi) > + goto error_param; > + > + for (int i = 0; i < qv_maps->num_qv_maps; i++) { > + struct virtchnl_queue_vector *qv_map = &qv_maps->qv_maps[i]; > + struct ice_q_vector *q_vector; > + u16 vector_id; > + int vsi_q_id; > + > + vsi_q_id = qv_map->queue_id; > + vector_id = qv_map->vector_id; > + > + q_vector = vf->vf_ops->get_q_vector(vsi, vector_id); > + if (!q_vector) > + goto error_param; > + > + if (!ice_vc_isvalid_q_id(vsi, vsi_q_id)) > + goto error_param; > + > + /* keep SW state in sync with HW, as ice_cfg_interrupt() does */ > + if (qv_map->queue_type == VIRTCHNL_QUEUE_TYPE_RX) { > + struct ice_rx_ring *ring = vsi->rx_rings[vsi_q_id]; > + > + if (ring->q_vector && ring->q_vector->num_ring_rx) > + ring->q_vector->num_ring_rx--; > + ring->q_vector = q_vector; > + q_vector->num_ring_rx++; > + q_vector->rx.itr_idx = qv_map->itr_idx; > + ice_cfg_rxq_interrupt(vsi, vsi_q_id, > + q_vector->vf_reg_idx, > + qv_map->itr_idx); > + } else if (qv_map->queue_type == VIRTCHNL_QUEUE_TYPE_TX) { > + struct ice_tx_ring *ring = vsi->tx_rings[vsi_q_id]; > + > + if (ring->q_vector && ring->q_vector->num_ring_tx) > + ring->q_vector->num_ring_tx--; > + ring->q_vector = q_vector; > + q_vector->num_ring_tx++; [Severity: Medium] Can q_vector->num_ring_rx and q_vector->num_ring_tx overflow in ice_vc_map_q_vector_msg() in drivers/net/ethernet/intel/ice/virt/queues.c? In drivers/net/ethernet/intel/ice/ice.h, num_ring_rx and num_ring_tx are defined as u8 (max 255): struct ice_q_vector { ... u8 num_ring_rx; /* total number of Rx rings in vector */ u8 num_ring_tx; /* total number of Tx rings in vector */ With VIRTCHNL_VF_LARGE_NUM_QPAIRS enabled, a VF can be configured with up to ICE_MAX_QS_PER_VF (256) queue pairs. If the VF sends a VIRTCHNL_OP_MAP_QUEUE_VECTOR message mapping all 256 Rx or Tx queues to the same interrupt vector, incrementing q_vector->num_ring_rx or q_vector->num_ring_tx 256 times wraps the u8 counter from 255 to 0. If any of those queues are later remapped to another vector, the check: if (ring->q_vector && ring->q_vector->num_ring_rx) ring->q_vector->num_ring_rx--; will see 0 and skip the decrement, leaving the per-vector ring accounting out of sync. Should num_ring_rx and num_ring_tx in struct ice_q_vector be widened to u16? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=11
