On Thu, Feb 5, 2026 at 2:44 AM Michael S. Tsirkin <[email protected]> wrote: > > On Wed, Feb 04, 2026 at 12:55:59PM -0500, Willem de Bruijn wrote: > > Jason Wang wrote: > > > On Mon, Feb 2, 2026 at 5:06 AM Willem de Bruijn > > > <[email protected]> wrote: > > > > > > > > Steffen Trumtrar wrote: > > > > > Add optional hardware rx timestamp offload for virtio-net. > > > > > > > > > > Introduce virtio feature VIRTIO_NET_F_TSTAMP. If negotiated, the > > > > > virtio-net header is expanded with room for a timestamp. > > > > > > > > > > To get and set the hwtstamp the functions ndo_hwtstamp_set/get need > > > > > to be implemented. This allows filtering the packets and only time > > > > > stamp > > > > > the packets where the filter matches. This way, the timestamping can > > > > > be en/disabled at runtime. > > > > > > > > > > Tested: > > > > > guest: ./timestamping eth0 \ > > > > > SOF_TIMESTAMPING_RAW_HARDWARE \ > > > > > SOF_TIMESTAMPING_RX_HARDWARE > > > > > host: nc -4 -u 192.168.1.1 319 > > > > > > > > > > Signed-off-by: Steffen Trumtrar <[email protected]> > > > > > > > > > > -- > > > > > Changes to last version: > > > > > - rework series to use flow filters > > > > > - add new struct virtio_net_hdr_v1_hash_tunnel_ts > > > > > - original work done by: Willem de Bruijn <[email protected]> > > > > > --- > > > > > drivers/net/virtio_net.c | 136 > > > > > ++++++++++++++++++++++++++++++++++++---- > > > > > include/uapi/linux/virtio_net.h | 9 +++ > > > > > 2 files changed, 133 insertions(+), 12 deletions(-) > > > > > > > > > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > > > > > index 1bb3aeca66c6e..4e8d9b20c1b34 100644 > > > > > --- a/drivers/net/virtio_net.c > > > > > +++ b/drivers/net/virtio_net.c > > > > > @@ -429,6 +429,9 @@ struct virtnet_info { > > > > > struct virtio_net_rss_config_trailer rss_trailer; > > > > > u8 rss_hash_key_data[VIRTIO_NET_RSS_MAX_KEY_SIZE]; > > > > > > > > > > + /* Device passes time stamps from the driver */ > > > > > + bool has_tstamp; > > > > > + > > > > > /* Has control virtqueue */ > > > > > bool has_cvq; > > > > > > > > > > @@ -475,6 +478,8 @@ struct virtnet_info { > > > > > > > > > > struct control_buf *ctrl; > > > > > > > > > > + struct kernel_hwtstamp_config tstamp_config; > > > > > + > > > > > /* Ethtool settings */ > > > > > u8 duplex; > > > > > u32 speed; > > > > > @@ -511,6 +516,7 @@ struct virtio_net_common_hdr { > > > > > struct virtio_net_hdr_mrg_rxbuf mrg_hdr; > > > > > struct virtio_net_hdr_v1_hash hash_v1_hdr; > > > > > struct virtio_net_hdr_v1_hash_tunnel tnl_hdr; > > > > > + struct virtio_net_hdr_v1_hash_tunnel_ts ts_hdr; > > > > > > > > Jason, Michael: creating a new struct for every field is not very > > > > elegant. Is it time to find a more forward looking approach to > > > > expanding with new fields? Like a TLV, or how netlink structs like > > > > tcp_info are extended with support for legacy users that only use > > > > a truncated struct? > > > > > > I fully agree, we need somebody to work on this. > > > > Great. I'll take a look when I have some time. > > > > The current approach infers struct virtio_net_hdr_$VARIANT size from > > negotiated features: > > > > - virtio_net: virtio_has_feature > > - vhost_net: virtio_features_test_bit > > - tun: tun_vnet_parse_size > > > > Tuntap however also explicitly negotiates length with TUNSETVNETHDRSZ. > > > > If we create a struct virtio_net_hdr_ext that can be continuously > > extended going forward, the host and guest will have to negotiate the > > length they both understand, and agree to the min of the two. > > > > We could use that to implicitly negotiate supported features. E.g., > > if len is sizeof(virtio_net_hdr_v1_hash_tunnel) then these features > > are supported: > > > > VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO > > VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO > > VIRTIO_NET_F_HASH_REPORT > > etc. > > > > But that would mean that all features up to the length have to be > > supported, no gaps. I think that is probably not preferable? So then > > they still need to negotiate features, as well as length. Does virtio > > have a mechanism to negotiate not boolean feature enable/disable but > > integers such as struct length?
I think not. > > > > Else the change will probably be simpler: keep existing negotiation, > > only change the code to once define a struct and keep adding fields. > > Rather than defining new structs with each feature and updating many > > function arguments and variables to switch to the new struct type. > > Right I am not sure why we keep doing this. Let's add a VLA at > the end of the struct and declare fields can be added at will. > > > > Again, for that see as example struct tcp_info, which is UAPI, but has > > been extended many times. > > Yes. Thanks > -- > MST >
