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

.browse for all subservice's #145

Open
oscartbeaumont opened this issue Oct 29, 2023 · 9 comments
Open

.browse for all subservice's #145

oscartbeaumont opened this issue Oct 29, 2023 · 9 comments

Comments

@oscartbeaumont
Copy link

I am looking into using sub-services with mdns-sd and am wondering if there is a way to subscribe to events for all subservices of a specific service.

For example, I want to callmdns_daemon.browse('my-service._udp.local.') and receive events for a._sub._my-service._udp.local., b._sub._my-service._udp.local. and etc. When subscribing to the "parent" service currently events are received, however, the sub_domain property on the Service will be None which means I don't know which subservice (a or b) the event came from which is super important for my usecase.

It seems like internally a._sub._my-service._udp.local. becomes an advertisement for a._sub._my-service._udp.local. and one for _my-service._udp.local. which explains this behavior but it still seems a little bit weird.

As a workaround, I am going to call .browse(...) for each subservice and join them together with StreamUnordered, however, this makes managing the whole thing a lot more complicated and error prone. Preferably I would like to have only a single flume::Reciever for all events which would simplify my code a heap.

Thanks for this library, it's been working super well!

@keepsimple1
Copy link
Owner

Thanks for opening the issue! It seems a valid use case. I've opened a PR to address this based on my understanding. If you got chance, let me know if the patch would work for you.

@oscartbeaumont
Copy link
Author

Thanks for the quick response. I have given the changes a try and they are working for the original issue, thanks heaps!

However, I have found another issue. The metadata for subservices is getting messed up.

I am advertising the services:

  • node._sub._my-service._udp.local. with the metadata { "i": "0" }
  • another._sub._my-service._udp.local. with the metadata { "i": "1" }

and the metadata value for node._sub._my-service._udp.local. will start as { "i": "0" } and then change to { "i": "1" }. I was also observing this behavior when using StreamUnordered, so I think this is an independent issue.

I feel like maybe this issue is related to service.full_name() not containing {srv}._sub. but I am not super familiar with the spec so I have no idea if this is expected or not.

@keepsimple1
Copy link
Owner

I am advertising the services:

  • node._sub._my-service._udp.local. with the metadata { "i": "0" }
  • another._sub._my-service._udp.local. with the metadata { "i": "1" }

do you advertise two different instance names for these two subtypes? or do you have one instance name that provides both subtype services?

Regarding the spec, the RFC6763 section 7.1 states the following:

...Note that the advertised web page's Service Instance Name is
   unchanged by the use of subtypes -- it is still something of the form
   "The Server._http._tcp.example.com.",

i.e. the service instance full name does not include {sub_type}._sub.. ( I don't use subtypes in my own services, but that's my understanding).

@oscartbeaumont
Copy link
Author

Yeah, I do have the same instance type for all subservices so that would explain the behaviour.

Working through potential solutions for my usecase, I don't think their is one right now.

  • I don't think I can extend the instance name to also include the subservice name because it will probs overflow the DNS record. Right now it would be 75 chars (a UUID for the subservice and a certificate hash for the instance) and the max length in the spec says 63.
  • I can't include the subservice name within the main service name (Eg. _{subservice}_{service}.local) because that would break the ability to .browse(...) for all subtypes. This solution would, however, be fine for DNS record length.

I wonder if you would be open for mdns-sd to have an API like browse_all() which would receive events for all incoming mDNS events regardless of service name? An API like that would allow me to include the subservice name within the main service name (_{subservice}_my-service._udp.local.) while not requiring me to call .browse for each service I would like the listen to (because properly knowing them ahead of time is annoying in my setup).

Thanks heaps for your help!

@keepsimple1
Copy link
Owner

Let me think a bit about browse_all() API.

And, it seems to me that there are 2 things in play:

  1. One service instance advertising more than 1 sub service types.

This could be supported by change ServiceInfo.sub_domain from a single string to a Vec of strings, a new method to set them and related changes. So that the user don't have to call register for each sub service type.

  1. One service instance might have different metadata (property settings) for different sub service types.

To allow this, what if including the sub service type name in the meta-data key itself? For example, instead of
{ "i" : "0"} , use `{"node._sub.i": "0"} ? would that work?

@keepsimple1
Copy link
Owner

  • I don't think I can extend the instance name to also include the subservice name because it will probs overflow the DNS record. Right now it would be 75 chars (a UUID for the subservice and a certificate hash for the instance) and the max length in the spec says 63.

Just a thought, how many bytes is the cert. hash? Is it possible to move the cert. hash into the metadata (TXT record)? The UUID of subservice gives uniqueness of each instance name, so you don't need to utilize "subtype / subdomain" any more. And there are different instances so that no more metadata overwrites.

@oscartbeaumont
Copy link
Author

oscartbeaumont commented Oct 31, 2023

For 1) I am not exactly sure what you're saying. Could you please clarify?

For 2) I did think about that but then figured it probably wouldn't work because I figured it would still result in only one of the metadata making it through properly to the other end. Without knowing the internals I, however, don't know if this is true or not.

I suspect using the UUID as the instance name by itself would not work because it's shared across nodes so multiple machines would end up advertising the same instance name. The certificate hash is unique for each node but the UUID is for a library which is a collection of resources that are synced between many nodes.

Something I just thought of is maybe I could have a global AtomicBool and join it to each instance's name (the public key hash) and then on the other node I can just split it off on the other end. That would add minimal length to it so should be fine from a record length perspective and I think that would allow me to use subservices for the discovery phase while still having the metadata not get messed up.

Thanks for taking the time to help!

@keepsimple1
Copy link
Owner

For 1) I am not exactly sure what you're saying. Could you please clarify?

Currently one service instance can only have one subtype (if any), which is stored in the sub_domain field of ServiceInfo.

In your use case, if you wanted to use one service instance for all sub service types, we would need to support multiple sub domains in one ServiceInfo, possibly a Vec<String>. This would introduce changes to some existing API like get_subtype() and add some new APIs to set the subtype Vec. But if it's a valid use case, we could add the support.

That said, from your descriptions, it seems like conceptually you have multiple different service instances (for one thing, the instances' metadata are different). So it might be cleaner to have one instance mapping to one sub service type. I don't really understand the use of AtomicBool unless there are only up to 2 different instance / sub service types ? But if that works for you, then go for it ;-).

For the PR, I think it's a good change anyway. I will merge it for now. If there are additional things we need to do for this issue, we can always open new PRs.

@keepsimple1
Copy link
Owner

Let me think a bit about browse_all() API.

Just wanted to follow up, sorry being a bit late, I feel that adding such API is too much hack for solving the original issue. (In mdns spec RFC 6763, there is meta-query to get a list of all services, but that's different, i.e. it only lists the service types, not instances of all services. And this lib already supports that).

I don't know whether you solved the issue by now, but if there is anything else you think this library can add / change to help, please comment. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants