Re: [MMUSIC] ietf-mmusic-mdns-ice-candidates-01 Nils review

[Youenn Fablet <[e-mail]>]

Thanks a lot for the review, Nils.
I filed https://github.com/rtcweb-wg/mdns-ice-candidates/issues/135 <https://github.com/rtcweb-wg/mdns-ice-candidates/issues/135> to keep track of your feedback and work on updating the draft accordingly.
Some comments inline.


> On 8 Jun 2021, at 22:31, Nils Ohlmeier <[e-mail]> wrote:
> 
> Hello,
> 
> I was asked to review https://datatracker.ietf.org/doc/html/draft-ietf-mmusic-mdns-ice-candidates-01 <https://datatracker.ietf.org/doc/html/draft-ietf-mmusic-mdns-ice-candidates-01>
> Here are my comment and questions:
> 
> Abstract:
> “To maximize the probability of a
>    direct peer-to-peer connection, client private IP addresses are
>    included in this candidate collection.”
> 
> I think it would make sense to replace “client” with ICE agent here, as client and server in the pure ICE context doesn’t make too much sense. I would suggest to change it to “..., private IP addresses of the ICE agent are included…”.
> 
Makes sense.
> 
> In the abstract the terms “private IP addresses” and “local IP addresses” are used. I would suggest sticking to just one term for consistency. Section 1. “Introduction” has the same mix of local vs private in it.
> 

OK, I would go with private IP addresses, following RFC1918.
> 
> Section 3.1.1 point 1
> I think it would be helpful if this draft at least offers the suggestion to replace all RFC1918 addresses, or put a reference to section 3.1.2.2 as a recommendation on how to determine which addresses to replace.
> 

We can add some more wording and refer both to section 3.1.2.2 as well as ICE agent modes.
> 
> Section 3.1.2.2 3rd paragraph
> “Regardless of results, …” confuses me. The last sentence in the paragraph before talks about the case of a public candidate. If I’m not mistaken in the case of a public address there shouldn’t be a server reflexive candidate generated.
> 
The idea is to generate a mDNS candidate for any host address.
That way, we do not need to wait for the STUN response to expose the mDNS candidate.

If there are STUN servers, this mDNS candidate gets complemented by a SRFLX candidate.
- In case the host address is private, this is useful to expose the public address as a SRFLX candidate.
- In case the host address is public, the mDNS candidate will probably not help and the SRFLX candidate will ensure connection setup.

I guess we could try to clarify the wording so it gets clearer.

> Section 3.1.2.2 4th paragraph
> While I agree that caching the result might be useful it can become dangerous in case the set of STUN servers changes. As the new STUN servers might return different results, cached results should be invalidated. Would it be worth adding a warning about this?
> 
Right, it makes sense to do so.
> Section 6.4
> I’m confused by the diagram showing that both agents send over their mDNS candidates before having checked with the STUN server if the interface IP addresses need to be filtered or not. I understand that section 3.1.2.2 recommends to first wait for the STUN binding response from the STUN server before deciding which addresses should be filtered and which not. Why is this example showing it differently?
> 
Section 3.1.2.2 does not really recommend to wait for the STUN binding responses.
It is up to the ICE agent to decide whether to limit generation of mDNS candidates or not to only when actually useful.

> Best regards
>   Nils Ohlmeier
> _______________________________________________
> mmusic mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/mmusic