Gracefully handle non-DNS-SD results from DNSServiceGetAddrInfo. #24715
Conversation
If we don't do this, then we'll add an entry to "interfaces" that does not have most of the necessary data defined, and if we pick that interface to then report to whatever started the resolve, things will fail out.
pullapprove
bot
requested review from
amitnj,
andy31415,
anush-apple,
arkq,
Byungjoo-Lee,
carol-apple,
chrisdecenzo,
chshu,
chulspro,
Damian-Nordic,
dhrishi,
electrocucaracha,
franck-apple,
gjc13,
harimau-qirex,
harsha-rajendran,
hawk248,
jelderton,
jepenven-silabs,
jmartinez-silabs,
jmeg-sfy,
jtung-apple,
ksperling-apple,
lazarkov,
lpbeliveau-silabs,
LuDuda and
mlepage-google
pullapprove
bot
requested review from
rgoliver,
saurabhst,
selissia,
tcarmelveilleux,
tecimovic,
tehampson,
turon,
vijs,
vivien-apple,
woody-apple,
xylophone21 and
yunhanw-google
|
PR #24715: Size comparison from fe24ee2 to d4dcaec Increases (6 builds for bl702, cc13x2_26x2, esp32, telink)
Decreases (4 builds for cc13x2_26x2, cyw30739, psoc6, qpg)
Full report (43 builds for bl602, bl702, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
|
I don’t think this is a good approach. If the network is answering dnssd queries using dns, that’s legitimate. Dnssd queries can go to unicast resolvers. So if you discard that data, it’s going to cause serious problems as unicast dnssd becomes more common, which we definitely want. I think it would be better to dig into why this response is actually causing a problem. |
That part I can tell you pretty easily. The current structure of the operational code is as follows:
What was causing problems is that we ended up with an entry in the map that had an IP address but none of the other information, so when we passed that to the API consumer it couldn't make sense of it. Hence this proposed fix: do not add entries to the map that are not already there from step 3 above. If we wanted to make use of this response, with the current structure of the code, we would need to match up the response with the right (port, TXT records, etc) metadata somehow. I suppose we could do that by changing what we use as the context for the |
|
Interesting. So if all the answers came back with interface zero, that would have worked? If so, then I think grouping by interface would be the right solution. But why didn't this work, given that you did have valid answers on the interface that the PTR/TXT/SRV came in on? |
If the answers to
Because in step 6 above we basically have to select one interface to hand back results for because the API surface there only allows one "result" containing TXT record, port, etc, with multiple IPs). That code was just checking for "has IPs", not "has all the other info", and ended up selecting interface 0 just because of the order the map got enumerated in: that one came first. We could have changed that code to ignore map entries for which we don't have the SRV/TXT/port bits, by the way; that is functionally identical to the fix here (which just avoids adding such entries to the map to start with). I think the concreate questions I have about the behavior of the underlying APIs are:
And if the answers to the above are "yes", then which interface index should be used as the scope for trying to talk to those link-local addresses in both cases? |
|
I’m not convinced that the current api behavior is correct. If you specify the interface, a response not on that interface seems problematic. But leaving that aside for the moment, because that’s a discussion for the service discovery team and not just me, I think grouping answers by interface should be correct. And not returning an incomplete answer is better than just dropping answers on interface 0. We should have a discussion about the interface issue separately. |
Just to make sure we are talking about the same thing: you mean grouping answers by the interface index we passed into |
|
I meant the info in the reply, actually. The info you got on index 0 seems like a non sequitur to me, and you got the same answer on the requested interface. The situation you are seeing here is actually a misconfigured local resolver. If you had gotten a real dnssd response on interface 0, it would have been complete. |
OK, that's what we are doing now....
The real question is what we should do with that info. The proposed change is to basically discard it if we did not get SRV/TXT records on the same interface.
As in, I would have gotten a response to |
|
Yes, unicast dnssd would deliver all the records with interface ==0. |
…ject-chip#24715) If we don't do this, then we'll add an entry to "interfaces" that does not have most of the necessary data defined, and if we pick that interface to then report to whatever started the resolve, things will fail out.
If we don't do this, then we'll add an entry to "interfaces" that does not have most of the necessary data defined, and if we pick that interface to then report to whatever started the resolve, things will fail out.