Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 15, 2025

The hardcoded limit of 10 concurrent ports per device was insufficient for gateway deployments handling hundreds of simultaneous requests to domains like ddriveupdate.diode.link.

Changes

  • Added -maxports flag to socksd, gateway, and publish commands (default: 10)
  • Created Config.GetMaxPortsPerDevice() helper method
  • Updated rate limiting in datapool.IncrementConnectionAttempt() and socks.doConnectDevice() to use configurable value
  • Optimized by caching config lookups outside loops and synchronized calls
  • Setting -maxports=0 disables rate limiting entirely (unlimited connections)

Usage

# Default behavior (10 ports per device)
diode gateway

# Gateway handling high traffic (unlimited)
diode gateway -maxports 0

# Custom limit for specific deployment
diode gateway -maxports 1000

# Conservative limit for testing
diode socksd -maxports 5

This allows operators to tune the limit based on deployment characteristics while maintaining safe defaults for typical use cases. Setting the value to 0 removes all rate limiting for high-traffic gateway deployments.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.


Note

Adds a configurable per-device concurrent port limit via -maxports (0=unlimited) and uses it for connection rate limiting in SOCKS/datapool; flags added to gateway, publish, and socksd.

  • Config/CLI:
    • Add cfg.MaxPortsPerDevice and Config.GetMaxPortsPerDevice() in config/flag.go.
    • Introduce -maxports flag (default 10, 0 = unlimited) in cmd/diode/gateway.go, cmd/diode/publish.go, and cmd/diode/socksd.go.
  • RPC/Rate Limiting:
    • Update rpc/datapool.go IncrementConnectionAttempt() to enforce MaxPortsPerDevice across active and in-progress connections; prefetch config before lock.
    • Update rpc/socks.go doConnectDevice() to skip candidates when active ports reach MaxPortsPerDevice (cached config read outside loop).

Written by Cursor Bugbot for commit 594261c. This will update automatically on new commits. Configure here.

Copilot AI and others added 3 commits December 15, 2025 21:03
Co-authored-by: dominicletz <2987674+dominicletz@users.noreply.github.com>
Co-authored-by: dominicletz <2987674+dominicletz@users.noreply.github.com>
Co-authored-by: dominicletz <2987674+dominicletz@users.noreply.github.com>
@dominicletz
Copy link
Member

@copilot Let's change the logic a little bit so that when specifying -maxports=0 it means "unlimited"

Copilot AI changed the title [WIP] Fix port open/close storms for stable connections Add configurable max ports per device limit via -maxports flag Dec 15, 2025
Copilot AI requested a review from dominicletz December 15, 2025 21:10
Co-authored-by: dominicletz <2987674+dominicletz@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Dec 15, 2025

@copilot Let's change the logic a little bit so that when specifying -maxports=0 it means "unlimited"

Done. Setting -maxports=0 now means unlimited (no rate limiting). The flag descriptions have been updated to indicate "(0 = unlimited)". Commit: 594261c

@dominicletz dominicletz marked this pull request as ready for review December 16, 2025 09:12
@dominicletz dominicletz merged commit dad47c6 into hansr/squash-port-openclose-storms Dec 16, 2025
25 checks passed
@dominicletz dominicletz deleted the copilot/sub-pr-195 branch December 16, 2025 09:12
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

This PR is being reviewed by Cursor Bugbot

Details

You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.

To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

if activePorts+inProgressAttempts < 10 {
// Total should not exceed MaxPortsPerDevice (active + in-progress)
// If maxPorts is 0, it means unlimited
if maxPorts == 0 || activePorts+inProgressAttempts < maxPorts {
Copy link

Choose a reason for hiding this comment

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

Bug: Inconsistent handling of negative maxports values blocks connections

The two rate limiting checks handle negative maxPorts values inconsistently. In socks.go, the condition maxPorts > 0 treats negative values as unlimited (skips rate limiting). However, in datapool.go, the condition maxPorts == 0 only matches exactly zero, so negative values fall through to activePorts+inProgressAttempts < maxPorts, which is always false when maxPorts is negative (since non-negative counts are never less than a negative number). This blocks all connection attempts when a negative value is passed. The datapool.go condition should use maxPorts <= 0 instead of maxPorts == 0 for consistency.

Additional Locations (1)

Fix in Cursor Fix in Web

dominicletz added a commit that referenced this pull request Dec 16, 2025
* fix port connection storms

* more port open madness

* ignore repeats

* Add configurable max ports per device limit via -maxports flag (#199)

* Initial plan

* Extract hardcoded threshold of 10 into command line flag

Co-authored-by: dominicletz <2987674+dominicletz@users.noreply.github.com>

* Refactor: Extract fallback logic into GetMaxPortsPerDevice helper

Co-authored-by: dominicletz <2987674+dominicletz@users.noreply.github.com>

* Optimize: Cache GetMaxPortsPerDevice calls for performance

Co-authored-by: dominicletz <2987674+dominicletz@users.noreply.github.com>

* Change -maxports=0 to mean unlimited instead of using default

Co-authored-by: dominicletz <2987674+dominicletz@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: dominicletz <2987674+dominicletz@users.noreply.github.com>

---------

Co-authored-by: hansr <hansrempel@gmail.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: dominicletz <2987674+dominicletz@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants