gPTP: use message type in combination with sequence ID to uniquely id…

[andrew-elder]

This pull request is for discussion.....

The following assertions motivate this commit

• gPTP packet Tx timestamps are collected sometime after the packet goes out on the wire
• slave and master sequenceIDs can in fact be the same breifly
• pDelay timer callback and packet receive processing occurs in different threads
• Path_Delay_Req_Message and Path_Delay_Resp_Message both require Tx timestamps

The idea here is to make available BOTH the message type and the sequence ID to the HAL timestamping layer. If the HAL layer can support message matching by both type and ID, then it can use them both. If it can only use the sequence ID, then it can stick with that.

Comments?

[christopher-s-hall]

My only question/criticism of the code: Why not make source port identity part of MessageId?

Otherwise, I see this to be more of a problem on the receive side. Currently the code serializes transmission. Maybe for some hardware this could be relaxed?

One scenario that I can't figure out a way around without additional info such as messageId:

dev #1 sends sync with sequence n
dev #1 receives pdelay req with sequence n from dev #2
dev #1 sends pdelay resp with sequence n

dev #2 receives pdelay resp and sync message with sequence n
- source port id, sequence are the same

Am I missing the point?

[pinealservo]

I'm not really following the purpose of this change either. I'm not sure this information is even checked at all in the Linux timestamping code path, and I'm not sure what the check is meant to guard against in the Windows path.

Although timestamps do get processed some very small time after their associated packet is processed, the code right now seems to just wait/block in the same code path as the data action to get the associated timestamp. There doesn't seem to be a way for multiple tx frames or multiple rx frames to get their timestamps confused right now, although it's entirely possible I missed something.

Could you elaborate some more on your assertions and how you're expecting that they might interact to produce an error condition?

[christopher-s-hall]

@pinealservo Agreed this isn't going to make any difference in standard Linux network stack where the driver makes the association between frame and timestamp. This would possibly apply to other OSs or non-standard timestamp interfaces.

On the receive side, I'm thinking of the case where the HW timestamp queue is small (1?) and the timestamp is matched using some data captured from the frame. If the HW was able to capture messageId, I think it would be useful and this patch would allow that in the application.

Hopefully, I'm not going off the rails. @andrew-elder?

[christopher-s-hall]

I'm also thinking that we should add domain to this list. It's not relevant now, but TSN will allow multiple domains.

[andrew-elder]

I think this issue arises due to a possibly incorrect (private) HAL implementation. The events from the TimerQueue that cause packet Tx operations are not serialized with other sources of packet Tx operations. Maybe Linux does that for you? When I read LinuxTimerQueueHandler() it is not obvious to me how the serialization of Tx packets and timestamp reading is implemented?

@christopher-s-hall - I think you are adding more functionality in to the MessageId than I originally intended, but if it helps solve other potential issues then I am not opposed.

[andrew-elder]

So, it looks like getTxLock() and putTxLock() should take care of serializing Tx timestamp operations....

[christopher-s-hall]

Yes, TxLock() serializes transmit operations. I think it works correctly. I did test pretty thoroughly for the transmit issue that was a problem at the time.

[christopher-s-hall]

I may be confused about the purpose of the MessageId class. My interpretation given the context of the patch: it's the minimal meta-data that uniquely identifies a PTP message. To me that's: (MessageId, PortIdentity, Sequence Number). PortIdentity is (UUID,Port Number). For Multiple domains / TSN, it would be: (MessageId, Domain, PortIdentity, Sequence Number). For Ethernet point-to-point, I think, you could optimize by dropping PortIdentity because that doesn't change. Does that sound right?

[andrew-elder]

There have been some conversations about this patch outside this email thread. Current plan is to merge it on Jan 26th unless someone has objections.