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

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] [day] [month] [year] [list]

Message-ID: <CAJcM6BE7qg464oLOJZtVEdYjaki422fxvUWFsA_=CjOAJeqZ_g@mail.gmail.com>
Date: Wed, 5 Nov 2025 10:31:32 -0800
From: Ankit Garg <nktgrg@...gle.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: Joshua Washington <joshwash@...gle.com>, netdev@...r.kernel.org, 
	Harshitha Ramamurthy <hramamurthy@...gle.com>, Jordan Rhee <jordanrhee@...gle.com>, 
	Willem de Bruijn <willemb@...gle.com>, Andrew Lunn <andrew+netdev@...n.ch>, 
	"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, 
	Paolo Abeni <pabeni@...hat.com>, Alexei Starovoitov <ast@...nel.org>, 
	Daniel Borkmann <daniel@...earbox.net>, Jesper Dangaard Brouer <hawk@...nel.org>, 
	John Fastabend <john.fastabend@...il.com>, Stanislav Fomichev <sdf@...ichev.me>, 
	Praveen Kaligineedi <pkaligineedi@...gle.com>, Ziwei Xiao <ziweixiao@...gle.com>, 
	open list <linux-kernel@...r.kernel.org>, 
	"open list:XDP (eXpress Data Path):Keyword:(?:b|_)xdp(?:b|_)" <bpf@...r.kernel.org>
Subject: Re: [PATCH net-next 2/3] gve: Allow ethtool to configure rx_buf_len

On Fri, Oct 24, 2025 at 11:17 AM Ankit Garg <nktgrg@...gle.com> wrote:
>
> On Thu, Oct 23, 2025 at 5:14 PM Jakub Kicinski <kuba@...nel.org> 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.
>

After taking another look, an additional check is still needed to
handle scenario when device doesn't advertise support for 4k buffers.
I reworked this check (and added a comment) in v2 which hopefully
conveys the intent better.


> > > +             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;
> > > +}