Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: Add WebRtcEndpoint.externalIPs configuration parameter #278

Closed
tyranron opened this issue Jul 12, 2018 · 10 comments
Closed

RFC: Add WebRtcEndpoint.externalIPs configuration parameter #278

tyranron opened this issue Jul 12, 2018 · 10 comments
Assignees

Comments

@tyranron
Copy link

@tyranron tyranron commented Jul 12, 2018

This RFC proposes to add WebRtcEndpoint.externalIPs configuration parameter which makes Kurento to gather its ICE candidates only on specified IPs.

Background and motivation

There is the long story about libnice gathering all possible IP addresses for its ICE candidates which introduces the problem for some Kurento many-networks deployments (like Amazon EC2, or Docker/Kubernetes): Kurento generates too much ICE candidates for itself and that results in the situation that sometimes (quite often, in practice) it fails to choose correct pair of ICE candidates and uses those ones from private networks, which leads to obvious bugs and video stability problems.

The first mention of this problem we found in this mailing list:

Also, the same problem is experienced by many users of Kurento:

  • #24
  • #102 (probably related too)
  • #126
  • #167 (probably, as we've experienced same behavior which has lead us to this investigation)

Problem

The main issue that libnice generates too many unnecessary candidates.

Currently, we're trying to avoid that by custom patched version of libnice where we're filtering unnecessary network interfaces in nice_interfaces_get_local_ips() function.
However, this is not good solution, as we never can predict all possible interfaces and libnice will never accept such PR to its sources. The "whitelist" way is better here than "blacklist".

Can we do better? I assume yes. Let's find out...

Solution

Diving into libnice showed that nice_interfaces_get_local_ips() is used, obviously, in nice_agent_gather_candidates().
The interesting part is that its used only when NiceAgent has no ->local_addresses field set.

Literally, this means that we can set IPs via this field we want Kurento to generate ICE candidates on and do not performs IPs lookup at all which will result also in performance boost (as optimizes "the time it takes for the connection check algorithm to move from state CONNECTED to state READY, on both peers").

No usage of ->local_addresses field was found in Kurento during NiceAgent initialization:

  1. nice_agent_new() in libnice
  2. kms_ice_nice_agent_new()
  3. kms_webrtc_session_init_ice_agent()

So, it's relatively easy to provide on Kurento side the config option WebRtcEndpoint.externalIPs which may hold list of IP addresses we want Kurento to create its ICE candidates on only:

  • If list is not empty, then fill with those IPs the ->local_addresses field of NiceAgent during KmsWebrtcSession initialization.
  • If list is empty (or not defined) the current behavior remains.

Profit

This will be a convenient way to make Kurento generate only those ICE candidates we want it to return. Though, remains optional (and opt-out by default) for those who does not need that.

flexconstructor added a commit to instrumentisto/kms-elements that referenced this issue Jan 17, 2019
@j1elo

This comment has been minimized.

Copy link
Member

@j1elo j1elo commented Oct 23, 2019

We are interested in adding such functionality to Kurento, as Openvidu itself (which builds upon Kurento) is also deployed in cloud provider environments.

The original idea was to wait until this issue got some more traction in the libnice project, at some point the added the compilation option --with-ignored-network-interface-prefix which we use in our libnice fork to exclude docker interfaces. However that option is clearly too limited, only allowing to specify a single interface pattern (and I wonder why they didn't add support to provide a comma-separated list of interface patterns, that's such an obvious improvement...)

Seems that libnice is not interested in solving this issue, so now that this RFC has been brought to our attention (sorry for the time it took!) I think we should work on it. I'd be interested in knowing if there are already working implementations of this out in the wild. I already found that the open-source project instrumentisto seems to have code written for this. If anyone else wants to chime in, she'll be welcome to do so.

@j1elo j1elo self-assigned this Oct 23, 2019
@tyranron

This comment has been minimized.

Copy link
Author

@tyranron tyranron commented Oct 23, 2019

@j1elo yup, this was implemented in our fork and works for us so far.

Here is the concrete commit: instrumentisto/kms-elements@6b4ad50

j1elo added a commit to Kurento/kms-elements that referenced this issue Nov 7, 2019
All rationale here: Kurento/bugtracker#278

There is the long story about libnice gathering all possible IP
addresses for its ICE candidates which introduces the problem for some
Kurento many-networks deployments (like Amazon EC2, or
Docker/Kubernetes): Kurento generates too much ICE candidates for itself
and that results in the situation that sometimes (quite often, in
practice) it fails to choose correct pair of ICE candidates and uses
those ones from private networks, which leads to obvious bugs and video
stability problems.

Turns out that libnice won't perform local address gathering if some
local address(es) have been set beforehand; so, by using this new
setting, we can provide the local addresses that libnice has to use,
skipping the actual gathering.

Thanks @alexlapa for the implementation at https://github.com/Kurento
/kms-elements/pull/20

Fixes #278
@j1elo

This comment has been minimized.

Copy link
Member

@j1elo j1elo commented Nov 7, 2019

Thanks @alexlapa for the implementation at
Kurento/kms-elements#20

@j1elo

This comment has been minimized.

Copy link
Member

