Skip to content

Conversation

haug-den-lucas
Copy link
Contributor

@haug-den-lucas haug-den-lucas commented May 28, 2025

This pull requests extends the gPTP implementation with clock servos, BMCA and hot standby.

gPTP bugfixes
The gPTP implementation had some bugs, which are fixed in this PR

  • As timestamps were taken after a frame was fully received/transmitted instead of at the start of a transmission, rare cases could occur, where a combination of PDelay and Sync messages at the same time leads to wrong timestamping => This is fixed by using he StreamingPhyLayer and taking the transmission/receptionStarted timestamps.
  • When a clock performs a clock jump (for whatever reason) outside of the gPTP module, the gPTP did not react on this change. Added a signal clocks emit when the perform a clock jump
  • The gmRR was calculated differently from the standard. Added a new method to calcualte the gmRR using the nrr but kept the old way to be also configurable
  • nrr was not used to calculate peer delays, added this as well (can be disabled by disabling the nrr calculation)

ClockServos
They only Servo-like clock existed in INET was the SettableClock.
Now there is a ServoClock base class, which provides the jumpClockTo function, which was the name suggest jumps to a specified clock time.
Additionally, there is the adjustClockTo function, which is implemented by the actual servo classes to smoothly adjust the clock to the provided time.
Curretnly the following implementations exist:

  • InstantServoClock: Previously the SettableClock, allows to specify if the clock only jumps or also estimates and adjusts the drift rate. I have replaced all instances of SettableClock in INET I could find with this new clock.
  • PiServoClock: Implements a PI-Servo
    => See showcases for more details

BMCA
Extended the gPTP implementation with the Best Master Clock Algorithm
=> See showcases for more details

HotStandby
Extended the MutliDomainGptp with an actual Hot Standby implementation that allows to automatically select a new time domain if one time domain goes offline. This can be enabled and disabled using a parameter.
This is based on the IEEE 802.1ASdm amendment, but uses a simplified version, which does not yet smoothly adjust when switching between two time domains.
This also fixes #1019
=> See showcases for more details

Showcases
I tried to update all showcases using the new implementation and compared the results of the old and new implementation.
When using the InstantServoClock the existing showcases provided the same results.
I've split the gPTP showcase into 3 showcases, a basic, a bmca and a hotstandby showcase.
The images for these showcases can be found in this repository:
https://github.com/haug-den-lucas/media

Further notes
Even though I've worked quite a bit with INET now, I am not fully aware which further side effects these changes might have.
As mentioned above, I checked all existing showcases if they are still valid and checked for all instances of the SettableClock if these simulations still provide the same results.
However, this is quite a big pull request and I'm happy to discuss the changes and further work on it to make merging possible.

The .cproject .oppbuildspecs etc. were not edited my me manually but by my OMNeT++ installation, if required I can remove the changes of these files, I am not exactly aware what the effect of these changes are.

I've extended the .clang-format file so other IDEs reformatting produces the same results as the OMNeT++ IDE

@levy levy requested review from avarga and levy May 29, 2025 08:07
@levy levy self-assigned this May 29, 2025
@levy levy added this to the INET 4.6.1 milestone May 29, 2025
@levy
Copy link
Contributor

levy commented May 29, 2025

Wow! Thank you for this pull request! It looks really good, and I'm pleasantly surprised how much you could pick up from the architecture and the code without a comprehensive documentation. This really is a huge pull request and it definitely goes in the right direction. It's going to take some effort from both sides to include this in INET, which we clearly want to do.

For the following two weeks I'll be quite busy with preparing my TSN presentation for the WFCS 2025 IEEE conference on factory automation in Rostock. I'll come back to this PR after that.

@haug-den-lucas
Copy link
Contributor Author

For the following two weeks I'll be quite busy with preparing my TSN presentation for the WFCS 2025 IEEE conference on factory automation in Rostock. I'll come back to this PR after that.

That's a nice coincidence, as we will also have a WiP paper presentation at WFCS on Wednesday based on this implementation.
I sadly won't make it to the TSN tutorial session on Tuesday, but if you're interested, you can drop by at our poster during the WiP session or we can have a talk sometime else during the conference.

@levy
Copy link
Contributor

levy commented May 29, 2025

Oh, it's good to know! We are coming back home on Wednesday evening, but let's meet if possible! I'll keep in mind that you are going to have a presentation.

