Re: Patch: update Bonjour support to the newer non-deprecated API

I've grown a bit tired of reading the "'DNSServiceRegistrationCreate' is
deprecated" warnings in OS X builds, and I'm also wondering whether any
of the recently reported problems with Snow Leopard might trace to our
use of an API that Apple has been deprecating since 2003. Hence,
attached is a patch that replaces our use of the DNSServiceDiscovery.h
API with the more modern dns_sd.h API. As-is, this would break Bonjour
functionality in OS X 10.2 and before, but I really doubt that anyone
still cares about that --- any objections out there?

Like the original coding, this will bind the Bonjour service name on
all available interfaces. I suspect that we ought to do something
different if we have been told to bind to a subset of interfaces,
but it's not entirely clear to me how to get the appropriate "interface
indexes" given the information we have (particularly, the listening
socket FDs). In any case, fixing that seems like added functionality.

In principle this might enable use of Bonjour on non-Apple OSes, but
I'm not personally interested enough to test that ...

Comments, objections?

regards, tom lane

On Sep 6, 2009, at 8:17 PM, Tom Lane wrote:

> In principle this might enable use of Bonjour on non-Apple OSes, but
> I'm not personally interested enough to test that ...
>
> Comments, objections?

+1 Seems like a no-brainer.

Best,

David

Tom Lane wrote:

> In principle this might enable use of Bonjour on non-Apple OSes, but
> I'm not personally interested enough to test that ...

With this patch and Avahi's compatibility headers I can successfully
compile --enable-bonjour on my Debian system. I haven't tested it.

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Alvaro Herrera wrote:
> Tom Lane wrote:
>
> > In principle this might enable use of Bonjour on non-Apple OSes, but
> > I'm not personally interested enough to test that ...
>
> With this patch and Avahi's compatibility headers I can successfully
> compile --enable-bonjour on my Debian system. I haven't tested it.

Hmm, but it doesn't link unless I manually add -ldns_sd to the link
command.

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Alvaro Herrera wrote:

> Hmm, but it doesn't link unless I manually add -ldns_sd to the link
> command.

And then it throws a warning on start, and fails to work anyway:

*** WARNING *** The program 'postgres' uses the Apple Bonjour compatibility layer of Avahi.
*** WARNING *** Please fix your application to use the native API of Avahi!
*** WARNING *** For more information see <http://0pointer.de/avahi-compat?s=libdns_sd&e=postgres>
LOG: DNSServiceRegister() failed: error code -65540

So we're not better than when we started, but it doesn't cause any
problem if not enabled.

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
> *** WARNING *** The program 'postgres' uses the Apple Bonjour compatibility layer of Avahi.
> *** WARNING *** Please fix your application to use the native API of Avahi!
> *** WARNING *** For more information see <http://0pointer.de/avahi-compat?s=libdns_sd&e=postgres>
> LOG: DNSServiceRegister() failed: error code -65540

Hmm, I read in their documentation that the dns_sd.h interface was
deprecated, but not that it had been intentionally disabled.
Seems like they want to drive users away rather than attract them.

> So we're not better than when we started, but it doesn't cause any
> problem if not enabled.

The patch as I gave it intentionally didn't change any user-visible
behavior, but one thing that is bothering me is that if USE_BONJOUR
is selected, the postmaster will *always* try to advertise itself
via DNS-SD. There's no provision for enabling the feature or not
at run time, which is a bad thing for packagers: they have to decide
for their users whether to turn it on. There was discussion in
connection with the Avahi patch last year to the effect that some
people thought advertising the postmaster might be a security issue
for them. So I'm thinking we ought to fix that while we're messing
with it.

The two possibilities for that seem to be to change the meaning of
bonjour_name = '' (have it mean "no advertisement" instead of
"default to service name = computer's name"), or to add a separate
boolean GUC. If the latter, is the default 'on' or 'off'? Opinions?

regards, tom lane

Tom Lane wrote:
> Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
> > *** WARNING *** The program 'postgres' uses the Apple Bonjour compatibility layer of Avahi.
> > *** WARNING *** Please fix your application to use the native API of Avahi!
> > *** WARNING *** For more information see <http://0pointer.de/avahi-compat?s=libdns_sd&e=postgres>
> > LOG: DNSServiceRegister() failed: error code -65540
>
> Hmm, I read in their documentation that the dns_sd.h interface was
> deprecated, but not that it had been intentionally disabled.
> Seems like they want to drive users away rather than attract them.

I think it is supposed to work; the code suggests that it should. I
can't quite find out what the error number is supposed to mean though.
The source is here:
http://avahi.sourcearchive.com/documentation/0.6.25-1ubuntu1/avahi-compat-libdns__sd_2compat_8c-source.html

... ah! here it is -- BadParam:
http://avahi.sourcearchive.com/documentation/0.6.25-1ubuntu1/dns__sd_8h-source.html

> The patch as I gave it intentionally didn't change any user-visible
> behavior, but one thing that is bothering me is that if USE_BONJOUR
> is selected, the postmaster will *always* try to advertise itself
> via DNS-SD. There's no provision for enabling the feature or not
> at run time, which is a bad thing for packagers: they have to decide
> for their users whether to turn it on. There was discussion in
> connection with the Avahi patch last year to the effect that some
> people thought advertising the postmaster might be a security issue
> for them. So I'm thinking we ought to fix that while we're messing
> with it.
>
> The two possibilities for that seem to be to change the meaning of
> bonjour_name = '' (have it mean "no advertisement" instead of
> "default to service name = computer's name"), or to add a separate
> boolean GUC. If the latter, is the default 'on' or 'off'? Opinions?

