chore: upgrade go-libp2p-kad-dht #10378
Conversation
hacdias
force-pushed
the
dht-0.25.2
branch
2 times, most recently
from
cf992ab
to
fe340cb
Compare
|
@guillaumemichel I can certainly add an option (it would be better than an environment variable). I think it's just important to understand whether or not it would be useful. The only case I can think of for the LAN DHT is if someone is running multiple Kubo nodes on the same machine. Disabling the private addresses in the WAN DHT can be useful for private networks maybe. Now that I think of it: if we don't add this option, aren't we literally breaking private IPFS clusters within private networks with no public IPs? |
|
Yes, it would essentially be useful for local ipfs clusters I guess (see libp2p/go-libp2p-kad-dht#839 (comment)) |
|
@guillaumemichel ok, then I will rework this to be an option in the Kubo configuration instead. |
| wanOptions := []dht.Option{ | ||
| dht.BootstrapPeers(args.BootstrapPeers...), | ||
| } | ||
| lanOptions := []dht.Option{} | ||
| if args.LoopbackAddressesOnLanDHT { | ||
| lanOptions = append(lanOptions, dht.AddressFilter(nil)) | ||
| } |
There was a problem hiding this comment.
💭 LoopbackAddressesOnLanDHT Feels overly specific given how generic go-libp2p config option is:
- 🔴 dht.RoutingTableFilter still applies
PrivateRoutingTableFilter - 🔴 dht.QueryFilter still applies
PrivateQueryFilter - 🟢 dht.AddressFilter applies no filters when
LoopbackAddressesOnLanDHTis set totruein Kubo
We have similar setup for WAN.
I would expect this a Flag to be named close to what we do in libp2p config, Routing.DisableNoAddressFiltersOnLanDHT and have similar one for WAN (Routing.DisableAddressFiltersOnWanDHT) just in case we break people running custom overlay networks that don't follow official RFCs (giving them an escape hatch to self-fix, without having to report bugs to us).
I don't know how real this problem is, we would have to itnterview someone working with IPFS and overlays, so don't want to block on this.
For now, if we make LoopbackAddressesOnLanDHT a Flag we can leave this as-is, and fix later, if/once someone reports we broke their setup.
There was a problem hiding this comment.
I changed to Flag. I would keep it as-is for now and see if someone complains. Otherwise we'll have to add many options. I had already discussed yesterday with @aschmahmann and we had decided on removing the WAN option for now.
hacdias
force-pushed
the
dht-0.25.2
branch
2 times, most recently
from
8b0a7ed
to
ab8c4ea
Compare
|
|
||
| Whether loopback addresses (e.g. 127.0.0.1) should not be ignored on the local LAN DHT. | ||
|
|
||
| Most users do not need this setting. It can be useful during testing, when multiple Kubo nodes run on the same machine but some of them do not have `Discovery.MDNS.Enabled`. |
There was a problem hiding this comment.
ℹ️ I played a bit with various setups and this seems to be useful only when mDNS discovery is not available. Had two nodes with MDNS, and they discovered each other and connected over /ip6/::1/udp/4001/quic-v1/ just fine.
Added this note so our config docs explain why and when the option matters, so users save time and not bother with it.
Supersedes and closes #10100.
My solution by using an environment variable to disable the address filtering in the DHT is definitely not the best. But something along this lines seems to be necessary for testing since we don't have multiple public IPs available. My other solution would be to add some test flag to Kubo (or config).