Re: [PATCH v1] virtio-net: Fix to avoid using reserved feature bits

On Tue, Apr 22, 2025 at 10:46 PM Michael S. Tsirkin <[email protected]> wrote:
>
> On Tue, Apr 22, 2025 at 07:49:41PM +0200, Paolo Abeni wrote:
> > On 1/26/25 7:20 AM, Parav Pandit wrote:
> > > @@ -136,6 +124,18 @@ \subsection{Feature bits}\label{sec:Device Types / Network Device / Feature bits
> > >  \item[VIRTIO_NET_F_SPEED_DUPLEX(63)] Device reports speed and duplex.
> > >
> > >  \item[VIRTIO_NET_F_RSS_CONTEXT(64)] Device supports multiple RSS contexts.
> > > +
> > > +\item[VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO (65)] Driver can receive GSO packets
> > > +  carried by a UDP tunnel.
> > > +
> > > +\item[VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO_CSUM (66)] Driver handles packets
> > > +  carried by a UDP tunnel with partial csum for the outer header.
> > > +
> > > +\item[VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO (67)] Device can receive GSO packets
> > > +  carried by a UDP tunnel.
> > > +
> > > +\item[VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO_CSUM (68)] Device handles packets
> > > +  carried by a UDP tunnel with partial csum for the outer header.
> > >  \end{description}
> >
> > I'm finally trying to finalize the implementation and I must admit I
> > underlooked the implication of using features bit >= 64.
> >
> > AFAICS all the virtio drivers in the Linux kernel and qemu assume that
> > the features can be represented with a single u64.
> >
> > Supporting the above requires major reworks in both user-space and
> > kernel space. At least on the kernel side a similar rework (extending
> > the _netdev_ features field) has proven to be complex enough to defeat
> > any implementation attempts for years and in the end it was dismissed in
> > favor of lower bits reuse.
> >
> > I'm wondering if we could reconsider using reserved bits here?
> >
> > Thanks!
> >
> > Paolo
>
> :(
>
> It's been there for 2 months now.
>
> And if you want to avoid using all bits > 63 then it's even more of
> a mess, you need to move VIRTIO_NET_F_RSS_CONTEXT which has been
> there for a year and a half.
>
> I'm afraid we'll have to bite the bullet.

One other issue with bits > 63 is that the vhost-user protocol
VHOST_USER_GET_FEATURES and VHOST_USER_SET_FEATURES messages use u64
to represent the features, so vhost-user-net devices can't query or
enable these features. vhost-user is outside the scope of the virtio
spec, though, and I think it's reasonable to extend the protocol to
enable high feature bits rather than avoiding them forever.

Thanks,
-- Daniel