[netdata] remove unallocated router ID entries on recovery after reset

[abtink]

This commit updates HandleNetworkDataRestoredAfterReset() which is called when a leader device is reset and recovers the Network Data from other routers. With the new code, leader will now check and remove entries in the restored Network Data that are from any currently unallocated router ID.

This change acts as a safeguard against an edge case where the leader is reset at an inopportune time, such as right after it removed an allocated router ID and sent MLE advertisement but before it got the chance to send the updated Network Data to other routers.

[size-report]

Size Report of OpenThread

Merging #9421 into main(df010bc).

name	branch	text	data	bss	total
ot-cli-ftd	main	462432	760	66252	529444
	#9421	462496	760	66252	529508
	+/-	+64	0	0	+64
ot-ncp-ftd	main	433732	760	61416	495908
	#9421	433796	760	61416	495972
	+/-	+64	0	0	+64
libopenthread-ftd.a	main	231947	0	40214	272161
	#9421	232015	0	40214	272229
	+/-	+68	0	0	+68
libopenthread-cli-ftd.a	main	55959	0	8051	64010
	#9421	55959	0	8051	64010
	+/-	0	0	0	0
libopenthread-ncp-ftd.a	main	31497	0	5852	37349
	#9421	31497	0	5852	37349
	+/-	0	0	0	0
ot-cli-mtd	main	361064	760	51132	412956
	#9421	361064	760	51132	412956
	+/-	0	0	0	0
ot-ncp-mtd	main	344748	760	46304	391812
	#9421	344748	760	46304	391812
	+/-	0	0	0	0
libopenthread-mtd.a	main	154588	0	25102	179690
	#9421	154588	0	25102	179690
	+/-	0	0	0	0
libopenthread-cli-mtd.a	main	39029	0	8043	47072
	#9421	39029	0	8043	47072
	+/-	0	0	0	0
libopenthread-ncp-mtd.a	main	24377	0	5852	30229
	#9421	24377	0	5852	30229
	+/-	0	0	0	0
ot-cli-ftd-br	main	529528	768	133868	664164
	#9421	529592	768	133868	664228
	+/-	+64	0	0	+64
libopenthread-ftd-br.a	main	293975	5	107806	401786
	#9421	294043	5	107806	401854
	+/-	+68	0	0	+68
libopenthread-cli-ftd-br.a	main	69160	0	8075	77235
	#9421	69160	0	8075	77235
	+/-	0	0	0	0
ot-rcp	main	61880	564	20532	82976
	#9421	61880	564	20532	82976
	+/-	0	0	0	0
libopenthread-rcp.a	main	9182	0	4988	14170
	#9421	9182	0	4988	14170
	+/-	0	0	0	0
libopenthread-radio.a	main	18528	0	206	18734
	#9421	18528	0	206	18734
	+/-	0	0	0	0

[codecov]

Codecov Report

Merging #9421 (572fe9d) into main (df010bc) will decrease coverage by 0.03%.
The diff coverage is 62.50%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #9421      +/-   ##
==========================================
- Coverage   85.86%   85.83%   -0.03%     
==========================================
  Files         559      559              
  Lines       73762    73770       +8     
==========================================
- Hits        63334    63324      -10     
- Misses      10428    10446      +18     

Files Changed	Coverage Δ
src/core/thread/network_data_leader_ftd.cpp	89.29% <62.50%> (-0.17%)	⬇️

... and 17 files with indirect coverage changes

[jwhui]

Can we simplify the nested while loops by setting iterator back to kIteratorInit here?

[abtink]

We can but I guess we would then be indirectly relying on behavior of GetNextServer() on how it changes (or does not change) the iterator when it returns an error (when net data is empty or we reach end of all entries). I think this may work with current implementation of GetNextServer(), but I feel may be better not to assume any specific behavior when we use it here.

[jwhui]

Don't we already rely on GetNextServer() to return error to break out of the loop? Can you provide an example on some alternative behavior that you have in mind?

[abtink]

For example, if network data is empty, then GetNextServer() may not change iterator and return error (if I am not mistaken, the current implementation does change the iterator in this case but it would be technically valid for it not to change it in this case).

Then if we use (iterator == kInteratorInit) we cannot tell the difference between whether we found an item and did RemoveRloc() or if the network data was empty.

Basically I feel it may be safer to not assume how iterator may or may not change when GetNextServer() returns an error.

[abtink]

@jwhui I think I misunderstood your suggestion. 😄... Yes. I think what you suggest would work nicely.

I assumed you want us to use if (iterator == kInteratorInit) as way to check if we exited early (replacing didChange check)

[abtink]

Changed it in new push (now we only have one loop). Thanks 😃