Hi Pauli, On Sun, Feb 9, 2025 at 6:40 PM Pauli Virtanen <pav@xxxxxx> wrote: > > Add SOF_TIMESTAMPING_TX_COMPLETION, for requesting a software timestamp > when hardware reports a packet completed. > > Completion tstamp is useful for Bluetooth, where hardware tx timestamps > cannot be obtained except for ISO packets, and the hardware has a queue Could you say more about why the hw timestamp cannot be obtained? > 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. It's worth mentioning the earlier discussion that took place last year in the commit log. It's hard to retrieve such an important discussion buried in the mailing list, which should be helpful for people to get to know/recall the exact background :) https://lore.kernel.org/all/6642c7f3427b5_20539c2949a@xxxxxxxxxxxxxxxxxxxxxx.notmuch/. https://lore.kernel.org/all/cover.1710440392.git.pav@xxxxxx/ And it's also good to attach the real use case from the following link for readers to know the exact case :) https://lore.kernel.org/all/7ade362f178297751e8a0846e0342d5086623edc.camel@xxxxxx/ Quoting here: " sendmsg() from user generates skbs to net/bluetooth side queue | * wait in net/bluetooth side queue until HW has free packet slot | * send to driver (-> SCM_TSTAMP_SCHED*) | * driver (usu. ASAP) queues to transport e.g. USB | * transport tx complete, skb freed | * packet waits in hardware-side buffers (usu. the largest delay) | * packet completion report from HW (-> SCM_TSTAMP_SND*) | * for one packet type, HW timestamp for last tx packet can queried The packet completion report does not imply the packet was received. " > Signed-off-by: Pauli Virtanen <pav@xxxxxx> > --- > Documentation/networking/timestamping.rst | 9 +++++++++ > include/linux/skbuff.h | 6 +++++- > include/uapi/linux/errqueue.h | 1 + > include/uapi/linux/net_tstamp.h | 6 ++++-- > net/ethtool/common.c | 1 + > net/socket.c | 3 +++ > 6 files changed, 23 insertions(+), 3 deletions(-) > > diff --git a/Documentation/networking/timestamping.rst b/Documentation/networking/timestamping.rst > index 61ef9da10e28..de2afed7a516 100644 > --- a/Documentation/networking/timestamping.rst > +++ b/Documentation/networking/timestamping.rst > @@ -140,6 +140,15 @@ 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 > + 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. The completion timestamps are > + currently implemented only for: Bluetooth L2CAP and ISO. This > + flag can be enabled via both socket options and control messages. > + I'd like to know if this flag can also be applied to NICs which have already implemented the hardware timestamp, like Intel i40e, no? Thanks, Jason > > 1.3.2 Timestamp Reporting > ^^^^^^^^^^^^^^^^^^^^^^^^^ > diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h > index bb2b751d274a..3707c9075ae9 100644 > --- a/include/linux/skbuff.h > +++ b/include/linux/skbuff.h > @@ -489,10 +489,14 @@ enum { > > /* generate software time stamp when entering packet scheduling */ > SKBTX_SCHED_TSTAMP = 1 << 6, > + > + /* generate software time stamp on packet tx completion */ > + SKBTX_COMPLETION_TSTAMP = 1 << 7, > }; > > #define SKBTX_ANY_SW_TSTAMP (SKBTX_SW_TSTAMP | \ > - SKBTX_SCHED_TSTAMP) > + SKBTX_SCHED_TSTAMP | \ > + SKBTX_COMPLETION_TSTAMP) > #define SKBTX_ANY_TSTAMP (SKBTX_HW_TSTAMP | \ > SKBTX_HW_TSTAMP_USE_CYCLES | \ > SKBTX_ANY_SW_TSTAMP) > diff --git a/include/uapi/linux/errqueue.h b/include/uapi/linux/errqueue.h > index 3c70e8ac14b8..1ea47309d772 100644 > --- a/include/uapi/linux/errqueue.h > +++ b/include/uapi/linux/errqueue.h > @@ -73,6 +73,7 @@ enum { > SCM_TSTAMP_SND, /* driver passed skb to NIC, or HW */ > SCM_TSTAMP_SCHED, /* data entered the packet scheduler */ > SCM_TSTAMP_ACK, /* data acknowledged by peer */ > + SCM_TSTAMP_COMPLETION, /* packet tx completion */ > }; > > #endif /* _UAPI_LINUX_ERRQUEUE_H */ > diff --git a/include/uapi/linux/net_tstamp.h b/include/uapi/linux/net_tstamp.h > index 55b0ab51096c..383213de612a 100644 > --- a/include/uapi/linux/net_tstamp.h > +++ b/include/uapi/linux/net_tstamp.h > @@ -44,8 +44,9 @@ enum { > SOF_TIMESTAMPING_BIND_PHC = (1 << 15), > SOF_TIMESTAMPING_OPT_ID_TCP = (1 << 16), > SOF_TIMESTAMPING_OPT_RX_FILTER = (1 << 17), > + SOF_TIMESTAMPING_TX_COMPLETION = (1 << 18), > > - SOF_TIMESTAMPING_LAST = SOF_TIMESTAMPING_OPT_RX_FILTER, > + SOF_TIMESTAMPING_LAST = SOF_TIMESTAMPING_TX_COMPLETION, > SOF_TIMESTAMPING_MASK = (SOF_TIMESTAMPING_LAST - 1) | > SOF_TIMESTAMPING_LAST > }; > @@ -58,7 +59,8 @@ enum { > #define SOF_TIMESTAMPING_TX_RECORD_MASK (SOF_TIMESTAMPING_TX_HARDWARE | \ > SOF_TIMESTAMPING_TX_SOFTWARE | \ > SOF_TIMESTAMPING_TX_SCHED | \ > - SOF_TIMESTAMPING_TX_ACK) > + SOF_TIMESTAMPING_TX_ACK | \ > + SOF_TIMESTAMPING_TX_COMPLETION) > > /** > * struct so_timestamping - SO_TIMESTAMPING parameter > diff --git a/net/ethtool/common.c b/net/ethtool/common.c > index 2bd77c94f9f1..75e3b756012e 100644 > --- a/net/ethtool/common.c > +++ b/net/ethtool/common.c > @@ -431,6 +431,7 @@ const char sof_timestamping_names[][ETH_GSTRING_LEN] = { > [const_ilog2(SOF_TIMESTAMPING_BIND_PHC)] = "bind-phc", > [const_ilog2(SOF_TIMESTAMPING_OPT_ID_TCP)] = "option-id-tcp", > [const_ilog2(SOF_TIMESTAMPING_OPT_RX_FILTER)] = "option-rx-filter", > + [const_ilog2(SOF_TIMESTAMPING_TX_COMPLETION)] = "completion-transmit", > }; > static_assert(ARRAY_SIZE(sof_timestamping_names) == __SOF_TIMESTAMPING_CNT); > > diff --git a/net/socket.c b/net/socket.c > index 4afe31656a2b..22b7f6f50889 100644 > --- a/net/socket.c > +++ b/net/socket.c > @@ -693,6 +693,9 @@ void __sock_tx_timestamp(__u32 tsflags, __u8 *tx_flags) > if (tsflags & SOF_TIMESTAMPING_TX_SCHED) > flags |= SKBTX_SCHED_TSTAMP; > > + if (tsflags & SOF_TIMESTAMPING_TX_COMPLETION) > + flags |= SKBTX_COMPLETION_TSTAMP; > + > *tx_flags = flags; > } > EXPORT_SYMBOL(__sock_tx_timestamp); > -- > 2.48.1 > >