Skip to content

Port Pool imrovements - #3990

Merged
Snawoot merged 6 commits into
masterfrom
fix_port_pool
Sep 27, 2021
Merged

Port Pool imrovements#3990
Snawoot merged 6 commits into
masterfrom
fix_port_pool

Conversation

@Snawoot

@Snawoot Snawoot commented Sep 24, 2021

Copy link
Copy Markdown
Contributor

Fix #3967

This PR contains following changes:

  • Safe use of random number generator in port pool (concurrent usage of rand.NewSource instances is not allowed! [1])
  • Also switched to concurrency-safe generator in other parts of node
  • Switched payments to CSPRNG
  • Revamped port distribution logic of static range port pool. Previously it was tending to seek from start of pool on any collision. This way there was high probability that eventually it will try own ports previously distributed from the beginning of range. Now it always picks random port and circles over entire pool range from random offset position.
  • Added more test cases for port pool to prove concurrent connections is not a problem (special for @creotiv)
  • Avoid collisions in AcquireMulti result

@codecov-commenter

codecov-commenter commented Sep 25, 2021

Copy link
Copy Markdown

Codecov Report

Merging #3990 (9eea329) into master (831d39c) will increase coverage by 0.02%.
The diff coverage is 54.83%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3990      +/-   ##
==========================================
+ Coverage   43.41%   43.43%   +0.02%     
==========================================
  Files         325      327       +2     
  Lines       17114    17147      +33     
==========================================
+ Hits         7430     7448      +18     
- Misses       8928     8941      +13     
- Partials      756      758       +2     
Impacted Files Coverage Δ
core/ip/fallbacks.go 0.00% <0.00%> (ø)
session/pingpong/hermes_promise_settler.go 37.23% <0.00%> (-0.13%) ⬇️
utils/random/timeseeded.go 0.00% <0.00%> (ø)
session/pingpong/invoice_tracker.go 62.76% <33.33%> (-0.72%) ⬇️
core/port/pool.go 51.61% <35.29%> (+11.61%) ⬆️
utils/random/concurrent.go 100.00% <100.00%> (ø)
core/policy/oracle.go 86.48% <0.00%> (-5.41%) ⬇️
nat/traversal/pinger.go 75.71% <0.00%> (-3.81%) ⬇️
requests/dialer_swarm.go 77.09% <0.00%> (ø)
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 831d39c...9eea329. Read the comment docs.

crand.Read(agreementID)
_, err := crand.Read(agreementID)
if err != nil {
panic(err)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nothing useful to log before panic?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

nope, except panic error itself. I don't expect this happening anyway, just a measure of precaution.

Comment thread core/port/pool.go Outdated
@Snawoot
Snawoot merged commit 13c775c into master Sep 27, 2021
@Snawoot
Snawoot deleted the fix_port_pool branch September 27, 2021 09:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cannot listen on UDP port :: error="listen udp :: bind: address already in use"

5 participants