Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[network-data] add leader override feature #9106

Closed
wants to merge 2 commits into from

Conversation

abtink
Copy link
Member

@abtink abtink commented May 31, 2023

This commit implements the leader override mechanism. When enabled, a border router (BR) monitors the following trigger conditions to start leader override:

  • The BR's leader weight is higher than the current partition's weight (as indicated in the current Leader Data).
  • The BR has pending local Network Data entries and has tried to register them with the leader at least 3 times, but failed each time.
  • Each attempt consisted of sending a SRV_DATA.ntf message to the leader, which was acknowledged but not integrated into the Thread Network Data within DATA_RESUBMIT_DELAY seconds (300 seconds).
  • The maximum size of the Thread Network Data has been such that the local Network Data entries would fit over the past period.

If all of these conditions are met, the BR starts the leader override procedure by selecting a random delay between 1 and 30 seconds. If the trigger conditions still hold after the random delay, the BR starts a new partition as the leader.

The OPENTHREAD_CONFIG_BORDER_ROUTER_LEADER_OVERRIDE_ENABLE configuration option or the OT_LEADER_OVERRIDE CMake option control whether this feature is included.

This commit also adds the test_leader_override.py script, which validates the behavior of the newly added leader override mechanism under different scenarios.


@size-report
Copy link

size-report bot commented May 31, 2023

Size Report of OpenThread

Merging #9106 into main(d56059e).

name branch text data bss total
ot-cli-ftd_1.1 main 470496 760 64252 535508
#9106 470528 760 64252 535540
+/- +32 0 0 +32
ot-cli-mtd_1.1 main 390200 760 54036 444996
#9106 390216 760 54036 445012
+/- +16 0 0 +16
ot-ncp-ftd_1.1 main 446124 760 59448 506332
#9106 446140 760 59448 506348
+/- +16 0 0 +16
ot-ncp-mtd_1.1 main 370068 760 49240 420068
#9106 370068 760 49240 420068
+/- 0 0 0 0
ot-rcp_1.1 main 60412 564 19964 80940
#9106 60412 564 19964 80940
+/- 0 0 0 0
libopenthread-cli-ftd.a_1.1 main 52271 0 8027 60298
#9106 52285 0 8027 60312
+/- +14 0 0 +14
libopenthread-cli-mtd.a_1.1 main 43659 0 8019 51678
#9106 43673 0 8019 51692
+/- +14 0 0 +14
libopenthread-ftd.a_1.1 main 245559 4 38782 284345
#9106 245583 4 38782 284369
+/- +24 0 0 +24
libopenthread-mtd.a_1.1 main 180602 4 28574 209180
#9106 180602 4 28574 209180
+/- 0 0 0 0
libopenthread-ncp-ftd.a_1.1 main 31539 0 5852 37391
#9106 31539 0 5852 37391
+/- 0 0 0 0
libopenthread-ncp-mtd.a_1.1 main 26541 0 5852 32393
#9106 26541 0 5852 32393
+/- 0 0 0 0
libopenthread-rcp.a_1.1 main 9028 0 4988 14016
#9106 9028 0 4988 14016
+/- 0 0 0 0
libopenthread-radio.a_1.1 main 18301 0 174 18475
#9106 18301 0 174 18475
+/- 0 0 0 0
ot-cli-ftd_1.3 main 493024 760 73748 567532
#9106 493056 760 73748 567564
+/- +32 0 0 +32
ot-cli-mtd_1.3 main 405480 760 55276 461516
#9106 405496 760 55276 461532
+/- +16 0 0 +16
ot-ncp-ftd_1.3 main 467484 760 68936 537180
#9106 467500 760 68936 537196
+/- +16 0 0 +16
ot-ncp-mtd_1.3 main 383820 760 50472 435052
#9106 383820 760 50472 435052
+/- 0 0 0 0
ot-rcp_1.3 main 62784 564 20532 83880
#9106 62784 564 20532 83880
+/- 0 0 0 0
libopenthread-cli-ftd.a_1.3 main 55318 0 8027 63345
#9106 55332 0 8027 63359
+/- +14 0 0 +14
libopenthread-cli-mtd.a_1.3 main 45973 0 8019 53992
#9106 45987 0 8019 54006
+/- +14 0 0 +14
libopenthread-ftd.a_1.3 main 264094 4 47734 311832
#9106 264118 4 47734 311856
+/- +24 0 0 +24
libopenthread-mtd.a_1.3 main 192281 4 29270 221555
#9106 192281 4 29270 221555
+/- 0 0 0 0
libopenthread-ncp-ftd.a_1.3 main 33377 0 5852 39229
#9106 33377 0 5852 39229
+/- 0 0 0 0
libopenthread-ncp-mtd.a_1.3 main 27907 0 5852 33759
#9106 27907 0 5852 33759
+/- 0 0 0 0
libopenthread-rcp.a_1.3 main 9194 0 4988 14182
#9106 9194 0 4988 14182
+/- 0 0 0 0
libopenthread-radio.a_1.3 main 19216 0 206 19422
#9106 19216 0 206 19422
+/- 0 0 0 0

@codecov
Copy link

