diff --git a/connectd/connectd.c b/connectd/connectd.c index 003ca90c0550..3fb4a24615bd 100644 --- a/connectd/connectd.c +++ b/connectd/connectd.c @@ -1674,6 +1674,7 @@ static void connect_init(struct daemon *daemon, const u8 *msg) struct wireaddr *announceable; char *tor_password; bool dev_disconnect, dev_throttle_gossip, dev_limit_connections_inflight; + u32 dev_gossip_cpu_budget; char *errstr; /* Fields which require allocation are allocated off daemon */ @@ -1696,6 +1697,7 @@ static void connect_init(struct daemon *daemon, const u8 *msg) &daemon->dev_no_ping_timer, &daemon->dev_handshake_no_reply, &dev_throttle_gossip, + &dev_gossip_cpu_budget, &daemon->dev_no_reconnect, &daemon->dev_fast_reconnect, &dev_limit_connections_inflight, @@ -1770,6 +1772,8 @@ static void connect_init(struct daemon *daemon, const u8 *msg) daemon->incoming_stream_limit = 1000; daemon->cpu_budget_usec_limit = 1500; } + if (dev_gossip_cpu_budget) + daemon->cpu_budget_usec_limit = dev_gossip_cpu_budget; if (dev_limit_connections_inflight) daemon->max_connect_in_flight = 1; diff --git a/connectd/connectd_wire.csv b/connectd/connectd_wire.csv index 03dccc94718c..9b9670704735 100644 --- a/connectd/connectd_wire.csv +++ b/connectd/connectd_wire.csv @@ -26,6 +26,7 @@ msgdata,connectd_init,dev_no_ping_timer,bool, # Allow incoming connections, but don't talk. msgdata,connectd_init,dev_noreply,bool, msgdata,connectd_init,dev_throttle_gossip,bool, +msgdata,connectd_init,dev_gossip_cpu_budget,u32, msgdata,connectd_init,dev_no_reconnect,bool, msgdata,connectd_init,dev_fast_reconnect,bool, msgdata,connectd_init,dev_limit_connections_inflight,bool, diff --git a/lightningd/connect_control.c b/lightningd/connect_control.c index 54b3d46c1180..99e417118a1b 100644 --- a/lightningd/connect_control.c +++ b/lightningd/connect_control.c @@ -760,6 +760,7 @@ int connectd_init(struct lightningd *ld) ld->dev_no_ping_timer, ld->dev_handshake_no_reply, ld->dev_throttle_gossip, + ld->dev_gossip_cpu_budget, !ld->reconnect, ld->dev_fast_reconnect, ld->dev_limit_connections_inflight, diff --git a/lightningd/lightningd.c b/lightningd/lightningd.c index 811b49542191..f1ad2089b0f6 100644 --- a/lightningd/lightningd.c +++ b/lightningd/lightningd.c @@ -128,6 +128,7 @@ static struct lightningd *new_lightningd(const tal_t *ctx) ld->dev_fast_gossip = false; ld->dev_fast_gossip_prune = false; ld->dev_throttle_gossip = false; + ld->dev_gossip_cpu_budget = 0; ld->dev_suppress_gossip = false; ld->dev_fast_reconnect = false; ld->dev_reject_closing_fee = false; diff --git a/lightningd/lightningd.h b/lightningd/lightningd.h index b60ea7bc9ae1..94bfba534fd8 100644 --- a/lightningd/lightningd.h +++ b/lightningd/lightningd.h @@ -317,6 +317,7 @@ struct lightningd { bool dev_fast_gossip; bool dev_fast_gossip_prune; bool dev_throttle_gossip; + u32 dev_gossip_cpu_budget; bool dev_suppress_gossip; /* How long to aim for low-priority commitment closes */ diff --git a/lightningd/options.c b/lightningd/options.c index 0294b8ff71d8..8c2ff366b2bf 100644 --- a/lightningd/options.c +++ b/lightningd/options.c @@ -949,6 +949,10 @@ static void dev_register_opts(struct lightningd *ld) opt_set_bool, &ld->dev_throttle_gossip, "Throttle gossip right down, for testing"); + clnopt_witharg("--dev-gossip-cpu-budget", OPT_DEV|OPT_SHOWINT, + opt_set_u32, opt_show_u32, + &ld->dev_gossip_cpu_budget, + "Total CPU usec per second for answering gossip queries (0: default)"); clnopt_noarg("--dev-limit-connections-inflight", OPT_DEV, opt_set_bool, &ld->dev_limit_connections_inflight, diff --git a/tests/test_gossip.py b/tests/test_gossip.py index 5df2f020471e..3144688c9f2b 100644 --- a/tests/test_gossip.py +++ b/tests/test_gossip.py @@ -2282,7 +2282,7 @@ def test_gossip_query_channel_range_cpu_throttle(node_factory, chainparams): 'disable-plugin': unneeded_plugins}, {'gossip_store_file': gsfile.name, 'disable-plugin': unneeded_plugins, - 'dev-throttle-gossip': None, + 'dev-gossip-cpu-budget': 10, 'broken_log': 'Throttling (incoming|outgoing) peer'}]) # A range far beyond any scid we generated: the reply matches @@ -2307,9 +2307,11 @@ def test_gossip_query_channel_range_cpu_throttle(node_factory, chainparams): check=True, timeout=TIMEOUT, stdout=subprocess.PIPE) assert time.time() - start < 10 - # l2 (--dev-throttle-gossip): the query itself is small to read, but - # *answering* it means walking the whole gossmap, which blows the - # tiny CPU budget: throttled even though the reply is minuscule. + # l2 (--dev-gossip-cpu-budget=10): the query itself is small to read, + # but *answering* it means walking the whole gossmap, which blows the + # tiny CPU budget: throttled even though the reply is minuscule. The + # traffic limits stay at their defaults, so only CPU can throttle it, + # and a 10 usec budget is blown by a full walk on any machine. # Ten queries down one connection, so we blow the budget by a wide # margin rather than by a hair. The earlier version asserted on wall # clock after a single query, which only held while the budget stayed @@ -2335,12 +2337,19 @@ def test_gossip_query_channel_range_cpu_throttle(node_factory, chainparams): def test_gossip_throttle_no_unrequested_stream(node_factory, chainparams): """When a throttle ends for a peer which never sent gossip_timestamp_filter, we must not start streaming gossip to it.""" - chans = [GenChannel(i, i + 1) for i in range(15000)] + # Few enough channels that walking them costs little, but enough that + # a full range reply is over --dev-throttle-gossip's 1000 bytes a + # second: that throttles by traffic for a couple of seconds, whatever + # the machine's speed. + chans = [GenChannel(i, i + 1) for i in range(300)] gsfile, nodemap = generate_gossip_store(chans) + # The huge CPU budget leaves only --dev-throttle-gossip's traffic + # limit, so the throttle is always the traffic one. l1 = node_factory.get_node(options={'disable-plugin': ['recover', 'cln-askrene', 'cln-renepay', 'bookkeeper', 'topology', 'cln-xpay'], - 'dev-throttle-gossip': None}, + 'dev-throttle-gossip': None, + 'dev-gossip-cpu-budget': 1000000000}, gossip_store_file=gsfile.name, broken_log='Throttling (incoming|outgoing) peer') @@ -2351,7 +2360,7 @@ def test_gossip_throttle_no_unrequested_stream(node_factory, chainparams): query = subprocess.run(['devtools/mkquery', 'query_channel_range', chainparams['chain_hash'], - '1000000', '1'], + '0', '4294967295'], check=True, timeout=TIMEOUT, stdout=subprocess.PIPE).stdout.strip().decode() @@ -2362,15 +2371,41 @@ def test_gossip_throttle_no_unrequested_stream(node_factory, chainparams): '--filter=256,257,258', '--max-messages=1', '{}@localhost:{}'.format(l1.info['id'], l1.port), - init] + [query] * 10, + init, query], stdout=subprocess.PIPE) - l1.daemon.wait_for_log(r'Throttling outgoing peer .*: too much CPU') + l1.daemon.wait_for_log(r'Throttling outgoing peer .*: too much traffic') l1.daemon.wait_for_log('Throttle over, not streaming: no gossip_timestamp_filter') gossipwith.kill() assert gossipwith.communicate()[0] == b'' +def test_gossip_cpu_throttle_only_queries(node_factory): + """Only answering gossip queries counts against the CPU budget: a + peer sending us ordinary messages (here pings) must not get its reads + throttled, even with a tiny CPU budget.""" + # A tiny CPU budget, but the normal traffic limits, so only CPU can + # trip the throttle. + l1 = node_factory.get_node(options={'dev-gossip-cpu-budget': 10}) + + # ping with num_pong_bytes 0, no padding. + ping = '0012' + '0000' + '0000' + out = subprocess.run(['devtools/gossipwith', + '--no-gossip', + '--hex', + '--network={}'.format(TEST_NETWORK), + '--filter=19', + '--max-messages=200', + '--timeout-after=30', + '{}@localhost:{}'.format(l1.info['id'], l1.port)] + + [ping] * 200, + timeout=TIMEOUT, stdout=subprocess.PIPE).stdout.split() + + # Every ping was answered, so all 200 really were read. + assert len(out) == 200 + assert not l1.daemon.is_in_log('Throttling incoming peer') + + def test_generate_gossip_store(node_factory): l1 = node_factory.get_node(start=False) chans = [GenChannel(0, 1),