Skip to content

p2p: size inbox reads by remaining capacity so InboxOverflow can't drop a peer we're applying backpressure to #7792

Open

Description

@brice-stacks

Background

#7784 caps the inbound work handled per network pass (max_inbound_messages_per_pass, max_inbound_processing_ms). While a conversation's inbox still has messages queued, its socket isn't read, so the remote peer is held back by TCP backpressure.

When the inbox is drained and we read again, ConnectionInbox::recv_bytes reads until WouldBlock, taking in everything that piled up in the kernel buffer. If that's more than inbox_maxlen messages, consume_messages returns InboxOverflow and the peer is dropped.

With node defaults (inbox_maxlen = 100, max_inbound_messages_per_pass = 64), a peer that keeps sending more than ~64 messages per pass can now eventually be dropped. Before #7784, that took more than ~100 messages in a single pass. Honest peers shouldn't send at these rates, but the backpressure should never end in a disconnect.

Separately, a burst of more than inbox_maxlen small messages arriving within one pass already caused InboxOverflow before #7784.

Why a check between reads isn't enough

Stopping the read loop once inbox.len() reaches a threshold doesn't prevent overflow. Each read() takes up to 64 KB, and consume_messages parses all of it at once. A P2P message is at least PREAMBLE_ENCODED_SIZE bytes (~165), so a single read can add ~385 messages.

Proposal

Size each read by the inbox space left. A read of N bytes can complete at most 1 + N / min_msg_size messages: one partial message already buffered, plus whole new ones. In recv_bytes:

let room = self.inbox_maxlen.saturating_sub(self.inbox.len());
if room <= 1 {
    self.read_paused = true; // leave the rest in the kernel buffer
    break;
}
let max_read = (room - 1).saturating_mul(min_msg_size).min(buf.len());
// read into buf[..max_read]
  • Add ProtocolFamily::min_message_size(). For StacksP2P it's PREAMBLE_ENCODED_SIZE. HTTP opts out and keeps the current behavior.
  • Expose read_paused through ConversationP2P::recv. process_p2p_conversation must keep revisiting a paused conversation (revisit = !convo_dead && (deferred_recv || read_paused || inbox_len() > 0)). Sockets are edge-triggered, so if a pass drains the inbox after a paused read, nothing would wake us for the bytes still on the socket.
  • Large messages are unaffected: a partially received message doesn't add to the inbox, so reads only shrink near capacity.

Behavior change to agree on

With this change, InboxOverflow can no longer happen for P2P conversations. A peer that floods us is throttled by TCP backpressure instead of being disconnected. Per-pass work and memory stay bounded, but the peer keeps its connection slot. The remaining defenses are the push-bandwidth limits and bans. We should confirm that's the behavior we want.

Tests

  • Unit test (connection.rs): feed a pipe more than inbox_maxlen tiny messages. The read pauses at capacity with no InboxOverflow, then resumes once the inbox is drained.
  • Integration test (net/tests/neighbors.rs): a peer with inbox_maxlen = 100 and max_inbound_messages_per_pass = 64 is sent a few hundred pings. The sender is never dropped and every ping gets a reply.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions