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

IPFS Improvements #284

Merged
merged 24 commits into from
Mar 13, 2019
Merged

IPFS Improvements #284

merged 24 commits into from
Mar 13, 2019

Conversation

makew0rld
Copy link
Contributor

@makew0rld makew0rld commented Feb 8, 2019

This PR adds several experimental IPFS improvements. It needs testing.
They are:

  • Relay hopping is enabled
    • If EnableRelayHop is enabled, the node will act as an intermediate (Hop Relay) node in relay circuits for connected peers. What does all this mean? Circuit relaying provides peers with the means to indirectly connect other peers who cannot directly connect to each other, either because of NAT or because of protocol incompatibilities, such as browser-based (js-ipfs) peers connecting to desktop (go-ipfs) peers. This is a nice feature to have if you want to be able to relay information for browser-based Dapps.

    • It seems it will help the overall health of the network
    • REMOVED
  • NAT port mapping is disabled as NATs do not come into play here
    • Perhaps this should be renabled, as sometimes at homes we will have these nodes under NAT?
    • REMOVED
  • Swarm connection management was setup
    • LowWater
      • Lowest # of connections that it attempts to maintain
      • At 600 by default, I changed it to 100
    • HighWater
      • Highest number of connections before they are discarded
      • At 900 by default, I changed it to 200
    • GracePeriod
      • How long connections last before they can be closed, even when HighWater has been reached. Prevents abrupt closure.
      • In low power situations, you might want to kick the GracePeriod up to 1 minute, but drastically reduce the LowWater and HighWater values.

      • At 30s by default, I changed it to 60s
    • These values need testing to be set properly, and I'd like your input
  • ipfs-swarm.sh was renamed to ipfs-setup.sh
    • The rename shows up like I've deleted one file and entirely created another, unfortunately
  • Swarm filtering was added
    • This is a CIDR blacklist, meaning IPFS will not try to connect to nodes with addresses within the blacklisted subnet, even if they show up in the DHT and stuff.
    • This is built into IPFS
    • ipfs-setup.sh checks if CJDNS, Yggdrasil, IPv4 Internet and IPv6 Internet are available, and blacklists them if they are not
    • This will hopefully make IPFS more efficient, and reduce the issues presented in P2P apps and mesh layering causing problems #283
    • It's now a loop
  • QUIC aka HTTP/3
    • QUIC uses UDP and aims to replace TCP
    • Less latency, attempts to avoid congestion
    • IPFS has experimental support for connecting to peers with it
    • This PR enables support for it
    • Adds a quic_enabled bool to nodeinfo.json
    • ipfs-setup.sh will try and use QUIC to peer with mesh peers, if they say they have enabled it in their nodeinfo
    • If you guys agree with supporting QUIC, then we could connect to the default CJDNS and Ygg peers over QUIC as well

@darkdrgn2k
Copy link
Contributor

Allot of these changes assume that we will be running a MESH ONLY network which is the exact opposite.

Additionally the ipfs-setup script does not take into account the fact that i may plug my computer into a my wired home network after the fact.

I think i'd be more comfortable with some sort of toggle to enable/disable internet peers instead of testing for internet.

echo "Blocked Yggdrasil network"
fi
# Clearnet or regular Internet IPv4 access
if ! [ "$(ping -c 3 1.1.1.1 &> /dev/null)" ]; then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure this works how you think.

Also in past experience this can return false data.

Also what if your tunnled over hyperbola (IP TUNNEL)? Do we care?

[root@localhost 1]# if ! [ "$(ping -c 3 99.99.99.99 &> /dev/null)" ]; then echo Good; fi
Good
[root@localhost 1]# if ! [ "$(ping -c 3 1.1.1.1 &> /dev/null)" ]; then echo Good; fi
Good

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❯ ping -w 2 99.99.99.99 &>/dev/null; if [ $? -eq 0 ]; then echo hello; fi

❯ ping -w 2 1.1.1.1 &>/dev/null; if [ $? -eq 0 ]; then echo hello; fi
hello

I will add this fix.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After shellcheck, I am going with:

if ! ping -w 2 1.1.1.1 &>/dev/null; then

# Enable Filestore for --nocopy capability
ipfs config --bool Experimental.FilestoreEnabled true

# Disable NAT port mapping used for NAT traversal
ipfs config --bool DisableNatPortMap true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about pis connected to the home internet, your killing their ability to try to gain more peer by nat port.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I mentioned that in my description but didn't change it. I will change it back.

@@ -29,9 +29,25 @@ ipfs init || true
# Enable gossipsub routing
ipfs config Pubsub.Router gossipsub

