Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[dnssd-server] implement DNS-SD discovery proxy functionality in core #10050

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

abtink
Copy link
Member

@abtink abtink commented Apr 19, 2024

This commit implements a generic discovery proxy in the DNS-SD server. It uses a set of newly added otPlatDnssd platform DNS-SD(mDNS) APIs to start or stop browsers, SRV/TXT resolvers, and IPv6/IPv4 address resolvers. These APIs closely match the native OpenThread mDNS implementation.

OpenThread DNS-SD's existing QueryCallback mechanism, enabling customized discovery proxy implementations, remains supported.

This commit includes a comprehensive unit test. This test validates the discovery proxy's behavior, covering: standard use cases, request timeout, s hared resolver/browser usage for multiple queries with the same name, filtering of invalid addresses, and various edge cases.

Copy link

size-report bot commented Apr 19, 2024

Size Report of OpenThread

Merging #10050 into main(0abc8d7).

name branch text data bss total
ot-cli-ftd main 466976 856 66348 534180
#10050 466976 856 66348 534180
+/- 0 0 0 0
ot-ncp-ftd main 435828 760 61560 498148
#10050 435828 760 61560 498148
+/- 0 0 0 0
libopenthread-ftd.a main 236153 95 40294 276542
#10050 236153 95 40294 276542
+/- 0 0 0 0
libopenthread-cli-ftd.a main 57533 0 8075 65608
#10050 57533 0 8075 65608
+/- 0 0 0 0
libopenthread-ncp-ftd.a main 31863 0 5916 37779
#10050 31863 0 5916 37779
+/- 0 0 0 0
ot-cli-mtd main 364464 760 51204 416428
#10050 364464 760 51204 416428
+/- 0 0 0 0
ot-ncp-mtd main 346996 760 46432 394188
#10050 346996 760 46432 394188
+/- 0 0 0 0
libopenthread-mtd.a main 158140 0 25166 183306
#10050 158140 0 25166 183306
+/- 0 0 0 0
libopenthread-cli-mtd.a main 39756 0 8059 47815
#10050 39756 0 8059 47815
+/- 0 0 0 0
libopenthread-ncp-mtd.a main 24743 0 5916 30659
#10050 24743 0 5916 30659
+/- 0 0 0 0
ot-cli-ftd-br main 550656 864 131188 682708
#10050 550672 864 131188 682724
+/- +16 0 0 +16
libopenthread-ftd-br.a main 324494 100 105110 429704
#10050 325087 100 105110 430297
+/- +593 0 0 +593
libopenthread-cli-ftd-br.a main 71394 0 8099 79493
#10050 71394 0 8099 79493
+/- 0 0 0 0
ot-rcp main 62248 564 20604 83416
#10050 62248 564 20604 83416
+/- 0 0 0 0
libopenthread-rcp.a main 9542 0 5052 14594
#10050 9542 0 5052 14594
+/- 0 0 0 0
libopenthread-radio.a main 18932 0 214 19146
#10050 18932 0 214 19146
+/- 0 0 0 0

@abtink abtink force-pushed the disc-proxy-new branch 3 times, most recently from c09d867 to 8bf2e5e Compare April 20, 2024 16:26
@abtink abtink marked this pull request as ready for review April 21, 2024 21:09
@jwhui jwhui requested a review from Vyrastas May 23, 2024 14:36
Comment on lines 558 to 559
* Refer to `otPlatDnssdAddressResolver` for documentation of member fields and `otMdnsStartIp6Resolver()` or
* `otMdnsStartIp4Resolver()` for how they are used.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

otMdnsStartIp6Resolver --> otMdnsStartIp6AddressResolver
otMdnsStartIp4Resolver --> otMdnsStartIp4AddressResolver

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in new push. Thanks.

Comment on lines 423 to 426
typedef struct otPlatDnssdBrowseResult otPlatDnssdBrowseResult;
typedef struct otPlatDnssdSrvResult otPlatDnssdSrvResult;
typedef struct otPlatDnssdTxtResult otPlatDnssdTxtResult;
typedef struct otPlatDnssdAddressResult otPlatDnssdAddressResult;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Documentation that you have for these below don't come through in Doxygen when they are typedef'd up here:

image

I assume you've done this for a reason... can the documentation comment for each also be put up here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the new push, I've re-arranged the type definitions and removed these.

These were intended as forward declarations so we can define the types in a more intuitive sequence. The challenge is that the Browser/Resolver types reference their corresponding Callback types, which in turn reference the Result type. We now define Result first, followed by Callback, and finally the Browser/Resolver structures.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for rearranging, everything comes through now 👍

This commit implements a generic discovery proxy in the DNS-SD server.
It uses a set of newly added `otPlatDnssd` platform DNS-SD(mDNS) APIs
to start or stop browsers, SRV/TXT resolvers, and IPv6/IPv4 address
resolvers. These APIs closely match the native OpenThread mDNS
implementation.

OpenThread DNS-SD's existing `QueryCallback` mechanism, enabling
customized discovery proxy implementations, remains supported.

This commit includes a comprehensive unit test. This test validates
the discovery proxy's behavior, covering: standard use cases, request
timeout, s hared resolver/browser usage for multiple queries with the
same name, filtering of invalid addresses, and various edge cases.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants