[dns-sd] Add support for service subtypes on Thread devices

Implementing commissionable node advertising for Thread
devices requires that our OpenThread-based DNS-SD backend
be able to register a set of service subtypes. It has become
possible recently once the support for service subtypes
was added to the OpenThread SRP client.

src/include/platform/ThreadStackManager.h

@@ -86,8 +86,9 @@ class ThreadStackManager
     CHIP_ERROR SetThreadEnabled(bool val);
 
 #if CHIP_DEVICE_CONFIG_ENABLE_THREAD_SRP_CLIENT
-    CHIP_ERROR AddSrpService(const char * aInstanceName, const char * aName, uint16_t aPort, chip::Mdns::TextEntry * aTxtEntries,
-                             size_t aTxtEntiresSize, uint32_t aLeaseInterval, uint32_t aKeyLeaseInterval);
+    CHIP_ERROR AddSrpService(const char * aInstanceName, const char * aName, uint16_t aPort,
+                             const Span<const char * const> & aSubTypes, const Span<const Mdns::TextEntry> & aTxtEntries,
+                             uint32_t aLeaseInterval, uint32_t aKeyLeaseInterval);
     CHIP_ERROR RemoveSrpService(const char * aInstanceName, const char * aName);
     CHIP_ERROR RemoveAllSrpServices();
     CHIP_ERROR SetupSrpHost(const char * aHostName);
