Re: [EXTERNAL] [PATCH 6.12.y] virtio_net: clamp rss_max_key_size to NETDEV_RSS_KEY_LEN

["Michael S. Tsirkin" <mst@redhat.com>]

On Wed, Apr 08, 2026 at 10:14:13AM -0400, Sasha Levin wrote:
> On Wed, Apr 08, 2026 at 01:49:48PM +0000, Srujana Challa wrote:
> > > From: Srujana Challa <schalla@marvell.com>
> > > 
> > > [ Upstream commit b4e5f04c58a29c499faa85d12952ca9a4faf1cb9 ]
> > > 
> > > rss_max_key_size in the virtio spec is the maximum key size supported by the
> > > device, not a mandatory size the driver must use. Also the value 40 is a spec
> > > minimum, not a spec maximum.
> > > 
> > > The current code rejects RSS and can fail probe when the device reports a
> > > larger rss_max_key_size than the driver buffer limit. Instead, clamp the
> > > effective key length to min(device rss_max_key_size, NETDEV_RSS_KEY_LEN)
> > > and keep RSS enabled.
> > > 
> > > This keeps probe working on devices that advertise larger maximum key sizes
> > > while respecting the netdev RSS key buffer size limit.
> > > 
> > > Fixes: 3f7d9c1964fc ("virtio_net: Add hash_key_length check")
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Srujana Challa <schalla@marvell.com>
> > > Acked-by: Michael S. Tsirkin <mst@redhat.com>
> > > Link: https://urldefense.proofpoint.com/v2/url?u=https-
> > > 3A__patch.msgid.link_20260326142344.1171317-2D1-2Dschalla-
> > > 40marvell.com&d=DwIDAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=Fj4OoD5hcKFp
> > > ANhTWdwQzjT1Jpf7veC5263T47JVpnc&m=0XuKVXgk9_1LUIZHeqL0znGhAh
> > > x5KvAOLvrCl-orVeQSt__4o6Djr-79rwCl6KNp&s=cfQpAcZTTE7nTYku-
> > > MVkfip0xUJoBBw4ikqm9iEdgcc&e=
> > > Signed-off-by: Jakub Kicinski <kuba@kernel.org> [ changed clamp target from
> > > NETDEV_RSS_KEY_LEN to VIRTIO_NET_RSS_MAX_KEY_SIZE ]
> > > Signed-off-by: Sasha Levin <sashal@kernel.org>
> > > ---
> > >  drivers/net/virtio_net.c | 16 ++++++++--------
> > >  1 file changed, 8 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index
> > > 5c83983f0eb3f..5a31ccdae2e22 100644
> > > --- a/drivers/net/virtio_net.c
> > > +++ b/drivers/net/virtio_net.c
> > > @@ -6502,6 +6502,7 @@ static int virtnet_probe(struct virtio_device *vdev)
> > >  	struct virtnet_info *vi;
> > >  	u16 max_queue_pairs;
> > >  	int mtu = 0;
> > > +	u16 key_sz;
> > > 
> > >  	/* Find if host supports multiqueue/rss virtio_net device */
> > >  	max_queue_pairs = 1;
> > > @@ -6624,14 +6625,13 @@ static int virtnet_probe(struct virtio_device
> > > *vdev)
> > >  		goto free;
> > > 
> > >  	if (vi->has_rss || vi->has_rss_hash_report) {
> > > -		vi->rss_key_size =
> > > -			virtio_cread8(vdev, offsetof(struct virtio_net_config,
> > > rss_max_key_size));
> > > -		if (vi->rss_key_size > VIRTIO_NET_RSS_MAX_KEY_SIZE) {
> > > -			dev_err(&vdev->dev, "rss_max_key_size=%u exceeds
> > > the limit %u.\n",
> > > -				vi->rss_key_size,
> > > VIRTIO_NET_RSS_MAX_KEY_SIZE);
> > > -			err = -EINVAL;
> > > -			goto free;
> > > -		}
> > > +		key_sz = virtio_cread8(vdev, offsetof(struct virtio_net_config,
> > > +rss_max_key_size));
> > > +
> > > +		vi->rss_key_size = min_t(u16, key_sz,
> > > VIRTIO_NET_RSS_MAX_KEY_SIZE);
> > > +		if (key_sz > vi->rss_key_size)
> > > +			dev_warn(&vdev->dev,
> > > +				 "rss_max_key_size=%u exceeds driver limit
> > > %u, clamping\n",
> > > +				 key_sz, vi->rss_key_size);
> > > 
> > >  		vi->rss_hash_types_supported =
> > >  		    virtio_cread32(vdev, offsetof(struct virtio_net_config,
> > > supported_hash_types));
> > > --
> > > 2.53.0
> > 
> > We used `NETDEV_RSS_KEY_LEN` intentionally for clamping.
> > `rss_max_key_size` is the maximum supported by the device,
> > while `40` is a spec minimum, not a maximum.
> > Clamping to `VIRTIO_NET_RSS_MAX_KEY_SIZE` would unnecessarily
> > limit valid devices(for example devices advertising 48/52 bytes) and
> > could reintroduce the original issue.
> > 
> > Could you please share the reason for changing the clamp target
> > from `NETDEV_RSS_KEY_LEN` to `VIRTIO_NET_RSS_MAX_KEY_SIZE`?
> 
> Hi Srujana,
> 
> In the upstream commit, the key buffer was also enlarged from 40 to 52 bytes as
> part of the `TRAILING_OVERLAP` / `rss_trailer` refactoring:
> 
> -		u8 rss_hash_key_data[VIRTIO_NET_RSS_MAX_KEY_SIZE];
> +		u8 rss_hash_key_data[NETDEV_RSS_KEY_LEN];
> 
> That refactoring isn't present in 6.12, so the key buffer is still `u8
> key[VIRTIO_NET_RSS_MAX_KEY_SIZE]` (40 bytes). Clamping to `NETDEV_RSS_KEY_LEN`
> (52) here would allow writing up to 52 bytes into a 40-byte buffer, so the
> clamp target had to match the actual buffer size in this tree.
> 
> -- 
> Thanks,
> Sasha


problem is, the commit log now says one thing and the patch does
a completely different, and in fact opposite, thing.


Why not pick that dependency then?

-- 
MST