-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Add header validity check for group and MCSP msg #11441
Add header validity check for group and MCSP msg #11441
Conversation
Handle Group message counter here spec 4.7.3connectedhomeip/src/transport/SessionManager.cpp Lines 489 to 494 in 06c3969
This comment was generated by todo based on a
|
06c3969
to
118b585
Compare
PR #11441: Size comparison from abcae0d to 118b585 Increases (8 builds for k32w, p6, qpg, telink)
Full report (9 builds for k32w, p6, qpg, telink)
|
118b585
to
6d58f89
Compare
PR #11441: Size comparison from 29747a2 to 6d58f89 Increases (33 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
Decreases (1 build for esp32)
Full report (38 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
|
/rebase |
6d58f89
to
9144e5d
Compare
PR #11441: Size comparison from f36b141 to 9144e5d Increases (34 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
Full report (38 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
|
Groupcast (multicast) messages aren't yet dispatched into anything as far as I can tell, so I don't believe ACks to groupcast could currently happen. But we do need code that prevents this. Just thought I would mention this. It does appear to me that if groupcast messages were injected into ExchangeManager::OnMessageReceived with the R flag set (needs ack), our current code might try to ack. |
Problem
Change overview
Add header validity check
Testing
Unit test added.