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

Config option for disabling NAT discovery #2964

Closed
ghost opened this issue Jul 11, 2016 · 8 comments
Closed

Config option for disabling NAT discovery #2964

ghost opened this issue Jul 11, 2016 · 8 comments
Assignees
Labels
exp/novice Someone with a little familiarity can pick up help wanted Seeking public contribution on this issue topic/nat Topic nat topic/repo Topic repo
Milestone

Comments

@ghost
Copy link

ghost commented Jul 11, 2016

In some locations (e.g. datacenters) you don't need NAT discovery. Sometimes it'll take its full couple of seconds though! If we can disable that, restarts of the daemon will be faster.

@ghost ghost added help wanted Seeking public contribution on this issue topic/repo Topic repo topic/nat Topic nat exp/novice Someone with a little familiarity can pick up labels Jul 11, 2016
@whyrusleeping whyrusleeping added this to the Ipfs 0.4.7 milestone Mar 6, 2017
@whyrusleeping
Copy link
Member

This should be pretty easy to do, should probably go under the Swarm config group and be called NatPortMap, default value true (if not present in the config, set it to true).

It needs to pass an option to the host construction method so that it can pass or not pass an option to the basichost.New here: https://github.com/ipfs/go-ipfs/blob/master/core/core.go#L731

Instead of passing a bool for 'NatPortMap' to this method with already way too many parameters, lets add an options struct as a parameter that we can extend later with more of these tweaks.

@kevina kevina self-assigned this Mar 9, 2017
@kevina
Copy link
Contributor

kevina commented Mar 9, 2017

@whyrusleeping

if not present in the config, set it to true

where would be a good place to do this? at the moment is is non obvious.

@whyrusleeping
Copy link
Member

in repo/fsrepo/serialize/serialize.go, at the bottom in the Load function, we just create an empty config before passing it to ReadConfigFile. If you set values in that, they will remain set to that value if the field is missing from the json.

Youre right, this is non-obvious, we should document it a bit while we're at it

@kevina
Copy link
Contributor

kevina commented Mar 9, 2017

Okay will do, It looks like this will be the first time we used a default value other than the go default of False, 0, or the empty string.

@whyrusleeping
Copy link
Member

Yeah, i don't think we knew how to do this before, otherwise the NoSync option would have been just Sync

@kevina
Copy link
Contributor

kevina commented Mar 10, 2017

Yeah, i don't think we knew how to do this before, otherwise the NoSync option would have been just Sync

So it turned out is was not sufficient to set the defaults in just the Load function, they also needed to be set in initConfig in fsrepo.go so I created a SetDefaults() method on the config structure. P.R. coming soon with some test to make sure the NoSync option really defaults to true.

@whyrusleeping
Copy link
Member

We have this now, #3798 was merged

The config option is Swarm.DisableNatPortMap

@ghost
Copy link
Author

ghost commented Mar 25, 2017

Awesome, thanks so much :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exp/novice Someone with a little familiarity can pick up help wanted Seeking public contribution on this issue topic/nat Topic nat topic/repo Topic repo
Projects
None yet
Development

No branches or pull requests

2 participants