[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250423064653.6db44e9a@kernel.org>
Date: Wed, 23 Apr 2025 06:46:53 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Dragos Tatulea <dtatulea@...dia.com>
Cc: davem@...emloft.net, 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 Wed, 23 Apr 2025 10:00:01 +0000 Dragos Tatulea wrote:
> > +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?
For bnxt rx_buf_len only applies to the "payload buffers".
I should document that, and retest with XDP.
I posted a doc recently with a "design guide" for API interfaces,
it said:
Visibility
==========
To simplify the implementations configuration parameters of disabled features
do not have to be hidden, or inaccessible.
Which I intended to mean that configuring something that isn't enabled
is okay. IIRC we also don't reject setting hds threshold if hds is off.
Hope I understood what you're getting at.
Powered by blists - more mailing lists