# Enable relay hop to help peers with connecting
ipfs config --bool Swarm.EnableRelayHop true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would increase bandwidth usage on internet connected peers.

Also how does this work when you block internet peers. Will that mean that a non-internet enabled peer will not want to relay through an internet relayed one?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would increase bandwidth usage on internet connected peers.

Possibly, but only if we manually set up the relay to be used. The current release does not automatically use relays. With this, we have the option too, and it will improve the overall health of the network.

Also how does this work when you block internet peers. Will that mean that a non-internet enabled peer will not want to relay through an internet relayed one?

As was said in chat by me:

Whether it will allow the connection to a blacklisted peer over a relay is a good question though, something to be tested. Just keep in mind IPFS won't try to do that on its own just yet

Relaying through an Internet peer would actually be a good thing for nodes to do, but we will have to wait until IPFS does it automatically, possibly in the next release. Once that happens, we can likely remove all the filtering entirely.

@makew0rld
Copy link
Contributor Author

In regards to LowWater and HighWater values, the IPFS lowpower profile sets them to 20 and 40 respectively. I hope that provides some reference, but to be honest I have no idea what it means for the Pis. Formal testing of resource use and performance with different values is probably required.

@makew0rld
Copy link
Contributor Author

makew0rld commented Feb 9, 2019

I made the script a loop, which is a big, potentially stupid, change. Check out the script now and before, the diff is hard to read.

@makew0rld makew0rld changed the title IPFS Improvements IPFS Improvements (After Release) Feb 12, 2019
@makew0rld makew0rld changed the title IPFS Improvements (After Release) IPFS Improvements (For after release) Feb 12, 2019
@makew0rld
Copy link
Contributor Author

develop will need to be merged back into this branch after the release.

@darkdrgn2k
Copy link
Contributor

@makeworld-the-better-one i would suggest looking at the latest release and doing a version bump

@makew0rld
Copy link
Contributor Author

@darkdrgn2k The latest IPFS release? Yes, that's the comment I made just above. I will also remerge from develop.

@makew0rld makew0rld removed the request for review from benhylau March 5, 2019 19:53
@makew0rld
Copy link
Contributor Author

I am going to move on to the testing stage for this PR, but before/as I do, does anyone (@benhylau and @darkdrgn2k) have issues with the script being a loop that blocks access to certain groups of peers as the environment changes? I'm not requesting a review yet.

I have confirmed that even with the latest IPFS release, you cannot access hidden networks (Yggdrasil on a CJDNS-only node, for example) by automatically using a relay. Manually connecting through a relay to access these networks could be possible, but should be done in a different PR.

@darkdrgn2k
Copy link
Contributor

darkdrgn2k commented Mar 6, 2019

@makeworld-the-better-one

being a loop that blocks access to certain groups of peers as the environment changes?

I can tell you from experience that something like "is there internet" by pinging 8.8.8.8 cannot guarantee an internet connection. Just the other day in the chat Ikata incorrectly though his internet was not working when 8.8.8.8 did not respond to pings.

Additional your code will block an Lan peering if there is no internet. You also cant assume that private ip addresses will be used.

Finally there has not been any established baseline of blacklisting a peer vs simply letting it fail the connection.

If you wish to peruse this method anyway i would suggest the following

  • Split it of into a separate optional service (probably separate module too)
  • Make sure you add a delay in the loop, you dont want it being seen as a d-dos
  • Be careful with overusing /opt/cjdns/publictoip6 This is a process intensive function
  • I would NOT put the peering support into a loop, especially in the means you have done it

Remember also that we ill need to separate the yggdrasil and cjdns stuff away from the main ipfs module to make it more "generic" for DEBifying.

@darkdrgn2k
Copy link
Contributor

Comment from IPFS channel

presumably the attempts to connect (incl bootstrap) via internet addresses will eventually time out. i guess blacklist will save on this delay, but only if all parallel attempts are taken up by the internet addressses, starving the cjdns/other addresses.

I think the key here was but only if all parallel attempts are taken up by the internet addresses

This means that there are so many internet peers trying to be contacted that there i no room for cjdns ones. I'm pretty sure this number is high so makes me question the effectiveness of this hack in the grand scheme of things.

@@ -38,6 +38,16 @@ ipfs config Pubsub.Router gossipsub
# Enable Filestore for --nocopy capability
ipfs config --bool Experimental.FilestoreEnabled true

# Setup connection management - Reduce connections to stress the Pi less
# XXX: These values need to be tweaked and tested
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@makeworld-the-better-one How will you measure whether these values provide an improvement or otherwise?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CPU and RAM usage can be looked, as well as file retrieval times and IPNS resolve. It will require a formal test, but I know that these values will be better than the ones we have now, especially since this is on such limited hardware.

