On Tue, Feb 9, 2021 at 11:15 PM Jason Wang <jasowang@xxxxxxxxxx> wrote:
>
>
> On 2021/2/10 上午10:36, Willem de Bruijn wrote:
> > On Tue, Feb 9, 2021 at 11:39 AM Michael S. Tsirkin<mst@xxxxxxxxxx> wrote:
> >> On Tue, Feb 09, 2021 at 01:45:11PM +0800, Jason Wang wrote:
> >>> On 2021/2/9 上午2:55, Willem de Bruijn wrote:
> >>>> From: Willem de Bruijn<willemb@xxxxxxxxxx>
> >>>>
> >>>> 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@xxxxxxxxxx>
> >>>> ---
> >>>> 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(-)
> >>>>
> >>>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> >>>> index ac44c5efa0bc..fc8ecd3a333a 100644
> >>>> --- a/drivers/net/virtio_net.c
> >>>> +++ b/drivers/net/virtio_net.c
> >>>> @@ -210,6 +210,12 @@ struct virtnet_info {
> >>>> /* Device will pass rx timestamp. Requires has_rx_tstamp */
> >>>> bool enable_rx_tstamp;
> >>>> + /* Device can pass CLOCK_TAI transmit time to the driver */
> >>>> + bool has_tx_tstamp;
> >>>> +
> >>>> + /* Device will pass tx timestamp. Requires has_tx_tstamp */
> >>>> + bool enable_tx_tstamp;
> >>>> +
> >>>> /* Has control virtqueue */
> >>>> bool has_cvq;
> >>>> @@ -1401,6 +1407,20 @@ static int virtnet_receive(struct receive_queue *rq, int budget,
> >>>> return stats.packets;
> >>>> }
> >>>> +static void virtnet_record_tx_tstamp(const struct send_queue *sq,
> >>>> + struct sk_buff *skb)
> >>>> +{
> >>>> + const struct virtio_net_hdr_hash_ts *h = skb_vnet_hdr_ht(skb);
> >>>> + const struct virtnet_info *vi = sq->vq->vdev->priv;
> >>>> + struct skb_shared_hwtstamps ts;
> >>>> +
> >>>> + 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.
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
