On Thu, May 18, 2017 at 9:06 AM, Miroslav Lichvar <mlich...@redhat.com> wrote:
> Add SOF_TIMESTAMPING_OPT_TX_SWHW option to allow an outgoing packet to
> be looped to the socket's error queue with a software timestamp even
> when a hardware transmit timestamp is expected to be provided by the
> driver.
>
> Applications using this option will receive two separate messages from
> the error queue, one with a software timestamp and the other with a
> hardware timestamp. As the hardware timestamp is saved to the shared skb
> info, which may happen before the first message with software timestamp
> is received by the application, the hardware timestamp is copied to the
> SCM_TIMESTAMPING control message only when the skb has no software
> timestamp or it is an incoming packet.
>
> While changing sw_tx_timestamp(), inline it in skb_tx_timestamp() as
> there are no other users.
>
> CC: Richard Cochran <richardcoch...@gmail.com>
> CC: Willem de Bruijn <will...@google.com>
> Signed-off-by: Miroslav Lichvar <mlich...@redhat.com>
> ---
> +/* On transmit, software and hardware timestamps are returned independently.
> + * As the two skb clones share the hardware timestamp, which may be updated
> + * before the software timestamp is received, a hardware TX timestamp may be
> + * returned only if there is no software TX timestamp. A false software
> + * timestamp made for SOCK_RCVTSTAMP when a real timestamp is missing must
> + * be ignored.
Please expand on why this case can be ignored. It is quite subtle. How about
something like
*
* A false software timestamp is one made inside the __sock_recv_timestamp
* call itself. These are generated whenever SO_TIMESTAMP(NS) is enabled
* on the socket, even when the timestamp reported is for another option, such
* as hardware tx timestamp.
*
* Ignore these when deciding whether a timestamp source is hw or sw.
*/
And perhaps move the comment to the branch itself.
> + */
> +static bool skb_is_swtx_tstamp(const struct sk_buff *skb,
> + const struct sock *sk, int false_tstamp)
> +{
> + if (false_tstamp && sk->sk_tsflags & SOF_TIMESTAMPING_OPT_TX_SWHW)
Also, why is it ignored only for the new mode?
> + return 0;
> +
> + return skb->tstamp && skb_is_err_queue(skb);
> +}
