On 2016年12月07日 18:24, Johannes Berg wrote:
And the flag is refered by 1) as you said. The purpose of the flag is to prevent 1) while 2) is ongoing. In other words, 1) has only read access authority to the flag. However, previous code updated the flag in 1). In addition, there is no code for 2). So I just remove the invalid accessing codes.I don't think 1) has read only access to that flag. A TSF adjust will by definition move the TBTT as well.It seems that the wording in the spec disagrees with that - it says (twice) to set the bit only while the TBTT adjustment procedure is ongoing, which isn't the case here? Then again, what exactly *is* this code doing?
It's called mesh_sync_offset_adjust_tbtt() which matches more closely "TBTT adjustment" than "neighbor offset synchronization"?
I think so. Because there is not any code creating "TBTT Adjustment Request frame" even though the frame is required by "TBTT adjustment".
The code looks more like offset synchronization though. Perhaps there's some confusing and it's kinda doing both?
In theory, updating the flag with 1) looks not correct because it is not clearly defined in spec.
In practice, I could consider extending the meaning of the flag over the spec to use it to avoid referring the updating TSF value by peer as Thomas said. I have took the statistics how many TSF drift (ifmsh->sync_offset_clockdrift_max) happens. The attached file shows the stats. The horizontal axis shows TSF drift time(usec) and vertical axis shows how many time the drift occurred. The graph shows almost drifts are under 20usec. In contrast, 2) could causes more than 1000usec drift. So 1) looks not so large enough to protect with the flag.
Masashi Honma.
Attachment:
clockdrift.png
Description: PNG image
