netdev - Re: [PATCH v5 1/5] net-timestamp: COMPLETION timestamp on packet tx completion

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <67dad6c5b6ce8_594829443@willemb.c.googlers.com.notmuch>
Date: Wed, 19 Mar 2025 10:37:57 -0400
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: Pauli Virtanen <pav@....fi>, 
 linux-bluetooth@...r.kernel.org
Cc: Pauli Virtanen <pav@....fi>, 
 Luiz Augusto von Dentz <luiz.dentz@...il.com>, 
 netdev@...r.kernel.org, 
 davem@...emloft.net, 
 kuba@...nel.org, 
 willemdebruijn.kernel@...il.com
Subject: Re: [PATCH v5 1/5] net-timestamp: COMPLETION timestamp on packet tx
 completion

Pauli Virtanen wrote:
> Add SOF_TIMESTAMPING_TX_COMPLETION, for requesting a software timestamp
> when hardware reports a packet completed.
> 
> Completion tstamp is useful for Bluetooth, as hardware timestamps do not
> exist in the HCI specification except for ISO packets, and the hardware
> has a queue where packets may wait.  In this case the software SND
> timestamp only reflects the kernel-side part of the total latency
> (usually small) and queue length (usually 0 unless HW buffers
> congested), whereas the completion report time is more informative of
> the true latency.
> 
> It may also be useful in other cases where HW TX timestamps cannot be
> obtained and user wants to estimate an upper bound to when the TX
> probably happened.
> 
> Signed-off-by: Pauli Virtanen <pav@....fi>
> ---
> 
> Notes:
>     v5:
>     - back to decoupled COMPLETION & SND, like in v3
>     - BPF reporting not implemented here
> 
>  Documentation/networking/timestamping.rst | 8 ++++++++
>  include/linux/skbuff.h                    | 7 ++++---
>  include/uapi/linux/errqueue.h             | 1 +
>  include/uapi/linux/net_tstamp.h           | 6 ++++--
>  net/core/skbuff.c                         | 2 ++
>  net/ethtool/common.c                      | 1 +
>  net/socket.c                              | 3 +++
>  7 files changed, 23 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/networking/timestamping.rst b/Documentation/networking/timestamping.rst
> index 61ef9da10e28..b8fef8101176 100644
> --- a/Documentation/networking/timestamping.rst
> +++ b/Documentation/networking/timestamping.rst
> @@ -140,6 +140,14 @@ SOF_TIMESTAMPING_TX_ACK:
>    cumulative acknowledgment. The mechanism ignores SACK and FACK.
>    This flag can be enabled via both socket options and control messages.
>  
> +SOF_TIMESTAMPING_TX_COMPLETION:
> +  Request tx timestamps on packet tx completion.  The completion

Minor: double space above, grammar issue below "receives [packet] a".

> +  timestamp is generated by the kernel when it receives packet a
> +  completion report from the hardware. Hardware may report multiple
> +  packets at once, and completion timestamps reflect the timing of the
> +  report and not actual tx time. This flag can be enabled via both
> +  socket options and control messages.
> +

Otherwise the patch LGTM.