[RFC] net: Allow setting clock type for network timestamps

[RFC] net: Allow setting clock type for network timestamps

[Ricardo Biehl Pasquali]

Network timestamps are always from REALTIME clock, which
may be adjusted by the user (mainly Network Time Protocol
corrections though).

For applications that do not want this behavior, an option
for setting clock type is needed.

The sound subsystem has already an option to set the clock
type for its timestamps. See SNDRV_PCM_TSTAMP_TYPE_* in
include/uapi/sound/asound.h and snd_pcm_gettime() in
include/sound/pcm.h .

While preparing a small set of changes I found some issues.

Brief of the changes:

- Add SO_TIMESTAMP_CLOCK socket option macro.

- Add macros for clock types:
	SOF_TIMESTAMP_CLOCK_REALTIME,
	SOF_TIMESTAMP_CLOCK_MONOTONIC,
	SOF_TIMESTAMP_CLOCK_MONOTONIC_RAW,

- Add set/get for the option. Initialize to REALTIME.

- In __net_timestamp(), get the time according to
  sk_timestamp_clock in 'struct sock'.

Here "CLOCK" is used instead of "TYPE" to avoid confusion
with receive/transmit, hardware/software options.

Issues:

- Are there problems or security issues that may arise if
  changing the clock in __net_timestamp()?

- __net_timestamp() has no access to 'struct sock'. One
  alternative is move it from linux/skbuff.h to net/sock.h

- Do every caller of __net_timestamp() has skb->sk set?

  If adding 'struct sock*' as argument to
  __net_timestamp(struct sk_buff*), in places like
  net/ipv6/exthdrs.c ipv6_dest_hao(), skb->sk must be set
  anyway. (see next)

- Should __net_timestamp() be used in __skb_tstamp_tx()?

  In __skb_tstamp_tx(), for example, skb->sk would not have
  been set at the moment of __net_timestamp() call. It is
  set in sock_queue_err_skb(), called inside
  __skb_complete_tx_timestamp().

In short, there must be a function which has access to
'struct sk_buff' and 'struct sock' and can safely set any
clock type. At the same time that this should keep the
code as simple as possible.

Non-implementation issues:

- Code and macro duplication existing in multiple
  subsystems that set clock type.

  The sound have SNDRV_PCM_TSTAMP_TYPE_* + snd_pcm_gettime()
  and the network SOF_TIMESTAMP_CLOCK_* + __net_timestamp().
  This is somewhat similar to the NONBLOCK flag (e.g.
  TFD_NONBLOCK, EFD_NONBLOCK), however both point to
  O_NONBLOCK. Could this be improved?

  What about using CLOCK_* macros? Note that the sound
  subsystem is not using them, otherwise this may break
  Application Binary Interface.

- Rename __net_timestamp() to skb_timestamp() or something
  more clear/specific and perhaps similar to other parts of
  the kernel.

	pasquali

Re: [RFC] net: Allow setting clock type for network timestamps

[Willem de Bruijn]

On Tue, Dec 11, 2018 at 10:20 AM Ricardo Biehl Pasquali
<pasqualirb@gmail.com> wrote:
>
> Network timestamps are always from REALTIME clock, which
> may be adjusted by the user (mainly Network Time Protocol
> corrections though).
>
> For applications that do not want this behavior, an option
> for setting clock type is needed.
>
> The sound subsystem has already an option to set the clock
> type for its timestamps. See SNDRV_PCM_TSTAMP_TYPE_* in
> include/uapi/sound/asound.h and snd_pcm_gettime() in
> include/sound/pcm.h .
>
> While preparing a small set of changes I found some issues.
>
> Brief of the changes:
>
> - Add SO_TIMESTAMP_CLOCK socket option macro.
>
> - Add macros for clock types:
>         SOF_TIMESTAMP_CLOCK_REALTIME,
>         SOF_TIMESTAMP_CLOCK_MONOTONIC,
>         SOF_TIMESTAMP_CLOCK_MONOTONIC_RAW,
>
> - Add set/get for the option. Initialize to REALTIME.
>
> - In __net_timestamp(), get the time according to
>   sk_timestamp_clock in 'struct sock'.

__net_timestamp is called early in the receive path before socket
lookup, in netif_receive_skb_internal through net_timestamp_check.

It is not possible to choose the clocktype based on destination
socket at this stage.

> - Do every caller of __net_timestamp() has skb->sk set?

See above, no.