From: Willem de Bruijn <willemdebruijn.kernel@gmail.com> Date: 2021-02-08 18:58:29
From: Willem de Bruijn <willemb@google.com>
RFCv2 for four new features to the virtio network device:
1. pass tx flow state to host, for routing + telemetry
2. pass rx tstamp to guest, for better RTT estimation
3. pass tx tstamp to guest, idem
3. pass tx delivery time to host, for accurate pacing
All would introduce an extension to the virtio spec.
Concurrently with code review I will write ballots to
https://www.oasis-open.org/committees/ballots.php?wg_abbrev=virtio
These changes are to the driver side. Evaluation additionally requires
achanges to qemu and at least one back-end. I implemented preliminary
support in Linux vhost-net. Both patches available through github at
https://github.com/wdebruij/linux/tree/virtio-net-txhash-2https://github.com/wdebruij/qemu/tree/virtio-net-txhash-2
Changes RFC -> RFCv2
- add transmit timestamp patch
- see individual patches for other changes
Willem de Bruijn (4):
virtio-net: support transmit hash report
virtio-net: support receive timestamp
virtio-net: support transmit timestamp
virtio-net: support future packet transmit time
drivers/net/virtio_net.c | 193 +++++++++++++++++++++++++++++++-
drivers/virtio/virtio_ring.c | 3 +-
include/linux/virtio.h | 1 +
include/uapi/linux/virtio_net.h | 24 +++-
4 files changed, 214 insertions(+), 7 deletions(-)
--
2.30.0.478.g8a0d178c01-goog
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com> Date: 2021-02-08 18:58:54
From: Willem de Bruijn <willemb@google.com>
Virtio-net supports sharing the flow hash from device to driver on rx.
Do the same in the other direction for robust routing and telemetry.
Linux derives ipv6 flowlabel and ECMP multipath from sk->sk_txhash,
and updates this field on error with sk_rethink_txhash. Allow the host
stack to do the same.
Concrete examples of error conditions that are resolved are
mentioned in the commits that add sk_rethink_txhash calls. Such as
commit 7788174e8726 ("tcp: change IPv6 flow-label upon receiving
spurious retransmission").
Experimental results mirror what the theory suggests: where IPv6
FlowLabel is included in path selection (e.g., LAG/ECMP), flowlabel
rotation on TCP timeout avoids the vast majority of TCP disconnects
that would otherwise have occurred during link failures in long-haul
backbones, when an alternative path is available.
Rotation can be applied to various bad connection signals, such as
timeouts and spurious retransmissions. In aggregate, such flow level
signals can help locate network issues. Reserve field hash_state to
share this info. For now, always set VIRTIO_NET_HASH_STATE_DEFAULT.
Passing information between TCP stack and driver is future work.
Changes RFC->RFCv2
- drop unused VIRTIO_NET_HASH_STATE_TIMEOUT_BIT
- convert from cpu_to_virtioXX to cpu_to_leXX
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
drivers/net/virtio_net.c | 26 +++++++++++++++++++++++---
include/uapi/linux/virtio_net.h | 9 ++++++++-
2 files changed, 31 insertions(+), 4 deletions(-)
@@ -201,6 +201,9 @@ struct virtnet_info {/* Host will merge rx buffers for big packets (shake it! shake it!) */boolmergeable_rx_bufs;+/* Driver will pass tx path info to the device */+boolhas_tx_hash;+/* Has control virtqueue */boolhas_cvq;
@@ -394,9 +397,9 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,hdr_len=vi->hdr_len;if(vi->mergeable_rx_bufs)-hdr_padded_len=sizeof(*hdr);+hdr_padded_len=max_t(unsignedint,hdr_len,sizeof(*hdr));else-hdr_padded_len=sizeof(structpadded_vnet_hdr);+hdr_padded_len=ALIGN(hdr_len,16);/* hdr_valid means no XDP, so we can copy the vnet header */if(hdr_valid)
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com> Date: 2021-02-08 18:59:30
From: Willem de Bruijn <willemb@google.com>
Add optional PTP hardware rx timestamp offload for virtio-net.
Accurate RTT measurement requires timestamps close to the wire.
Introduce virtio feature VIRTIO_NET_F_RX_TSTAMP. If negotiated, the
virtio-net header is expanded with room for a timestamp.
A device may pass receive timestamps for all or some packets. Flag
VIRTIO_NET_HDR_F_TSTAMP signals whether a timestamp is recorded.
A driver that supports hardware timestamping must also support
ioctl SIOCSHWTSTAMP. Implement that, as well as information getters
ioctl SIOCGHWTSTAMP and ethtool get_ts_info (`ethtool -T $DEV`).
The timestamp straddles (virtual) hardware domains. Like PTP, use
international atomic time (CLOCK_TAI) as global clock base. The driver
must sync with the device, e.g., through kvm-clock.
Tested:
guest: ./timestamping eth0 \
SOF_TIMESTAMPING_RAW_HARDWARE \
SOF_TIMESTAMPING_RX_HARDWARE
host: nc -4 -u 192.168.1.1 319
Changes RFC -> RFCv2
- rename virtio_net_hdr_v12 to virtio_net_hdr_hash_ts
- add ethtool .get_ts_info to query capabilities
- add ioctl SIOC[GS]HWTSTAMP to configure feature
- add vi->enable_rx_tstamp to store configuration
- convert virtioXX_to_cpu to leXX_to_cpu
- convert reserved to __u32
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
drivers/net/virtio_net.c | 113 +++++++++++++++++++++++++++++++-
include/uapi/linux/virtio_net.h | 13 ++++
2 files changed, 124 insertions(+), 2 deletions(-)
@@ -204,6 +204,12 @@ struct virtnet_info {/* Driver will pass tx path info to the device */boolhas_tx_hash;+/* Device can pass CLOCK_TAI receive time to the driver */+boolhas_rx_tstamp;++/* Device will pass rx timestamp. Requires has_rx_tstamp */+boolenable_rx_tstamp;+/* Has control virtqueue */boolhas_cvq;
@@ -126,6 +127,7 @@ struct virtio_net_hdr_v1 {#define VIRTIO_NET_HDR_F_NEEDS_CSUM 1 /* Use csum_start, csum_offset */#define VIRTIO_NET_HDR_F_DATA_VALID 2 /* Csum is valid */#define VIRTIO_NET_HDR_F_RSC_INFO 4 /* rsc info in csum_ fields */+#define VIRTIO_NET_HDR_F_TSTAMP 8 /* timestamp is recorded */__u8flags;#define VIRTIO_NET_HDR_GSO_NONE 0 /* Not a GSO frame */#define VIRTIO_NET_HDR_GSO_TCPV4 1 /* GSO frame, IPv4 TCP (TSO) */
@@ -181,6 +183,17 @@ struct virtio_net_hdr_v1_hash {};};+structvirtio_net_hdr_hash_ts{+structvirtio_net_hdr_v1hdr;+struct{+__le32value;+__le16report;+__le16flow_state;+}hash;+__u32reserved;+__le64tstamp;+};+#ifndef VIRTIO_NET_NO_LEGACY/* This header comes first in the scatter-gather list.*Forlegacyvirtio,ifVIRTIO_F_ANY_LAYOUTisnotnegotiated,itmust
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com> Date: 2021-02-08 18:59:56
From: Willem de Bruijn <willemb@google.com>
Add optional transmit time (SO_TXTIME) offload for virtio-net.
The Linux TCP/IP stack tries to avoid bursty transmission and network
congestion through pacing: computing an skb delivery time based on
congestion information. Userspace protocol implementations can achieve
the same with SO_TXTIME. This may also reduce scheduling jitter and
improve RTT estimation.
Pacing can be implemented in ETF or FQ qdiscs or offloaded to NIC
hardware. Allow virtio-net driver to offload for the same reasons.
The timestamp straddles (virtual) hardware domains. Like PTP, use
international atomic time (CLOCK_TAI) as global clock base. The driver
must sync with the device, e.g., through kvm-clock.
Changes RFC - RFCv2
- rename from transmit timestamp to future packet transmit time
- convert cpu_to_virtioXX to cpu_to_leXX
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
drivers/net/virtio_net.c | 13 ++++++++++++-
include/uapi/linux/virtio_net.h | 1 +
2 files changed, 13 insertions(+), 1 deletion(-)
@@ -216,6 +216,9 @@ struct virtnet_info {/* Device will pass tx timestamp. Requires has_tx_tstamp */boolenable_tx_tstamp;+/* Driver will pass CLOCK_TAI delivery time to the device */+boolhas_tx_time;+/* Has control virtqueue */boolhas_cvq;
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com> Date: 2021-02-08 19:00:16
From: Willem de Bruijn <willemb@google.com>
Add optional PTP hardware tx timestamp offload for virtio-net.
Accurate RTT measurement requires timestamps close to the wire.
Introduce virtio feature VIRTIO_NET_F_TX_TSTAMP, the transmit
equivalent to VIRTIO_NET_F_RX_TSTAMP.
The driver sets VIRTIO_NET_HDR_F_TSTAMP to request a timestamp
returned on completion. If the feature is negotiated, the device
either places the timestamp or clears the feature bit.
The timestamp straddles (virtual) hardware domains. Like PTP, use
international atomic time (CLOCK_TAI) as global clock base. The driver
must sync with the device, e.g., through kvm-clock.
Modify can_push to ensure that on tx completion the header, and thus
timestamp, is in a predicatable location at skb_vnet_hdr.
RFC: this implementation relies on the device writing to the buffer.
That breaks DMA_TO_DEVICE semantics. For now, disable when DMA is on.
The virtio changes should be a separate patch at the least.
Tested: modified txtimestamp.c to with h/w timestamping:
- sock_opt = SOF_TIMESTAMPING_SOFTWARE |
+ sock_opt = SOF_TIMESTAMPING_RAW_HARDWARE |
+ do_test(family, SOF_TIMESTAMPING_TX_HARDWARE);
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
drivers/net/virtio_net.c | 61 ++++++++++++++++++++++++++++-----
drivers/virtio/virtio_ring.c | 3 +-
include/linux/virtio.h | 1 +
include/uapi/linux/virtio_net.h | 1 +
4 files changed, 56 insertions(+), 10 deletions(-)
@@ -210,6 +210,12 @@ struct virtnet_info {/* Device will pass rx timestamp. Requires has_rx_tstamp */boolenable_rx_tstamp;+/* Device can pass CLOCK_TAI transmit time to the driver */+boolhas_tx_tstamp;++/* Device will pass tx timestamp. Requires has_tx_tstamp */+boolenable_tx_tstamp;+/* Has control virtqueue */boolhas_cvq;
@@ -1401,6 +1407,20 @@ static int virtnet_receive(struct receive_queue *rq, int budget,returnstats.packets;}+staticvoidvirtnet_record_tx_tstamp(conststructsend_queue*sq,+structsk_buff*skb)+{+conststructvirtio_net_hdr_hash_ts*h=skb_vnet_hdr_ht(skb);+conststructvirtnet_info*vi=sq->vq->vdev->priv;+structskb_shared_hwtstampsts;++if(h->hdr.flags&VIRTIO_NET_HDR_F_TSTAMP&&+vi->enable_tx_tstamp){+ts.hwtstamp=ns_to_ktime(le64_to_cpu(h->tstamp));+skb_tstamp_tx(skb,&ts);+}+}+staticvoidfree_old_xmit_skbs(structsend_queue*sq,boolin_napi){unsignedintlen;
@@ -1567,7 +1588,8 @@ static int xmit_skb(struct send_queue *sq, struct sk_buff *skb)can_push=vi->any_header_sg&&!((unsignedlong)skb->data&(__alignof__(*hdr)-1))&&-!skb_header_cloned(skb)&&skb_headroom(skb)>=hdr_len;+!skb_header_cloned(skb)&&skb_headroom(skb)>=hdr_len&&+!vi->enable_tx_tstamp;/* Even if we can, don't push here yet as this would skew*csum_startoffsetbelow.*/if(can_push)
From: Jason Wang <hidden> Date: 2021-02-09 04:19:09
On 2021/2/9 上午2:55, Willem de Bruijn wrote:
quoted hunk
From: Willem de Bruijn <willemb@google.com>
Add optional PTP hardware rx timestamp offload for virtio-net.
Accurate RTT measurement requires timestamps close to the wire.
Introduce virtio feature VIRTIO_NET_F_RX_TSTAMP. If negotiated, the
virtio-net header is expanded with room for a timestamp.
A device may pass receive timestamps for all or some packets. Flag
VIRTIO_NET_HDR_F_TSTAMP signals whether a timestamp is recorded.
A driver that supports hardware timestamping must also support
ioctl SIOCSHWTSTAMP. Implement that, as well as information getters
ioctl SIOCGHWTSTAMP and ethtool get_ts_info (`ethtool -T $DEV`).
The timestamp straddles (virtual) hardware domains. Like PTP, use
international atomic time (CLOCK_TAI) as global clock base. The driver
must sync with the device, e.g., through kvm-clock.
Tested:
guest: ./timestamping eth0 \
SOF_TIMESTAMPING_RAW_HARDWARE \
SOF_TIMESTAMPING_RX_HARDWARE
host: nc -4 -u 192.168.1.1 319
Changes RFC -> RFCv2
- rename virtio_net_hdr_v12 to virtio_net_hdr_hash_ts
- add ethtool .get_ts_info to query capabilities
- add ioctl SIOC[GS]HWTSTAMP to configure feature
- add vi->enable_rx_tstamp to store configuration
- convert virtioXX_to_cpu to leXX_to_cpu
- convert reserved to __u32
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
drivers/net/virtio_net.c | 113 +++++++++++++++++++++++++++++++-
include/uapi/linux/virtio_net.h | 13 ++++
2 files changed, 124 insertions(+), 2 deletions(-)
@@ -204,6 +204,12 @@ struct virtnet_info {/* Driver will pass tx path info to the device */boolhas_tx_hash;+/* Device can pass CLOCK_TAI receive time to the driver */+boolhas_rx_tstamp;++/* Device will pass rx timestamp. Requires has_rx_tstamp */+boolenable_rx_tstamp;+/* Has control virtqueue */boolhas_cvq;
Any reason for not embedding structure virtio_net_hdr_v1_hash?
Thanks
+ __u32 reserved;
+ __le64 tstamp;
+};
+
#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
From: Jason Wang <hidden> Date: 2021-02-09 05:47:10
On 2021/2/9 上午2:55, Willem de Bruijn wrote:
quoted hunk
From: Willem de Bruijn <willemb@google.com>
Add optional PTP hardware tx timestamp offload for virtio-net.
Accurate RTT measurement requires timestamps close to the wire.
Introduce virtio feature VIRTIO_NET_F_TX_TSTAMP, the transmit
equivalent to VIRTIO_NET_F_RX_TSTAMP.
The driver sets VIRTIO_NET_HDR_F_TSTAMP to request a timestamp
returned on completion. If the feature is negotiated, the device
either places the timestamp or clears the feature bit.
The timestamp straddles (virtual) hardware domains. Like PTP, use
international atomic time (CLOCK_TAI) as global clock base. The driver
must sync with the device, e.g., through kvm-clock.
Modify can_push to ensure that on tx completion the header, and thus
timestamp, is in a predicatable location at skb_vnet_hdr.
RFC: this implementation relies on the device writing to the buffer.
That breaks DMA_TO_DEVICE semantics. For now, disable when DMA is on.
The virtio changes should be a separate patch at the least.
Tested: modified txtimestamp.c to with h/w timestamping:
- sock_opt = SOF_TIMESTAMPING_SOFTWARE |
+ sock_opt = SOF_TIMESTAMPING_RAW_HARDWARE |
+ do_test(family, SOF_TIMESTAMPING_TX_HARDWARE);
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
drivers/net/virtio_net.c | 61 ++++++++++++++++++++++++++++-----
drivers/virtio/virtio_ring.c | 3 +-
include/linux/virtio.h | 1 +
include/uapi/linux/virtio_net.h | 1 +
4 files changed, 56 insertions(+), 10 deletions(-)
@@ -210,6 +210,12 @@ struct virtnet_info {/* Device will pass rx timestamp. Requires has_rx_tstamp */boolenable_rx_tstamp;+/* Device can pass CLOCK_TAI transmit time to the driver */+boolhas_tx_tstamp;++/* Device will pass tx timestamp. Requires has_tx_tstamp */+boolenable_tx_tstamp;+/* Has control virtqueue */boolhas_cvq;
@@ -1401,6 +1407,20 @@ static int virtnet_receive(struct receive_queue *rq, int budget,returnstats.packets;}+staticvoidvirtnet_record_tx_tstamp(conststructsend_queue*sq,+structsk_buff*skb)+{+conststructvirtio_net_hdr_hash_ts*h=skb_vnet_hdr_ht(skb);+conststructvirtnet_info*vi=sq->vq->vdev->priv;+structskb_shared_hwtstampsts;++if(h->hdr.flags&VIRTIO_NET_HDR_F_TSTAMP&&+vi->enable_tx_tstamp){+ts.hwtstamp=ns_to_ktime(le64_to_cpu(h->tstamp));+skb_tstamp_tx(skb,&ts);
This probably won't work since the buffer is read-only from the device.
(See virtqueue_add_outbuf()).
Another issue that I vaguely remember that the virtio spec forbids out
buffer after in buffer.
@@ -57,6 +57,7 @@*Steering*/#define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */+#define VIRTIO_NET_F_TX_TSTAMP 54 /* Device sends TAI transmit time */#define VIRTIO_NET_F_RX_TSTAMP 55 /* Device sends TAI receive time */
I wonder how much value to split into two features.
Thanks
From: "Michael S. Tsirkin" <mst@redhat.com> Date: 2021-02-09 09:47:03
On Mon, Feb 08, 2021 at 01:55:57PM -0500, Willem de Bruijn wrote:
From: Willem de Bruijn <willemb@google.com>
Add optional PTP hardware tx timestamp offload for virtio-net.
Accurate RTT measurement requires timestamps close to the wire.
Introduce virtio feature VIRTIO_NET_F_TX_TSTAMP, the transmit
equivalent to VIRTIO_NET_F_RX_TSTAMP.
The driver sets VIRTIO_NET_HDR_F_TSTAMP to request a timestamp
returned on completion. If the feature is negotiated, the device
either places the timestamp or clears the feature bit.
The timestamp straddles (virtual) hardware domains. Like PTP, use
international atomic time (CLOCK_TAI) as global clock base. The driver
must sync with the device, e.g., through kvm-clock.
Modify can_push to ensure that on tx completion the header, and thus
timestamp, is in a predicatable location at skb_vnet_hdr.
RFC: this implementation relies on the device writing to the buffer.
That breaks DMA_TO_DEVICE semantics. For now, disable when DMA is on.
If you do something like this, please do it in the validate
callback and clear the features you aren't using.
quoted hunk
The virtio changes should be a separate patch at the least.
Tested: modified txtimestamp.c to with h/w timestamping:
- sock_opt = SOF_TIMESTAMPING_SOFTWARE |
+ sock_opt = SOF_TIMESTAMPING_RAW_HARDWARE |
+ do_test(family, SOF_TIMESTAMPING_TX_HARDWARE);
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
drivers/net/virtio_net.c | 61 ++++++++++++++++++++++++++++-----
drivers/virtio/virtio_ring.c | 3 +-
include/linux/virtio.h | 1 +
include/uapi/linux/virtio_net.h | 1 +
4 files changed, 56 insertions(+), 10 deletions(-)
@@ -210,6 +210,12 @@ struct virtnet_info {/* Device will pass rx timestamp. Requires has_rx_tstamp */boolenable_rx_tstamp;+/* Device can pass CLOCK_TAI transmit time to the driver */+boolhas_tx_tstamp;++/* Device will pass tx timestamp. Requires has_tx_tstamp */+boolenable_tx_tstamp;+/* Has control virtqueue */boolhas_cvq;
@@ -1401,6 +1407,20 @@ static int virtnet_receive(struct receive_queue *rq, int budget,returnstats.packets;}+staticvoidvirtnet_record_tx_tstamp(conststructsend_queue*sq,+structsk_buff*skb)+{+conststructvirtio_net_hdr_hash_ts*h=skb_vnet_hdr_ht(skb);+conststructvirtnet_info*vi=sq->vq->vdev->priv;+structskb_shared_hwtstampsts;++if(h->hdr.flags&VIRTIO_NET_HDR_F_TSTAMP&&+vi->enable_tx_tstamp){+ts.hwtstamp=ns_to_ktime(le64_to_cpu(h->tstamp));+skb_tstamp_tx(skb,&ts);+}+}+staticvoidfree_old_xmit_skbs(structsend_queue*sq,boolin_napi){unsignedintlen;
@@ -1567,7 +1588,8 @@ static int xmit_skb(struct send_queue *sq, struct sk_buff *skb)can_push=vi->any_header_sg&&!((unsignedlong)skb->data&(__alignof__(*hdr)-1))&&-!skb_header_cloned(skb)&&skb_headroom(skb)>=hdr_len;+!skb_header_cloned(skb)&&skb_headroom(skb)>=hdr_len&&+!vi->enable_tx_tstamp;/* Even if we can, don't push here yet as this would skew*csum_startoffsetbelow.*/if(can_push)
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com> Date: 2021-02-09 13:56:23
On Mon, Feb 8, 2021 at 11:13 PM Jason Wang [off-list ref] wrote:
On 2021/2/9 上午2:55, Willem de Bruijn wrote:
quoted
From: Willem de Bruijn <willemb@google.com>
Add optional PTP hardware rx timestamp offload for virtio-net.
Accurate RTT measurement requires timestamps close to the wire.
Introduce virtio feature VIRTIO_NET_F_RX_TSTAMP. If negotiated, the
virtio-net header is expanded with room for a timestamp.
A device may pass receive timestamps for all or some packets. Flag
VIRTIO_NET_HDR_F_TSTAMP signals whether a timestamp is recorded.
A driver that supports hardware timestamping must also support
ioctl SIOCSHWTSTAMP. Implement that, as well as information getters
ioctl SIOCGHWTSTAMP and ethtool get_ts_info (`ethtool -T $DEV`).
The timestamp straddles (virtual) hardware domains. Like PTP, use
international atomic time (CLOCK_TAI) as global clock base. The driver
must sync with the device, e.g., through kvm-clock.
Tested:
guest: ./timestamping eth0 \
SOF_TIMESTAMPING_RAW_HARDWARE \
SOF_TIMESTAMPING_RX_HARDWARE
host: nc -4 -u 192.168.1.1 319
Changes RFC -> RFCv2
- rename virtio_net_hdr_v12 to virtio_net_hdr_hash_ts
- add ethtool .get_ts_info to query capabilities
- add ioctl SIOC[GS]HWTSTAMP to configure feature
- add vi->enable_rx_tstamp to store configuration
- convert virtioXX_to_cpu to leXX_to_cpu
- convert reserved to __u32
Signed-off-by: Willem de Bruijn <willemb@google.com>
Does this mean even if the device doesn't pass timestamp, the header
still contains the timestamp fields.
Yes. As implemented, the size of the header is constant across
packets. If both sides negotiate the feature, then all headers reserve
space, whether or not the specific packet has a timestamp.
So far headers are fixed size. I suppose we could investigate variable
size headers. This goes back to our discussion in the previous
patchset, that we can always add a packed-header feature later, if the
number of optional features reaches a size that makes the complexity
worthwhile.
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com> Date: 2021-02-09 14:46:48
On Tue, Feb 9, 2021 at 4:43 AM Michael S. Tsirkin [off-list ref] wrote:
On Mon, Feb 08, 2021 at 01:55:57PM -0500, Willem de Bruijn wrote:
quoted
From: Willem de Bruijn <willemb@google.com>
Add optional PTP hardware tx timestamp offload for virtio-net.
Accurate RTT measurement requires timestamps close to the wire.
Introduce virtio feature VIRTIO_NET_F_TX_TSTAMP, the transmit
equivalent to VIRTIO_NET_F_RX_TSTAMP.
The driver sets VIRTIO_NET_HDR_F_TSTAMP to request a timestamp
returned on completion. If the feature is negotiated, the device
either places the timestamp or clears the feature bit.
The timestamp straddles (virtual) hardware domains. Like PTP, use
international atomic time (CLOCK_TAI) as global clock base. The driver
must sync with the device, e.g., through kvm-clock.
Modify can_push to ensure that on tx completion the header, and thus
timestamp, is in a predicatable location at skb_vnet_hdr.
RFC: this implementation relies on the device writing to the buffer.
That breaks DMA_TO_DEVICE semantics. For now, disable when DMA is on.
If you do something like this, please do it in the validate
callback and clear the features you aren't using.
Ah yes. Thanks for the tip. I'll do that ..
.. once I'm sure that this approach of using an outbuf for I/O is
actually allowed behavior. I'm not entirely convinced yet myself.
Jason also pointed out more specific concerns. I'll look into that
further.
From: "Michael S. Tsirkin" <mst@redhat.com> Date: 2021-02-09 16:41:23
On Tue, Feb 09, 2021 at 01:45:11PM +0800, Jason Wang wrote:
On 2021/2/9 上午2:55, Willem de Bruijn wrote:
quoted
From: Willem de Bruijn <willemb@google.com>
Add optional PTP hardware tx timestamp offload for virtio-net.
Accurate RTT measurement requires timestamps close to the wire.
Introduce virtio feature VIRTIO_NET_F_TX_TSTAMP, the transmit
equivalent to VIRTIO_NET_F_RX_TSTAMP.
The driver sets VIRTIO_NET_HDR_F_TSTAMP to request a timestamp
returned on completion. If the feature is negotiated, the device
either places the timestamp or clears the feature bit.
The timestamp straddles (virtual) hardware domains. Like PTP, use
international atomic time (CLOCK_TAI) as global clock base. The driver
must sync with the device, e.g., through kvm-clock.
Modify can_push to ensure that on tx completion the header, and thus
timestamp, is in a predicatable location at skb_vnet_hdr.
RFC: this implementation relies on the device writing to the buffer.
That breaks DMA_TO_DEVICE semantics. For now, disable when DMA is on.
The virtio changes should be a separate patch at the least.
Tested: modified txtimestamp.c to with h/w timestamping:
- sock_opt = SOF_TIMESTAMPING_SOFTWARE |
+ sock_opt = SOF_TIMESTAMPING_RAW_HARDWARE |
+ do_test(family, SOF_TIMESTAMPING_TX_HARDWARE);
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
drivers/net/virtio_net.c | 61 ++++++++++++++++++++++++++++-----
drivers/virtio/virtio_ring.c | 3 +-
include/linux/virtio.h | 1 +
include/uapi/linux/virtio_net.h | 1 +
4 files changed, 56 insertions(+), 10 deletions(-)
@@ -210,6 +210,12 @@ struct virtnet_info {/* Device will pass rx timestamp. Requires has_rx_tstamp */boolenable_rx_tstamp;+/* Device can pass CLOCK_TAI transmit time to the driver */+boolhas_tx_tstamp;++/* Device will pass tx timestamp. Requires has_tx_tstamp */+boolenable_tx_tstamp;+/* Has control virtqueue */boolhas_cvq;
@@ -1401,6 +1407,20 @@ static int virtnet_receive(struct receive_queue *rq, int budget,returnstats.packets;}+staticvoidvirtnet_record_tx_tstamp(conststructsend_queue*sq,+structsk_buff*skb)+{+conststructvirtio_net_hdr_hash_ts*h=skb_vnet_hdr_ht(skb);+conststructvirtnet_info*vi=sq->vq->vdev->priv;+structskb_shared_hwtstampsts;++if(h->hdr.flags&VIRTIO_NET_HDR_F_TSTAMP&&+vi->enable_tx_tstamp){+ts.hwtstamp=ns_to_ktime(le64_to_cpu(h->tstamp));+skb_tstamp_tx(skb,&ts);
This probably won't work since the buffer is read-only from the device. (See
virtqueue_add_outbuf()).
Another issue that I vaguely remember that the virtio spec forbids out
buffer after in buffer.
Both Driver Requirements: Message Framing and Driver Requirements: Scatter-Gather Support
have this statement:
The driver MUST place any device-writable descriptor elements after any device-readable descriptor ele-
ments.
similarly
Device Requirements: The Virtqueue Descriptor Table
A device MUST NOT write to a device-readable buffer, and a device SHOULD NOT read a device-writable
buffer.
@@ -57,6 +57,7 @@*Steering*/#define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */+#define VIRTIO_NET_F_TX_TSTAMP 54 /* Device sends TAI transmit time */#define VIRTIO_NET_F_RX_TSTAMP 55 /* Device sends TAI receive time */
I wonder how much value to split into two features.
Thanks
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com> Date: 2021-02-10 02:40:08
On Tue, Feb 9, 2021 at 11:39 AM Michael S. Tsirkin [off-list ref] wrote:
On Tue, Feb 09, 2021 at 01:45:11PM +0800, Jason Wang wrote:
quoted
On 2021/2/9 上午2:55, Willem de Bruijn wrote:
quoted
From: Willem de Bruijn <willemb@google.com>
Add optional PTP hardware tx timestamp offload for virtio-net.
Accurate RTT measurement requires timestamps close to the wire.
Introduce virtio feature VIRTIO_NET_F_TX_TSTAMP, the transmit
equivalent to VIRTIO_NET_F_RX_TSTAMP.
The driver sets VIRTIO_NET_HDR_F_TSTAMP to request a timestamp
returned on completion. If the feature is negotiated, the device
either places the timestamp or clears the feature bit.
The timestamp straddles (virtual) hardware domains. Like PTP, use
international atomic time (CLOCK_TAI) as global clock base. The driver
must sync with the device, e.g., through kvm-clock.
Modify can_push to ensure that on tx completion the header, and thus
timestamp, is in a predicatable location at skb_vnet_hdr.
RFC: this implementation relies on the device writing to the buffer.
That breaks DMA_TO_DEVICE semantics. For now, disable when DMA is on.
The virtio changes should be a separate patch at the least.
Tested: modified txtimestamp.c to with h/w timestamping:
- sock_opt = SOF_TIMESTAMPING_SOFTWARE |
+ sock_opt = SOF_TIMESTAMPING_RAW_HARDWARE |
+ do_test(family, SOF_TIMESTAMPING_TX_HARDWARE);
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
drivers/net/virtio_net.c | 61 ++++++++++++++++++++++++++++-----
drivers/virtio/virtio_ring.c | 3 +-
include/linux/virtio.h | 1 +
include/uapi/linux/virtio_net.h | 1 +
4 files changed, 56 insertions(+), 10 deletions(-)
@@ -210,6 +210,12 @@ struct virtnet_info {/* Device will pass rx timestamp. Requires has_rx_tstamp */boolenable_rx_tstamp;+/* Device can pass CLOCK_TAI transmit time to the driver */+boolhas_tx_tstamp;++/* Device will pass tx timestamp. Requires has_tx_tstamp */+boolenable_tx_tstamp;+/* Has control virtqueue */boolhas_cvq;
@@ -1401,6 +1407,20 @@ static int virtnet_receive(struct receive_queue *rq, int budget,returnstats.packets;}+staticvoidvirtnet_record_tx_tstamp(conststructsend_queue*sq,+structsk_buff*skb)+{+conststructvirtio_net_hdr_hash_ts*h=skb_vnet_hdr_ht(skb);+conststructvirtnet_info*vi=sq->vq->vdev->priv;+structskb_shared_hwtstampsts;++if(h->hdr.flags&VIRTIO_NET_HDR_F_TSTAMP&&+vi->enable_tx_tstamp){+ts.hwtstamp=ns_to_ktime(le64_to_cpu(h->tstamp));+skb_tstamp_tx(skb,&ts);
This probably won't work since the buffer is read-only from the device. (See
virtqueue_add_outbuf()).
Another issue that I vaguely remember that the virtio spec forbids out
buffer after in buffer.
Both Driver Requirements: Message Framing and Driver Requirements: Scatter-Gather Support
have this statement:
The driver MUST place any device-writable descriptor elements after any device-readable descriptor ele-
ments.
similarly
Device Requirements: The Virtqueue Descriptor Table
A device MUST NOT write to a device-readable buffer, and a device SHOULD NOT read a device-writable
buffer.
Thanks. That's clear. So the clean solution would be to add a
device-writable descriptor after the existing device-readable ones.
And the device must be aware that this is to return the tstamp only.
In the example implementation of vhost, it has to exclude this last
descriptor from the msg->msg_iter iovec array with packet data
initialized at get_tx_bufs/init_iov_iter.
From: Jason Wang <hidden> Date: 2021-02-10 04:12:31
On 2021/2/9 下午9:53, Willem de Bruijn wrote:
On Mon, Feb 8, 2021 at 11:13 PM Jason Wang [off-list ref] wrote:
quoted
On 2021/2/9 上午2:55, Willem de Bruijn wrote:
quoted
From: Willem de Bruijn <willemb@google.com>
Add optional PTP hardware rx timestamp offload for virtio-net.
Accurate RTT measurement requires timestamps close to the wire.
Introduce virtio feature VIRTIO_NET_F_RX_TSTAMP. If negotiated, the
virtio-net header is expanded with room for a timestamp.
A device may pass receive timestamps for all or some packets. Flag
VIRTIO_NET_HDR_F_TSTAMP signals whether a timestamp is recorded.
A driver that supports hardware timestamping must also support
ioctl SIOCSHWTSTAMP. Implement that, as well as information getters
ioctl SIOCGHWTSTAMP and ethtool get_ts_info (`ethtool -T $DEV`).
The timestamp straddles (virtual) hardware domains. Like PTP, use
international atomic time (CLOCK_TAI) as global clock base. The driver
must sync with the device, e.g., through kvm-clock.
Tested:
guest: ./timestamping eth0 \
SOF_TIMESTAMPING_RAW_HARDWARE \
SOF_TIMESTAMPING_RX_HARDWARE
host: nc -4 -u 192.168.1.1 319
Changes RFC -> RFCv2
- rename virtio_net_hdr_v12 to virtio_net_hdr_hash_ts
- add ethtool .get_ts_info to query capabilities
- add ioctl SIOC[GS]HWTSTAMP to configure feature
- add vi->enable_rx_tstamp to store configuration
- convert virtioXX_to_cpu to leXX_to_cpu
- convert reserved to __u32
Signed-off-by: Willem de Bruijn <willemb@google.com>
static const struct net_device_ops virtnet_netdev = {
.ndo_open = virtnet_open,
.ndo_stop = virtnet_close,
Does this mean even if the device doesn't pass timestamp, the header
still contains the timestamp fields.
Yes. As implemented, the size of the header is constant across
packets. If both sides negotiate the feature, then all headers reserve
space, whether or not the specific packet has a timestamp.
So far headers are fixed size. I suppose we could investigate variable
size headers. This goes back to our discussion in the previous
patchset, that we can always add a packed-header feature later, if the
number of optional features reaches a size that makes the complexity
worthwhile.
Right, so for timstamp it's probably OK but we probably need to do as
you said here if we want to add more in the header. Let's see how
Michael think about this.
From: Jason Wang <hidden> Date: 2021-02-10 04:14:06
On 2021/2/10 上午12:38, Michael S. Tsirkin wrote:
On Tue, Feb 09, 2021 at 01:45:11PM +0800, Jason Wang wrote:
quoted
On 2021/2/9 上午2:55, Willem de Bruijn wrote:
quoted
From: Willem de Bruijn<willemb@google.com>
Add optional PTP hardware tx timestamp offload for virtio-net.
Accurate RTT measurement requires timestamps close to the wire.
Introduce virtio feature VIRTIO_NET_F_TX_TSTAMP, the transmit
equivalent to VIRTIO_NET_F_RX_TSTAMP.
The driver sets VIRTIO_NET_HDR_F_TSTAMP to request a timestamp
returned on completion. If the feature is negotiated, the device
either places the timestamp or clears the feature bit.
The timestamp straddles (virtual) hardware domains. Like PTP, use
international atomic time (CLOCK_TAI) as global clock base. The driver
must sync with the device, e.g., through kvm-clock.
Modify can_push to ensure that on tx completion the header, and thus
timestamp, is in a predicatable location at skb_vnet_hdr.
RFC: this implementation relies on the device writing to the buffer.
That breaks DMA_TO_DEVICE semantics. For now, disable when DMA is on.
The virtio changes should be a separate patch at the least.
Tested: modified txtimestamp.c to with h/w timestamping:
- sock_opt = SOF_TIMESTAMPING_SOFTWARE |
+ sock_opt = SOF_TIMESTAMPING_RAW_HARDWARE |
+ do_test(family, SOF_TIMESTAMPING_TX_HARDWARE);
Signed-off-by: Willem de Bruijn<willemb@google.com>
---
drivers/net/virtio_net.c | 61 ++++++++++++++++++++++++++++-----
drivers/virtio/virtio_ring.c | 3 +-
include/linux/virtio.h | 1 +
include/uapi/linux/virtio_net.h | 1 +
4 files changed, 56 insertions(+), 10 deletions(-)
@@ -210,6 +210,12 @@ struct virtnet_info {/* Device will pass rx timestamp. Requires has_rx_tstamp */boolenable_rx_tstamp;+/* Device can pass CLOCK_TAI transmit time to the driver */+boolhas_tx_tstamp;++/* Device will pass tx timestamp. Requires has_tx_tstamp */+boolenable_tx_tstamp;+/* Has control virtqueue */boolhas_cvq;
@@ -1401,6 +1407,20 @@ static int virtnet_receive(struct receive_queue *rq, int budget,returnstats.packets;}+staticvoidvirtnet_record_tx_tstamp(conststructsend_queue*sq,+structsk_buff*skb)+{+conststructvirtio_net_hdr_hash_ts*h=skb_vnet_hdr_ht(skb);+conststructvirtnet_info*vi=sq->vq->vdev->priv;+structskb_shared_hwtstampsts;++if(h->hdr.flags&VIRTIO_NET_HDR_F_TSTAMP&&+vi->enable_tx_tstamp){+ts.hwtstamp=ns_to_ktime(le64_to_cpu(h->tstamp));+skb_tstamp_tx(skb,&ts);
This probably won't work since the buffer is read-only from the device. (See
virtqueue_add_outbuf()).
Another issue that I vaguely remember that the virtio spec forbids out
buffer after in buffer.
Both Driver Requirements: Message Framing and Driver Requirements: Scatter-Gather Support
have this statement:
The driver MUST place any device-writable descriptor elements after any device-readable descriptor ele-
ments.
similarly
Device Requirements: The Virtqueue Descriptor Table
A device MUST NOT write to a device-readable buffer, and a device SHOULD NOT read a device-writable
buffer.
Exactly. But I wonder what's the rationale behinds those requirements?
Thanks
From: Jason Wang <hidden> Date: 2021-02-10 04:17:34
On 2021/2/10 上午10:36, Willem de Bruijn wrote:
On Tue, Feb 9, 2021 at 11:39 AM Michael S. Tsirkin[off-list ref] wrote:
quoted
On Tue, Feb 09, 2021 at 01:45:11PM +0800, Jason Wang wrote:
quoted
On 2021/2/9 上午2:55, Willem de Bruijn wrote:
quoted
From: Willem de Bruijn<willemb@google.com>
Add optional PTP hardware tx timestamp offload for virtio-net.
Accurate RTT measurement requires timestamps close to the wire.
Introduce virtio feature VIRTIO_NET_F_TX_TSTAMP, the transmit
equivalent to VIRTIO_NET_F_RX_TSTAMP.
The driver sets VIRTIO_NET_HDR_F_TSTAMP to request a timestamp
returned on completion. If the feature is negotiated, the device
either places the timestamp or clears the feature bit.
The timestamp straddles (virtual) hardware domains. Like PTP, use
international atomic time (CLOCK_TAI) as global clock base. The driver
must sync with the device, e.g., through kvm-clock.
Modify can_push to ensure that on tx completion the header, and thus
timestamp, is in a predicatable location at skb_vnet_hdr.
RFC: this implementation relies on the device writing to the buffer.
That breaks DMA_TO_DEVICE semantics. For now, disable when DMA is on.
The virtio changes should be a separate patch at the least.
Tested: modified txtimestamp.c to with h/w timestamping:
- sock_opt = SOF_TIMESTAMPING_SOFTWARE |
+ sock_opt = SOF_TIMESTAMPING_RAW_HARDWARE |
+ do_test(family, SOF_TIMESTAMPING_TX_HARDWARE);
Signed-off-by: Willem de Bruijn<willemb@google.com>
---
drivers/net/virtio_net.c | 61 ++++++++++++++++++++++++++++-----
drivers/virtio/virtio_ring.c | 3 +-
include/linux/virtio.h | 1 +
include/uapi/linux/virtio_net.h | 1 +
4 files changed, 56 insertions(+), 10 deletions(-)
@@ -210,6 +210,12 @@ struct virtnet_info {/* Device will pass rx timestamp. Requires has_rx_tstamp */boolenable_rx_tstamp;+/* Device can pass CLOCK_TAI transmit time to the driver */+boolhas_tx_tstamp;++/* Device will pass tx timestamp. Requires has_tx_tstamp */+boolenable_tx_tstamp;+/* Has control virtqueue */boolhas_cvq;
@@ -1401,6 +1407,20 @@ static int virtnet_receive(struct receive_queue *rq, int budget,returnstats.packets;}+staticvoidvirtnet_record_tx_tstamp(conststructsend_queue*sq,+structsk_buff*skb)+{+conststructvirtio_net_hdr_hash_ts*h=skb_vnet_hdr_ht(skb);+conststructvirtnet_info*vi=sq->vq->vdev->priv;+structskb_shared_hwtstampsts;++if(h->hdr.flags&VIRTIO_NET_HDR_F_TSTAMP&&+vi->enable_tx_tstamp){+ts.hwtstamp=ns_to_ktime(le64_to_cpu(h->tstamp));+skb_tstamp_tx(skb,&ts);
This probably won't work since the buffer is read-only from the device. (See
virtqueue_add_outbuf()).
Another issue that I vaguely remember that the virtio spec forbids out
buffer after in buffer.
Both Driver Requirements: Message Framing and Driver Requirements: Scatter-Gather Support
have this statement:
The driver MUST place any device-writable descriptor elements after any device-readable descriptor ele-
ments.
similarly
Device Requirements: The Virtqueue Descriptor Table
A device MUST NOT write to a device-readable buffer, and a device SHOULD NOT read a device-writable
buffer.
Thanks. That's clear. So the clean solution would be to add a
device-writable descriptor after the existing device-readable ones.
I think so, but a question is the format for this tailer. I think it
might be better to post a spec patch to discuss.
Thanks
And the device must be aware that this is to return the tstamp only.
In the example implementation of vhost, it has to exclude this last
descriptor from the msg->msg_iter iovec array with packet data
initialized at get_tx_bufs/init_iov_iter.
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com> Date: 2021-02-10 14:42:12
On Tue, Feb 9, 2021 at 11:15 PM Jason Wang [off-list ref] wrote:
On 2021/2/10 上午10:36, Willem de Bruijn wrote:
quoted
On Tue, Feb 9, 2021 at 11:39 AM Michael S. Tsirkin[off-list ref] wrote:
quoted
On Tue, Feb 09, 2021 at 01:45:11PM +0800, Jason Wang wrote:
quoted
On 2021/2/9 上午2:55, Willem de Bruijn wrote:
quoted
From: Willem de Bruijn<willemb@google.com>
Add optional PTP hardware tx timestamp offload for virtio-net.
Accurate RTT measurement requires timestamps close to the wire.
Introduce virtio feature VIRTIO_NET_F_TX_TSTAMP, the transmit
equivalent to VIRTIO_NET_F_RX_TSTAMP.
The driver sets VIRTIO_NET_HDR_F_TSTAMP to request a timestamp
returned on completion. If the feature is negotiated, the device
either places the timestamp or clears the feature bit.
The timestamp straddles (virtual) hardware domains. Like PTP, use
international atomic time (CLOCK_TAI) as global clock base. The driver
must sync with the device, e.g., through kvm-clock.
Modify can_push to ensure that on tx completion the header, and thus
timestamp, is in a predicatable location at skb_vnet_hdr.
RFC: this implementation relies on the device writing to the buffer.
That breaks DMA_TO_DEVICE semantics. For now, disable when DMA is on.
The virtio changes should be a separate patch at the least.
Tested: modified txtimestamp.c to with h/w timestamping:
- sock_opt = SOF_TIMESTAMPING_SOFTWARE |
+ sock_opt = SOF_TIMESTAMPING_RAW_HARDWARE |
+ do_test(family, SOF_TIMESTAMPING_TX_HARDWARE);
Signed-off-by: Willem de Bruijn<willemb@google.com>
---
drivers/net/virtio_net.c | 61 ++++++++++++++++++++++++++++-----
drivers/virtio/virtio_ring.c | 3 +-
include/linux/virtio.h | 1 +
include/uapi/linux/virtio_net.h | 1 +
4 files changed, 56 insertions(+), 10 deletions(-)
@@ -210,6 +210,12 @@ struct virtnet_info {/* Device will pass rx timestamp. Requires has_rx_tstamp */boolenable_rx_tstamp;+/* Device can pass CLOCK_TAI transmit time to the driver */+boolhas_tx_tstamp;++/* Device will pass tx timestamp. Requires has_tx_tstamp */+boolenable_tx_tstamp;+/* Has control virtqueue */boolhas_cvq;
@@ -1401,6 +1407,20 @@ static int virtnet_receive(struct receive_queue *rq, int budget,returnstats.packets;}+staticvoidvirtnet_record_tx_tstamp(conststructsend_queue*sq,+structsk_buff*skb)+{+conststructvirtio_net_hdr_hash_ts*h=skb_vnet_hdr_ht(skb);+conststructvirtnet_info*vi=sq->vq->vdev->priv;+structskb_shared_hwtstampsts;++if(h->hdr.flags&VIRTIO_NET_HDR_F_TSTAMP&&+vi->enable_tx_tstamp){+ts.hwtstamp=ns_to_ktime(le64_to_cpu(h->tstamp));+skb_tstamp_tx(skb,&ts);
This probably won't work since the buffer is read-only from the device. (See
virtqueue_add_outbuf()).
Another issue that I vaguely remember that the virtio spec forbids out
buffer after in buffer.
Both Driver Requirements: Message Framing and Driver Requirements: Scatter-Gather Support
have this statement:
The driver MUST place any device-writable descriptor elements after any device-readable descriptor ele-
ments.
similarly
Device Requirements: The Virtqueue Descriptor Table
A device MUST NOT write to a device-readable buffer, and a device SHOULD NOT read a device-writable
buffer.
Thanks. That's clear. So the clean solution would be to add a
device-writable descriptor after the existing device-readable ones.
I think so, but a question is the format for this tailer. I think it
might be better to post a spec patch to discuss.
Okay I'll do that. I want to get something that works first, to make
sure that whatever I propose in spec is actually implementable.
From: Willem de Bruijn <willemb@google.com> Date: 2021-05-13 22:50:03
On Mon, Feb 8, 2021 at 1:56 PM Willem de Bruijn
[off-list ref] wrote:
From: Willem de Bruijn <willemb@google.com>
RFCv2 for four new features to the virtio network device:
1. pass tx flow state to host, for routing + telemetry
2. pass rx tstamp to guest, for better RTT estimation
3. pass tx tstamp to guest, idem
3. pass tx delivery time to host, for accurate pacing
All would introduce an extension to the virtio spec.
Concurrently with code review I will write ballots to
https://www.oasis-open.org/committees/ballots.php?wg_abbrev=virtio
These changes are to the driver side. Evaluation additionally requires
achanges to qemu and at least one back-end. I implemented preliminary
support in Linux vhost-net. Both patches available through github at
https://github.com/wdebruij/linux/tree/virtio-net-txhash-2https://github.com/wdebruij/qemu/tree/virtio-net-txhash-2
Changes RFC -> RFCv2
- add transmit timestamp patch
- see individual patches for other changes
Willem de Bruijn (4):
virtio-net: support transmit hash report
virtio-net: support receive timestamp
virtio-net: support transmit timestamp
virtio-net: support future packet transmit time
Seeing Yuri's patchset adding new features reminded me that I did not
follow-up on this patch series on the list.
The patches themselves are mostly in good shape. The last tx tstamp
issue can be resolved.
But the device implementation I target only supports legacy mode.
Below conversation that we had in one of the patches makes clear that
supporting this in legacy is not feasible. Nor is upgrading that
device in the short term. Until there is a device implementation that
implements these offloads, these features are a dead letter. Not moving
forward for now.
Somewhat related: is there a plan for when we run out of 64 feature bits?
quoted
quoted
Actually, would it be possible to make new features available on
legacy devices? There is nothing in the features bits precluding it.
I think it won't be possible: you are using feature bit 55,
legacy devices have up to 32 feature bits. And of course the
header looks a bit differently for legacy, you would have to add special
code to handle that when mergeable buffers are off.
I think I can make the latter work. I did start without a dependency
on the v1 header initially.
Feature bit array length I had not considered. Good point. Need to
think about that. It would be very appealing if in particular the
tx-hash feature could work in legacy mode.
From: Jason Wang <hidden> Date: 2021-05-14 07:12:31
On Fri, May 14, 2021 at 6:50 AM Willem de Bruijn [off-list ref] wrote:
On Mon, Feb 8, 2021 at 1:56 PM Willem de Bruijn
[off-list ref] wrote:
quoted
From: Willem de Bruijn <willemb@google.com>
RFCv2 for four new features to the virtio network device:
1. pass tx flow state to host, for routing + telemetry
2. pass rx tstamp to guest, for better RTT estimation
3. pass tx tstamp to guest, idem
3. pass tx delivery time to host, for accurate pacing
All would introduce an extension to the virtio spec.
Concurrently with code review I will write ballots to
https://www.oasis-open.org/committees/ballots.php?wg_abbrev=virtio
These changes are to the driver side. Evaluation additionally requires
achanges to qemu and at least one back-end. I implemented preliminary
support in Linux vhost-net. Both patches available through github at
https://github.com/wdebruij/linux/tree/virtio-net-txhash-2https://github.com/wdebruij/qemu/tree/virtio-net-txhash-2
Changes RFC -> RFCv2
- add transmit timestamp patch
- see individual patches for other changes
Willem de Bruijn (4):
virtio-net: support transmit hash report
virtio-net: support receive timestamp
virtio-net: support transmit timestamp
virtio-net: support future packet transmit time
Seeing Yuri's patchset adding new features reminded me that I did not
follow-up on this patch series on the list.
The patches themselves are mostly in good shape. The last tx tstamp
issue can be resolved.
But the device implementation I target only supports legacy mode.
Below conversation that we had in one of the patches makes clear that
supporting this in legacy is not feasible. Nor is upgrading that
device in the short term. Until there is a device implementation that
implements these offloads, these features are a dead letter. Not moving
forward for now.
Somewhat related: is there a plan for when we run out of 64 feature bits?
A quick thought: we need add (or reserve) a new feature bit to
indicate that we need more bits, and have transport specific
implementation of those extra bits negotiation. E.g for PCI, we can
introduce new fields in the capability.
Thanks
quoted
quoted
quoted
Actually, would it be possible to make new features available on
legacy devices? There is nothing in the features bits precluding it.
I think it won't be possible: you are using feature bit 55,
legacy devices have up to 32 feature bits. And of course the
header looks a bit differently for legacy, you would have to add special
code to handle that when mergeable buffers are off.
I think I can make the latter work. I did start without a dependency
on the v1 header initially.
Feature bit array length I had not considered. Good point. Need to
think about that. It would be very appealing if in particular the
tx-hash feature could work in legacy mode.
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com> Date: 2021-05-14 12:47:41
On Fri, May 14, 2021 at 3:12 AM Jason Wang [off-list ref] wrote:
On Fri, May 14, 2021 at 6:50 AM Willem de Bruijn [off-list ref] wrote:
quoted
On Mon, Feb 8, 2021 at 1:56 PM Willem de Bruijn
[off-list ref] wrote:
quoted
From: Willem de Bruijn <willemb@google.com>
RFCv2 for four new features to the virtio network device:
1. pass tx flow state to host, for routing + telemetry
2. pass rx tstamp to guest, for better RTT estimation
3. pass tx tstamp to guest, idem
3. pass tx delivery time to host, for accurate pacing
All would introduce an extension to the virtio spec.
Concurrently with code review I will write ballots to
https://www.oasis-open.org/committees/ballots.php?wg_abbrev=virtio
These changes are to the driver side. Evaluation additionally requires
achanges to qemu and at least one back-end. I implemented preliminary
support in Linux vhost-net. Both patches available through github at
https://github.com/wdebruij/linux/tree/virtio-net-txhash-2https://github.com/wdebruij/qemu/tree/virtio-net-txhash-2
Changes RFC -> RFCv2
- add transmit timestamp patch
- see individual patches for other changes
Willem de Bruijn (4):
virtio-net: support transmit hash report
virtio-net: support receive timestamp
virtio-net: support transmit timestamp
virtio-net: support future packet transmit time
Seeing Yuri's patchset adding new features reminded me that I did not
follow-up on this patch series on the list.
The patches themselves are mostly in good shape. The last tx tstamp
issue can be resolved.
But the device implementation I target only supports legacy mode.
Below conversation that we had in one of the patches makes clear that
supporting this in legacy is not feasible. Nor is upgrading that
device in the short term. Until there is a device implementation that
implements these offloads, these features are a dead letter. Not moving
forward for now.
Somewhat related: is there a plan for when we run out of 64 feature bits?
A quick thought: we need add (or reserve) a new feature bit to
indicate that we need more bits, and have transport specific
implementation of those extra bits negotiation. E.g for PCI, we can
introduce new fields in the capability.
Thanks Jason. Yes, that makes sense to me.
The difference from 32 to 64 bit between virtio_pci_legacy.c and
virtio_pci_modern.c is a good example:
static u64 vp_get_features(struct virtio_device *vdev)
{
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
/* When someone needs more than 32 feature bits, we'll need to
* steal a bit to indicate that the rest are somewhere else. */
return ioread32(vp_dev->ioaddr + VIRTIO_PCI_HOST_FEATURES);
}
u64 vp_modern_get_features(struct virtio_pci_modern_device *mdev)
{
struct virtio_pci_common_cfg __iomem *cfg = mdev->common;
u64 features;
vp_iowrite32(0, &cfg->device_feature_select);
features = vp_ioread32(&cfg->device_feature);
vp_iowrite32(1, &cfg->device_feature_select);
features |= ((u64)vp_ioread32(&cfg->device_feature) << 32);
return features;
}
device_feature_select is a 32-bit field, of which only values 0 and 1
are defined so far, per the virtio 1.1 spec:
"
device_feature_select
The driver uses this to select which feature bits device_feature
shows. Value 0x0 selects Feature Bits 0 to 31, 0x1 selects Feature
Bits 32 to 63, etc.
"
That leaves plenty of room for expansion, at least for pci devices.
quoted
quoted
quoted
quoted
Actually, would it be possible to make new features available on
legacy devices? There is nothing in the features bits precluding it.
I think it won't be possible: you are using feature bit 55,
legacy devices have up to 32 feature bits. And of course the
header looks a bit differently for legacy, you would have to add special
code to handle that when mergeable buffers are off.
I think I can make the latter work. I did start without a dependency
on the v1 header initially.
Feature bit array length I had not considered. Good point. Need to
think about that. It would be very appealing if in particular the
tx-hash feature could work in legacy mode.