Re: [PATCH 2/2] net/packet: send and receive pkt with given vnet_hdr_sz

[Willem de Bruijn <willemdebruijn.kernel@gmail.com>]

> >>> You're right. This is configured from userspace before the FD is passed
> >>> to vhost-net, so indeed this will require packet socket UAPI support.
> >>
> >> Thanks for quick reply. We will go with adding an extra UAPI here then.
> >>
> >>
> >> Another discussion for designing this UAPI is, whether it will be better to
> >> support setting only vnet header size, just like what TAP does in its ioctl,
> >> or to support setting a virtio feature bit-map.
> >>
> >>
> >> UAPI setting only vnet header size
> >>
> >> Pros:
> >>
> >> 1. It aligns with how other virito backend devices communicate with
> >> virtio-user
> >>
> >> 2. We can use the holes in struct packet_socket (net/packet/internal.h:120)
> >> to record the extra information since the size info only takes 8 bits.
> >>
> >> Cons:
> >>
> >> 1. It may have more information that virtio-user needs to communicate with
> >> packet socket in the future and needs to add more UAPI supports here.
> >>
> >> To Michael: Is there any other information that backend device needs and
> >> will be given from virtio-user?
> >
> > Yes e.g. I already mentioned virtio 1.0 wrt LE versus native endian
> > format.
> >
> >
> >> UAPI setting a virtio feature bit-map
> >>
> >> Pros:
> >>
> >> 1. It is more general and may reduce future UAPI changes.
> >>
> >> Cons:
> >>
> >> 1. A virtio feature bit-map needs 64 bits, which needs to add an extra field
> >> in packet_sock struct

Accepting a bitmap in the ABI does not have to imply storing a bitmap.

> >>
> >> 2. Virtio-user needs to aware that using packet socket as backend supports
> >> different approach to negotiate the vnet header size.
> >>
> >>
> >> We really appreciate any suggestion or discussion on this design choice of
> >> UAPI.
> > In the end it's ok with just size too, you just probably shouldn't say
> > you support VERSION_1 if you are not passing that bit.
> >
> 
> Sorry for the confusion here that we mentioned VERSION_1 in the commit 
> log. We actually just attended to give an example of what features that 
> may need 12-byte vnet header. We will remove it from the commit log in 
> patch v2 to avoid confusion here. Thanks a lot for your suggestions.

The question hinges on which features are expected to have to be
supported in the future. So far we have

- extra num_buffers field
- little endian (V1)

Given the rate of change in the spec, I don't think this should
be over designed. If V1 is not planned to be supported, just
configure header length. If it is, then perhaps instead a feature
bitmap.