codecov bot commented May 31, 2023

Codecov Report

Merging #9106 (4dde33a) into main (47f15e3) will increase coverage by 3.06%.
The diff coverage is 85.10%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9106      +/-   ##
==========================================
+ Coverage   82.34%   85.41%   +3.06%     
==========================================
  Files         527      544      +17     
  Lines       69940    71612    +1672     
==========================================
+ Hits        57592    61164    +3572     
+ Misses      12348    10448    -1900     
Impacted Files Coverage Δ
src/core/common/time_ticker.cpp 95.83% <ø> (ø)
src/core/thread/network_data_local.cpp 86.29% <33.33%> (-2.70%) ⬇️
src/cli/cli_network_data.cpp 79.00% <83.33%> (+3.59%) ⬆️
src/core/thread/network_data_notifier.cpp 88.64% <83.78%> (-2.50%) ⬇️
src/core/thread/network_data_leader_ftd.cpp 89.04% <86.53%> (+0.66%) ⬆️
src/cli/cli.cpp 74.91% <88.88%> (-0.86%) ⬇️
src/cli/cli_network_data.hpp 100.00% <100.00%> (ø)
src/core/api/border_router_api.cpp 89.47% <100.00%> (+2.80%) ⬆️
src/core/api/thread_ftd_api.cpp 77.38% <100.00%> (+3.76%) ⬆️
src/core/thread/network_data_leader_ftd.hpp 100.00% <100.00%> (ø)
... and 1 more

... and 108 files with indirect coverage changes

@jwhui jwhui requested a review from ndyck14 May 31, 2023 18:07
*
*/
#ifndef OPENTHREAD_CONFIG_BORDER_ROUTER_LEADER_OVERRIDE_ENABLE
#define OPENTHREAD_CONFIG_BORDER_ROUTER_LEADER_OVERRIDE_ENABLE OPENTHREAD_CONFIG_BORDER_ROUTING_ENABLE
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also to include the condition here that Thread Version >= 1.3.1

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since it is only added behavior and requires feature to be explicitly enabled (through API call otBorderRouterSetLeaderOverrideEnabled() and by default it is disabled), I think we can skip version check (we usually avoid version check for new additive feature, e.g. SRP clinet/sever also don't have version check).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, if the feature is available and by default disabled then it's fine to include for the standard build. A BR vendor can anyway overrule the setting if wanted, e.g. to reduce some code if needed.


void Leader::CheckForNetDataGettingFull(const NetworkData &aNetworkData, uint16_t aOldRloc16)
{
// Determines whether there is still room in Network Data to register
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with a check method, some result of the check needs to be provided either a boolean or an object whose size can be verified. It's not clear from the comments how the result is delivered?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added new text in comment to clarify this. We use callback by invoking Get<Notifier>().SignalNetworkDataFull() (which then may call user callback and inform any other module that may be intrested).

The reason went for this model (callback) is that it can be triggered from different paths in the code (e.g. local netdata getting full, etc).

This commit adds a new mechanism to detect when Network Data (local or
leader) becomes full. When this happens, a callback function is
invoked. This callback function can be set using a newly added OT
API. The callback is invoked whenever:
- The device is acting as the leader and receives a Network Data
  registration from a Border Router (BR) that it cannot add to
  Network Data (running out of space).
- The device is acting as a BR and new entries cannot be added to its
  local Network Data.
- The device is acting as a BR and tries to register its local Network
  Data entries with the leader, but determines that its local entries
  will not fit.

The `OPENTHREAD_CONFIG_BORDER_ROUTER_SIGNAL_NETWORK_DATA_FULL` config
controls the new mechanism and its API. It is enabled by default when
`OPENTHREAD_CONFIG_BORDER_ROUTING_ENABLE` is enabled.

This commit also adds CLI support for the new mechanism and adds a
test case to validate its behavior.
This commit implements the leader override mechanism. When enabled, a
border router (BR) monitors the following trigger conditions to start
leader override:

- The BR's leader weight is higher than the current partition's
  weight (as indicated in the current Leader Data).
- The BR has pending local Network Data entries and has tried to
  register them with the leader at least 3 times, but failed each
  time.
- Each attempt consisted of sending a SRV_DATA.ntf message to the
  leader, which was acknowledged but not integrated into the Thread
  Network Data within `DATA_RESUBMIT_DELAY` seconds (300 seconds).
 - The maximum size of the Thread Network Data has been such that the
   local Network Data entries would fit over the past period.

If all of these conditions are met, the BR starts the leader override
procedure by selecting a random delay between 1 and 30 seconds. If
the trigger conditions still hold after the random delay, the BR
starts a new partition as the leader.

The `OPENTHREAD_CONFIG_BORDER_ROUTER_LEADER_OVERRIDE_ENABLE`
configuration option or the `OT_LEADER_OVERRIDE` CMake option control
whether this feature is included.

This commit also adds the `test_leader_override.py` script, which
validates the behavior of the newly added leader override mechanism
under different scenarios.
@abtink
Copy link
Member Author

abtink commented Jun 12, 2024

Closing this PR for now.

@abtink abtink closed this Jun 12, 2024
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.

None yet

2 participants