RE: [EXTERNAL] Re: [PATCH net-next,2/2] virtio_net: replace RSS key size max... | netdev

RE: [EXTERNAL] Re: [PATCH net-next,2/2] virtio_net: replace RSS key size max check with BUILD_BUG_ON

From: Srujana Challa <schalla@marvell.com>
Date: 2026-02-25 12:29:57
Also in: virtualization

quoted

ZjQcmQRYFpfptBannerEnd
On Wed, Feb 25, 2026 at 05:52:29PM +0800, Xuan Zhuo wrote:

quoted

On Wed, 25 Feb 2026 04:47:22 -0500, "Michael S. Tsirkin"

[off-list ref] wrote:

quoted

On Wed, Feb 25, 2026 at 05:36:06PM +0800, Xuan Zhuo wrote:

quoted

On Wed, 25 Feb 2026 04:33:57 -0500, "Michael S. Tsirkin"

[off-list ref] wrote:

quoted

On Wed, Feb 25, 2026 at 05:30:33PM +0800, Xuan Zhuo wrote:

quoted

On Wed, 25 Feb 2026 04:24:14 -0500, "Michael S. Tsirkin"

[off-list ref] wrote:

quoted

On Wed, Feb 25, 2026 at 05:11:42PM +0800, Xuan Zhuo wrote:

quoted

On Tue, 24 Feb 2026 12:28:50 +0530, Srujana Challa

[off-list ref] wrote:

quoted

Since NETDEV_RSS_KEY_LEN was increased to 256 in
net-next, use BUILD_BUG_ON to enforce the limit at
compile time and remove the redundant runtime max check.

Signed-off-by: Srujana Challa <schalla@marvell.com>
---
 drivers/net/virtio_net.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/net/virtio_net.c

b/drivers/net/virtio_net.c index
eeefe8abc122..768ad5523dfa 100644

--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c

@@ -6639,13 +6639,7 @@ static int

virtnet_validate(struct

virtio_device *vdev)

quoted

quoted
quoted
quoted
quoted
quoted
quoted
quoted
 			__virtio_clear_bit(vdev,

VIRTIO_NET_F_RSS);

quoted

quoted
quoted
quoted
quoted
quoted
quoted
quoted
quoted
quoted
 			__virtio_clear_bit(vdev,
VIRTIO_NET_F_HASH_REPORT);
quoted
quoted
quoted
quoted
quoted
quoted
quoted
quoted
 		}
-		if (key_sz > NETDEV_RSS_KEY_LEN) {
-			dev_warn(&vdev->dev,
-				 "rss_max_key_size=%u

exceeds driver

quoted

quoted
limit %u, disabling RSS\n",
quoted
quoted
quoted
quoted
quoted
quoted
quoted
quoted
-				 key_sz,

NETDEV_RSS_KEY_LEN);

quoted

quoted
quoted
quoted
quoted
quoted
quoted
quoted
quoted
quoted
-			__virtio_clear_bit(vdev,

VIRTIO_NET_F_RSS);

quoted

quoted
quoted
quoted
quoted
quoted
quoted
quoted
-			__virtio_clear_bit(vdev,

VIRTIO_NET_F_HASH_REPORT);

quoted

quoted
quoted
quoted
quoted
quoted
quoted
quoted
-		}
+		BUILD_BUG_ON(type_max(key_sz) >=

NETDEV_RSS_KEY_LEN);

quoted

Do we really need this check?

If I understand correctly, the intention is to cap
key_sz at 256. However, since key_sz is of type u8,
its maximum value is inherently 255, making this check
redundant. This is not only limited by this kernel
code, the virtio-net spec defines

this.

quoted

That's why it's BUILD_BUG_ON. It checks it has the right type.

We never *need* BUILD_BUG_ON by definition, what this
does is document the assumption.

quoted

Moreover, if NETDEV_RSS_KEY_LEN is ever reduced to a
value smaller than 256 in the future, this check would
no longer enforce

the intended limit correctly.

quoted

then it would fail build.

So, does this mean we don't need to account for the case
where NETDEV_RSS_KEY_LEN is 128, but the key_sz reported
by the device is

64?

quoted

quoted
Thanks.
yes.

Why?

If NETDEV_RSS_KEY_LEN is 128 but the device reports a key_sz
of 64, does this violate the spec?

not the value of key_sz. If type of key_sz


i actually do not understand the question. this is not what
BUILD_BUG_ON checks.

So this is the issue. Originally, the code checked whether the
value of key_sz was less than NETDEV_RSS_KEY_LEN. However,
switching to a type_max check means it no longer covers the scenario I

described.

quoted

Therefore, I think this is unreasonable.

Thanks


patch 1 is unreasonable i think.

Patch 1 is targeted for net, addressing an issue where
VIRTIO_NET_RSS_MAX_KEY_SIZE is fixed at 40, which is only the minimum
required by the spec. This led to virtio‑net probe failures when
devices reported an RSS key size greater than 40. However, the driver
also cannot handle keys larger than NETDEV_RSS_KEY_LEN (previously 52)
due to the BUG_ON(len > sizeof(netdev_rss_key)) in netdev_rss_key_fill. To

resolve both issues, VIRTIO_NET_RSS_MAX_KEY_SIZE has been replaced with
NETDEV_RSS_KEY_LEN.

but where would driver *get* keys larger than sizeof(netdev_rss_key), even if
device supports more.

In virtio‑net, we rely on netdev_rss_key_fill() during virtnet_init_default_rss() to populate the RSS hash key,
which uses device supported length.
The code path is:
static void virtnet_init_default_rss(struct virtnet_info *vi)
{
        vi->rss_hdr->hash_types = cpu_to_le32(vi->rss_hash_types_supported);
        vi->rss_hash_types_saved = vi->rss_hash_types_supported;
        vi->rss_hdr->indirection_table_mask = vi->rss_indir_table_size
                                                ? cpu_to_le16(vi->rss_indir_table_size - 1) : 0;
        vi->rss_hdr->unclassified_queue = 0;

        virtnet_rss_update_by_qpairs(vi, vi->curr_queue_pairs);

        vi->rss_trailer.hash_key_length = vi->rss_key_size;

        netdev_rss_key_fill(vi->rss_hash_key_data, vi->rss_key_size);
}

quoted

Patch 2 is added as per your suggestion to address following warning
in net-next after  NETDEV_RSS_KEY_LEN was increased to 256.
+../drivers/net/virtio_net.c:6642:14: warning: result of comparison of

constant 256 with expression of type 'u8' (aka 'unsigned char') is always false
[-Wtautological-constant-out-of-range-compare]

quoted

+ 6642 |                 if (key_sz > NETDEV_RSS_KEY_LEN) {
+      |                     ~~~~~~ ^ ~~~~~~~~~~~~~~~~~~
+1 warning generated.
Sorry, I forgot the cover page.

Thanks!

quoted

which is why patchsets should have a cover letter btw so one can
reply to just patch 1.

quoted

the code makes assumptions but it documents them and not
just documents them, build will fail if they are violated.

About this, I am ok.

Thanks.

quoted

Moreover, you should add a cover letter.

Thanks.

quoted

 	}

 	return 0;
--
2.25.1

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help