Fixing stale Tx timestamp fetch.

[mgalka]

It fixes daemon_cl behavior in the following situation.

1. Sent gPTP message.
2. Try to get the timestamp.
3. Repeat no. 2 until timeout occurs.
4. Timeout occurs, continue operation.
5. Timestamp comes (but it's not read).
6. Sent another gPTP message.
7. Try to get the timestamp.
8. Read the timestamp -> Timestamp from message no. 1 is being read.
9. Time synchronization fails due to the wrong Tx timestamp.

This fix discards stale Tx timestamps. It checks message type and sequence id of the reflected packet to see if it contains the timestamp for the frame being sent at the moment. In case it doesn't the timestamp is discarded.

[andrew-elder]

@mgalka, thank you for this pull request at the detailed notes. I assume you have actually observed what you are fixing here?

[mgalka]

@andrew-elder Yes, I've observed the behaviour on a non-PC device. I couldn't reproduce the it though on a regular PC with i210 NIC, probably due to being unable to reproduce I/O load high enough to get the PDelay timeout to occur.

[pinealservo]

Thanks for the fix!

If you wouldn't mind, it would be helpful to add some sort of documentation about what you're doing; e.g. why pick 46 for the length of reflected_bytes, or why add 14 to reflected_bytes to get gptpCommonHeader? Comments/#defines would be fine.

I think I know the answers, but it took a while to check up on them and it would be helpful to future reviewers to not have to wonder about them.

[andrew-elder]

@mgalka - are you going to be able to address @pinealservo's comments? I really do appreciate this pull request and look forward to getting it merged!

[mgalka]

@pinealservo @andrew-elder
I changed the code so it's a bit more self-explanatory.

Let's keep the explanation here also for the sake of clarity:

The size of the reflected_bytes is just enough to store the ethernet and ptp common headers. It's enough to get the Sequence Id and Message Type. At first, it took just 46 bytes, because we don't even need the whole PTP Common Header, now I changed it to the full header (2 bytes more than previously) to keep the code more clear.

I added a define ETHER_HDR_LEN for the Ethernet Header length. Previously it was kept as as a magic number 14, which was not the best idea.

I used PTP_COMMON_HDR_TRANSSPEC_MSGTYPE to get the Message Type from the frame. This macro basically does nothing, it's just there to keep the code more consistent.

[andrew-elder]

@mgalka - looks good to me. I'm merge in the next 24 hours if I don't see any other comments. @christopher-s-hall - do you have any comments on this?

[pinealservo]

Thanks for the update; it's definitely clearer now.

[christopher-s-hall]

This looks fine to me assuming that it still works with I210. I don't have the time to test right now.

[andrew-elder]

Thanks for the feedback @christopher-s-hall, merging...