search

keepsimple1 / mdns-sd

Rust library for mDNS based Service Discovery
Apache License 2.0
88 stars 38 forks source link

question about interaction between record refresh and cache flush bit #203

Closed keepsimple1 closed 2 months ago

keepsimple1 commented 2 months ago

In a recent PR https://github.com/keepsimple1/mdns-sd/pull/201, we started to support CACHE_FLUSH bit in a RR (resource record). I'm wondering what should happen if the RR received is a response to RR refresh.

In RFC 6762 section 5.2, it says: 

The querier should plan to issue a query at 80% of the record
   lifetime, and then if no answer is received, at 85%, 90%, and 95%.
   If an answer is received, then the remaining TTL is reset to the
   value given in the answer, and this process repeats for as long as
   the Multicast DNS querier has an ongoing interest in the record.

It seems to me that it suggests that when an answer received, the existing record should be updated with TTL reset to the new value.

However, suppose rrdata is the same, what if the answer has CACHE_FLUSH bit set? Should we expire the existing record as required by CACHE_FLUSH, or update the existing record?

(I also have a similar question about interaction between record refresh and know-answer suppression, but that's a different issue)

lyager commented 2 months ago

From my short recollection, what I read into it is that the CACHE_FLUSH bit is set after client probing has been done, meaning that the client 'knows' that it has authoritative knowledge of the information it is sending out. The example given is for example when a device has been offline due to network reconfiguration - so not as an answer to a respone. IIRC

keepsimple1 commented 2 months ago

I thought out this and think that we should only apply cache-flush bit when rdata is different, otherwise we will keep getting new records with the same data and unnecessary ServiceResolved events. I opted with this approach when working on PR #180. My testing looks good. If there are any problems, please open a new issue. Thanks!

lyager commented 2 months ago

@keepsimple1 I think this is definitely a step in the right direction. One concern I have is: If a device has 2 IP addresses, and one interface goes down, but the other interface keeps its current IP. That would result in a CACHE-FLUSH, but with the check for CACHE-FLUSH now being inside the 'find', the first IP can be found in the array, and thus the expiry of the cached entries not being called?

keepsimple1 commented 2 months ago

One concern I have is: If a device has 2 IP addresses, and one interface goes down, but the other interface keeps its current IP. That would result in a CACHE-FLUSH, but with the check for CACHE-FLUSH now being inside the 'find', the first IP can be found in the array, and thus the expiry of the cached entries not being called?

That's a good point! Indeed I think we should always check for cache-flush bit, and at later time when the records actually expire, we can resolve the instances again if applicable. I fixed the code again and also added a test case to cover the case you asked above, see PR #211 .

lyager commented 1 month ago

I'll give it a testspin asap. Nicely done