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