Ticket #6 (closed defect: invalid)

Opened 3 years ago

Last modified 3 years ago

doesn't seem to update txt record

Reported by: anonymous Assigned to: somebody
Priority: major Milestone:
Component: component1 Version:
Keywords: Cc:

Description

Rhythmbox publishes a DAAP service. We are adding a password capability to it which will be published in a txt record. This capability is able to be changed by the user (ie. to either require a password or not). When updating the service name we call avahi_entry_group_add_service_strlst with the AVAHI_PUBLISH_UPDATE flag and that works fine. However, when we try to update the txt record "Password=false" to "Password=true" we always get a collision. The daemon debug says:

Recieved conflicting record [None._daap._tcp.local IN TXT "Password=true" "org.freedesktop.Avahi.cookie=1683616268" ; ttl=4500]. Resetting our record.

Recieved conflicting record [None._daap._tcp.local IN TXT "Password=true" "org.freedesktop.Avahi.cookie=1683616268" ; ttl=4500] with local record to be. Withdrawing.

Change History

01/27/06 19:54:57 changed by lennart

Works fine here.

When renaming a service it doesn't make sense to set AVAHI_PUBLISH_UPDATE and readd the service name. This should fail. AVAHI_PUBLISH_UPDATE is useful when an existing record should be replaced with a new one, where the name, the type and the class stay the same but the rdata doesn't.

In short: when you rename a service you should first reset the entry group using avahi_entry_group_reset() and than readd your services. When you need to modify just the SRV data you should use AVAHI_PUBLISH_UPDATE. When you need to modify the TXT data, you should use avahi_entry_group_update_service_txt().

And make sure to always check the return value of these calls.

Could you point me to the code that resulted in these issues you reported?

01/27/06 20:07:55 changed by mccann@jhu.edu

I don't really know what the SRV data is. I was just going on what it says on the main page of the docs: http://avahi.org/download/doxygen/

"

  • If the entry group enters AVAHI_ENTRY_GROUP_COLLISION state the services of the entry group are automatically removed from the server. You may immediately add your services back to the entry group (but with new names, perhaps using avahi_alternative_service_name()) and commit again. Please do not free the entry group and create a new one. This would inhibit some traffic limiting algorithms in mDNS.
  • When you need to modify your services, use the AVAHI_PUBLISH_UPDATE flag. Please do not free the entry group and create a new one. This would inhibit some traffic limiting algorithms in mDNS.

"

So, we have two cases where we want to change things: 1. when we get a collision 2. when the user changes a setting (service name or password-required) or both.

So, it sounds like I don't need to use the UPDATE flag?

I guess the strange part is when the name changes and TXT changes. Shouldn't I just be able to use add_service?

The code isn't in CVS yet. An older version of the patch is in: http://bugzilla.gnome.org/show_bug.cgi?id=322966

01/27/06 20:16:20 changed by anonymous

Oh, the port number can change also depending on where libsoup server decides to listen.

01/27/06 20:20:49 changed by mccann@jhu.edu

So, I guess what I'm asking is how do I unconditionally update any and all of the information (name, TXT, port)?

01/27/06 20:44:05 changed by anonymous

FYI, I uploaded the most recent patch the RB bug.

01/27/06 20:46:33 changed by lennart

Not quite.

In case 1, the collision, just readd your service with the new name. Don't set AVAHI_PUBLISH_UPDATE or anything.

In case 2a, the user changes the service name. Call avahi_entry_group_reset() manually. And than readd the service with the new name, just like in case 1.

In case 2b, the user adds/removes a password. Call avahi_entry_group_update_txt() and pass the new data.

If you're lazy you could handle the three cases the same, like in 2a. However, this will cause slightly more traffic. But I guess this doesn't matter. avahi_entry_group_update_txt() is mostly there to cut down the traffic in apps that change their TXT really, really often. (i.e. more often than once every minute)