# Wait for IPFS to initalize
attempts=15
until [[ $(curl http://localhost:5001/api/v0/id -s 2>/dev/null) || ${attempts} -eq 0 ]]; do
sleep 1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need to test so often and I don't think 15 s is enough in some cases?

echo "Connecting to ${addr} with QUIC"
else
ipfs swarm connect "/ip6/${addr}/tcp/4001/ipfs/${id}"
echo "Connecting to ${addr} regularly"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
echo "Connecting to ${addr} regularly"
echo "Connecting to ${addr} over TCP"

# Check for QUIC connections first
if [ "$(echo "${res}" | jq -r -M '.services.IPFS.quic_enabled')" == 'true' ]; then
# ID is not needed for QUIC connections
ipfs swarm connect "/ip6/${addr}/udp/4001/quic"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the log should come before the command

ipfs swarm filters add '/ip6/fc00::/ipcidr/8'
echo "Blocked CJDNS network"
fi

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# Check if Yggdrasil is running

while read -r ygg_peer; do
addPeer "${ygg_peer}"
done <<< "$(sudo yggdrasilctl getPeers | grep -v "(self)" | awk '{print $1}' | grep -v bytes_recvd | xargs)"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# Remove a filter if there was one

# Clearnet or regular Internet IPv4 access
if ! ping -w 2 1.1.1.1 &>/dev/null; then
# Block all IPv4 - it's not used anywhere else
ipfs swarm filters add '/ip4/0.0.0.0/ipcidr/0'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if I am on a off-Internet LAN local meshnet plugged into my local switch? Now my IPFS won't swarm?

fi

# Clearnet or regular Internet IPv4 access
if ! ping -w 2 1.1.1.1 &>/dev/null; then
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This type of Internet check is usually problematic. What is the motivation behind adding this filter at all?

echo "Enabled access to all of IPv4"
fi

# Clearnet IPv6 access
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as IPv4. Recommend removing both.

else
ipfs swarm filters rm '/ip6/2000::/ipcidr/3'
echo "Enabled access to IPv6 internet"
fi
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would at least so a 60 s sleep on this loop. Probably more.

}

# Continually mesh with peers and add or remove filter rules as networks go up and down
while true; do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont know how postexec will work for long running processes. Upon service restart I dont think the PID will be killed but it will spawn again.

This will created multiple scripts running in parallel.

Better off running in a service

@makew0rld
Copy link
Contributor Author

Thanks for the comments on the PR!

I will edit it and remove the loop, and instead just leave the tweaking of some settings and doing a version update.

@makew0rld
Copy link
Contributor Author

makew0rld commented Mar 12, 2019

I've now updated the PR, it is much smaller now. Here are the changes as compared to develop

  • Version update to 0.4.19
  • Set config so that the daemon connects with fewer peers, to save on resources
  • Added connecting to peers over QUIC, if they have it enabled
  • The IPFS API check now takes a maximum of 30 seconds, with 3 second breaks, previously it was 15 secs max with 1 sec breaks.

Copy link
Contributor

@darkdrgn2k darkdrgn2k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks alright!

Copy link
Member

@benhylau benhylau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may still need to test the optimization values but this looks okay overall.

@makew0rld makew0rld merged commit 96e3980 into develop Mar 13, 2019
@makew0rld makew0rld deleted the ipfs-config branch March 13, 2019 18:01
darkdrgn2k added a commit that referenced this pull request Mar 26, 2020
    Yggdrasil IPTunnel now does not change config files
    Yggdrasil IPtunnel drop in service adjustment
    Yggdrsail IPTunnel supports IPv6 and routed IPv6
    Yggdrsail version bump
    Support for x86 and x64
    Profile selection menu format changed in Dialog
    Rewritten and simplified board detection
    IPFS Improvments #284
    CJDNS now module
    Prototype can run without CJDNS now
    Better docs
    Grafan database now can be removed when uninstalling
    Moved network config to interface.d model
    Removed Network Manager
    Added Modules.md
    Added confSet function and implemented confget/confset config files
    NodeJS now shared module
    NodeJS version bump
    MESH_NAME now a config
    Added ipv6 netcat option
    Localized Patch Foo in TOMESH repo to prevent version conflicts and outage
    Added support for PI4
    Added support for Buster
    Fixed ETH0 vs BR0 issue on espressoBIN
    Prometheus version bump
    Raspberry Pi Watch Dog Timer
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants