ippfind.c: Make multiple simultaneous searches in Avahi reliable

[zdohnal]

Currently we mark data from Avahi as ready if there are data on a specific descriptor - this does not work if ippfind is called to do multiple simultaneous searches, and the first search result is empty, so ippfind returns nothing even if the other search(es) return(s) data.

The patch works with 'avahi_got_data' global variable, which is now incremented everytime 'browse_callback()' processes event ALL_FOR_NOW, which indicates the end of result for one search. ippfind will start the processing the data once 'avahi_got_data' matches with number of searches ippfind was told to do. The change slows the processing, but it will give use more reliable answers.

[michaelrsweet]

OK, so this is something like:

ippfind _foo._tcp _bar._tcp

and you want ippfind to wait for at least one match for _foo and _bar? The problem is that we can't depend on finding any of the named services/types. The current code waits a minimum of 1 second for responses and then stops when all of the found services have been queried/processed. If a service doesn't respond within 1 second then that probably points to a network congestion issue or a problem with the device, but you can also use the -T seconds option to specify a larger initial timeout.

[zdohnal]

OK, so this is something like:

ippfind _foo._tcp _bar._tcp

and you want ippfind to wait for at least one match for _foo and _bar? The problem is that we can't depend on finding any of the named services/types.

No, my original idea (the one I had implemented in this PR) was to wait until we have data from Avahi which matches the number of searches we started, because Avahi always sends ALL_FOR_NOW every time when it finished the search (regardless it found a match or not) - so the idea was to wait until we saw a specific number of ALL_FOR_NOW events and then start processing.

However your review gave me an idea to introduce it only as default which can be overriden by setting a timeout via '-T', in case a script runs ippfind and expects it to finish in time.
IMO ippfind is a CLI tool, so it should give a reliable answer in default settings, sacrificing the short length of processing.

The current code waits a minimum of 1 second for responses and then stops when all of the found services have been queried/processed. If a service doesn't respond within 1 second

Unfortunately this is a problem if empty response arrives sooner than the response with match or Avahi sends both responses in one write() (unfortunately Avahi sometimes bundles responses if they are sent from the same source...) - ippfind processes the empty response and finishes before the match is added into services array, resulting into empty output.

My main goal with this PR is that driverless backend (@tillkamppeter) could give reliable answers as well (if it changes to run ippfind without -T), so driverless devices can be shown reliably in CUPS Web UI during installing a permanent queue for sharing - currently driverless sometimes returns the device and sometimes not, which is annoying for users and it is difficult to explain it in documentation :) . cups-deviced handles this device discovery and has a timeout on its own, so if driverless would take too much time, cups-deviced kills them afterwards, so it looks good to me on this side.

[michaelrsweet]

Hmm, I'm still not happy with the proposed "fix". The "all for now" flag isn't even reliable with mDNSResponder so the strategy ippfind (and dnssd) use is to keep the browse going until all of the found services have been resolved/queried or the timeout expires, with a minimum of 1 second to find services with the browse.

So let's actually try to fix the Avahi issue and not work around it...

WRT the "driverless" backend, if it ran the "dnssd" backend to find printers then it wouldn't wake up every printer it finds and would choose the best/preferred protocol based on the priority key in the TXT records.

[zdohnal]

Hmm, I'm still not happy with the proposed "fix". The "all for now" flag isn't even reliable with mDNSResponder

You're right - when I was testing the fix, I saw ALL_FOR_NOW event in strace output, but now I don't see it in the response... so we can't rely on it :( .

so the strategy ippfind (and dnssd) use is to keep the browse going until all of the found services have been resolved/queried or the timeout expires, with a minimum of 1 second to find services with the browse.

Ok :( since we don't have a reliable way how to know whether we have all data from Avahi...

So let's actually try to fix the Avahi issue and not work around it...

I've reported it upstream as avahi/avahi#442 - @pemensik and some other people are working on reviving Avahi now.

WRT the "driverless" backend, if it ran the "dnssd" backend to find printers then it wouldn't wake up every printer it finds and would choose the best/preferred protocol based on the priority key in the TXT records.

@tillkamppeter WDYT? It would be great if driverless was reliable in most situations (right now it isn't in case there is no IPPS printer and Avahi bundles the response) and this sounds feasible - either way we can raise ippfind timeout a little (a default cups-deviced timeout is 15s) in driverless in the meantime.

[pemensik]

Wait, what TXT priority key in TXT record? Doesn't it already use SRV records, which have both priority and weight fields already? Why would it need priority duplicated again in its TXT record?

[tillkamppeter]

Using ippfind to make driverless work was simply my first most intuitive idea in the first place, seeing that it lists all IPP services and also has sphisticated filtering to select the ones one is interested in, in my case the driverless printers, which are slected by the criteria that they use IPP 2.x and support at least one of the PDLs PDF, Apple Raster, PWG Raster, and PCLm.

I already saw that it was a little slow and did many attempts to get it faster, partially also with the help of several GSoC contributors, in 2022 even having one whose task was optimizing Avahi usage in general.

If the dnssd CUPS backend is faster and more reliable I have no problem to switch driverless to use it, but important is that only actual driverless IPP printers get listed, IPP 1.x printers or printers which do not understand any of the 4 mentioned PDLs, and also not printers which do not do IPP at all, like older LPD, port-9100, or whatever printers which dnssddetects. So dnssd needs to do the full filtering, to avoid that nay printer has to get DNS-SD-resolved twice (by dnssd and by driverless) as DNS-SD-resolving is a slow, time-consuming process.

[michaelrsweet]

@pemensik

Wait, what TXT priority key in TXT record? Doesn't it already use SRV records, which have both priority and weight fields already? Why would it need priority duplicated again in its TXT record?

The Bonjour Printing Specification defines the "priority" key in TXT records to tell network clients which print protocol to prefer for that printer. The DNS SRV priority/weight mechanism is for choosing amongst equivalent services, for example choosing between NTP servers for a domain.

[michaelrsweet]

@tillkamppeter

If the dnssd CUPS backend is faster and more reliable I have no problem to switch driverless to use it, but important is that only actual driverless IPP printers get listed, IPP 1.x printers or printers which do not understand any of the 4 mentioned PDLs, and also not printers which do not do IPP at all, like older LPD, port-9100, or whatever printers which dnssddetects. So dnssd needs to do the full filtering, to avoid that nay printer has to get DNS-SD-resolved twice (by dnssd and by driverless) as DNS-SD-resolving is a slow, time-consuming process.

That is pretty easy - just drop any device URI that doesn't contain _ipp or _ipps. Since the backend doesn't do a full resolve, it should be faster than ippfind in general. An alternate solution is to use cupsEnumDests with CUPS_PRINTER_DISCOVERED for both type and mask.

[zdohnal]

FYI an interesting finding - Avahi actually does not bundle the response, I see two separate messages in DBUS monitoring, but they end up in one sendmsg() syscall, which triggers my issue. So not much in Avahi for fixing - I will try to spawn separate Avahi clients for searches and see whether it fixes the issue.