I have a mild preference towards having a new GUC to shut it off
explicitely; and the default should be off to avoid the possible
security hole (equivalent to having listen_addresses default to
localhost, I think. On the other hand, if listen_addresses is set to
that, there is no security hole. I assume we're only publishing on
addresses we're listening on, not all addresses?)

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

On Mon, Sep 07, 2009 at 12:50:37PM -0400, Tom Lane wrote:
>
> The two possibilities for that seem to be to change the meaning of
> bonjour_name = '' (have it mean "no advertisement" instead of
> "default to service name = computer's name"), or to add a separate
> boolean GUC. If the latter, is the default 'on' or 'off'?
> Opinions?

+1 for separate boolean GUC, default off. I know extra GUCs aren't
great, but I don't see another way :/

Cheers,
David.
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david(dot)fetter(at)gmail(dot)com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

David Fetter <david(at)fetter(dot)org> writes:
> On Mon, Sep 07, 2009 at 12:50:37PM -0400, Tom Lane wrote:
>> The two possibilities for that seem to be to change the meaning of
>> bonjour_name = '' (have it mean "no advertisement" instead of
>> "default to service name = computer's name"), or to add a separate
>> boolean GUC. If the latter, is the default 'on' or 'off'?
>> Opinions?

> +1 for separate boolean GUC, default off. I know extra GUCs aren't
> great, but I don't see another way :/

Yeah, the default service name seems like a potentially useful behavior
in some contexts, so taking that behavior away doesn't seem nice.
Separate bool it is...

regards, tom lane

Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
> Tom Lane wrote:
>> Hmm, I read in their documentation that the dns_sd.h interface was
>> deprecated, but not that it had been intentionally disabled.

> I think it is supposed to work; the code suggests that it should. I
> can't quite find out what the error number is supposed to mean though.
> The source is here:
> http://avahi.sourcearchive.com/documentation/0.6.25-1ubuntu1/avahi-compat-libdns__sd_2compat_8c-source.html

> ... ah! here it is -- BadParam:
> http://avahi.sourcearchive.com/documentation/0.6.25-1ubuntu1/dns__sd_8h-source.html

Interesting. I coded the call to default everything it could, but maybe
Avahi is a little less forgiving about NULL parameters. In particular
it wouldn't surprise me if they thought that there *must* be a callback
function --- we're playing fast and loose with the API by not bothering
to check for a response from the daemon.

regards, tom lane

Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
> The source is here:
> http://avahi.sourcearchive.com/documentation/0.6.25-1ubuntu1/avahi-compat-libdns__sd_2compat_8c-source.html

After reading that code, I am content to remain forcibly non compatible
with it. If we relied on this it would inject threading into the
postmaster, which was precisely the thing that got the previous Avahi
patch rejected last year :-(

Apple's implementation of the same API does not appear to require
any threading:
http://www.opensource.apple.com/source/mDNSResponder/mDNSResponder-176.3/mDNSShared/dnssd_clientstub.c
Hm, seems to be Apache license ...

regards, tom lane

Tom Lane wrote:
> Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
> > The source is here:
> > http://avahi.sourcearchive.com/documentation/0.6.25-1ubuntu1/avahi-compat-libdns__sd_2compat_8c-source.html
>
> After reading that code, I am content to remain forcibly non compatible
> with it. If we relied on this it would inject threading into the
> postmaster, which was precisely the thing that got the previous Avahi
> patch rejected last year :-(

Should we inject some sort of compile-time rejection of the
compatibility API? Like, say

#ifdef AVAHI_SERVICE_COOKIE
#error The Avahi implementation is incompatible
#endif

inside some #ifdef BONJOUR block?

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

I wrote:
> I've grown a bit tired of reading the "'DNSServiceRegistrationCreate' is
> deprecated" warnings in OS X builds, and I'm also wondering whether any
> of the recently reported problems with Snow Leopard might trace to our
> use of an API that Apple has been deprecating since 2003. Hence,
> attached is a patch that replaces our use of the DNSServiceDiscovery.h
> API with the more modern dns_sd.h API.

BTW, I cannot find any evidence that our old Bonjour code doesn't work
on Snow Leopard: it compiles, with just the same deprecation warning
as before, and the postmaster runs, and you can see the Bonjour
registration appear with something like 'dns-sd -B _postgresql' ---
and disappear again when the postmaster exits. I had guessed that
Jerry LeVan's DNS issues might be somehow related to this code, but
apparently not.

So I'm inclined to change the code now that we've got a replacement
that doesn't cause the deprecation warning, but there doesn't seem to
be any need to back-patch it.

I will also see about adding a separate boolean control GUC, as
discussed.

regards, tom lane

Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
> Should we inject some sort of compile-time rejection of the
> compatibility API? Like, say

> #ifdef AVAHI_SERVICE_COOKIE
> #error The Avahi implementation is incompatible
> #endif

> inside some #ifdef BONJOUR block?

I looked into this and couldn't find any obvious candidate symbol to
test for. I'm not that excited about throwing roadblocks in the way
of somebody who wants to try it, anyway --- my feeling is more like
"if it breaks you get to keep both pieces".

regards, tom lane

Tom Lane wrote:
> Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
> > Should we inject some sort of compile-time rejection of the
> > compatibility API? Like, say
>
> > #ifdef AVAHI_SERVICE_COOKIE
> > #error The Avahi implementation is incompatible
> > #endif
>
> > inside some #ifdef BONJOUR block?
>
> I looked into this and couldn't find any obvious candidate symbol to
> test for. I'm not that excited about throwing roadblocks in the way
> of somebody who wants to try it, anyway --- my feeling is more like
> "if it breaks you get to keep both pieces".

Agreed -- that seems reasonable considering that one needs to
explicitely pass --enable-bonjour anyway.

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.