Make all peer messages constant message size

.github/scripts/setup.sh

@@ -52,9 +52,11 @@ sudo apt-get -qq install --no-install-recommends --allow-unauthenticated -yy \
      sudo \
      tcl \
      tclsh \
+     tshark \
      unzip \
      valgrind \
      wget \
+     wireshark-common \
      xsltproc \
      systemtap-sdt-dev \
      zlib1g-dev
@@ -97,3 +99,14 @@ export PROTOC=/usr/local/bin/protoc
 export PATH=$PATH:/usr/local/bin
 env
 ls -lha /usr/local/bin
+
+# wireshark-common normally does this, but GH runners are special, so we
+# do it explicitly
+sudo groupadd -f wireshark
+sudo chgrp wireshark /usr/bin/dumpcap
+sudo chmod 750 /usr/bin/dumpcap
+sudo setcap cap_net_raw,cap_net_admin=eip /usr/bin/dumpcap
+
+# Add ourselves to the wireshark group (still need "sg wireshark..." for it to take effect)
+sudo usermod -aG wireshark "$(id -nu)"
+

.github/workflows/ci.yaml

@@ -323,7 +323,7 @@ jobs:
         run: |
           env
           cat config.vars
-          uv run eatmydata pytest tests/test_downgrade.py -n ${PYTEST_PAR} ${PYTEST_OPTS}
+          sg wireshark "uv run eatmydata pytest tests/test_downgrade.py -n ${PYTEST_PAR} ${PYTEST_OPTS}"
       - name: Upload test results
         if: always()
         uses: actions/upload-artifact@v4
@@ -438,7 +438,7 @@ jobs:
         run: |
           env
           cat config.vars
-          VALGRIND=0 uv run eatmydata pytest tests/ -n ${PYTEST_PAR} ${PYTEST_OPTS}
+          VALGRIND=0 sg wireshark "uv run eatmydata pytest tests/ -n ${PYTEST_PAR} ${PYTEST_OPTS}"
       - name: Upload test results
         if: always()
         uses: actions/upload-artifact@v4
@@ -526,7 +526,7 @@ jobs:
           TEST_DEBUG: 1
           PYTEST_PAR: 2
         run: |
-          VALGRIND=1 uv run eatmydata pytest tests/ -n ${PYTEST_PAR} ${PYTEST_OPTS}  ${{ matrix.PYTEST_OPTS }}
+          VALGRIND=1 sg wireshark "uv run eatmydata pytest tests/ -n ${PYTEST_PAR} ${PYTEST_OPTS}  ${{ matrix.PYTEST_OPTS }}"
       - name: Upload test results
         if: always()
         uses: actions/upload-artifact@v4
@@ -617,7 +617,7 @@ jobs:
         env:
           PYTEST_PAR: 2
         run: |
-          uv run eatmydata pytest tests/ -n ${PYTEST_PAR} ${PYTEST_OPTS}  ${{ matrix.PYTEST_OPTS }}
+          sg wireshark "uv run eatmydata pytest tests/ -n ${PYTEST_PAR} ${PYTEST_OPTS}  ${{ matrix.PYTEST_OPTS }}"
       - name: Upload test results
         if: always()
         uses: actions/upload-artifact@v4
@@ -748,7 +748,7 @@ jobs:
         run: |
           env
           cat config.vars
-          VALGRIND=0 uv run eatmydata pytest tests/ -n ${PYTEST_PAR} ${PYTEST_OPTS}
+          VALGRIND=0 sg wireshark "uv run eatmydata pytest tests/ -n ${PYTEST_PAR} ${PYTEST_OPTS}"
       - name: Upload test results
         if: always()
         uses: actions/upload-artifact@v4

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>
@@ -127,8 +128,13 @@ static struct peer *new_peer(struct daemon *daemon,
 	peer->cs = *cs;
 	peer->subds = tal_arr(peer, struct subd *, 0);
 	peer->peer_in = NULL;
-	peer->sent_to_peer = NULL;
-	peer->urgent = false;
+	membuf_init(&peer->encrypted_peer_out,
+		    tal_arr(peer, u8, 0), 0,
+		    membuf_tal_resize);
+	peer->encrypted_peer_out_sent = 0;
+	peer->nonurgent_flush_timer = NULL;
+	peer->peer_out_urgent = 0;
+	peer->flushing_nonurgent = false;
 	peer->draining_state = NOT_DRAINING;
 	peer->peer_in_lastmsg = -1;
 	peer->peer_outq = msg_queue_new(peer, false);
@@ -505,6 +511,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 {
@@ -526,6 +545,10 @@ static struct io_plan *conn_in(struct io_conn *conn,
 			       time_from_sec(daemon->timeout_secs),
 			       conn_timeout, conn);
 
+	/* Don't try to set TCP options on local socket! */
+	if (!conn_in_arg->is_websocket)
+		set_tcp_no_delay(io_conn_fd(conn));
+
 	/*~ The crypto handshake differs depending on whether you received or
 	 * initiated the socket connection, so there are two entry points.
 	 * Note, again, the notleak() to avoid our simplistic leak detection
@@ -1173,6 +1196,7 @@ static void try_connect_one_addr(struct connecting *connect)
 		goto next;
 	}
 
+	set_tcp_no_delay(fd);
 	connect->connect_attempted = true;
 
 	/* This creates the new connection using our fd, with the initialization

connectd/connectd.h

@@ -3,6 +3,7 @@
 #include "config.h"
 #include <bitcoin/short_channel_id.h>
 #include <ccan/htable/htable_type.h>
+#include <ccan/membuf/membuf.h>
 #include <ccan/timer/timer.h>
 #include <common/bigsize.h>
 #include <common/crypto_state.h>
@@ -79,17 +80,18 @@ struct peer {
 	/* Connections to the subdaemons */
 	struct subd **subds;
 
-	/* When socket has Nagle overridden */
-	bool urgent;
-
 	/* Input buffer. */
 	u8 *peer_in;
 
 	/* Output buffer. */
 	struct msg_queue *peer_outq;
 
-	/* Peer sent buffer (for freeing after sending) */
-	const u8 *sent_to_peer;
+	/* Encrypted peer sending buffer */
+	MEMBUF(u8) encrypted_peer_out;
+	size_t encrypted_peer_out_sent;
+	size_t peer_out_urgent;
+	bool flushing_nonurgent;
+	struct oneshot *nonurgent_flush_timer;
 
 	/* We stream from the gossip_store for them, when idle */
 	struct gossip_state gs;