netdev - Re: [RFC net-next 19/22] eth: bnxt: use queue op config validate

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5nar53qzx3oyphylkiv727rnny7cdu5qlvgyybl2smopa6krb4@jzdm3jr22zkc>
Date: Thu, 12 Jun 2025 11:56:26 +0000
From: Dragos Tatulea <dtatulea@...dia.com>
To: Jakub Kicinski <kuba@...nel.org>, davem@...emloft.net
Cc: netdev@...r.kernel.org, edumazet@...gle.com, pabeni@...hat.com, 
	andrew+netdev@...n.ch, horms@...nel.org, donald.hunter@...il.com, sdf@...ichev.me, 
	almasrymina@...gle.com, dw@...idwei.uk, asml.silence@...il.com, ap420073@...il.com, 
	jdamato@...tly.com, michael.chan@...adcom.com
Subject: Re: [RFC net-next 19/22] eth: bnxt: use queue op config validate

On Mon, Apr 21, 2025 at 03:28:24PM -0700, Jakub Kicinski wrote:
> Move the rx-buf-len config validation to the queue ops.
> 
> Signed-off-by: Jakub Kicinski <kuba@...nel.org>
> ---
>  drivers/net/ethernet/broadcom/bnxt/bnxt.c     | 40 +++++++++++++++++++
>  .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 12 ------
>  2 files changed, 40 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> index 43497b335329..a772ffaf3e5b 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> @@ -16052,8 +16052,46 @@ static int bnxt_queue_stop(struct net_device *dev, void *qmem, int idx)
>  	return 0;
>  }
>  
> +static int
> +bnxt_queue_cfg_validate(struct net_device *dev, int idx,
> +			struct netdev_queue_config *qcfg,
> +			struct netlink_ext_ack *extack)
> +{
> +	struct bnxt *bp = netdev_priv(dev);
> +
> +	/* Older chips need MSS calc so rx_buf_len is not supported,
> +	 * but we don't set queue ops for them so we should never get here.
> +	 */
> +	if (qcfg->rx_buf_len != bp->rx_page_size &&
> +	    !(bp->flags & BNXT_FLAG_CHIP_P5_PLUS)) {
> +		NL_SET_ERR_MSG_MOD(extack, "changing rx-buf-len not supported");
> +		return -EINVAL;
> +	}
> +
> +	if (!is_power_of_2(qcfg->rx_buf_len)) {
> +		NL_SET_ERR_MSG_MOD(extack, "rx-buf-len is not power of 2");
> +		return -ERANGE;
> +	}
> +	if (qcfg->rx_buf_len < BNXT_RX_PAGE_SIZE ||
> +	    qcfg->rx_buf_len > BNXT_MAX_RX_PAGE_SIZE) {
> +		NL_SET_ERR_MSG_MOD(extack, "rx-buf-len out of range");
> +		return -ERANGE;
> +	}
> +	return 0;
> +}
> +

For the hypothetical situation when the user configures a larger buffer
than the ring size * MTU. Should the check happen in validate or should
the max buffer size be dynamic depending on ring size and MTU?

It is currently hypothetical as BNXT_MAX_RX_PAGE_SIZE is 32K. But for
reference it would be good to know/document.

Thanks,
Dragos