Bluetooth: Mesh: do not send time status often than 30 seconds

[alxelax]

Commit adds restriction for the Time model to not send Time Status more often than 30 seconds if this is aresult of state update after Time Set or Time Status commands.

[NordicBuilder]

Test specification

CI/Jenkins/NRF

• Integration Platforms

CI/Jenkins/integration

Test Module	File based changes	Manually selected	West overwrite
test-fw-nrfconnect-ble_mesh	X

Detailed information of selected test modules

Note: This message is automatically posted and updated by the CI

[NordicBuilder]

You can find the documentation preview for this PR at this link. It will be updated about 10 minutes after the documentation build succeeds.

Note: This comment is automatically posted by the Documentation Publishing GitHub Action.

[alxelax]

PR has been converted into draft. I want to implement unit tests for the changes.

[ludvigsj]

I have a few questions.

1. Why does the time relaying introduce randomness, whereas the time set response does not?
2. If I read the current code correctly, the time set handling will just not send a response if it is less than 30 seconds since last response was sent. In my opinion, we should always send one response after the "last" time set, potentially having to delay this response (between 1 and 30 seconds, depending on when the last status was sent).
3. Also, if I read correctly, the relay handling will first wait until 30 seconds have passed and then schedule a delayed status at 20-50 seconds later after this, meaning there will be 50-80 seconds between these two relays. Is this on purpuse? If so: why? Sorry, I misread. The delay will of course be pending if the handler is called again while waiting to send the first randomly delayed relayed status.

[alxelax]

I have a few questions.

1. Why does the time relaying introduce randomness, whereas the time set response does not?

This is an errata requirement. Please take a look errata reference in JIRA task since Omkar asked to not reference the errata number and text here. Changes to this subclause: 5.3.1.2.3 Receiving a Time Status message.
The general idea is to prevent the broadcast storm of Time Statuses by Time Relays roles when Authorities change for example Time Zone simultaneously by GPS etc.

2. If I read the current code correctly, the time set handling will just not send a response if it is less than 30 seconds since last response was sent. In my opinion, we should always send one response after the "last" time set, potentially having to delay this response (between 1 and 30 seconds, depending on when the last status was sent).

Time Set will always send a response to the sender here: https://github.com/nrfconnect/sdk-nrf/pull/11997/files#diff-626675688a86ce1b261757ffd4f251e64c8b334403c2432102a99ec505fa8c32R273
The only state change publishing is restricted by 30 seconds limitation.

The time model had the bug and didn't publish state change at all. Luckily errata coincided with this code. I fixed the bug according to the errata requirement.

[ludvigsj]

Time Set will always send a response to the sender here: https://github.com/nrfconnect/sdk-nrf/pull/11997/files#diff-626675688a86ce1b261757ffd4f251e64c8b334403c2432102a99ec505fa8c32R273 The only state change publishing is restricted by 30 seconds limitation.

My point is, it should probably sent one more status update than it does now. Meaning, the behavior should be this, I think:

t = 0, time_set
t = 0, publishes time because of time_set
t = 10, time_set
t = 20, time_set
t = 30, publish time a final time to inform of latest state change.

The last step will not be done with the current code.

[alxelax]

Time Set will always send a response to the sender here: https://github.com/nrfconnect/sdk-nrf/pull/11997/files#diff-626675688a86ce1b261757ffd4f251e64c8b334403c2432102a99ec505fa8c32R273 The only state change publishing is restricted by 30 seconds limitation.

My point is, it should probably sent one more status update than it does now. Meaning, the behavior should be this, I think:

t = 0, time_set t = 0, publishes time because of time_set t = 10, time_set t = 20, time_set t = 30, publish time a final time to inform of latest state change.

The last step will not be done with the current code.

I asked this question to @omkar3141 during implementation. Should the server skip or postpone publishing? I got answered it should skip.
@omkar3141, could you clarify @ludvigsj's concern?

[omkar3141]

The spec does not have clarity about whether 30 second limit applies to all kinds of publishing or only to periodic publishing. The intention in the spec seems to be to prevent flooding of network by Time_Status messages if someone sets the publish period to a low value. We will ask for clarification.

Meanwhile it is enough to implement rate limitation for handing of 'Time_Status' messages (in handle_time_status). So, if other server is misbehaving, this server does not start misbehaving as well.

[alxelax]

The spec does not have clarity about whether 30 second limit applies to all kinds of publishing or only to periodic publishing. The intention in the spec seems to be to prevent flooding of network by Time_Status messages if someone sets the publish period to a low value. We will ask for clarification.

Meanwhile it is enough to implement rate limitation for handing of 'Time_Status' messages (in handle_time_status). So, if other server is misbehaving, this server does not start misbehaving as well.

Hm... Seems spec clarifies the time restriction only for one kind of publishing. This is state change publishing. I added periodic here to follow 30 seconds rules. I'll remove considering time in periodic publishing. If user wants to do DDoS attack of network over the time service, let's allow to do this. :)

[alxelax]

Discussed IRL. For now, we decided to apply publishing time restriction rules for handling Time Status messages and not do for Time Set messages. At the current moment, publishing the last Time model state depends on configured periodic publishing time of the Time model. If Time Set is received during 30 seconds restriction time it is not clear what is better to do: publish immediately or postpone until the end of the already started 30 seconds timeout.

If clarification for the Time Set message is gotten from Bluetooth SIG it will be applied later as a separate task\PR.