connectd: fix nagle disabling logic.

Our CORK logic was wrong, and it's better to use Nagle anyway:
we disable Nagle just before sending timing-critical messages.

Time for 100 (failed) payments:

Before:
	148.8573575

After:
	10.7356977

Note this revealed a timing problem in test_reject_invalid_payload: we would
miss the cause of the sendonion failure, and waitsendpay would be called
*after* it had failed, so simply returns "Payment failure reason unknown".

Signed-off-by: Rusty Russell <[email protected]>
Changelog-Fixed: Protocol: Removed 200ms latency from sending commit/revoke messages.

Author: rustyrussell

connectd/multiplex.c

@@ -307,8 +307,6 @@ void setup_peer_gossip_store(struct peer *peer,
 static void set_urgent_flag(struct peer *peer, bool urgent)
 {
 	int val;
-	int opt;
-	const char *optname;
 
 	if (urgent == peer->urgent)
 		return;
@@ -318,23 +316,13 @@ static void set_urgent_flag(struct peer *peer, bool urgent)
 	if (peer->is_websocket != NORMAL_SOCKET)
 		return;
 
-#ifdef TCP_CORK
-	opt = TCP_CORK;
-	optname = "TCP_CORK";
-#elif defined(TCP_NODELAY)
-	opt = TCP_NODELAY;
-	optname = "TCP_NODELAY";
-#else
-#error "Please report platform with neither TCP_CORK nor TCP_NODELAY?"
-#endif
-
 	val = urgent;
 	if (setsockopt(io_conn_fd(peer->to_peer),
-		       IPPROTO_TCP, opt, &val, sizeof(val)) != 0
+		       IPPROTO_TCP, TCP_NODELAY, &val, sizeof(val)) != 0
 	    /* This actually happens in testing, where we blackhole the fd */
 	    && peer->daemon->dev_disconnect_fd == -1) {
-		status_broken("setsockopt %s=1 fd=%u: %s",
-			      optname, io_conn_fd(peer->to_peer),
+		status_broken("setsockopt TCP_NODELAY=1 fd=%u: %s",
+			      io_conn_fd(peer->to_peer),
 			      strerror(errno));
 	}
 	peer->urgent = urgent;

tests/test_connection.py

@@ -4822,7 +4822,6 @@ def test_private_channel_no_reconnect(node_factory):
     assert only_one(l1.rpc.listpeers()['peers'])['connected'] is False
 
 
-@pytest.mark.xfail(strict=True)
 @unittest.skipIf(VALGRIND, "We assume machine is reasonably fast")
 def test_no_delay(node_factory):
     """Is our Nagle disabling for critical messages working?"""

tests/test_pay.py

@@ -3423,12 +3423,11 @@ def test_reject_invalid_payload(node_factory):
                      first_hop=first_hop,
                      payment_hash=inv['payment_hash'],
                      shared_secrets=onion['shared_secrets'])
-
-    l2.daemon.wait_for_log(r'Failing HTLC because of an invalid payload')
-
     with pytest.raises(RpcError, match=r'WIRE_INVALID_ONION_PAYLOAD'):
         l1.rpc.waitsendpay(inv['payment_hash'])
 
+    l2.daemon.wait_for_log(r'Failing HTLC because of an invalid payload')
+
 
 @unittest.skip("Test is flaky causing CI to be unusable.")
 def test_excluded_adjacent_routehint(node_factory, bitcoind):