[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <willemdebruijn.kernel.302a7f5489992@gmail.com>
Date: Wed, 04 Feb 2026 12:55:59 -0500
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: Jason Wang <jasowang@...hat.com>,
Willem de Bruijn <willemdebruijn.kernel@...il.com>
Cc: Steffen Trumtrar <s.trumtrar@...gutronix.de>,
"Michael S. Tsirkin" <mst@...hat.com>,
Xuan Zhuo <xuanzhuo@...ux.alibaba.com>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>,
Richard Cochran <richardcochran@...il.com>,
Andrew Lunn <andrew+netdev@...n.ch>,
Eugenio Pérez <eperezma@...hat.com>,
virtualization@...ts.linux.dev,
netdev@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH RFC v2 2/2] virtio-net: support receive timestamp
Jason Wang wrote:
> On Mon, Feb 2, 2026 at 5:06 AM Willem de Bruijn
> <willemdebruijn.kernel@...il.com> 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 <s.trumtrar@...gutronix.de>
> > >
> > > --
> > > 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 <willemb@...gle.com>
> > > ---
> > > 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.
Powered by blists - more mailing lists