[low-power] resync SSED with its parent after retransmission #6342

brianljt · 2021-03-24T04:08:20Z

Per 15.4 spec, transmitter cannot modify frame content in retransmission which will leads to out-of-date CSL IE being sent to CSL transmitter. This PR fixes this issue by,

Send a follow up data poll with CSL IE to resync the CSL transmitter.
Send periodic data poll with CSL IE to keep SSED's parent in sync.
Move PrepareDataRequest to data_poll_sender.
Add test cases to cover.

IEEE Std 802.15.4 - 6.7.4.3 Retransmission,

When not using TSCH mode and a frame with the Security Enabled field set to one is retransmitted, the frame shall be retransmitted without changes and without passing through the outgoing frame security procedure, as defined in 9.2.1.

Thread Spec V1.2 - 4.6.5.1.1 Communication between Parent and SSED Child

A SSED Device MUST include IEEE 802.15.4 CSL Information Elements (CSL Phase, CSL Period) in all the IEEE 802.15.4 Commands, ACKs, and Data frames unicast to its Parent router, as defined in [IEEE 802.15.4-2015] and Table 3-7.

included

…nsmission

size-report · 2021-03-24T04:15:50Z

Size Report of OpenThread

Merging #6342 into main(62ae1a1).

name	branch	text	data	bss	total
ot-cli-ftd_1.1	main	427168	780	56216	484164
	#6342	427200	780	56216	484196
	+/-	+32	0	0	+32
ot-cli-mtd_1.1	main	344944	780	46096	391820
	#6342	344992	780	46096	391868
	+/-	+48	0	0	+48
ot-ncp-ftd_1.1	main	416024	780	59048	475852
	#6342	416056	780	59048	475884
	+/-	+32	0	0	+32
ot-ncp-mtd_1.1	main	337148	780	48936	386864
	#6342	337196	780	48936	386912
	+/-	+48	0	0	+48
ot-rcp_1.1	main	56780	564	19940	77284
	#6342	56780	564	19940	77284
	+/-	0	0	0	0
libopenthread-cli-ftd.a_1.1	main	43042	0	452	43494
	#6342	43042	0	452	43494
	+/-	0	0	0	0
libopenthread-cli-mtd.a_1.1	main	35083	0	444	35527
	#6342	35083	0	444	35527
	+/-	0	0	0	0
libopenthread-ftd.a_1.1	main	228260	20	38352	266632
	#6342	228264	20	38352	266636
	+/-	+4	0	0	+4
libopenthread-mtd.a_1.1	main	159940	20	28240	188200
	#6342	159964	20	28240	188224
	+/-	+24	0	0	+24
libopenthread-ncp-ftd.a_1.1	main	31236	0	5852	37088
	#6342	31236	0	5852	37088
	+/-	0	0	0	0
libopenthread-ncp-mtd.a_1.1	main	26084	0	5852	31936
	#6342	26084	0	5852	31936
	+/-	0	0	0	0
libopenthread-rcp.a_1.1	main	8703	0	4988	13691
	#6342	8703	0	4988	13691
	+/-	0	0	0	0
libopenthread-radio.a_1.1	main	14303	0	150	14453
	#6342	14303	0	150	14453
	+/-	0	0	0	0
ot-cli-ftd_1.2	main	449724	780	65060	515564
	#6342	449852	780	65068	515700
	+/-	+128	0	+8	+136
ot-cli-mtd_1.2	main	359892	780	46628	407300
	#6342	360052	780	46636	407468
	+/-	+160	0	+8	+168
ot-ncp-ftd_1.2	main	437472	780	67900	506152
	#6342	437584	780	67908	506272
	+/-	+112	0	+8	+120
ot-ncp-mtd_1.2	main	350416	780	49476	400672
	#6342	350544	780	49484	400808
	+/-	+128	0	+8	+136
ot-rcp_1.2	main	58032	564	20000	78596
	#6342	58016	564	20000	78580
	+/-	-16	0	0	-16
libopenthread-cli-ftd.a_1.2	main	45862	0	452	46314
	#6342	45862	0	452	46314
	+/-	0	0	0	0
