virtio-net: add flow filter for receive timestamps

[PATCH RFC v2 2/2] virtio-net: support receive timestamp

Posted by Steffen Trumtrar 1 week, 4 days ago

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@pengutronix.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@google.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;
 	};
 };
 
@@ -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;
+	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;
+};
+
 #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

Re: [PATCH RFC v2 2/2] virtio-net: support receive timestamp

Posted by Willem de Bruijn 1 week, 1 day ago

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@pengutronix.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@google.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?

>  	};
>  };
>  
> @@ -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
>

Re: [PATCH RFC v2 2/2] virtio-net: support receive timestamp

Posted by Jason Wang 6 days, 21 hours ago

On Mon, Feb 2, 2026 at 5:06 AM Willem de Bruijn
<willemdebruijn.kernel@gmail.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@pengutronix.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@google.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.

Thanks

Re: [PATCH RFC v2 2/2] virtio-net: support receive timestamp

Posted by Willem de Bruijn 5 days, 7 hours ago

Jason Wang wrote:
> On Mon, Feb 2, 2026 at 5:06 AM Willem de Bruijn
> <willemdebruijn.kernel@gmail.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@pengutronix.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@google.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.

Re: [PATCH RFC v2 2/2] virtio-net: support receive timestamp

Posted by Michael S. Tsirkin 5 days, 6 hours ago

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
> > <willemdebruijn.kernel@gmail.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@pengutronix.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@google.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.

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.


-- 
MST

Re: [PATCH RFC v2 2/2] virtio-net: support receive timestamp

Posted by Jason Wang 4 days, 22 hours ago

On Thu, Feb 5, 2026 at 2:44 AM Michael S. Tsirkin <mst@redhat.com> 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
> > > <willemdebruijn.kernel@gmail.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@pengutronix.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@google.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?

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
>

Re: [PATCH RFC v2 2/2] virtio-net: support receive timestamp

Posted by Michael S. Tsirkin 1 week ago

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 <s.trumtrar@pengutronix.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@google.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 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
> > 
>

Re: [PATCH RFC v2 2/2] virtio-net: support receive timestamp

Posted by Willem de Bruijn 1 week ago

Michael S. Tsirkin wrote:
> 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 <s.trumtrar@pengutronix.de>

> > > @@ -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.

Great thanks.

Agreed that TLV was probably the wrong suggestion.

We can definitely have a required order of fields. My initial thought
is as said like many user/kernel structures: where both sides agree on
the basic order of the struct, and pass along the length, so that they
agree only to process the min of both their supported lengths. New
fields are added at the tail of the struct. See for instance getsockopt
TCP_INFO.

Re: [PATCH RFC v2 2/2] virtio-net: support receive timestamp

Posted by Steffen Trumtrar 1 week ago

On 2026-02-01 at 16:05 -05, Willem de Bruijn <willemdebruijn.kernel@gmail.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@pengutronix.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@google.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?
>

Yes, this gets complicated real fast and leads to really long calls for all the nested fields. If there is a different way, I'd prefer that.

> >  	};
> >  };
> >  
> > @@ -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.

Should have added a comment, but this is based on this patch

    c3838262b824c71c145cd3668722e99a69bc9cd9
    virtio_net: fix alignment for virtio_net_hdr_v1_hash
      
    Changing alignment of header would mean it's no longer safe to cast a
    2 byte aligned pointer between formats. Use two 16 bit fields to make
    it 2 byte aligned as previously.

> 
> More broadly: can my old patchset be dusted off as is. Does it require
> significant changes?
>

This is the dusted off version ;) With the flow filter it should be possible to turn the timestamps on and off during runtime.


Best regards,
Steffen

-- 
Pengutronix e.K.                | Dipl.-Inform. Steffen Trumtrar |
Steuerwalder Str. 21            | https://www.pengutronix.de/    |
31137 Hildesheim, Germany       | Phone: +49-5121-206917-0       |
Amtsgericht Hildesheim, HRA 2686| Fax:   +49-5121-206917-5555    |

Re: [PATCH RFC v2 2/2] virtio-net: support receive timestamp

Posted by Xuan Zhuo 1 week, 4 days ago

On Thu, 29 Jan 2026 09:06:42 +0100, Steffen Trumtrar <s.trumtrar@pengutronix.de> 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@pengutronix.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@google.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;
>  	};
>  };
>
> @@ -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;
> +	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;
> +};


Since patch #1 used this struct, this one should be placed first in the series.
Also, has the virtio specification process accepted such a draft proposal?

Thanks


> +
>  #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
>

Re: [PATCH RFC v2 2/2] virtio-net: support receive timestamp

Posted by Steffen Trumtrar 1 week, 4 days ago

Hi,

On 2026-01-29 at 17:48 +08, Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> 
> Since patch #1 used this struct, this one should be placed first in the series.

oh, you are right, the order should be the other way around.

> Also, has the virtio specification process accepted such a draft proposal?

I haven't sent the draft yet, because I'm unsure if I understood the way this should be implemented with the flow filter correctly.
If the direction is correct, I'd try and get the specification process going again.
(That is not that easy, if you're not used to it and not that deep into the whole virtio universe ;))

Best regards,
Steffen

-- 
Pengutronix e.K.                | Dipl.-Inform. Steffen Trumtrar |
Steuerwalder Str. 21            | https://www.pengutronix.de/    |
31137 Hildesheim, Germany       | Phone: +49-5121-206917-0       |
Amtsgericht Hildesheim, HRA 2686| Fax:   +49-5121-206917-5555    |

Re: [PATCH RFC v2 2/2] virtio-net: support receive timestamp

Posted by Xuan Zhuo 1 week, 4 days ago

On Thu, 29 Jan 2026 11:08:27 +0100, Steffen Trumtrar <s.trumtrar@pengutronix.de> wrote:
>
> Hi,
>
> On 2026-01-29 at 17:48 +08, Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> >
> > Since patch #1 used this struct, this one should be placed first in the series.
>
> oh, you are right, the order should be the other way around.
>
> > Also, has the virtio specification process accepted such a draft proposal?
>
> I haven't sent the draft yet, because I'm unsure if I understood the way this should be implemented with the flow filter correctly.
> If the direction is correct, I'd try and get the specification process going again.
> (That is not that easy, if you're not used to it and not that deep into the whole virtio universe ;))


There have been many historical attempts in this area- you may want to take a
look first.

Thanks.

>
>
> Best regards,
> Steffen
>
> --
> Pengutronix e.K.                | Dipl.-Inform. Steffen Trumtrar |
> Steuerwalder Str. 21            | https://www.pengutronix.de/    |
> 31137 Hildesheim, Germany       | Phone: +49-5121-206917-0       |
> Amtsgericht Hildesheim, HRA 2686| Fax:   +49-5121-206917-5555    |

[PATCH RFC v2 1/2] tun: support rx-tstamp
[PATCH RFC v2 2/2] virtio-net: support receive timestamp