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

Add option to bind to interface/ip address on ICE adapter #1461

Closed
BenKluwe opened this issue Oct 7, 2019 · 18 comments
Closed

Add option to bind to interface/ip address on ICE adapter #1461

BenKluwe opened this issue Oct 7, 2019 · 18 comments
Labels
bug S3 normal severity 3 - normal - regular issue, some loss of functionality under specific circumstances waiting for other projects

Comments

@BenKluwe
Copy link

BenKluwe commented Oct 7, 2019

Is your feature request related to a problem? Please describe.
On Windows, the ICE adapter sometimes times out repeatedly and indefinitely because it attempts to get ICE candidates from adapters that are not connected to the internet. There is a workaround to fix this issue by disabling all other adapters, but this is quite annoying and a solution is at hand.

Describe the solution you'd like
This feature already exists in the ICE project (NOT faf ICE adapter project), specifically to allow/block specific interfaces and/or ip addresses from candidate gathering.

Describe alternatives you've considered
none, this is already quasi-implemented and not bubbled through to the FAF client.

Wanna have the bug fixed quickly?
Visit Issue hunt...
Issue hunt

@BenKluwe
Copy link
Author

BenKluwe commented Oct 7, 2019

See also FAForever/java-ice-adapter#23

@1-alex98
Copy link
Member

1-alex98 commented Oct 8, 2019

This is not client related. If @Geosearchef merges PR I am happy to update version.

@1-alex98 1-alex98 closed this as completed Oct 8, 2019
@1-alex98
Copy link
Member

1-alex98 commented Oct 8, 2019

oh this would need client to tell it which ones to ignore

@BenKluwe
Copy link
Author

BenKluwe commented Oct 8, 2019

yes, which ones to blacklist and/or which ones to whitelist. This should not be done automatically and left to the user because it might otherwise screw things up badly.

@BenKluwe
Copy link
Author

BenKluwe commented Oct 8, 2019

Obviously if none are given then just allow all, which is the default for ice4j and the faf-ice-adapter

@1-alex98
Copy link
Member

1-alex98 commented Oct 8, 2019

If it is known how to create such a virtual inteface that makes ICE run forever one might as well investigate that 🤔 tho ICE4J does seem a little left behind

@BenKluwe
Copy link
Author

BenKluwe commented Oct 8, 2019

The idea behind this is not to let ICE run forever but instead tell faf-ice-adapter every time it runs which interfaces to use and the faf-ice-adapter can then pass that message on to ice4j :)

@1-alex98
Copy link
Member

1-alex98 commented Oct 8, 2019

Tho this is a work arround for an ice4j bug right

@1-alex98 1-alex98 reopened this Oct 8, 2019
@BenKluwe
Copy link
Author

BenKluwe commented Oct 8, 2019

Yes, I guess you could call it a workaround. It might not even be a bug, it's just that the timeout of 5 seconds in the faf-ice-adapter is probably not enough for ice4j to finish what they call harvesting. Then again, if you have to wait a minute or so for all interfaces to timeout so a connection can be found then one might call that a bug!

@1-alex98
Copy link
Member

1-alex98 commented Oct 8, 2019

So why not increase timeout then and try again?

@BenKluwe
Copy link
Author

BenKluwe commented Oct 8, 2019

The timeout is hard-coded

@1-alex98
Copy link
Member

1-alex98 commented Oct 8, 2019

Well lets try change that before before we start building workarounds right?

@1-alex98
Copy link
Member

1-alex98 commented Oct 8, 2019

If u want to u can join us on Zulip discord etc. and discuss with @Geosearchef that does the Ice Adapter (mostly)

@BenKluwe
Copy link
Author

BenKluwe commented Oct 8, 2019

Well... it's not really a workaround though because it is part of the configuration of ice4j, see here and thus unlikely to change. Binding to an interface will provide a much better UX i.e. faster loading time. I'm on discord as FF764.

@1-alex98
Copy link
Member

1-alex98 commented Oct 8, 2019

Not sure having users select them seems wrong

@BenKluwe
Copy link
Author

BenKluwe commented Oct 9, 2019

I tried out a higher timeout in the faf-ice-adapter yesterday (60s) to no avail. Instead of the faf-ice-adapter timing out the underlying ice4j implementation timed out. There is the possibility of using the address that has the default route to automatically detect the binding interface, but this should be done only once at setup-time. The drawback is the user then has to set the binding interface if it ever changes (consider for example VPNs) yet from the users perspective there will be no knowledge of this because the user never set it in the first place.

@1-alex98
Copy link
Member

ok no idea what u mean now @Geosearchef needs to discuss with u there

@1-alex98 1-alex98 added the bug label Oct 16, 2019
@1-alex98 1-alex98 added the S3 normal severity 3 - normal - regular issue, some loss of functionality under specific circumstances label Nov 1, 2019
@BenKluwe
Copy link
Author

As far as I can tell this isn't needed anymore. I recently installed the FAF client again and networking worked out of the box even with multiple adapters active, some of which not connected to the internet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug S3 normal severity 3 - normal - regular issue, some loss of functionality under specific circumstances waiting for other projects
Projects
None yet
Development

No branches or pull requests

3 participants