Re: [PATCH net-next v2 03/15] bnxt_en: Set default MPC ring count

[Jakub Kicinski <kuba@kernel.org>]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
bnxt_en: Set default MPC ring count

This patch adds bnxt_set_dflt_mpc_rings() to compute a default number of
MPC channels when the firmware supports MPC and CONFIG_BNXT_TLS is set,
and bnxt_trim_mpc_rings() to cap the per-type counts to the current
tx_nr_rings_per_tc.

> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_mpc.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_mpc.c
> index 9859a5f86268..cce73d56e46e 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_mpc.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_mpc.c
> @@ -45,3 +46,64 @@ int bnxt_mpc_cp_rings_in_use(struct bnxt *bp)
>  		return 0;
>  	return mpc->mpc_cp_rings;
>  }
> +
> +void bnxt_trim_mpc_rings(struct bnxt *bp)
> +{
> +	struct bnxt_mpc_info *mpc = bp->mpc_info;
> +	int max = bp->tx_nr_rings_per_tc;
> +	u8 max_cp = 0;
> +	int i;
> +
> +	if (!mpc)
> +		return;
> +
> +	for (i = 0; i < BNXT_MPC_TYPE_MAX; i++) {
> +		mpc->mpc_ring_count[i] = min_t(u8, mpc->mpc_ring_count[i], max);

[Medium]
Can the u8 cast in this min_t() truncate the cap?  bp->tx_nr_rings_per_tc
is an int and max is declared as int here, but min_t(u8, ...) casts both
operands to u8 before comparing.

For values of bp->tx_nr_rings_per_tc that are a multiple of 256, the
truncated max becomes 0 and every mpc_ring_count[i] gets zeroed, which
also makes mpc_cp_rings 0 and disables MPC even though plenty of TX rings
are available.  For values in the [257..263] range the truncated max is
[1..7], which would cap the count below BNXT_MAX_MPC (8).  The same
pattern repeats every 256.

Modern P5/P7 deployments with high CPU counts and large user-selected TX
channel counts via ethtool -L can land in those ranges.

Would using min_t(int, ...) be more appropriate here, given the result is
bounded by BNXT_MAX_MPC == 8 and therefore still fits in u8?

> +		max_cp = max(max_cp, mpc->mpc_ring_count[i]);
> +	}
> +	mpc->mpc_cp_rings = max_cp;
> +}
> +

[ ... ]
-- 
pw-bot: cr