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

From: Nils Ohlmeier <[email-masked]>
Date: Tue, 8 Jun 2021 13:31:35 -0700

Hello,

I was asked to review 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…”.

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.

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.

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.

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?

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?

Best regards
  Nils Ohlmeier

---

From: Youenn Fablet <[email-masked]>
Date: Mon, 14 Jun 2021 15:54:28 +0200

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 <[email-masked]> 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>
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…

OK, I would go with private IP addresses, following RFC1918.
> 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 addres…

We can add some more wording and refer both to section 3.1.2.2 as well as ICE agent modes.
> “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…
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.

> 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…
Right, it makes sense to do so.
> 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 understan…
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
> [email-masked]
> https://www.ietf.org/mailman/listinfo/mmusic

---

From: Christer Holmberg <[email-masked]>
Date: Thu, 17 Jun 2021 18:44:45 +0000

Hi,

...

>> 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.
>

I didn't check how the terms are used, but aren't "private IP address" and "local IP address" separate things?

For example, the host candidate represents a local IP address, which may or may not be private.

So, you can use "private IP address" in cases where you talk about private IP addresses, but if you talk about host candidates in general I think you should use "local IP address".

Regards,

Christer