@j1elo j1elo commented Nov 11, 2019

Hi there. I'm reopening this issue because we found that the feature is not working for pretty basic use cases, i.e. Amazon AWS EC2 instances.

Indicating the external IP in the WebRtcEndpoint configuration file is actually working for the basic local network tests; e.g. these are the candidates that libnice generates for localhost:

Video:
[IceCandidateFound] local: 'candidate:1 1 UDP 2013266431 127.0.0.1 16046 typ host'
[IceCandidateFound] local: 'candidate:2 1 TCP 1015022079 127.0.0.1 9 typ host tcptype active'
[IceCandidateFound] local: 'candidate:3 1 TCP 1010827775 127.0.0.1 36311 typ host tcptype passive'

Audio:
[IceCandidateFound] local: 'candidate:1 2 UDP 2013266430 127.0.0.1 22531 typ host'
[IceCandidateFound] local: 'candidate:2 2 TCP 1015022078 127.0.0.1 9 typ host tcptype active'
[IceCandidateFound] local: 'candidate:3 2 TCP 1010827774 127.0.0.1 43763 typ host tcptype passive'

And these ones are generated when testing in LAN:

Video:
[IceCandidateFound] local: 'candidate:1 1 UDP 2013266431 192.168.1.19 29321 typ host'
[IceCandidateFound] local: 'candidate:2 1 TCP 1015021823 192.168.1.19 9 typ host tcptype active'
[IceCandidateFound] local: 'candidate:3 1 TCP 1010827519 192.168.1.19 25998 typ host tcptype passive'

Audio:
[IceCandidateFound] local: 'candidate:1 2 UDP 2013266430 192.168.1.2 26732 typ host'
[IceCandidateFound] local: 'candidate:2 2 TCP 1015021822 192.168.1.2 9 typ host tcptype active'
[IceCandidateFound] local: 'candidate:3 2 TCP 1010827518 192.168.1.2 63880 typ host tcptype passive'

However, we have a pretty different behavior when running in an empty, clean EC2 instance, where all ports are open and KMS is being installed locally with apt-get:

Video:
[IceCandidateFound] local: 'candidate:64513 1 TCP 1015022847 1.2.3.4 9 typ host tcptype active'

Audio:
[IceCandidateFound] local: 'candidate:64513 2 TCP 1015022846 1.2.3.4 9 typ host tcptype active'

I'll be studying why libnice behaves like this. Any feedback or insight would be awesome.

@j1elo

This comment has been minimized.

Copy link
Member

@j1elo j1elo commented Nov 12, 2019

Turns out libnice generates all 3 types of candidates when the local address that was added matches with one of the local network interfaces. That's why it was working in the two first tests, where the IP addresses correspond to addresses of the hardware interfaces.

If the local IP doesn't match any, then libnice only generates the TCP active candidate.

@j1elo

This comment has been minimized.

Copy link
Member

@j1elo j1elo commented Nov 14, 2019

So libnice won't generate local candidates for external IP addresses, because it will fail here:
https://github.com/Kurento/libnice/blob/a5b506996955413b92e049748dde40b94b594c6e/socket/udp-bsd.c#L126

It makes sense: it's simply not possible to bind a local socket to an unknown IP address that doesn't exist in the system. And an external IP (such as the public part of the LAN's NAT) is, indeed, an unknown IP address for the local network.

So, @tyranron @alexlapa I would like to know in what sense is the externalIp parameter working for you, as it seems that it can only be provided with local addresses in order to make it work as one would expect (to generate all kinds of local candidates: UDP, TCP passive, TCP active).

If there is no clear way to make this work for the general case of telling libnice about the public IP address, then we'll have to revert this change and instead look into different ways to accomplish the objective.

@j1elo

This comment has been minimized.

Copy link
Member

@j1elo j1elo commented Nov 14, 2019

I think maybe this should be reduced to just a matter of terminology, so just renaming the parameter to localAddress because the "external" name made me think that it would be possible to provide an actual external IP address, which doesn't work

@alexlapa

This comment has been minimized.

Copy link

@alexlapa alexlapa commented Nov 18, 2019

@j1elo ,

We are running kurento in docker with network=host, so it has access to all server's network interfaces. IP that we specify as externalIp is server's eth0, i.e. servers public IP.

Yes terminology might be a little tricky here.

@j1elo

This comment has been minimized.

Copy link
Member

@j1elo j1elo commented Nov 18, 2019

After some conversation I think or best option right now is to follow the initiative from GStreamer-plugins-bad, and instead of an IP address, accept a network interface name or list of names in the parameter: webrtcbin: add ice-nics property

There is a good reasoning for allowing an interface name instead of an IP address: in virtual and dynamic environments, the interface names are pretty much known beforehand, but the IP addresses might change. So it's safer to give eth0 to Kurento and then have the corresponding IP address found with libnice's nice_interfaces_get_ip_for_interface().

@j1elo

This comment has been minimized.

Copy link
Member

@j1elo j1elo commented Nov 21, 2019

Parameter externalAddresses changed to networkInterfaces in commit Kurento/kms-elements@cbe7516

@j1elo j1elo closed this Nov 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.