libopenthread-cli-mtd.a_1.2	main	37402	0	444	37846
	#6342	37402	0	444	37846
	+/-	0	0	0	0
libopenthread-ftd.a_1.2	main	247624	20	47008	294652
	#6342	247720	20	47016	294756
	+/-	+96	0	+8	+104
libopenthread-mtd.a_1.2	main	172001	20	28584	200605
	#6342	172115	20	28592	200727
	+/-	+114	0	+8	+122
libopenthread-ncp-ftd.a_1.2	main	32960	0	5852	38812
	#6342	32960	0	5852	38812
	+/-	0	0	0	0
libopenthread-ncp-mtd.a_1.2	main	27344	0	5852	33196
	#6342	27344	0	5852	33196
	+/-	0	0	0	0
libopenthread-rcp.a_1.2	main	8703	0	4988	13691
	#6342	8703	0	4988	13691
	+/-	0	0	0	0
libopenthread-radio.a_1.2	main	15225	0	182	15407
	#6342	15217	0	182	15399
	+/-	-8	0	0	-8

codecov · 2021-03-24T06:43:51Z

Codecov Report

Merging #6342 (a184271) into main (5bc74b7) will decrease coverage by 0.20%.
The diff coverage is 85.10%.

❗ Current head a184271 differs from pull request most recent head 0f271fe. Consider uploading reports for the commit 0f271fe to get more accurate results

@@            Coverage Diff             @@
##             main    #6342      +/-   ##
==========================================
- Coverage   86.55%   86.35%   -0.21%     
==========================================
  Files         428      429       +1     
  Lines       54746    55018     +272     
==========================================
+ Hits        47388    47509     +121     
- Misses       7358     7509     +151

Impacted Files	Coverage Δ
src/cli/cli.hpp	`100.00% <ø> (ø)`
src/core/api/channel_manager_api.cpp	`86.66% <0.00%> (-13.34%)`	⬇️
src/core/api/channel_monitor_api.cpp	`57.14% <ø> (-28.58%)`	⬇️
src/core/api/child_supervision_api.cpp	`100.00% <ø> (ø)`
src/core/api/coap_api.cpp	`67.20% <ø> (ø)`
src/core/api/coap_secure_api.cpp	`70.42% <ø> (ø)`
src/core/api/commissioner_api.cpp	`93.75% <ø> (ø)`
src/core/api/dataset_api.cpp	`83.33% <ø> (ø)`
src/core/api/dataset_ftd_api.cpp	`100.00% <ø> (ø)`
src/core/api/dataset_updater_api.cpp	`40.00% <ø> (ø)`
... and 164 more

Irving-cl

Overall LGTM. Some small issues.

src/core/mac/data_poll_sender.cpp

src/core/mac/data_poll_sender.hpp

…nsmission

Irving-cl

LGTM 👍

abtink

Looks great overall. Thanks. 👍

One suggestion below to check and protect if the calculation may roll-over on default period (just to be safe with all config values).

src/core/mac/data_poll_sender.cpp

src/core/mac/data_poll_sender.hpp

src/core/thread/mle.cpp

abtink

LGTM. 👍

…ead#6342) Per 15.4 spec, transmitter cannot modify frame content in retransmission which will leads to out-of-date CSL IE being sent to CSL transmitter. This commit fixes this issue by, 1. Send a follow up data poll with CSL IE to resync the CSL transmitter. 2. Send periodic data poll with CSL IE to keep SSED's parent in sync. 3. Move PrepareDataRequest to data_poll_sender. 4. Add test cases to cover.

brianljt added 13 commits March 16, 2021 18:04

[low-power] resync SSED with its parent after retransmission with CSL IE

9984909

included

CI fix

8900385

Merge remote-tracking branch 'upstream/main' into issue/csl/csl_retra…

7816e87

…nsmission

add CI test steps to verify CSL sync message after retransmission

c4d63ed

minor fixes

b9e7017

Merge remote-tracking branch 'upstream/main' into issue/csl/csl_retra…

fbe410b

…nsmission

fix merge conflicts

2766ae0

