With the ongoing work to add BQL to Socket CAN, I figured out that it
would be nice to have an easy way to mesure the latency.
And one easy way to do so it to check the round trip time of the
packet by doing the difference between the software rx timestamp and
the software tx timestamp.
rx timestamps are already available. This patch gives the missing
piece: add a tx software timestamp feature to the CAN devices.
Of course, the tx software timestamp might also be used for other
purposes such as performance measurements of the different queuing
disciplines (e.g. by checking the difference between the kernel tx
software timestamp and the userland tx software timestamp).
v2 was a mistake, please ignore it (fogot to do git add, changes were
not reflected...)
v3 reflects the comments that Jeroen made in
https://lkml.org/lkml/2021/1/10/54
Vincent Mailhol (1):
can: dev: add software tx timestamps
drivers/net/can/dev.c | 1 +
1 file changed, 1 insertion(+)
--
2.26.2
Call skb_tx_timestamp() within can_put_echo_skb() so that a software
tx timestamp gets attached on the skb.
There two main reasons to include this call in can_put_echo_skb():
* It easily allow to enable the tx timestamp on all devices with
just one small change.
* According to Documentation/networking/timestamping.rst, the tx
timestamps should be generated in the device driver as close as
possible, but always prior to passing the packet to the network
interface. During the call to can_put_echo_skb(), the skb gets
cloned meaning that the driver should not dereference the skb
variable anymore after can_put_echo_skb() returns. This makes
can_put_echo_skb() the very last place we can use the skb without
having to access the echo_skb[] array.
Remarks:
* By default, skb_tx_timestamp() does nothing. It needs to be
activated by passing the SOF_TIMESTAMPING_TX_SOFTWARE flag either
through socket options or control messages.
* The hardware rx timestamp of a local loopback message is the
hardware tx timestamp. This means that there are no needs to
implement SOF_TIMESTAMPING_TX_HARDWARE for CAN sockets.
References:
Support for the error queue in CAN RAW sockets (which is needed for tx
timestamps) was introduced in:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=eb88531bdbfaafb827192d1fc6c5a3fcc4fadd96
Put the call to skb_tx_timestamp() just before adding it to the array:
https://lkml.org/lkml/2021/1/10/54
Signed-off-by: Vincent Mailhol <[email protected]>
---
drivers/net/can/dev.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index 3486704c8a95..850759c7677f 100644
--- a/drivers/net/can/dev.c
+++ b/drivers/net/can/dev.c
@@ -481,6 +481,7 @@ int can_put_echo_skb(struct sk_buff *skb, struct net_device *dev,
skb->pkt_type = PACKET_BROADCAST;
skb->ip_summed = CHECKSUM_UNNECESSARY;
skb->dev = dev;
+ skb_tx_timestamp(skb);
/* save this skb for tx interrupt echo handling */
priv->echo_skb[idx] = skb;
--
2.26.2
On 1/10/21 1:49 PM, Vincent Mailhol wrote:
> Call skb_tx_timestamp() within can_put_echo_skb() so that a software
> tx timestamp gets attached on the skb.
>
> There two main reasons to include this call in can_put_echo_skb():
>
> * It easily allow to enable the tx timestamp on all devices with
> just one small change.
>
> * According to Documentation/networking/timestamping.rst, the tx
> timestamps should be generated in the device driver as close as
> possible, but always prior to passing the packet to the network
> interface. During the call to can_put_echo_skb(), the skb gets
> cloned meaning that the driver should not dereference the skb
> variable anymore after can_put_echo_skb() returns. This makes
> can_put_echo_skb() the very last place we can use the skb without
> having to access the echo_skb[] array.
>
> Remarks:
>
> * By default, skb_tx_timestamp() does nothing. It needs to be
> activated by passing the SOF_TIMESTAMPING_TX_SOFTWARE flag either
> through socket options or control messages.
>
> * The hardware rx timestamp of a local loopback message is the
> hardware tx timestamp. This means that there are no needs to
> implement SOF_TIMESTAMPING_TX_HARDWARE for CAN sockets.
>
> References:
>
> Support for the error queue in CAN RAW sockets (which is needed for tx
> timestamps) was introduced in:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=eb88531bdbfaafb827192d1fc6c5a3fcc4fadd96
>
> Put the call to skb_tx_timestamp() just before adding it to the array:
> https://lkml.org/lkml/2021/1/10/54
>
> Signed-off-by: Vincent Mailhol <[email protected]>
Applied to linux-can-next/testing (ontop of my dev infrastructure cleanup).
regards,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung West/Dortmund | Phone: +49-231-2826-924 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On Sun, Jan 10, 2021 at 09:49:03PM +0900, Vincent Mailhol wrote:
> * The hardware rx timestamp of a local loopback message is the
> hardware tx timestamp. This means that there are no needs to
> implement SOF_TIMESTAMPING_TX_HARDWARE for CAN sockets.
I can't agree with that statement. The local loopback is a special
"feature" of CAN sockets, and some programs turn it off. Furthermore,
requiring user space to handle CAN sockets differently WRT Tx time
stamps is user-unfriendly. So I would strongly support adding
SOF_TIMESTAMPING_TX_HARDWARE to the CAN layer in the future.
(This isn't a criticism of the current patch, though.)
Thanks,
Richard
On Tue. 12 Jan 2021 at 02:11, Richard Cochran <[email protected]> wrote:
>
> On Sun, Jan 10, 2021 at 09:49:03PM +0900, Vincent Mailhol wrote:
> > * The hardware rx timestamp of a local loopback message is the
> > hardware tx timestamp. This means that there are no needs to
> > implement SOF_TIMESTAMPING_TX_HARDWARE for CAN sockets.
>
> I can't agree with that statement. The local loopback is a special
> "feature" of CAN sockets, and some programs turn it off. Furthermore,
> requiring user space to handle CAN sockets differently WRT Tx time
> stamps is user-unfriendly. So I would strongly support adding
> SOF_TIMESTAMPING_TX_HARDWARE to the CAN layer in the future.
>
> (This isn't a criticism of the current patch, though.)
Fair enough. Implementing SOF_TIMESTAMPING_TX_HARDWARE would
result into having the timestamp being duplicated for the
loopback frames but allowing existing programs to work as
with no modifications is a good enough reason.
Out of curiosity, which programs do you use? I guess wireshark
but please let me know if you use any other programs (I just use
to write a small C program to do the stuff).
Mark: do you want me to send a v4 of that patch with above
comment removed or can you directly do the change in your testing
branch?
Yours sincerely,
Vincent
On Tue, Jan 12, 2021 at 09:00:33AM +0900, Vincent MAILHOL wrote:
> Out of curiosity, which programs do you use? I guess wireshark
> but please let me know if you use any other programs (I just use
> to write a small C program to do the stuff).
I was thinking of PTP over DeviceNET (which, in turn, is over CAN).
This is specified in Annex G of IEEE 1588.
The linuxptp stack has modular design and could one day support
DeviceNET. It would be much easier for linuxptp if CAN interfaces
support hardware time stamping in the same way as other network
interfaces.
Thanks,
Richard
On Tue. 12 Jan 2021 at 11:14, Richard Cochran <[email protected]> wrote:
>
> On Tue, Jan 12, 2021 at 09:00:33AM +0900, Vincent MAILHOL wrote:
> > Out of curiosity, which programs do you use? I guess wireshark
> > but please let me know if you use any other programs (I just use
> > to write a small C program to do the stuff).
>
> I was thinking of PTP over DeviceNET (which, in turn, is over CAN).
> This is specified in Annex G of IEEE 1588.
>
> The linuxptp stack has modular design and could one day support
> DeviceNET. It would be much easier for linuxptp if CAN interfaces
> support hardware time stamping in the same way as other network
> interfaces.
I actually also thought of implementing PTP but for a slightly
different goal: synchronise the clock of the different CAN
controllers connected to a same Linux host. But so far, it is
just a rough idea and I never looked really deep into the
technical details of linuxptp.
However, I did not know about DeviceNET. I am not really aware of
the use cases for industrial automation applications.
Glad to have asked, learnt something :)
Yours sincerely,
Vincent
On 1/12/21 1:00 AM, Vincent MAILHOL wrote:
[...]
> Mark: do you want me to send a v4 of that patch with above
> comment removed or can you directly do the change in your testing
> branch?
Please send a patch on-top of linux-can-next/testing
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung West/Dortmund | Phone: +49-231-2826-924 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On Tue. 12 Jan 2021 at 16:58, Marc Kleine-Budde <[email protected]> wrote:
>
> On 1/12/21 1:00 AM, Vincent MAILHOL wrote:
> [...]
>
> > Mark: do you want me to send a v4 of that patch with above
> > comment removed or can you directly do the change in your testing
> > branch?
>
> Please send a patch on-top of linux-can-next/testing
Done: https://lore.kernel.org/linux-can/[email protected]/
Yours sincerely,
Vincent