connectd: don't toggle nagle on and off, leave it always off.

We're doing our own buffering now.

We leave the is_urgent() function for two commits in the future though.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

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

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
+static bool UNNEEDED is_urgent(enum peer_wire type)
- * 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)
 {
 	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);