From 20e053016dae4a38c9f616e2582a59701f3df6ae Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Thu, 15 Sep 2022 19:05:41 -0400 Subject: [PATCH] Disable MDNS lookup on first message deliver failure be default (#22678) * Disable MDNS lookup on first message deliver failure * Restyle * Reorder config entry * Address PR comments and clean up config comment * Address PR comments --- src/messaging/BUILD.gn | 6 ++++++ src/messaging/ReliableMessageMgr.cpp | 2 ++ src/messaging/ReliableMessageProtocolConfig.h | 20 +++++++++++++++++++ 3 files changed, 28 insertions(+) diff --git a/src/messaging/BUILD.gn b/src/messaging/BUILD.gn index f01b749d3f6d17..bf363c7239db71 100644 --- a/src/messaging/BUILD.gn +++ b/src/messaging/BUILD.gn @@ -17,6 +17,8 @@ import("//build_overrides/chip.gni") declare_args() { # Allows to change time between retries during the case session chip_case_retry_delta = "" + + chip_config_resolve_peer_on_first_transmit_failure = "" } defines = [] @@ -26,6 +28,10 @@ if (chip_case_retry_delta != "") { [ "CHIP_CONFIG_MRP_LOCAL_ACTIVE_RETRY_INTERVAL=${chip_case_retry_delta}" ] } +if (chip_config_resolve_peer_on_first_transmit_failure != "") { + defines += [ "CHIP_CONFIG_RESOLVE_PEER_ON_FIRST_TRANSMIT_FAILURE=${chip_config_resolve_peer_on_first_transmit_failure}" ] +} + source_set("messaging_mrp_config") { sources = [ "ReliableMessageProtocolConfig.h" ] diff --git a/src/messaging/ReliableMessageMgr.cpp b/src/messaging/ReliableMessageMgr.cpp index e745e54d46665e..f1b297b7b8d799 100644 --- a/src/messaging/ReliableMessageMgr.cpp +++ b/src/messaging/ReliableMessageMgr.cpp @@ -316,6 +316,7 @@ CHIP_ERROR ReliableMessageMgr::SendFromRetransTable(RetransTableEntry * entry) if (err == CHIP_NO_ERROR) { +#if CHIP_CONFIG_RESOLVE_PEER_ON_FIRST_TRANSMIT_FAILURE const ExchangeManager * exchangeMgr = entry->ec->GetExchangeMgr(); // TODO: investigate why in ReliableMessageMgr::CheckResendApplicationMessageWithPeerExchange unit test released exchange // context with mExchangeMgr==nullptr is used. @@ -329,6 +330,7 @@ CHIP_ERROR ReliableMessageMgr::SendFromRetransTable(RetransTableEntry * entry) mSessionUpdateDelegate->UpdatePeerAddress(entry->ec->GetSessionHandle()->GetPeer()); } } +#endif // CHIP_CONFIG_RESOLVE_PEER_ON_FIRST_TRANSMIT_FAILURE } else { diff --git a/src/messaging/ReliableMessageProtocolConfig.h b/src/messaging/ReliableMessageProtocolConfig.h index f05f4b05d9cc97..2e253c10bdc3ac 100644 --- a/src/messaging/ReliableMessageProtocolConfig.h +++ b/src/messaging/ReliableMessageProtocolConfig.h @@ -71,6 +71,26 @@ namespace chip { #define CHIP_CONFIG_RMP_DEFAULT_ACK_TIMEOUT (200_ms32) #endif // CHIP_CONFIG_RMP_DEFAULT_ACK_TIMEOUT +/** + * @def CHIP_CONFIG_RESOLVE_PEER_ON_FIRST_TRANSMIT_FAILURE + * + * @brief + * Should an address lookup of the peer happen on every first message that fails + * to send on the link. + * + * The default value to not perform lookup was selected because most implementations + * of address lookup are not cache the and a request is sent on the link. Failing + * to deliver the first message is far more likely to happen due to lossy link + * than an actual address change where the peer did not reset. In the lossy link + * situation, doing further DNS resolutions on a degraded link can exacerbate that + * problem greatly. Additionally, every message that arrives from a peer updates the + * address. If the peer has fallen off the link due to any other reason, a re-resolve + * may not achieve an address that is reachable, even if a resolve response occurs. + */ +#ifndef CHIP_CONFIG_RESOLVE_PEER_ON_FIRST_TRANSMIT_FAILURE +#define CHIP_CONFIG_RESOLVE_PEER_ON_FIRST_TRANSMIT_FAILURE 0 +#endif // CHIP_CONFIG_RESOLVE_PEER_ON_FIRST_TRANSMIT_FAILURE + /** * @def CHIP_CONFIG_RMP_RETRANS_TABLE_SIZE *