On Mon, Feb 27, 2017 at 10:23 AM, Miroslav Lichvar <mlich...@redhat.com> wrote:
> On Tue, Feb 07, 2017 at 02:32:04PM -0800, Willem de Bruijn wrote:
>> >> 4) allow sockets to use both SW and HW TX timestamping at the same time
>> >>
>> >> When using a socket which is not bound to a specific interface, it
>> >> would be nice to get transmit SW timestamps when HW timestamps are
>> >> missing. I suspect it's difficult to predict if a HW timestamp will
>> >> be available. Maybe it would be acceptable to get from the error
>> >> queue two messages per transmission if the interface supports both
>> >> SW and HW timestamping?
>> >
>> >
>> > This seems useful,
>>
>> Agreed, as long as it is optional so that it does not change the
>> behavior for existing applications.
>
> Do you think it is safe to assume that no application enabled both SW
> and HW TX timestamping?
We cannot rule out that a process set both flags.
> Do we need a new option for this?
Similar to OPT_TSONLY or OPT_ID, but to signal the intent of
receiving both timestamps. Yes, agreed.
>> > but not sure how best to implement it.
>>
>> It might be sufficient to just remove the second line in sw_tx_timestamp
>>
>> static inline void sw_tx_timestamp(struct sk_buff *skb)
>> {
>> if (skb_shinfo(skb)->tx_flags & SKBTX_SW_TSTAMP &&
>> !(skb_shinfo(skb)->tx_flags & SKBTX_IN_PROGRESS))
>> skb_tstamp_tx(skb, NULL);
>> }
>
> With this change I'm getting two error messages per transmission, but
> it looks like it may need some additional changes.
>
> If the first error message is received after the HW timestamp was
> captured,
When does this happen? The first timestamp is generated from
skb_tx_timestamp in the device driver's ndo_start_xmit before
passing the packet to the NIC, the second when the device
driver cleans the tx descriptor on completion.
Is this for drivers that do not have skb_tx_timestamp, as you
mention below? Then the solution is to add that call.
> it contains both timestamps as the HW timestamp is in the
> shared info of the skb. Is it possible it could contain a partially
> updated HW timestamp? I'm not sure how locking works here. Is
> scm_timestamping actually allowed to contain more than one timestamp?
> The timestamping.txt document says "Only one field is non-zero at any
> time.", but that wasn't true even before if both SW and HW RX
> timestamping was enabled.
>
> If SO_TIMESTAMP{,NS} is enabled, ts[0] in the second error message
> will contain a bogus SW timestamp added by __sock_recv_timestamp() for
> a "Race occurred between timestamp enabling and packet receiving". Is
Good point. That should not be set on transmit timestamps.
> there a guarantee applications will get a timestamp for all messages
> after enabling SO_TIMESTAMP? The original code is older than the git
> repo, so I'm not sure what was the reason for this. To me it would
> make more sense to not add any SCM_TIMESTAMP (and SW timestamp in
> SCM_TIMESTAMPING) when the the timestamp is missing. If that's not
> always acceptable, maybe it could be restricted to sockets that have
> HW timestamping enabled?
I would limit scope to tx timestamping and leave rx semantics as is.
> Some drivers don't call skb_tx_timestamp() when HW timestamp was
> requested. From a cursory look it is e1000e, xgbe, sxgbe, and stmmac.
> This should hopefully be an easy fix.
Indeed. that should be added, then.