@@ -228,10 +229,11 @@ inline CHIP_ERROR ThreadStackManager::SetThreadEnabled(bool val)
 
 #if CHIP_DEVICE_CONFIG_ENABLE_THREAD_SRP_CLIENT
 inline CHIP_ERROR ThreadStackManager::AddSrpService(const char * aInstanceName, const char * aName, uint16_t aPort,
-                                                    chip::Mdns::TextEntry * aTxtEntries, size_t aTxtEntiresSize,
-                                                    uint32_t aLeaseInterval = 0, uint32_t aKeyLeaseInterval = 0)
+                                                    const Span<const char * const> & aSubTypes,
+                                                    const Span<const Mdns::TextEntry> & aTxtEntries, uint32_t aLeaseInterval = 0,
+                                                    uint32_t aKeyLeaseInterval = 0)
 {
-    return static_cast<ImplClass *>(this)->_AddSrpService(aInstanceName, aName, aPort, aTxtEntries, aTxtEntiresSize, aLeaseInterval,
+    return static_cast<ImplClass *>(this)->_AddSrpService(aInstanceName, aName, aPort, aSubTypes, aTxtEntries, aLeaseInterval,
                                                           aKeyLeaseInterval);
 }
 

src/lib/mdns/Advertiser.h

@@ -44,12 +44,14 @@ static constexpr size_t kKeyRotatingIdMaxLength         = 100;
 static constexpr size_t kKeyPairingInstructionMaxLength = 128;
 static constexpr size_t kKeyPairingHintMaxLength        = 10;
 
-static constexpr size_t kSubTypeShortDiscriminatorMaxLength = 3;
-static constexpr size_t kSubTypeLongDiscriminatorMaxLength  = 4;
-static constexpr size_t kSubTypeVendorMaxLength             = 5;
-static constexpr size_t kSubTypeDeviceTypeMaxLength         = 5;
-static constexpr size_t kSubTypeCommissioningModeMaxLength  = 1;
-static constexpr size_t kSubTypeAdditionalPairingMaxLength  = 1;
+static constexpr size_t kSubTypeShortDiscriminatorMaxLength = 4; // _S<dd>
+static constexpr size_t kSubTypeLongDiscriminatorMaxLength  = 6; // _L<dddd>
+static constexpr size_t kSubTypeVendorMaxLength             = 7; // _V<ddddd>
+static constexpr size_t kSubTypeDeviceTypeMaxLength         = 5; // _T<ddd>
+static constexpr size_t kSubTypeCommissioningModeMaxLength  = 3; // _C<d>
+static constexpr size_t kSubTypeAdditionalPairingMaxLength  = 3; // _A<d>
+static constexpr size_t kSubTypeMaxNumber                   = 6;
+static constexpr size_t kSubTypeMaxLength                   = 7;
 
 enum class CommssionAdvertiseMode : uint8_t
 {

src/lib/mdns/Discovery_ImplPlatform.cpp

@@ -136,14 +136,14 @@ CHIP_ERROR DiscoveryImplPlatform::Advertise(const CommissionAdvertisingParameter
     TextEntry textEntries[9];
     size_t textEntrySize = 0;
     // add underscore, character and newline to lengths for sub types (ex. _S<ddd>)
-    char shortDiscriminatorSubtype[kSubTypeShortDiscriminatorMaxLength + 3];
-    char longDiscriminatorSubtype[kSubTypeLongDiscriminatorMaxLength + 4];
-    char vendorSubType[kSubTypeVendorMaxLength + 3];
-    char commissioningModeSubType[kSubTypeCommissioningModeMaxLength + 3];
-    char openWindowSubType[kSubTypeAdditionalPairingMaxLength + 3];
-    char deviceTypeSubType[kSubTypeDeviceTypeMaxLength + 3];
+    char shortDiscriminatorSubtype[kSubTypeShortDiscriminatorMaxLength + 1];
+    char longDiscriminatorSubtype[kSubTypeLongDiscriminatorMaxLength + 1];
+    char vendorSubType[kSubTypeVendorMaxLength + 1];
+    char commissioningModeSubType[kSubTypeCommissioningModeMaxLength + 1];
+    char openWindowSubType[kSubTypeAdditionalPairingMaxLength + 1];
+    char deviceTypeSubType[kSubTypeDeviceTypeMaxLength + 1];
     // size of subTypes array should be count of SubTypes above
-    const char * subTypes[6];
+    const char * subTypes[kSubTypeMaxNumber];
     size_t subTypeSize = 0;
 
     if (!mMdnsInitialized)

src/lib/support/Span.h

@@ -35,6 +35,7 @@ class Span
 {
 public:
     using pointer = T *;
+    using const_pointer = const T *;
 
     constexpr Span() : mDataBuf(nullptr), mDataLen(0) {}
     constexpr Span(pointer databuf, size_t datalen) : mDataBuf(databuf), mDataLen(datalen) {}
@@ -45,6 +46,10 @@ class Span
     constexpr pointer data() const { return mDataBuf; }
     constexpr size_t size() const { return mDataLen; }
     constexpr bool empty() const { return size() == 0; }
+    constexpr const_pointer begin() const { return data(); }
+    constexpr const_pointer end() const { return data() + size(); }
+    constexpr pointer begin() { return data(); }
+    constexpr pointer end() { return data() + size(); }
 
     // Allow data_equal for spans that are over the same type up to const-ness.
     template <class U, typename = std::enable_if_t<std::is_same<std::remove_const_t<T>, std::remove_const_t<U>>::value>>

src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.cpp

@@ -1068,12 +1068,14 @@ void GenericThreadStackManagerImpl_OpenThread<ImplClass>::OnSrpClientStateChange
 
 template <class ImplClass>
 CHIP_ERROR GenericThreadStackManagerImpl_OpenThread<ImplClass>::_AddSrpService(const char * aInstanceName, const char * aName,
-                                                                               uint16_t aPort, chip::Mdns::TextEntry * aTxtEntries,
-                                                                               size_t aTxtEntriesSize, uint32_t aLeaseInterval,
-                                                                               uint32_t aKeyLeaseInterval)
+                                                                               uint16_t aPort,
+                                                                               const Span<const char * const> & aSubTypes,
+                                                                               const Span<const Mdns::TextEntry> & aTxtEntries,
+                                                                               uint32_t aLeaseInterval, uint32_t aKeyLeaseInterval)
 {
     CHIP_ERROR error                         = CHIP_NO_ERROR;
     typename SrpClient::Service * srpService = nullptr;
+    size_t entryId                           = 0;
 
     Impl()->LockThreadStack();
 
@@ -1089,9 +1091,7 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread<ImplClass>::_AddSrpService(c
         // Remove possible existing entry
         if ((strcmp(service.mInstanceName, aInstanceName) == 0) && (strcmp(service.mName, aName) == 0))
         {
-            VerifyOrExit(MapOpenThreadError(otSrpClientClearService(mOTInst, &(service.mService))) == CHIP_NO_ERROR,
-                         error = MapOpenThreadError(OT_ERROR_FAILED));
-
+            SuccessOrExit(error = MapOpenThreadError(otSrpClientClearService(mOTInst, &service.mService)));
             // Clear memory immediately, as OnSrpClientNotification will not be called.
             memset(&service, 0, sizeof(service));
         }
@@ -1119,35 +1119,54 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread<ImplClass>::_AddSrpService(c
 
     srpService->mService.mPort = aPort;
 
-    // Check if there are some optional text entries to add.
-    if (aTxtEntries && aTxtEntriesSize != 0)
+#if OPENTHREAD_API_VERSION >= 132
+    VerifyOrExit(aSubTypes.size() <= ArraySize(srpService->mSubTypeBuffers), error = CHIP_ERROR_BUFFER_TOO_SMALL);
+    entryId = 0;
+
+    for (const char * subType : aSubTypes)
     {
-        VerifyOrExit(aTxtEntriesSize <= SrpClient::kMaxTxtEntriesNumber, error = CHIP_ERROR_INVALID_LIST_LENGTH);
+        auto & destBuffer = srpService->mSubTypeBuffers[entryId];
+        VerifyOrExit(strlen(subType) < ArraySize(destBuffer), error = CHIP_ERROR_BUFFER_TOO_SMALL);
+        strcpy(destBuffer, subType);
 
-        srpService->mService.mNumTxtEntries = static_cast<uint8_t>(aTxtEntriesSize);
+        srpService->mSubTypes[entryId++] = destBuffer;
+    }
 
-        for (uint8_t entryId = 0; entryId < aTxtEntriesSize; entryId++)
-        {
-            VerifyOrExit(aTxtEntries[entryId].mDataSize <= SrpClient::kMaxTxtValueSize, error = CHIP_ERROR_BUFFER_TOO_SMALL);
-            VerifyOrExit((strlen(aTxtEntries[entryId].mKey) + 1) <= SrpClient::kMaxTxtKeySize, error = CHIP_ERROR_BUFFER_TOO_SMALL);
+    srpService->mSubTypes[entryId]      = nullptr;
+    srpService->mService.mSubTypeLabels = srpService->mSubTypes;
+#endif
 
-            srpService->mTxtEntries[entryId].mValueLength = static_cast<uint8_t>(aTxtEntries[entryId].mDataSize);
-            memcpy(&(srpService->mTxtValueBuffers[entryId][0]), aTxtEntries[entryId].mData, aTxtEntries[entryId].mDataSize);
-            srpService->mTxtEntries[entryId].mValue = &(srpService->mTxtValueBuffers[entryId][0]);
+    // Check if there are some optional text entries to add.
+    VerifyOrExit(aTxtEntries.size() <= ArraySize(srpService->mTxtEntries), error = CHIP_ERROR_BUFFER_TOO_SMALL);
+    entryId = 0;
 
-            memcpy(&(srpService->mTxtKeyBuffers[entryId][0]), aTxtEntries[entryId].mKey, strlen(aTxtEntries[entryId].mKey) + 1);
-            srpService->mTxtEntries[entryId].mKey = &(srpService->mTxtKeyBuffers[entryId][0]);
-        }
+    for (const chip::Mdns::TextEntry & entry : aTxtEntries)
+    {
+        VerifyOrExit(strlen(entry.mKey) < SrpClient::kMaxTxtKeySize, error = CHIP_ERROR_BUFFER_TOO_SMALL);
+        strcpy(srpService->mTxtKeyBuffers[entryId], entry.mKey);
+
+        VerifyOrExit(entry.mDataSize <= SrpClient::kMaxTxtValueSize, error = CHIP_ERROR_BUFFER_TOO_SMALL);
+        memcpy(srpService->mTxtValueBuffers[entryId], entry.mData, entry.mDataSize);
 
-        srpService->mService.mTxtEntries = srpService->mTxtEntries;
+        srpService->mTxtEntries[entryId].mKey         = srpService->mTxtKeyBuffers[entryId];
+        srpService->mTxtEntries[entryId].mValue       = srpService->mTxtValueBuffers[entryId];
+        srpService->mTxtEntries[entryId].mValueLength = static_cast<uint8_t>(entry.mDataSize);
+        ++entryId;
     }
 
+    srpService->mService.mNumTxtEntries = static_cast<uint8_t>(aTxtEntries.size());
+    srpService->mService.mTxtEntries    = srpService->mTxtEntries;
+
     ChipLogProgress(DeviceLayer, "advertising srp service: %s.%s", srpService->mService.mInstanceName, srpService->mService.mName);
     error = MapOpenThreadError(otSrpClientAddService(mOTInst, &(srpService->mService)));
 
 exit:
-    Impl()->UnlockThreadStack();
+    if (srpService != nullptr && error != CHIP_NO_ERROR)
+    {
+        memset(srpService, 0, sizeof(*srpService));
+    }
 
+    Impl()->UnlockThreadStack();
     return error;
 }
 

src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.h

@@ -93,8 +93,9 @@ class GenericThreadStackManagerImpl_OpenThread
     void _OnWoBLEAdvertisingStop(void);
 
 #if CHIP_DEVICE_CONFIG_ENABLE_THREAD_SRP_CLIENT
-    CHIP_ERROR _AddSrpService(const char * aInstanceName, const char * aName, uint16_t aPort, chip::Mdns::TextEntry * aTxtEntries,
-                              size_t aTxtEntiresSize, uint32_t aLeaseInterval, uint32_t aKeyLeaseInterval);
+    CHIP_ERROR _AddSrpService(const char * aInstanceName, const char * aName, uint16_t aPort,
+                              const Span<const char * const> & aSubTypes, const Span<const Mdns::TextEntry> & aTxtEntries,
+                              uint32_t aLeaseInterval, uint32_t aKeyLeaseInterval);
     CHIP_ERROR _RemoveSrpService(const char * aInstanceName, const char * aName);
     CHIP_ERROR _RemoveAllSrpServices();
     CHIP_ERROR _SetupSrpHost(const char * aHostName);
@@ -133,6 +134,11 @@ class GenericThreadStackManagerImpl_OpenThread
             otSrpClientService mService;
             char mInstanceName[kMaxInstanceNameSize + 1];
             char mName[kMaxNameSize + 1];
+#if OPENTHREAD_API_VERSION >= 132
+            // TODO: use fixed buffer allocator to reduce the memory footprint from N*M to sum(M_i)
+            char mSubTypeBuffers[chip::Mdns::kSubTypeMaxNumber][chip::Mdns::kSubTypeMaxLength + 1];
+            const char * mSubTypes[chip::Mdns::kSubTypeMaxNumber + 1]; // extra entry for nullptr at the end
+#endif
             otDnsTxtEntry mTxtEntries[kMaxTxtEntriesNumber];
             uint8_t mTxtValueBuffers[kMaxTxtEntriesNumber][kMaxTxtValueSize];
             char mTxtKeyBuffers[kMaxTxtEntriesNumber][kMaxTxtKeySize];

src/platform/OpenThread/MdnsImpl.cpp

@@ -39,23 +39,18 @@ const char * GetProtocolString(MdnsServiceProtocol protocol)
 
 CHIP_ERROR ChipMdnsPublishService(const MdnsService * service)
 {
-    CHIP_ERROR result = CHIP_NO_ERROR;
-
-    VerifyOrExit(service, result = CHIP_ERROR_INVALID_ARGUMENT);
+    ReturnErrorCodeIf(service == nullptr, CHIP_ERROR_INVALID_ARGUMENT);
     if (strcmp(service->mHostName, "") != 0)
     {
-        CHIP_ERROR hostNameErr = ThreadStackMgr().SetupSrpHost(service->mHostName);
-        VerifyOrExit(hostNameErr == CHIP_NO_ERROR, result = hostNameErr);
+        ReturnErrorOnFailure(ThreadStackMgr().SetupSrpHost(service->mHostName));
     }
 
     char serviceType[chip::Mdns::kMdnsTypeAndProtocolMaxSize + 1];
     snprintf(serviceType, sizeof(serviceType), "%s.%s", service->mType, GetProtocolString(service->mProtocol));
 
-    result =
-        ThreadStackMgr().AddSrpService(service->mName, serviceType, service->mPort, service->mTextEntries, service->mTextEntrySize);
-
-exit:
-    return result;
+    Span<const char * const> subTypes(service->mSubTypes, service->mSubTypeSize);
+    Span<const TextEntry> textEntries(service->mTextEntries, service->mTextEntrySize);
+    return ThreadStackMgr().AddSrpService(service->mName, serviceType, service->mPort, subTypes, textEntries);
 }
 
 CHIP_ERROR ChipMdnsStopPublish()