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:13:45
Also in: virtualization

-----Original Message-----
From: Michael S. Tsirkin <mst@redhat.com>
Sent: Wednesday, February 25, 2026 3:35 PM
To: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
Cc: Srujana Challa <schalla@marvell.com>; pabeni@redhat.com;
jasowang@redhat.com; eperezma@redhat.com; davem@davemloft.net;
edumazet@google.com; kuba@kernel.org; Nithin Kumar Dabilpuram
[off-list ref]; Shiva Shankar Kommula
[off-list ref]; netdev@vger.kernel.org;
virtualization@lists.linux.dev
Subject: [EXTERNAL] Re: [PATCH net-next,2/2] virtio_net: replace RSS key size
max check with BUILD_BUG_ON

On Wed, Feb 25, 2026 at 05: 52: 29PM +0800, Xuan Zhuo wrote: > On Wed,
25 Feb 2026 04: 47: 22 -0500, "Michael S. Tsirkin" <mst@ redhat. com> wrote:

quoted

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

Wed, ZjQcmQRYFpfptBannerStart Prioritize security for external emails:
Confirm sender and content safety before clicking links or opening
attachments <https://us-phishalarm-
ewt.proofpoint.com/EWT/v1/CRVmXkqW!tc3Z1f8UYnWatK-
8Gd3a7iLAAhjzwNasdLDRrDCP-M_xUEAECz9j2fwxWlWq3EugFfzY8uKAhQ-
Q2XIdQs89ldnlzrpsl9A$>
Report Suspicious

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);
 			__virtio_clear_bit(vdev,

VIRTIO_NET_F_HASH_REPORT);

quoted

 		}
-		if (key_sz > NETDEV_RSS_KEY_LEN) {
-			dev_warn(&vdev->dev,
-				 "rss_max_key_size=%u exceeds driver

limit %u, disabling RSS\n",

quoted

-				 key_sz, NETDEV_RSS_KEY_LEN);
-			__virtio_clear_bit(vdev, VIRTIO_NET_F_RSS);
-			__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.
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.

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]
+ 6642 |                 if (key_sz > NETDEV_RSS_KEY_LEN) {
+      |                     ~~~~~~ ^ ~~~~~~~~~~~~~~~~~~
+1 warning generated.
Sorry, I forgot the cover page.

Thanks!

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