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

[trel-dnssd] prioritize wider-scope IPv6 address of TREL peers #2118

Merged
merged 1 commit into from
Dec 8, 2023

Conversation

abtink
Copy link
Member

@abtink abtink commented Dec 5, 2023

This commit updates TrelDnssd to select the numerically smallest IPv6 address when a peer advertises multiple IPv6 addresses. This prioritizes globally-unique addresses (GUA) over unique-local addresses (ULA) with fc00::/7 prefix, followed by link-local addresses (fe80::/10).


Should address #2116.

This commit updates `TrelDnssd` to select the numerically smallest
IPv6 address when a peer advertises multiple IPv6 addresses. This
prioritizes globally-unique addresses (GUA) over unique-local
addresses (ULA) with `fc00::/7` prefix, followed by link-local
addresses (`fe80::/10`).
Copy link
Contributor

@librasungirl librasungirl left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

@superwhd superwhd left a comment

Choose a reason for hiding this comment

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

LGTM regarding the logic of selecting address.

However, I think this alone may not be able to resolve the issue. Today in OTBR the mDNS resolvers won't return multiple addresses in one callback. When a host has multiple addresses, mDNS resolvers will trigger a callback for every address.

To fully fix the issue, we may also need to enhance the mDNS resolvers to make it return all resolved addresses at one time.

@abtink
Copy link
Member Author

abtink commented Dec 5, 2023

To fully fix the issue, we may also need to enhance the mDNS resolvers to make it return all resolved addresses at one time.

@superwhd, I think Publisher behavior to return the first address during Broswe/ServiceDiscovery may be fine and was intended behavior.

But there is a missing behavior (never implemented?) in trel_dnssd platform code which is mentioned/required in trel.h API documentation and I expect would address this. Upon discovery of a peer service, the platform should start an ongoing DNS-SD query for AAA records on the hostname of service (which should then discover all addresses on host):
trel.h

 * events are not guaranteed, however. When a TREL service instance is discovered, a new ongoing DNS-SD query for an
 * AAAA record should be started on the hostname indicated in the SRV record of the discovered instance. If multiple
 * host IPv6 addressees are discovered for a peer, one with highest scope among all addresses MUST be reported (if
 * there are multiple address at same scope, one must be selected randomly).

This would address cases where the host addresses get changed (which wont work today).

@jwhui jwhui merged commit 9f34b9d into openthread:main Dec 8, 2023
30 checks passed
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

4 participants