Add option to enable/disable heuristic early-return logic

[bundasmanu]

This PR is related to the following RTPEngine Forum question: (https://groups.google.com/g/rtpengine/c/YsTU99HGd2o).

After 13.5 (including) the following commit was introduced: (afce798).

The idea of this commit is quiet good, but could affect some scenarios, like: when clients are behind some VPN's.

So, the proposal here, is to include a new parameter: endpoint-learning-heuristic-disable-early-return. This parameter by default assumes false as value (so, the current behavior is preserved). But, passing true as value, reverts the current behavior and preserves the old logic (<= 13.4).

The following PR was already tested locally, and allows the correct turn on/off.

If possible, a backport should be made for versions >= 13.5 (14.x also).

[rfuchs]

I don't think making this an option is a good solution. You said this is affecting scenarios with ICE - I'm thinking endpoint learning in general should be defused or even disabled altogether when ICE is in use, as ICE should be in charge of determining endpoint address. I was under the impression that this is already done. Let me check.

[rfuchs]

Have you tried what happens if you just disable endpoint learning?

[bundasmanu]

Have you tried what happens if you just disable endpoint learning?

Yes.
Inside the topic in the forum, i describe that. If, i switch to delayed it solves the problem. But, switching that value, could lead to problems, that i'm not aware. As, this is totally dependent also on the clients environment side. Switching to off does not solve the problem.

And, using this eventual parameter introduces some help, for people that wants to keep using heuristic, but without that early-return. By default, assumes false, so will not affect current deployments.

[rfuchs]

Have you tried what happens if you just disable endpoint learning?

Yes. Inside the topic in the forum, i describe that. If, i switch to delayed it solves the problem. But, switching that value, could lead to problems, that i'm not aware. As, this is totally dependent also on the clients environment side. Switching to off does not solve the problem.

That's odd. ICE should still adjust the endpoint addresses based on the outcome of the negotiations, basically obsoleting the other learning mechanism. Perhaps there's some other underlying issue lurking. Sadly I have no way to reproduce this myself.

And, using this eventual parameter introduces some help, for people that wants to keep using heuristic, but without that early-return. By default, assumes false, so will not affect current deployments.

I can see it as a feasible solution for a backport, but for future versions going forward I'd rather avoid having it. Determining the endpoint should be delegated to ICE - the regular learning mechanism should be just a fallback.

[bundasmanu]

Have you tried what happens if you just disable endpoint learning?

Yes. Inside the topic in the forum, i describe that. If, i switch to delayed it solves the problem. But, switching that value, could lead to problems, that i'm not aware. As, this is totally dependent also on the clients environment side. Switching to off does not solve the problem.

That's odd. ICE should still adjust the endpoint addresses based on the outcome of the negotiations, basically obsoleting the other learning mechanism. Perhaps there's some other underlying issue lurking. Sadly I have no way to reproduce this myself.

And, using this eventual parameter introduces some help, for people that wants to keep using heuristic, but without that early-return. By default, assumes false, so will not affect current deployments.

I can see it as a feasible solution for a backport, but for future versions going forward I'd rather avoid having it. Determining the endpoint should be delegated to ICE - the regular learning mechanism should be just a fallback.

@rfuchs, i completely see your point.

And its indeed an interesting case. But, that commit is the root cause that switch the behavior between the 13.4 and >= 13.5. The only difference between success and failure, is on the rtpengine version. I dunno, if the person that introduce that commit, evaluates the full impact of that change. Maybe more people in the future, when they start using >= 13.5, could start complain about it.