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 for Android: free password hash cracker in your pocket

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

Message-ID: <a5uokb5qgp5atz2cakap2idwhepu5uxkmhj43guf5t3abhyu4n@7xaxugulyng2>
Date: Wed, 23 Apr 2025 10:00:01 +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;
> +}
> +
HDS off and rx_buf_len > 4K seems to be accepted. Is this inteded?

> +static void
> +bnxt_queue_cfg_defaults(struct net_device *dev, int idx,
> +			struct netdev_queue_config *qcfg)
> +{
> +	qcfg->rx_buf_len	= BNXT_RX_PAGE_SIZE;
> +}
> +
>  static const struct netdev_queue_mgmt_ops bnxt_queue_mgmt_ops = {
>  	.ndo_queue_mem_size	= sizeof(struct bnxt_rx_ring_info),
> +
> +	.ndo_queue_cfg_defaults	= bnxt_queue_cfg_defaults,
> +	.ndo_queue_cfg_validate = bnxt_queue_cfg_validate,
>  	.ndo_queue_mem_alloc	= bnxt_queue_mem_alloc,
>  	.ndo_queue_mem_free	= bnxt_queue_mem_free,
>  	.ndo_queue_start	= bnxt_queue_start,
> @@ -16061,6 +16099,8 @@ static const struct netdev_queue_mgmt_ops bnxt_queue_mgmt_ops = {
>  };
>  
>  static const struct netdev_queue_mgmt_ops bnxt_queue_mgmt_ops_unsupp = {
> +	.ndo_queue_cfg_defaults	= bnxt_queue_cfg_defaults,
> +	.ndo_queue_cfg_validate = bnxt_queue_cfg_validate,
>  };
>  
>  static void bnxt_remove_one(struct pci_dev *pdev)
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> index 956f51449709..8842390f687f 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> @@ -867,18 +867,6 @@ static int bnxt_set_ringparam(struct net_device *dev,
>  	if (!kernel_ering->rx_buf_len)	/* Zero means restore default */
>  		kernel_ering->rx_buf_len = BNXT_RX_PAGE_SIZE;
>  
> -	if (kernel_ering->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(kernel_ering->rx_buf_len) ||
> -	    kernel_ering->rx_buf_len < BNXT_RX_PAGE_SIZE ||
> -	    kernel_ering->rx_buf_len > BNXT_MAX_RX_PAGE_SIZE) {
> -		NL_SET_ERR_MSG_MOD(extack, "rx-buf-len out of range, or not power of 2");
> -		return -ERANGE;
> -	}
> -
>  	if (netif_running(dev))
>  		bnxt_close_nic(bp, false, false);
>  
> -- 
> 2.49.0
>

Thanks,
Dragos