@levy
Copy link
Contributor

levy commented Jun 25, 2025

Looking at this PR again, I would like to ask you to perhaps consider submitting several smaller PRs instead of this large one.

Preferably each PR should contain only a few patches and describe what is changed and why. The patches would be carefully selected and squashed from the ones present in this PR. This would allow me to review these smaller PRs one by one and merge them into master over a longer period of time.

I also assume there are some patches which need additional consideration and perhaps discussion and modification, but in its current form it's really difficult to figure out. Otherwise I feel like it would take too long to go through everything that is included here.

@levy
Copy link
Contributor

levy commented Aug 4, 2025

I'm sorry for coming back this late...

So, I started to work on the time synchronization related features of INET. There are several known issues with the current implementation, some of which are already added as issues on GitHub. More specifically I'm working on the following topics:

  • clock implementation correctness and efficiency
  • separation of SettableClock from the underlying OscillatorBasedClock, this would allow having the monotonous clock time for peer delay measurement even while the clock is being adjusted
  • adding a clock servo API (not really focusing on the actual servos themselves)
  • gPTP correctness, fixing glitches caused by overlapping peer delay measurement and time synchronization, switching to using the first bit transmission/reception times, properly forwarding sync messages, fixing sequence ID handling, adding state machines to follow the gPTP peer delay measurement and time synchronization processes, adding exponential averaging for peer delay measurement, etc.

I currently have two work-in-progress branches. They are called topic/clock, and on top of that topic/gptp. My biggest concern currently is that it's very difficult to validate the clock model and gPTP implementation due to its numerical nature.

I would really like to include as much as possible from your work, but having such a large pull request makes this nearly impossible to do. Do you think that we could work together to make INET much better for simulating time-sensitive networking? I could see going through the changes of this PR and making sure that each fix and new feature is included in same shape or form.

@haug-den-lucas
Copy link
Contributor Author

haug-den-lucas commented Aug 4, 2025

I'm sorry for coming back this late...

So, I started to work on the time synchronization related features of INET. There are several known issues with the current implementation, some of which are already added as issues on GitHub. More specifically I'm working on the following topics:

* clock implementation correctness and efficiency

* separation of SettableClock from the underlying OscillatorBasedClock, this would allow having the monotonous clock time for peer delay measurement even while the clock is being adjusted

* adding a clock servo API (not really focusing on the actual servos themselves)

* gPTP correctness, fixing glitches caused by overlapping peer delay measurement and time synchronization, switching to using the first bit transmission/reception times, properly forwarding sync messages, fixing sequence ID handling, adding state machines to follow the gPTP peer delay measurement and time synchronization processes, adding exponential averaging for peer delay measurement, etc.

I currently have two work-in-progress branches. They are called topic/clock, and on top of that topic/gptp. My biggest concern currently is that it's very difficult to validate the clock model and gPTP implementation due to its numerical nature.

I would really like to include as much as possible from your work, but having such a large pull request makes this nearly impossible to do. Do you think that we could work together to make INET much better for simulating time-sensitive networking? I could see going through the changes of this PR and making sure that each fix and new feature is included in same shape or form.

Hello, sorry also for not reacting to your previous message.
I sadly was busy with other work and did not have time to look further into this at the moment.
But I agree this is quite a large PR which cannot be included at once.
Sadly it was kind of developed in parallel and not that independent from each other, so splitting up might not be too easy, but what could work is the following:

  • The ClockServo API implementation could be extracted and be merged first.
  • Based on that the updated gPTP can be merged (I'm not sure if I can extract BMCA and the other gPTP fixes seperately as these changes are kind of dependent on each other in some ways)
  • HotStandby however is pretty independent and could probably easily become it's own PR after the rest is done.

However, I'm unsure about the work that is going on in your two branches and what could help you to make merging easier.
Maybe it might be possible to have a short online meeting within the next days and discuss these things then it might be easier for me to know where to continue working from.
You can reach me at [email protected] if that is an option.

@haug-den-lucas
Copy link
Contributor Author

Small addition, some of these glitches are also resolved in my PR, for example using the transmissionStarted and receptionStarted instead of transmissionEnded and receptionEnded signals etc.
I think for me it would be really helpful if we could discuss what kind of help I could provide specifically such that my changes are compatible with your current branches.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

multidomain in gptp
4 participants