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

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

On Wed, Feb 25, 2026 at 12:13:01PM +0000, Srujana Challa wrote:
> 
> 
> > -----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
> > <ndabilpuram@marvell.com>; Shiva Shankar Kommula
> > <kshankar@marvell.com>; 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:
> > > > 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"
> > <mst@redhat.com> 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"
> > <mst@redhat.com> 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"
> > <mst@redhat.com> 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
> > <schalla@marvell.com> 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 <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)
> > > > > > > > > >  			__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.

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



> 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
> > > > > > > > > >
> > > > > > > >
> > > > > >
> > > >
>