> -----Original Message-----
> From: Michael S. Tsirkin <[email protected]>
> Sent: Wednesday, February 25, 2026 3:35 PM
> To: Xuan Zhuo <[email protected]>
> Cc: Srujana Challa <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; Nithin Kumar Dabilpuram
> <[email protected]>; Shiva Shankar Kommula
> <[email protected]>; [email protected];
> [email protected]
> 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:
> > > 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:
> > On Wed, 25 Feb 2026 04:47:22 -0500, "Michael S. Tsirkin"
> <[email protected]> wrote:
> > > On Wed, Feb 25, 2026 at 05:36:06PM +0800, Xuan Zhuo wrote:
> > > > On Wed, 25 Feb 2026 04:33:57 -0500, "Michael S. Tsirkin"
> <[email protected]> wrote:
> > > > > On Wed, Feb 25, 2026 at 05:30:33PM +0800, Xuan Zhuo wrote:
> > > > > > On Wed, 25 Feb 2026 04:24:14 -0500, "Michael S. Tsirkin"
> <[email protected]> wrote:
> > > > > > > On Wed, Feb 25, 2026 at 05:11:42PM +0800, Xuan Zhuo wrote:
> > > > > > > > On Tue, 24 Feb 2026 12:28:50 +0530, Srujana Challa
> <[email protected]> wrote:
> > > > > > > > > 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 <[email protected]>
> > > > > > > > > ---
> > > > > > > > > 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)
> > > > > > > > > __virtio_clear_bit(vdev,
> > > > > > > > > VIRTIO_NET_F_RSS);
> > > > > > > > > __virtio_clear_bit(vdev,
> VIRTIO_NET_F_HASH_REPORT);
> > > > > > > > > }
> > > > > > > > > - if (key_sz > NETDEV_RSS_KEY_LEN) {
> > > > > > > > > - dev_warn(&vdev->dev,
> > > > > > > > > - "rss_max_key_size=%u exceeds
> > > > > > > > > driver
> limit %u, disabling RSS\n",
> > > > > > > > > - key_sz, NETDEV_RSS_KEY_LEN);
> > > > > > > > > - __virtio_clear_bit(vdev,
> > > > > > > > > VIRTIO_NET_F_RSS);
> > > > > > > > > - __virtio_clear_bit(vdev,
> VIRTIO_NET_F_HASH_REPORT);
> > > > > > > > > - }
> > > > > > > > > + BUILD_BUG_ON(type_max(key_sz) >=
> NETDEV_RSS_KEY_LEN);
> > > > > > > >
> > > > > > > > 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.
> > > > > > >
> > > > > > > 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.
> > > > > > >
> > > > > > >
> > > > > > > > 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.
> > > > > > >
> > > > > > > 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?
> > > > > >
> > > > > > 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.
>
>
> > >
> > > > > 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.
> > > >
> > > >
> > > > >
> > > > >
> > > > > > >
> > > > > > > >
> > > > > > > > Moreover, you should add a cover letter.
> > > > > > > >
> > > > > > > > Thanks.
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > > }
> > > > > > > > >
> > > > > > > > > return 0;
> > > > > > > > > --
> > > > > > > > > 2.25.1
> > > > > > > > >
> > > > > > >
> > > > >
> > >
