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

On Wed, Apr 23, 2025 at 09:29:11AM -0700, Daniel Verkamp wrote:
> 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


Yes you would use VHOST_USER_SET_PROTOCOL_FEATURES to make VHOST_USER_GET_FEATURES
return two u64s, or even a new message returning an array.