connectd: don't toggle nagle on and off, leave it always off.
What changed, and why it matters
This change simplifies how Core Lightning's networking layer handles TCP packet batching. Previously, the code repeatedly turned a kernel feature called Nagle's algorithm on and off for each message depending on whether it was 'urgent.' Now it just leaves Nagle disabled all the time. The practical effect is lower latency for time-sensitive Lightning messages and less risk of inconsistent behavior across operating systems. It is not a fix for a known exploit, but it removes a fragile, platform-dependent mechanism that could have caused message delays or odd edge cases.
Treat as a hardening/cleanup commit. Review whether any remaining references to peer->urgent or is_urgent() are cleaned up in the promised follow-up commits. No immediate security response is indicated, but operators should ensure they run a version that includes this change to avoid the prior inconsistent cross-platform behavior.
Security signals we found
Removal of dynamic TCP_NODELAY toggling reduces per-message setsockopt surface
New helper centralizes socket option setting at socket creation time
Guard added to avoid setting TCP options on UNIX domain sockets
Commentary in removed code acknowledged platform-specific behavior and potential for one extra packet on non-Linux platforms
Evidence from the diff
The commit removes the dynamic toggling of TCP_NODELAY in connectd/multiplex.c and instead sets TCP_NODELAY once when a TCP socket is created in connectd/connectd.c. The old set_urgent_flag() path toggled NODELAY per message type, with platform-specific comments about Linux TCP_CORK vs FreeBSD behavior. The new set_tcp_no_delay() helper is called for both outgoing and incoming TCP sockets (ADDR_INTERNAL_WIREADDR), but skipped for UNIX sockets. is_urgent() is retained but marked UNNEEDED for a future commit. The change is framed as a cleanup because connectd now does its own buffering.
Changed components
connectd/connectd.cconnectd/multiplex.cTCP socket setup for peer connectionsMessage send path in peer multiplexingInspect captured patch +22 / −39
diff --git a/connectd/connectd.c b/connectd/connectd.c
index 792b472f..8421b905 100644
--- a/connectd/connectd.c
+++ b/connectd/connectd.c
@@ -46,6 +46,7 @@
#include <fcntl.h>
#include <netdb.h>
#include <netinet/in.h>
+#include <netinet/tcp.h>
#include <signal.h>
#include <sodium.h>
#include <sys/stat.h>
@@ -507,6 +508,19 @@ static bool get_remote_address(struct io_conn *conn,
return true;
}
+/* Nagle had a good idea of making networking more efficient by
+ * inserting a delay, creating a trap for every author of network code
+ * everywhere.
+ */
+static void set_tcp_no_delay(int fd)
+{
+ int val = 1;
+ if (setsockopt(fd, IPPROTO_TCP, TCP_NODELAY, &val, sizeof(val)) != 0) {
+ status_broken("setsockopt TCP_NODELAY=1 fd=%u: %s",
+ fd, strerror(errno));
+ }
+}
+
/*~ As so common in C, we need to bundle two args into a callback, so we
* allocate a temporary structure to hold them: */
struct conn_in {
@@ -639,6 +653,10 @@ static struct io_plan *connection_in(struct io_conn *conn,
if (!get_remote_address(conn, &conn_in_arg.addr))
return io_close(conn);
+ /* Don't try to set TCP options on UNIX socket! */
+ if (conn_in_arg.addr.itype == ADDR_INTERNAL_WIREADDR)
+ set_tcp_no_delay(io_conn_fd(conn));
+
conn_in_arg.daemon = daemon;
conn_in_arg.is_websocket = false;
return conn_in(conn, &conn_in_arg);
@@ -1175,6 +1193,9 @@ static void try_connect_one_addr(struct connecting *connect)
goto next;
}
+ /* Don't try to set TCP options on UNIX socket! */
+ if (addr->itype == ADDR_INTERNAL_WIREADDR)
+ set_tcp_no_delay(fd);
connect->connect_attempted = true;
/* This creates the new connection using our fd, with the initialization
diff --git a/connectd/multiplex.c b/connectd/multiplex.c
index b19bffe9..abc31d3b 100644
--- a/connectd/multiplex.c
+++ b/connectd/multiplex.c
@@ -22,7 +22,6 @@
#include <errno.h>
#include <inttypes.h>
#include <netinet/in.h>
-#include <netinet/tcp.h>
#include <wire/peer_wire.h>
#include <wire/wire_io.h>
@@ -294,42 +293,7 @@ void setup_peer_gossip_store(struct peer *peer,
return;
}
-/* We're happy for the kernel to batch update and gossip messages, but a
- * commitment message, for example, should be instantly sent. There's no
- * great way of doing this, unfortunately.
- *
- * Setting TCP_NODELAY on Linux flushes the socket, which really means
- * we'd want to toggle on then off it *after* sending. But Linux has
- * TCP_CORK. On FreeBSD, it seems (looking at source) not to, so
- * there we'd want to set it before the send, and reenable it
- * afterwards. Even if this is wrong on other non-Linux platforms, it
- * only means one extra packet.
- */
-static void set_urgent_flag(struct peer *peer, bool urgent)
-{
- int val;
-
- if (urgent == peer->urgent)
- return;
-
- /* FIXME: We can't do this on websockets, but we could signal our
- * websocket proxy via some magic message to do so! */
- if (peer->is_websocket != NORMAL_SOCKET)
- return;
-
- val = urgent;
- if (setsockopt(io_conn_fd(peer->to_peer),
- 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 TCP_NODELAY=1 fd=%u: %s",
- io_conn_fd(peer->to_peer),
- strerror(errno));
- }
- peer->urgent = urgent;
-}
-
-static bool is_urgent(enum peer_wire type)
+static bool UNNEEDED is_urgent(enum peer_wire type)
{
switch (type) {
case WIRE_INIT:
@@ -510,8 +474,6 @@ static struct io_plan *encrypt_and_send(struct peer *peer, const u8 *msg TAKES)
{
int type = fromwire_peektype(msg);
- set_urgent_flag(peer, is_urgent(type));
-
/* Special message type directing us to process batch items. */
if (type == WIRE_PROTOCOL_BATCH_ELEMENT) {
peer->encrypted_peer_out = process_batch_elements(peer, msg);
Why this scored 23/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.