Skip to content

Commit 01aaa69

Browse files
committed
connectd: limit incoming traffic to 1MB per second.
Decryption is pretty efficient, but incoming traffic can bog down connectd, especially on smaller nodes, so simply limit it to 1MB per second. This triggers in various tests, which is good: shows that it's working, and that we continue to (slowly!) process traffic. Changelog-Fixed: connectd: throttle incoming peers to give fairer peer handling under stress. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
1 parent 7c2d322 commit 01aaa69

7 files changed

Lines changed: 67 additions & 4 deletions

File tree

connectd/connectd.c

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,10 @@ static struct peer *new_peer(struct daemon *daemon,
128128
peer->cs = *cs;
129129
peer->subds = tal_arr(peer, struct subd *, 0);
130130
peer->peer_in = NULL;
131+
peer->bytes_rcvd_this_second = 0;
132+
peer->bytes_rcvd_start_time = time_mono();
133+
peer->recv_timer = NULL;
134+
peer->throttle_warned = false;
131135
membuf_init(&peer->encrypted_peer_out,
132136
tal_arr(peer, u8, 0), 0,
133137
membuf_tal_resize);
@@ -1751,8 +1755,10 @@ static void connect_init(struct daemon *daemon, const u8 *msg)
17511755
}
17521756

17531757
/* 500 bytes per second, not 1M per second */
1754-
if (dev_throttle_gossip)
1758+
if (dev_throttle_gossip) {
17551759
daemon->gossip_stream_limit = 500;
1760+
daemon->incoming_stream_limit = 500;
1761+
}
17561762

17571763
if (dev_limit_connections_inflight)
17581764
daemon->max_connect_in_flight = 1;
@@ -2565,6 +2571,7 @@ int main(int argc, char *argv[])
25652571
daemon->dev_keep_nagle = false;
25662572
/* We generally allow 1MB per second per peer, except for dev testing */
25672573
daemon->gossip_stream_limit = 1000000;
2574+
daemon->incoming_stream_limit = 1000000;
25682575
daemon->scid_htable = new_htable(daemon, scid_htable);
25692576

25702577
/* stdin == control */

connectd/connectd.h

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,15 @@ struct peer {
8383
/* Input buffer. */
8484
u8 *peer_in;
8585

86+
/* Bytes received in the last second. */
87+
size_t bytes_rcvd_this_second;
88+
/* When that second starts */
89+
struct timemono bytes_rcvd_start_time;
90+
/* Timer when we're throttling input */
91+
struct oneshot *recv_timer;
92+
/* Only send message once if peer gets throttled */
93+
bool throttle_warned;
94+
8695
/* Output buffer. */
8796
struct msg_queue *peer_outq;
8897

@@ -309,9 +318,12 @@ struct daemon {
309318
/* Allow localhost to be considered "public", only with --developer */
310319
bool dev_allow_localhost;
311320

312-
/* How much to gossip allow a peer every 60 seconds (bytes) */
321+
/* How much to gossip allow a peer every second (bytes) */
313322
size_t gossip_stream_limit;
314323

324+
/* How much incomign traffic do we allow per peer every second (bytes) */
325+
size_t incoming_stream_limit;
326+
315327
/* We support use of a SOCKS5 proxy (e.g. Tor) */
316328
struct addrinfo *proxyaddr;
317329

connectd/multiplex.c

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1395,6 +1395,8 @@ static struct io_plan *read_body_from_peer_done(struct io_conn *peer_conn,
13951395
tal_bytelen(peer->peer_in));
13961396
return io_close(peer_conn);
13971397
}
1398+
1399+
peer->bytes_rcvd_this_second += tal_bytelen(peer->peer_in);
13981400
tal_free(peer->peer_in);
13991401

14001402
type = fromwire_peektype(decrypted);
@@ -1511,16 +1513,52 @@ static struct io_plan *read_body_from_peer(struct io_conn *peer_conn,
15111513
if (!cryptomsg_decrypt_header(&peer->cs, peer->peer_in, &len))
15121514
return io_close(peer_conn);
15131515

1516+
peer->bytes_rcvd_this_second += tal_bytelen(peer->peer_in);
1517+
15141518
tal_resize(&peer->peer_in, (u32)len + CRYPTOMSG_BODY_OVERHEAD);
15151519
return io_read(peer_conn, peer->peer_in, tal_count(peer->peer_in),
15161520
read_body_from_peer_done, peer);
15171521
}
15181522

1523+
static void recv_throttle_timeout(struct peer *peer)
1524+
{
1525+
peer->recv_timer = tal_free(peer->recv_timer);
1526+
io_wake(&peer->peer_in);
1527+
}
1528+
15191529
static struct io_plan *read_hdr_from_peer(struct io_conn *peer_conn,
15201530
struct peer *peer)
15211531
{
1532+
struct timemono now = time_mono();
15221533
assert(peer->to_peer == peer_conn);
15231534

1535+
/* If it's been over a second, make a fresh start. */
1536+
if (time_to_sec(timemono_between(now, peer->bytes_rcvd_start_time)) > 0) {
1537+
peer->bytes_rcvd_start_time = now;
1538+
peer->bytes_rcvd_this_second = 0;
1539+
}
1540+
1541+
/* You sent too much this second? */
1542+
if (peer->bytes_rcvd_this_second > peer->daemon->incoming_stream_limit) {
1543+
status_unusual_once(&peer->throttle_warned,
1544+
CI_UNEXPECTED
1545+
"Throttling incoming peer %s:"
1546+
" too much traffic",
1547+
fmt_node_id(tmpctx, &peer->id));
1548+
1549+
/* Set timer for next second (if not already) */
1550+
if (!peer->recv_timer) {
1551+
peer->recv_timer = new_abstimer(&peer->daemon->timers,
1552+
peer,
1553+
timemono_add(peer->bytes_rcvd_start_time,
1554+
time_from_sec(1)),
1555+
recv_throttle_timeout,
1556+
peer);
1557+
}
1558+
return io_wait(peer_conn, &peer->peer_in,
1559+
read_hdr_from_peer, peer);
1560+
}
1561+
15241562
/* BOLT #8:
15251563
*
15261564
* ### Receiving and Decrypting Messages

tests/test_askrene.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1463,6 +1463,7 @@ def test_real_data(node_factory, bitcoind, executor):
14631463
opts=[{'gossip_store_file': outfile.name,
14641464
'allow_warning': True,
14651465
'dev-throttle-gossip': None,
1466+
'broken_log': 'Throttling incoming peer',
14661467
# This can be slow!
14671468
'askrene-timeout': TIMEOUT},
14681469
{'allow_warning': True}])
@@ -1573,6 +1574,7 @@ def test_real_biases(node_factory, bitcoind, executor):
15731574
l1, l2 = node_factory.line_graph(2, fundamount=AMOUNT,
15741575
opts=[{'gossip_store_file': outfile.name,
15751576
'allow_warning': True,
1577+
'broken_log': 'Throttling incoming peer',
15761578
'dev-throttle-gossip': None},
15771579
{'allow_warning': True}])
15781580

@@ -1685,6 +1687,7 @@ def test_askrene_fake_channeld(node_factory, bitcoind):
16851687
opts=[{'gossip_store_file': outfile.name,
16861688
'subdaemon': 'channeld:../tests/plugins/channeld_fakenet',
16871689
'allow_warning': True,
1690+
'broken_log': 'Throttling incoming peer',
16881691
'dev-throttle-gossip': None},
16891692
{'allow_bad_gossip': True}])
16901693

tests/test_gossip.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2164,7 +2164,9 @@ def test_dump_own_gossip(node_factory):
21642164
def test_gossip_throttle(node_factory, bitcoind, chainparams):
21652165
"""Make some gossip, test it gets throttled"""
21662166
l1, l2, l3, l4 = node_factory.line_graph(4, wait_for_announce=True,
2167-
opts=[{}, {}, {}, {'dev-throttle-gossip': None}])
2167+
opts=[{}, {}, {},
2168+
{'broken_log': 'Throttling incoming peer',
2169+
'dev-throttle-gossip': None}])
21682170

21692171
# We expect: self-advertizement (3 messages for l1 and l4) plus
21702172
# 4 node announcements, 3 channel announcements and 6 channel updates.

tests/test_plugin.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3160,7 +3160,7 @@ def test_commando(node_factory, executor):
31603160
@pytest.mark.slow_test
31613161
def test_commando_stress(node_factory, executor):
31623162
"""Stress test to slam commando with many large queries"""
3163-
nodes = node_factory.get_nodes(5)
3163+
nodes = node_factory.get_nodes(5, opts=[{'broken_log': 'Throttling incoming peer'}, {}, {}, {}, {}])
31643164

31653165
rune = nodes[0].rpc.createrune()['rune']
31663166
for n in nodes[1:]:

tests/test_xpay.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,7 @@ def test_xpay_fake_channeld(node_factory, bitcoind, chainparams, slow_mode):
241241
'allow_warning': True,
242242
'dev-throttle-gossip': None,
243243
'log-level': 'info',
244+
'broken_log': 'Throttling incoming peer',
244245
# xpay gets upset if it's aging when we remove cln-askrene!
245246
'dev-xpay-no-age': None,
246247
},

0 commit comments

Comments
 (0)