Re: [PATCH net-next 06/10] ice: Flesh out implementation of support for SRIOV on bonded interface

[Simon Horman <simon.horman@corigine.com>]

On Wed, Jul 26, 2023 at 11:21:37AM -0700, Tony Nguyen wrote:
> From: Dave Ertman <david.m.ertman@intel.com>
> 
> Add in the functions that will allow a VF created on the primary interface
> of a bond to "fail-over" to another PF interface in the bond and continue
> to Tx and Rx.
> 
> Add in an ordered take-down path for the bonded interface.
> 
> Reviewed-by: Daniel Machon <daniel.machon@microchip.com>
> Signed-off-by: Dave Ertman <david.m.ertman@intel.com>
> Tested-by: Sujai Buvaneswaran <sujai.buvaneswaran@intel.com>
> Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_lag.c | 822 ++++++++++++++++++++++-
>  1 file changed, 812 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_lag.c b/drivers/net/ethernet/intel/ice/ice_lag.c

...

> @@ -233,6 +303,119 @@ static void ice_display_lag_info(struct ice_lag *lag)
>  		upper, role, primary);
>  }
>  
> +/**
> + * ice_lag_qbuf_recfg - generate a buffer of queues for a reconfigure command
> + * @hw: HW struct that contains the queue contexts
> + * @qbuf: pointer to buffer to populate
> + * @vsi_num: index of the VSI in PF space
> + * @numq: number of queues to search for
> + * @tc: traffic class that contains the queues
> + *
> + * function returns the number of valid queues in buffer
> + */
> +static u16
> +ice_lag_qbuf_recfg(struct ice_hw *hw, struct ice_aqc_cfg_txqs_buf *qbuf,
> +		   u16 vsi_num, u16 numq, u8 tc)
> +{
> +	struct ice_q_ctx *q_ctx;
> +	u16 qid, count = 0;
> +	struct ice_pf *pf;
> +	int i;
> +
> +	pf = hw->back;
> +	for (i = 0; i < numq; i++) {
> +		q_ctx = ice_get_lan_q_ctx(hw, vsi_num, tc, i);
> +		if (q_ctx->q_teid == ICE_INVAL_TEID) {

Hi Tony and Dave,

sorry for not noticing this earlier.

Here q_ctx is dereferenced...

> +			dev_dbg(ice_hw_to_dev(hw), "%s queue %d INVAL TEID\n",
> +				__func__, i);
> +			continue;
> +		}
> +
> +		if (!q_ctx || q_ctx->q_handle == ICE_INVAL_Q_HANDLE) {

...but here it is assumed that q_ctx may be NULL.

Flagged by Smatch.

> +			dev_dbg(ice_hw_to_dev(hw), "%s queue %d %s\n", __func__,
> +				i, q_ctx ? "INVAL Q HANDLE" : "NO Q CONTEXT");
> +			continue;
> +		}
> +
> +		qid = pf->vsi[vsi_num]->txq_map[q_ctx->q_handle];
> +		qbuf->queue_info[count].q_handle = cpu_to_le16(qid);
> +		qbuf->queue_info[count].tc = tc;
> +		qbuf->queue_info[count].q_teid = cpu_to_le32(q_ctx->q_teid);
> +		count++;
> +	}
> +
> +	return count;
> +}

...