[dnssd] deduplicate service instance updates#3461
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3461 +/- ##
===========================================
- Coverage 55.77% 33.69% -22.09%
===========================================
Files 87 143 +56
Lines 6890 17286 +10396
Branches 0 1417 +1417
===========================================
+ Hits 3843 5824 +1981
- Misses 3047 11155 +8108
- Partials 0 307 +307 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
When OTBR_MDNS=mDNSResponder is enabled, redundant service browse and resolve callbacks can occur for already discovered TREL peers. Because TrelDnssd::OnTrelServiceInstanceAdded unconditionally called OnTrelServiceInstanceRemoved right at the start of the function, each duplicate mDNS update triggered peer removal (kDnssdRemoved) followed by peer re-addition (kReAdded). This continuous state cycling in OpenThread's TrelPeerTable caused unnecessary multi-radio link probing and out-of-order fragment delivery between 15.4 and TREL interfaces, leading to kErrorDuplicated drops on valid ICMPv6 Echo Reply fragments during integration tests. Before calling OnTrelServiceInstanceRemoved, check if the peer entry already exists with identical socket address and TXT data. If unchanged, update mDiscoverTime and exit without triggering peer removal or re-addition in OpenThread.
cca8db0 to
488cbb1
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request optimizes peer discovery and handling in the TREL DNSSD implementation. It introduces a Matches helper method in the Peer class to compare socket addresses and TXT data. In OnTrelServiceInstanceAdded, it replaces unconditional peer removal with a check to see if the peer already exists; if it matches, it updates the discovery time and exits early, and if the extended address differs, it removes the instance. Additionally, it ensures proper initialization of otPlatTrelPeerInfo structures using memset and refactors socket address comparisons to avoid direct struct comparison which could be affected by padding. There are no review comments, so I have no feedback to provide.
When OTBR_MDNS=mDNSResponder is enabled, redundant service browse and resolve callbacks can occur for already discovered TREL peers. Because TrelDnssd::OnTrelServiceInstanceAdded unconditionally called OnTrelServiceInstanceRemoved right at the start of the function, each duplicate mDNS update triggered peer removal (kDnssdRemoved) followed by peer re-addition (kReAdded).
This continuous state cycling in OpenThread's TrelPeerTable caused unnecessary multi-radio link probing and out-of-order fragment delivery between 15.4 and TREL interfaces, leading to kErrorDuplicated drops on valid ICMPv6 Echo Reply fragments during integration tests.
Before calling OnTrelServiceInstanceRemoved, check if the peer entry already exists with identical socket address and TXT data. If unchanged, update mDiscoverTime and exit without triggering peer removal or re-addition in OpenThread.