Question related to otPlatDns API documentation

[Cristib05]

Hello,
In https://github.com/openthread/openthread/blob/main/include/openthread/platform/dns.h, I can see that for otPlatDnsStartUpstreamQuery documentation, the following is stated:
The OpenThread core may cancel a (possibly timeout) query transaction by calling otPlatDnsCancelUpstreamQuery, the platform must not call otPlatDnsUpstreamQueryDone on a cancelled transaction. https://github.com/openthread/openthread/blob/main/include/openthread/platform/dns.h#L83
But for otPlatDnsCancelUpstreamQuery documentation, it is stated:
The platform must call otPlatDnsUpstreamQueryDone to release the resources.

Also, in simultion code, it seems that the call to otPlatDnsUpstreamQueryDone is performed: https://github.com/openthread/openthread/blob/main/examples/platforms/simulation/dns.c#L49

I would like to understand if there is a documentation error for otPlatDnsStartUpstreamQuery documentation and if the platform is expected to call otPlatDnsUpstreamQueryDone when otPlatDnsCancelUpstreamQuery is called.

Thanks!

[abtink]

@Cristib05, thanks for reporting this.

I am not the original author of the "upstream DNS code" and am not super familiar with it. To understand this, I read the code to see how it works and also tracked the git history to see what was added/changed and when.

This whole feature was added in this PR:

• [dnssd] support forwarding DNS queries to platform upstream #8668

And the same inconsistent documentation statements are present in this original PR. So it was unfortunately there since the beginning!

---

From reading the code, it looks like the implementation does require that the otPlatDnsUpstreamQueryDone() callback be invoked when otPlatDnsCancelUpstreamQuery is called.

Here are the details:

• otPlatDnsCancelUpstreamQuery() is called in HandleTimer() when the query timeout timer expires.
• It does not reset the allocated transaction in HandleTimer().
• The ResetUpstreamQueryTransaction() (which resets/deallocates the transaction) is called from Server::OnUpstreamQueryDone(), which is itself called from otPlatDnsUpstreamQueryDone().

Also, as you pointed out, the simulation platform implementation does call the Done callback on Cancel.
The same is the case for the posix implementation:

openthread/src/posix/platform/resolver.cpp

Lines 315 to 327 in 47f0417

	void Resolver::Cancel(otPlatDnsUpstreamQuery *aTxn)
	{
	Transaction *txn = GetTransaction(aTxn);
	if (txn != nullptr)
	{
	CloseTransaction(txn);
	}
	otPlatDnsUpstreamQueryDone(gInstance, aTxn, nullptr);
	}

[abtink]

Submitted

• [dnssd] correct DNS upstream query cancellation documentation #12210

[Cristib05]

Thanks @abtink for taking time to look on this!