Re: [PATCH net-next 2/3] gve: Allow ethtool to configure rx_buf_len

[Ankit Garg]

On Thu, Oct 23, 2025 at 5:14 PM Jakub Kicinski <kuba@xxxxxxxxxx> wrote:
>
> On Wed, 22 Oct 2025 11:22:24 -0700 Joshua Washington wrote:
> > +     if (priv->rx_cfg.packet_buffer_size != SZ_2K) {
> > +             netdev_warn(dev,
> > +                         "XDP is not supported for Rx buf len %d. Set Rx buf len to %d before using XDP.\n",
> > +                         priv->rx_cfg.packet_buffer_size, SZ_2K);
> > +             return -EOPNOTSUPP;
> > +     }
>
> Please plumb extack thru to here. It's inside struct netdev_bpf
>

Using extack just for this log will make it inconsistent with other
logs in this method. Would it be okay if I send a fast follow patch to
use exstack in this method and others?

> >       max_xdp_mtu = priv->rx_cfg.packet_buffer_size - sizeof(struct ethhdr);
> >       if (priv->queue_format == GVE_GQI_QPL_FORMAT)
> >               max_xdp_mtu -= GVE_RX_PAD;
> > @@ -2050,6 +2057,44 @@ bool gve_header_split_supported(const struct gve_priv *priv)
> >               priv->queue_format == GVE_DQO_RDA_FORMAT && !priv->xdp_prog;
> >  }
> >
> > +int gve_set_rx_buf_len_config(struct gve_priv *priv, u32 rx_buf_len,
> > +                           struct netlink_ext_ack *extack,
> > +                           struct gve_rx_alloc_rings_cfg *rx_alloc_cfg)
> > +{
> > +     u32 old_rx_buf_len = rx_alloc_cfg->packet_buffer_size;
> > +
> > +     if (rx_buf_len == old_rx_buf_len)
> > +             return 0;
> > +
> > +     if (!gve_is_dqo(priv)) {
> > +             NL_SET_ERR_MSG_MOD(extack,
> > +                                "Modifying Rx buf len is only supported with DQO format");
> > +             return -EOPNOTSUPP;
> > +     }
> > +
> > +     if (priv->xdp_prog && rx_buf_len != SZ_2K) {
> > +             NL_SET_ERR_MSG_MOD(extack,
> > +                                "Rx buf len can only be 2048 when XDP is on");
> > +             return -EINVAL;
> > +     }
> > +
> > +     if (rx_buf_len > priv->max_rx_buffer_size) {
>
> This check looks kinda pointless given the check right below against
> the exact sizes?
>

My intent was to code defensively against device accidently advertising
anything in [2k+1,4k) as max buffer size. I will remove this check.

> > +             NL_SET_ERR_MSG_FMT_MOD(extack,
> > +                                    "Rx buf len exceeds the max supported value of %u",
> > +                                    priv->max_rx_buffer_size);
> > +             return -EINVAL;
> > +     }
> > +
> > +     if (rx_buf_len != SZ_2K && rx_buf_len != SZ_4K) {
> > +             NL_SET_ERR_MSG_MOD(extack,
> > +                                "Rx buf len can only be 2048 or 4096");
> > +             return -EINVAL;
> > +     }
> > +     rx_alloc_cfg->packet_buffer_size = rx_buf_len;
> > +
> > +     return 0;
> > +}