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? 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. Again, for that see as example struct tcp_info, which is UAPI, but has been extended many times.