BTW: I had a quick look on your code, some notes:

  • There's no need to build an AvahiStringList?? when publishing in your case. Just pass the TXT data you want to publish (e.g. "Password=true") as last argument to avahi_entry_group_add_service() (and a trailing NULL).
  • Please don't specify AVAHI_PUBLISH_USE_MULTICAST unless you have a good reason to do so. Just pass 0. If you specify AVAHI_PUBLISH_USE_MULTICAST you force mDNS in all cases, which doesn't make sense when unicast DNS would do, too.
  • This is nonsense: str_list = avahi_string_list_new ("", NULL). This creates a string list with a single empty string. An empty list is just a NULL, just like glib's GSList.
  • What's the point in allocating a variable "flags" on the stack when calling avahi_client_new()? You can simply pass 0 directly.
  • Why do you limit your stuff to IPv4 only? You pass AVAHI_PROTO_INET when registering your stuff. Use AVAHI_PROTO_UNSPEC and you'll be found by IPv6 clients too
  • It's a waste of resources to create two AvahiClient?? objects, one for browsing and the other for publishing
  • The intended parsing of TXT records is a single loop which iterates through all entries of the AvahiStringList?? and parses the records one-by-one, with a complexity of O(n). Instead you look for each of the records you need manually, with a complexity of O(n*m). (n = number of records in TXT, m = keys understood). Please note that AvahiStringList?? is a public structure, you're welcome to modify and iterate through it.
  • If you call avahi_set_allocator (avahi_glib_allocator()), avahi_malloc() and avahi_free() are mostly identical to g_malloc() and g_free(). Hence you are welcome to free the data returned by avahi_string_list_get_pair() with g_free(), hence you need to do that copying you currently do
  • The code "host = g_malloc0 (16);" will make BOOM if your service is an IPv6 service. Consider using AVAHI_ADDRESS_STR_MAX as length for such a buffer. And consider allocating it on the stack instead of the heap
  • Please do not pass "local" as domain to browse in. This limits Avahi to multicast DNS where unicast DNS would do, too. Just pass NULL, the default.

Nonetheless, good work. And I guess Avahi ist just too complicated as API. :-(

01/27/06 20:49:14 changed by lennart

humm that should read: "... hence you need not to do that copying you currently do ..."

01/27/06 20:55:55 changed by lennart

If the port number changes you need to modify the SRV record. This cannot be done with avahi_entry_group_update_txt() (obviously...)

To keep things simple, just do as I suggested and reset the entry group and readd your stuff.

I.e.

* Case 1, in case of conflict, just readd your service

* Case 2, in case of configuration change, just call avahi_entry_group_reset() and readd your service, just like in case 1.

That's the simplest solution, i guess.

01/27/06 20:57:06 changed by anonymous

Thanks for the reply. Most of the issues you mention are things that are unmodified by this patch - they were there before - but I'm glad you mentioned them.

So, I think we'll be lazy in RB and use case 2a for everything since we don't change the TXT entry hardly ever. In fact, that is what the most recent patch should do. However, it doesn't seem to work it complains about a collision.

When I don't use a TXT at all it works. So, it seems like reset doesn't reset the TXT or something...

01/27/06 21:37:41 changed by anonymous

Thank you so much for the review. Now that I've fixed all the problems you mentioned the collision problem seems to be gone. The only reason I can think of is the change from using avahi_entry_group_add_service_strlst to avahi_entry_group_add_service. Maybe there is a problem with the former?

Thanks again!

01/27/06 21:42:15 changed by lennart

Mhmm, I still cannot reproduce the problems you experience here. If I reset an existing service and readd it with a different name it works. If i readd it with the same name but different TXT data, it works too. Even if i readd exactly the same data it works fine.

May I ask what version of Avahi you are running?

I updated the example source:trunk/examples/client-publish-service.c to include some code to show how this reset algorithm should look like. (In fact this was the program I did my tests as described above with)

01/27/06 21:45:53 changed by lennart

Mmm, i guess my last comment is obsolete now.

Mmmh, the two functions avahi_entry_group_add_service_strlst() and avahi_entry_group_add_service() are actually the same, the latter just unwraps the arguments into an AvahiStringList? and calls the former.

I think your issues was related to the empty string you had in your TXT data. Did you fix that? This is something which is not allowed by the DNS-SD spec, and we currently filter that out. I guess this filtering doesn't work correctly. I will have a look into this.

May I close this bug now?

01/27/06 22:14:32 changed by anonymous

Yes I'm using a const char * for the txt record now. Feel free to close. Thanks.

01/27/06 23:48:24 changed by lennart

  • status changed from new to closed.
  • resolution set to invalid.