On Sun, Feb 01, 2026 at 04:05:54PM -0500, Willem de Bruijn 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 certainly wouldn't mind, though I suspect tlv is too complex as
hardware implementations can't efficiently follow linked lists. I'll
try to ping some hardware designers for what works well for offloads.
> > };
> > };
> >
> > @@ -682,6 +688,13 @@ skb_vnet_common_hdr(struct sk_buff *skb)
> > return (struct virtio_net_common_hdr *)skb->cb;
> > }
> >
> > +static inline struct virtio_net_hdr_v1_hash_tunnel_ts
> > *skb_vnet_hdr_ts(struct sk_buff *skb)
> > +{
> > + BUILD_BUG_ON(sizeof(struct virtio_net_hdr_v1_hash_tunnel_ts) >
> > sizeof(skb->cb));
> > +
> > + return (void *)skb->cb;
> > +}
> > +
> > /*
> > * private is used to chain pages for big packets, put the whole
> > * most recent used list in the beginning for reuse
> > @@ -2560,6 +2573,15 @@ virtio_net_hash_value(const struct
> > virtio_net_hdr_v1_hash *hdr_hash)
> > (__le16_to_cpu(hdr_hash->hash_value_hi) << 16);
> > }
> >
> > +static inline u64
> > +virtio_net_tstamp_value(const struct virtio_net_hdr_v1_hash_tunnel_ts
> > *hdr_hash_ts)
> > +{
> > + return (u64)__le16_to_cpu(hdr_hash_ts->tstamp_0) |
> > + ((u64)__le16_to_cpu(hdr_hash_ts->tstamp_1) << 16) |
> > + ((u64)__le16_to_cpu(hdr_hash_ts->tstamp_2) << 32) |
> > + ((u64)__le16_to_cpu(hdr_hash_ts->tstamp_3) << 48);
> > +}
> > +
> > static void virtio_skb_set_hash(const struct virtio_net_hdr_v1_hash
> > *hdr_hash,
> > struct sk_buff *skb)
> > {
> > @@ -2589,6 +2611,18 @@ static void virtio_skb_set_hash(const struct
> > virtio_net_hdr_v1_hash *hdr_hash,
> > skb_set_hash(skb, virtio_net_hash_value(hdr_hash), rss_hash_type);
> > }
> >
> > +static inline void virtnet_record_rx_tstamp(const struct virtnet_info *vi,
> > + struct sk_buff *skb)
> > +{
> > + struct skb_shared_hwtstamps *shhwtstamps = skb_hwtstamps(skb);
> > + const struct virtio_net_hdr_v1_hash_tunnel_ts *h = skb_vnet_hdr_ts(skb);
> > + u64 ts;
> > +
> > + ts = virtio_net_tstamp_value(h);
> > + memset(shhwtstamps, 0, sizeof(struct skb_shared_hwtstamps));
> > + shhwtstamps->hwtstamp = ns_to_ktime(ts);
> > +}
> > +
> > static void virtnet_receive_done(struct virtnet_info *vi, struct
> > receive_queue *rq,
> > struct sk_buff *skb, u8 flags)
> > {
> > @@ -2617,6 +2651,8 @@ static void virtnet_receive_done(struct virtnet_info
> > *vi, struct receive_queue *
> > goto frame_err;
> > }
> >
> > + if (vi->has_tstamp && vi->tstamp_config.rx_filter !=
> > HWTSTAMP_FILTER_NONE)
> > + virtnet_record_rx_tstamp(vi, skb);
> > skb_record_rx_queue(skb, vq2rxq(rq->vq));
> > skb->protocol = eth_type_trans(skb, dev);
> > pr_debug("Receiving skb proto 0x%04x len %i type %i\n",
> > @@ -3321,7 +3357,7 @@ static int xmit_skb(struct send_queue *sq, struct
> > sk_buff *skb, bool orphan)
> > {
> > const unsigned char *dest = ((struct ethhdr *)skb->data)->h_dest;
> > struct virtnet_info *vi = sq->vq->vdev->priv;
> > - struct virtio_net_hdr_v1_hash_tunnel *hdr;
> > + struct virtio_net_hdr_v1_hash_tunnel_ts *hdr;
> > int num_sg;
> > unsigned hdr_len = vi->hdr_len;
> > bool can_push;
> > @@ -3329,8 +3365,8 @@ static int xmit_skb(struct send_queue *sq, struct
> > sk_buff *skb, bool orphan)
> > pr_debug("%s: xmit %p %pM\n", vi->dev->name, skb, dest);
> >
> > /* Make sure it's safe to cast between formats */
> > - BUILD_BUG_ON(__alignof__(*hdr) != __alignof__(hdr->hash_hdr));
> > - BUILD_BUG_ON(__alignof__(*hdr) != __alignof__(hdr->hash_hdr.hdr));
> > + BUILD_BUG_ON(__alignof__(*hdr) != __alignof__(hdr->tnl.hash_hdr));
> > + BUILD_BUG_ON(__alignof__(*hdr) != __alignof__(hdr->tnl.hash_hdr.hdr));
> >
> > can_push = vi->any_header_sg &&
> > !((unsigned long)skb->data & (__alignof__(*hdr) - 1)) &&
> > @@ -3338,18 +3374,18 @@ static int xmit_skb(struct send_queue *sq, struct
> > sk_buff *skb, bool orphan)
> > /* Even if we can, don't push here yet as this would skew
> > * csum_start offset below. */
> > if (can_push)
> > - hdr = (struct virtio_net_hdr_v1_hash_tunnel *)(skb->data -
> > - hdr_len);
> > + hdr = (struct virtio_net_hdr_v1_hash_tunnel_ts *)(skb->data -
> > + hdr_len);
> > else
> > - hdr = &skb_vnet_common_hdr(skb)->tnl_hdr;
> > + hdr = &skb_vnet_common_hdr(skb)->ts_hdr;
> >
> > - if (virtio_net_hdr_tnl_from_skb(skb, hdr, vi->tx_tnl,
> > + if (virtio_net_hdr_tnl_from_skb(skb, &hdr->tnl, vi->tx_tnl,
> > virtio_is_little_endian(vi->vdev), 0,
> > false))
> > return -EPROTO;
> >
> > if (vi->mergeable_rx_bufs)
> > - hdr->hash_hdr.hdr.num_buffers = 0;
> > + hdr->tnl.hash_hdr.hdr.num_buffers = 0;
> >
> > sg_init_table(sq->sg, skb_shinfo(skb)->nr_frags + (can_push ? 1 : 2));
> > if (can_push) {
> > @@ -5563,6 +5599,22 @@ static int virtnet_get_per_queue_coalesce(struct
> > net_device *dev,
> > return 0;
> > }
> >
> > +static int virtnet_get_ts_info(struct net_device *dev,
> > + struct kernel_ethtool_ts_info *info)
> > +{
> > + /* setup default software timestamp */
> > + ethtool_op_get_ts_info(dev, info);
> > +
> > + info->rx_filters = (BIT(HWTSTAMP_FILTER_NONE) |
> > + BIT(HWTSTAMP_FILTER_PTP_V1_L4_SYNC) |
> > + BIT(HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ) |
> > + BIT(HWTSTAMP_FILTER_ALL));
> > +
> > + info->tx_types = HWTSTAMP_TX_OFF;
> > +
> > + return 0;
> > +}
> > +
> > static void virtnet_init_settings(struct net_device *dev)
> > {
> > struct virtnet_info *vi = netdev_priv(dev);
> > @@ -5658,7 +5710,7 @@ static const struct ethtool_ops virtnet_ethtool_ops =
> > {
> > .get_ethtool_stats = virtnet_get_ethtool_stats,
> > .set_channels = virtnet_set_channels,
> > .get_channels = virtnet_get_channels,
> > - .get_ts_info = ethtool_op_get_ts_info,
> > + .get_ts_info = virtnet_get_ts_info,
> > .get_link_ksettings = virtnet_get_link_ksettings,
> > .set_link_ksettings = virtnet_set_link_ksettings,
> > .set_coalesce = virtnet_set_coalesce,
> > @@ -6242,6 +6294,58 @@ static void virtnet_tx_timeout(struct net_device
> > *dev, unsigned int txqueue)
> > jiffies_to_usecs(jiffies - READ_ONCE(txq->trans_start)));
> > }
> >
> > +static int virtnet_hwtstamp_get(struct net_device *dev,
> > + struct kernel_hwtstamp_config *tstamp_config)
> > +{
> > + struct virtnet_info *vi = netdev_priv(dev);
> > +
> > + if (!netif_running(dev))
> > + return -EINVAL;
> > +
> > + *tstamp_config = vi->tstamp_config;
> > +
> > + return 0;
> > +}
> > +
> > +static int virtnet_hwtstamp_set(struct net_device *dev,
> > + struct kernel_hwtstamp_config *tstamp_config,
> > + struct netlink_ext_ack *extack)
> > +{
> > + struct virtnet_info *vi = netdev_priv(dev);
> > +
> > + if (!netif_running(dev))
> > + return -EINVAL;
> > +
> > + switch (tstamp_config->rx_filter) {
> > + case HWTSTAMP_FILTER_PTP_V1_L4_SYNC:
> > + case HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ:
> > + break;
> > + case HWTSTAMP_FILTER_PTP_V2_EVENT:
> > + case HWTSTAMP_FILTER_PTP_V2_L2_EVENT:
> > + case HWTSTAMP_FILTER_PTP_V2_L4_EVENT:
> > + case HWTSTAMP_FILTER_PTP_V2_SYNC:
> > + case HWTSTAMP_FILTER_PTP_V2_L2_SYNC:
> > + case HWTSTAMP_FILTER_PTP_V2_L4_SYNC:
> > + case HWTSTAMP_FILTER_PTP_V2_DELAY_REQ:
> > + case HWTSTAMP_FILTER_PTP_V2_L2_DELAY_REQ:
> > + case HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ:
> > + tstamp_config->rx_filter = HWTSTAMP_FILTER_PTP_V2_EVENT;
> > + break;
> > + case HWTSTAMP_FILTER_NONE:
> > + break;
> > + case HWTSTAMP_FILTER_ALL:
> > + tstamp_config->rx_filter = HWTSTAMP_FILTER_ALL;
> > + break;
>
> It's fine to implement filters, but also fine to only support ALL or
> NONE for simplicity.
>
> In the end it probably depends on what the underlying physical device
> supports.
>
> > + default:
> > + tstamp_config->rx_filter = HWTSTAMP_FILTER_ALL;
> > + return -ERANGE;
> > + }
> > +
> > + vi->tstamp_config = *tstamp_config;
> > +
> > + return 0;
> > +}
> > +
> > static int virtnet_init_irq_moder(struct virtnet_info *vi)
> > {
> > u8 profile_flags = 0, coal_flags = 0;
> > @@ -6289,6 +6393,8 @@ static const struct net_device_ops virtnet_netdev = {
> > .ndo_get_phys_port_name = virtnet_get_phys_port_name,
> > .ndo_set_features = virtnet_set_features,
> > .ndo_tx_timeout = virtnet_tx_timeout,
> > + .ndo_hwtstamp_set = virtnet_hwtstamp_set,
> > + .ndo_hwtstamp_get = virtnet_hwtstamp_get,
> > };
> >
> > static void virtnet_config_changed_work(struct work_struct *work)
> > @@ -6911,6 +7017,9 @@ static int virtnet_probe(struct virtio_device *vdev)
> > if (virtio_has_feature(vdev, VIRTIO_NET_F_HASH_REPORT))
> > vi->has_rss_hash_report = true;
> >
> > + if (virtio_has_feature(vdev, VIRTIO_NET_F_TSTAMP))
> > + vi->has_tstamp = true;
> > +
> > if (virtio_has_feature(vdev, VIRTIO_NET_F_RSS)) {
> > vi->has_rss = true;
> >
> > @@ -6945,8 +7054,10 @@ static int virtnet_probe(struct virtio_device *vdev)
> > dev->xdp_metadata_ops = &virtnet_xdp_metadata_ops;
> > }
> >
> > - if (virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO) ||
> > - virtio_has_feature(vdev, VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO))
> > + if (vi->has_tstamp)
> > + vi->hdr_len = sizeof(struct virtio_net_hdr_v1_hash_tunnel_ts);
> > + else if (virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO) ||
> > + virtio_has_feature(vdev, VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO))
> > vi->hdr_len = sizeof(struct virtio_net_hdr_v1_hash_tunnel);
> > else if (vi->has_rss_hash_report)
> > vi->hdr_len = sizeof(struct virtio_net_hdr_v1_hash);
> > @@ -7269,7 +7380,8 @@ static struct virtio_device_id id_table[] = {
> > VIRTIO_NET_F_SPEED_DUPLEX, VIRTIO_NET_F_STANDBY, \
> > VIRTIO_NET_F_RSS, VIRTIO_NET_F_HASH_REPORT, VIRTIO_NET_F_NOTF_COAL, \
> > VIRTIO_NET_F_VQ_NOTF_COAL, \
> > - VIRTIO_NET_F_GUEST_HDRLEN, VIRTIO_NET_F_DEVICE_STATS
> > + VIRTIO_NET_F_GUEST_HDRLEN, VIRTIO_NET_F_DEVICE_STATS, \
> > + VIRTIO_NET_F_TSTAMP
> >
> > static unsigned int features[] = {
> > VIRTNET_FEATURES,
> > diff --git a/include/uapi/linux/virtio_net.h
> > b/include/uapi/linux/virtio_net.h
> > index 1db45b01532b5..9f967575956b8 100644
> > --- a/include/uapi/linux/virtio_net.h
> > +++ b/include/uapi/linux/virtio_net.h
> > @@ -56,6 +56,7 @@
> > #define VIRTIO_NET_F_MQ 22 /* Device supports Receive Flow
> > * Steering */
> > #define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */
> > +#define VIRTIO_NET_F_TSTAMP 49 /* Device sends TAI receive
> > time */
> > #define VIRTIO_NET_F_DEVICE_STATS 50 /* Device can provide
> > device-level statistics. */
> > #define VIRTIO_NET_F_VQ_NOTF_COAL 52 /* Device supports virtqueue
> > notification coalescing */
> > #define VIRTIO_NET_F_NOTF_COAL 53 /* Device supports
> > notifications coalescing */
> > @@ -215,6 +216,14 @@ struct virtio_net_hdr_v1_hash_tunnel {
> > __le16 inner_nh_offset;
> > };
> >
> > +struct virtio_net_hdr_v1_hash_tunnel_ts {
> > + struct virtio_net_hdr_v1_hash_tunnel tnl;
> > + __le16 tstamp_0;
> > + __le16 tstamp_1;
> > + __le16 tstamp_2;
> > + __le16 tstamp_3;
> > +};
>
> Why the multiple fields, rather than u64.
>
> More broadly: can my old patchset be dusted off as is. Does it require
> significant changes?
>
> I only paused it at the time, because I did not have a real device
> back-end that was going to support it.
>
> > +
> > #ifndef VIRTIO_NET_NO_LEGACY
> > /* This header comes first in the scatter-gather list.
> > * For legacy virtio, if VIRTIO_F_ANY_LAYOUT is not negotiated, it must
> >
> > --
> > 2.52.0
> >
>