[low-power] fix retransmission frame with invalid CSL IE included

d83b07e

CI fix

fd8f7c5

Merge remote-tracking branch 'upstream/main' into issue/csl/csl_retra…

67e5286

…nsmission

add more CI test cases

210c9e7

fix initial state

ae5571c

remove unused variable

495648f

brianljt added bug P3 Thread 1.2 labels Mar 24, 2021

google-cla bot added the cla: yes label Mar 24, 2021

brianljt requested review from Irving-cl and bukepo March 24, 2021 04:09

brianljt linked an issue Mar 24, 2021 that may be closed by this pull request

[low-power] retransmission frame CSL IE handling #6286

Closed

brianljt requested a review from abtink March 25, 2021 04:03

bukepo approved these changes Mar 31, 2021

View reviewed changes

Irving-cl reviewed Apr 2, 2021

View reviewed changes

address review comments

7170fd0

brianljt requested a review from Irving-cl April 6, 2021 05:04

brianljt added 2 commits April 6, 2021 13:06

Merge remote-tracking branch 'upstream/main' into issue/csl/csl_retra…

0d48add

…nsmission

fix merge conficts

20ea399

brianljt force-pushed the issue/csl/csl_retransmission branch from 3849885 to 20ea399 Compare April 6, 2021 05:25

edmont approved these changes Apr 7, 2021

View reviewed changes

Irving-cl approved these changes Apr 8, 2021

View reviewed changes

brianljt requested review from jwhui and removed request for abtink April 9, 2021 01:51

jwhui requested a review from abtink April 9, 2021 20:39

abtink approved these changes Apr 13, 2021

View reviewed changes

src/core/mac/data_poll_sender.cpp Outdated Show resolved Hide resolved

src/core/mac/data_poll_sender.cpp Show resolved Hide resolved

src/core/mac/data_poll_sender.hpp Show resolved Hide resolved

brianljt added 2 commits April 13, 2021 10:17

guard against roll-over

2314fb9

make pretty

037a19c

abtink approved these changes Apr 13, 2021

View reviewed changes

jwhui reviewed Apr 13, 2021

View reviewed changes

src/core/thread/mle.cpp Outdated Show resolved Hide resolved

brianljt added 2 commits April 13, 2021 14:11

update poll period when setting CSL timeout

0f271fe

fix CI

3b6f84b

brianljt requested a review from jwhui April 13, 2021 07:32

abtink approved these changes Apr 13, 2021

View reviewed changes

jwhui approved these changes Apr 13, 2021

View reviewed changes

jwhui merged commit d514373 into openthread:main Apr 13, 2021

jwhui mentioned this pull request Sep 1, 2021

[csl] Phase update on frame retries? #6976

Closed

edmont mentioned this pull request Sep 3, 2021

Broken OT 1.2 support in openthread/ot-nrf528xx openthread/ot-nrf528xx#148

Closed

George-Stefan mentioned this pull request Sep 16, 2021

Remove the check for "secured" enhanced ACK for CSL calculations #6994

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[low-power] resync SSED with its parent after retransmission #6342

[low-power] resync SSED with its parent after retransmission #6342

brianljt commented Mar 24, 2021 •

edited

size-report bot commented Mar 24, 2021 •

edited

codecov bot commented Mar 24, 2021 •

edited

Irving-cl left a comment

Irving-cl left a comment

abtink left a comment

abtink left a comment

[low-power] resync SSED with its parent after retransmission #6342

[low-power] resync SSED with its parent after retransmission #6342

Conversation

brianljt commented Mar 24, 2021 • edited

size-report bot commented Mar 24, 2021 • edited

Size Report of OpenThread

codecov bot commented Mar 24, 2021 • edited

Codecov Report

Irving-cl left a comment

Choose a reason for hiding this comment

Irving-cl left a comment

Choose a reason for hiding this comment

abtink left a comment

Choose a reason for hiding this comment

abtink left a comment

Choose a reason for hiding this comment

brianljt commented Mar 24, 2021 •

edited

size-report bot commented Mar 24, 2021 •

edited

codecov bot commented Mar 24, 2021 •

edited