Skip to content

add setting allow_multiple_connections_per_peer#8372

Open
milahu wants to merge 2 commits into
arvidn:RC_2_1from
milahu:add-setting-allow_multiple_connections_per_peer
Open

add setting allow_multiple_connections_per_peer#8372
milahu wants to merge 2 commits into
arvidn:RC_2_1from
milahu:add-setting-allow_multiple_connections_per_peer

Conversation

@milahu
Copy link
Copy Markdown

@milahu milahu commented May 14, 2026

fix #8367

@arvidn
Copy link
Copy Markdown
Owner

arvidn commented May 15, 2026

this seems like a bug in the recent allow_multiple_connections_per_pid feature. It's not clear why we would need another setting, or how it differs from peer_id.

what does "peer" refer to in that name, if it's not IP nor peer-ID?

@milahu
Copy link
Copy Markdown
Author

milahu commented May 15, 2026

what does "peer" refer to in that name, if it's not IP nor peer-ID?

peer = IP address + port

include/libtorrent/settings_pack.hpp

			// allow multiple connections
			// from the same IP address and port
			allow_multiple_connections_per_peer,

so we could rename

-allow_multiple_connections_per_peer
+allow_multiple_connections_per_ip_and_port

so in total, would have these settings

allow_multiple_connections_per_ip
allow_multiple_connections_per_ip_and_port
allow_multiple_connections_per_peer_id [*]

[*] with #8373

@glassez
Copy link
Copy Markdown
Contributor

glassez commented May 15, 2026

peer = IP address + port

IP address + port is usually called endpoint, no?

@milahu
Copy link
Copy Markdown
Author

milahu commented May 15, 2026

that would give

allow_multiple_connections_per_ip
allow_multiple_connections_per_endpoint
allow_multiple_connections_per_peer_id

but ip_and_port is more explicit than endpoint
in effect, more user-friendly

also because setting
allow_multiple_connections_per_ip_and_port = True
would imply
allow_multiple_connections_per_ip = True
(TODO implement)

@arvidn
Copy link
Copy Markdown
Owner

arvidn commented May 15, 2026

TCP/IP does not allow two TCP connections to have the same (src-ip, src-port, dst-ip, dst-port) tuple. I don't believe it's possible for an IP and port to connect multiple times to the same IP and port.

@milahu
Copy link
Copy Markdown
Author

milahu commented May 16, 2026

TCP/IP does not allow two TCP connections to have the same (src-ip, src-port, dst-ip, dst-port) tuple.

yes, this allows us to detect stale connections

in d95d007 i have replaced the setting

-allow_multiple_connections_per_peer
+replace_stale_connections

for now this is experimental, so default false
but in the future, this should be changed to default true

the beauty is this:

  1. seeder is listening for leechers
  2. leecher1 connects to seeder
  3. leecher1 crashes and cannot cleanup its connection to seeder
  4. seeder thinks that leecher1 is still connected
  5. leecher2 tries to re-use the listening port of leecher1
  6. the operating system detects that leecher1 is no longer using its listening port, so leecher2 can successfully re-use the listening port
  7. leecher2 connects to seeder
  8. seeder in peer_list::new_connection receives a new connection from leecher2, using the exact same connection tuple (src_ipaddr, src_port, dst_ipaddr, dst_port) as in the old connection from leecher1, which is not possible in TCP/IP, so the old connection entry from leecher1 must be stale, therefore it is 100% safe to close the old connection from leecher1 and replace it with the new connection from leecher2

this may be an edge case, because all peers are running on localhost
but my hope is that also not-so-edge cases will profit from this change

currently the seeder log shows

*** DUPLICATE_PEER [ this: "127.0.0.1" that: "127.0.0.1" ]
*** CONNECTION_CLOSED [ op: 1 ERROR: (libtorrent:220) replaced stale connection ]

ideally we should remove the DUPLICATE_PEER message
because in this case, it is a "false duplicate" peer

todo: squash commits (or make a new PR)

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.

3 participants