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

On Mon, Apr 28, 2025 at 10:39:59AM +0200, Paolo Abeni wrote:
> On 4/23/25 8:07 PM, Michael S. Tsirkin wrote:
> > 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:
> >>> 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.
> > 
> > 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.
> 
> I think that additionally the VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET command
> will need some clarification, as in the current text looks a bit
> inconsistent:
> 
> """
> // in Offloads State Configuration / Setting Offloads State:
> 
> #define VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO  46
> 
> // ...
> 
> The class VIRTIO_NET_CTRL_GUEST_OFFLOADS has one command:
> VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET applies the new offloads configuration.
> 
> le64 value passed as command data is a bitmask, bits set define
> offloads to be enabled, bits cleared - offloads to be disabled.
> 
> There is a corresponding device feature for each offload. Upon feature
> negotiation corresponding offload gets enabled to preserve backward
> compatibility
> """
> 
> The "corresponding device feature" has the same numerical value of the
> selected offloads, except for UDP tunnels related one (which are mapped
> to bits corresponding to reserved features).
> 
> It's unclear to me which should be the better way to address this
> inconsistency.
> 
> /P


Parav, what's your take here? Given your change broke
VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET, do you want to revert it?

-- 
MST