On Wed, Feb 25, 2026 at 12:13:01PM +0000, Srujana Challa wrote:
>
>
> > -----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.
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
> > > > > > > > > >
> > > > > > > >
> > > > > >
> > > >
>
