Skip to content

Commit

Permalink
Fix unsolicited announcements with minimal mDNS
Browse files Browse the repository at this point in the history
As of 97d9f7e ("Address 9154 and CM/AC DNS-SD convergence (project-chip#9348)")
operational discovery during chip-tool's commissioning flow typically
fails with minimal mDNS. This occurs because this change unintentionally
broke the logic to send unsolicited mDNS announcements as required by
RFC 6762.

During commissioning, the initial mDNS query to resolve the operational
service on the network is generally sent before the device is able to
join the network. The is perfectly fine because RFC 6762 accounts for
this case and requires devices joining the network to advertise all of
their records. These unsolicited responses provide the IP address and
allow the final commissioning steps to execute.

The minimal mDNS implementation sends its unsolicited announcements in
Start() and not at any other time. In the above change, we started
calling StopPublishDevice() immediately prior to (re-)starting the
server, leaving no records to announce. The records are added
immediately afterwards, but currently adding records does not trigger
additional announcements. Thus, a response never arrives and
commissioning times out.

Fix this by adding an announcement after the records are added.
Unfortunately this will quite spammy since we announce everything and
advertise is called several times in a row. What should happen is
announcements only happen with new or updated records, or when new
interfaces are enabled.

(Note that this problem is greatly exacerbated by the lack of
retransmissions of queries, which is being addressed in
project-chip#9900).
  • Loading branch information
mspang committed Sep 24, 2021
1 parent bc4d711 commit 127fade
Showing 1 changed file with 8 additions and 0 deletions.
8 changes: 8 additions & 0 deletions src/lib/mdns/Advertiser_ImplMinimalMdns.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,10 @@ CHIP_ERROR AdvertiserMinMdns::Advertise(const OperationalAdvertisingParameters &

ChipLogProgress(Discovery, "CHIP minimal mDNS configured as 'Operational device'.");

// Advertise the records we just added as required by RFC 6762.
// TODO - Don't announce records that haven't been updated.
AdvertiseRecords();

return CHIP_NO_ERROR;
}

Expand Down Expand Up @@ -609,6 +613,10 @@ CHIP_ERROR AdvertiserMinMdns::Advertise(const CommissionAdvertisingParameters &
ChipLogProgress(Discovery, "CHIP minimal mDNS configured as 'Commissioner device'.");
}

// Advertise the records we just added as required by RFC 6762.
// TODO - Don't announce records that haven't been updated.
AdvertiseRecords();

return CHIP_NO_ERROR;
}

Expand Down

0 comments on commit 127fade

Please sign in to comment.