MEDIUM: connection: remove the intermediary polling state from the connection

Historically we used to require that the connections held the desired
polling states for the data layer and the socket layer. Then with muxes
these were more or less merged into the transport layer, and now it
happens that with all transport layers having their own state, the
"transport layer state" as we have it in the connection (XPRT_RD_ENA,
XPRT_WR_ENA) is only an exact copy of the undelying file descriptor
state, but with a delay. All of this is causing some difficulties at
many places in the code because there are still some locations which
use the conn_want_* API to remain clean and only rely on connection,
and count on a later collection call to conn_cond_update_polling(),
while others need an immediate action and directly use the FD updates.

Since our updates are now much cheaper, most of them being only an
atomic test-and-set operation, and since our I/O callbacks are deferred,
there's no benefit anymore in trying to "cache" the transient state
change in the connection flags hoping to cancel them before they
become an FD event. Better make such calls transparent indirections
to the FD layer instead and get rid of the deferred operations which
needlessly complicate the logic inside.

This removes flags CO_FL_XPRT_{RD,WR}_ENA and CO_FL_WILL_UPDATE.
A number of functions related to polling updates were either greatly
simplified or removed.

Two places were using CO_FL_XPRT_WR_ENA as a hint to know if more data
were expected to be sent after a PROXY protocol or SOCKSv4 header. These
ones were simply replaced with a check on the subscription which is
where we ought to get the autoritative information from.

Now the __conn_xprt_want_* and their conn_xprt_want_* counterparts
are the same. conn_stop_polling() and conn_xprt_stop_both() are the
same as well. conn_cond_update_polling() only causes errors to stop
polling. It also becomes way more obvious that muxes should not at
all employ conn_xprt_{want|stop}_{recv,send}(), and that the call
to __conn_xprt_stop_recv() in case a mux failed to allocate a buffer
is inappropriate, it ought to unsubscribe from reads instead. All of
this definitely requires a serious cleanup.

contrib/debug/flags.c

@@ -163,11 +163,8 @@ void show_conn_flags(unsigned int f)
 	SHOW_FLAG(f, CO_FL_ADDR_TO_SET);
 	SHOW_FLAG(f, CO_FL_ADDR_FROM_SET);
 	SHOW_FLAG(f, CO_FL_WAIT_ROOM);
-	SHOW_FLAG(f, CO_FL_WILL_UPDATE);
 	SHOW_FLAG(f, CO_FL_XPRT_READY);
 	SHOW_FLAG(f, CO_FL_CTRL_READY);
-	SHOW_FLAG(f, CO_FL_XPRT_WR_ENA);
-	SHOW_FLAG(f, CO_FL_XPRT_RD_ENA);
 
 	if (f) {
 		printf("EXTRA(0x%08x)", f);

include/proto/connection.h

@@ -163,48 +163,20 @@ static inline void conn_stop_tracking(struct connection *conn)
 	conn->flags &= ~CO_FL_XPRT_TRACKED;
 }
 
-/* Update polling on connection <c>'s file descriptor depending on its current
- * state as reported in the connection's CO_FL_XPRT_* flags. The connection
- * flags are updated with the new flags at the end of the operation. Polling
- * is totally disabled if an error was reported.
- */
-void conn_update_xprt_polling(struct connection *c);
-
-/* Automatically updates polling on connection <c> depending on the XPRT flags.
- * It does nothing if CO_FL_WILL_UPDATE is present, indicating that an upper
- * caller is going to do it again later.
- */
-static inline void conn_cond_update_xprt_polling(struct connection *c)
-{
-	if (!(c->flags & CO_FL_WILL_UPDATE))
-		conn_update_xprt_polling(c);
-}
-
 /* Stop all polling on the fd. This might be used when an error is encountered
- * for example. It does not propage the change to the fd layer if
- * CO_FL_WILL_UPDATE is present, indicating that an upper caller is going to do
- * it later.
+ * for example.
  */
 static inline void conn_stop_polling(struct connection *c)
 {
-	c->flags &= ~(CO_FL_XPRT_RD_ENA | CO_FL_XPRT_WR_ENA);
-	if (!(c->flags & CO_FL_WILL_UPDATE) && conn_ctrl_ready(c))
+	if (conn_ctrl_ready(c))
 		fd_stop_both(c->handle.fd);
 }
 
-/* Automatically update polling on connection <c> depending on the XPRT and
- * SOCK flags, and on whether a handshake is in progress or not. This may be
- * called at any moment when there is a doubt about the effectiveness of the
- * polling state, for instance when entering or leaving the handshake state.
- * It does nothing if CO_FL_WILL_UPDATE is present, indicating that an upper
- * caller is going to do it again later.
- */
+/* Stops polling in case of error on the connection. */
 static inline void conn_cond_update_polling(struct connection *c)
 {
 	if (unlikely(c->flags & CO_FL_ERROR))
 		conn_stop_polling(c);
-	else if (!(c->flags & CO_FL_WILL_UPDATE))
-		conn_update_xprt_polling(c);
 }
 
 /***** Event manipulation primitives for use by DATA I/O callbacks *****/
@@ -214,57 +186,57 @@ static inline void conn_cond_update_polling(struct connection *c)
  */
 static inline void __conn_xprt_want_recv(struct connection *c)
 {
-	c->flags |= CO_FL_XPRT_RD_ENA;
+	if (conn_ctrl_ready(c))
+		fd_want_recv(c->handle.fd);
 }
 
 static inline void __conn_xprt_stop_recv(struct connection *c)
 {
-	c->flags &= ~CO_FL_XPRT_RD_ENA;
+	if (conn_ctrl_ready(c))
+		fd_stop_recv(c->handle.fd);
 }
 
 static inline void __conn_xprt_want_send(struct connection *c)
 {
-	c->flags |= CO_FL_XPRT_WR_ENA;
+	if (conn_ctrl_ready(c))
+		fd_want_send(c->handle.fd);
 }
 
 static inline void __conn_xprt_stop_send(struct connection *c)
 {
-	c->flags &= ~CO_FL_XPRT_WR_ENA;
+	if (conn_ctrl_ready(c))
+		fd_stop_send(c->handle.fd);
 }
 
 static inline void __conn_xprt_stop_both(struct connection *c)
 {
-	c->flags &= ~(CO_FL_XPRT_WR_ENA | CO_FL_XPRT_RD_ENA);
+	if (conn_ctrl_ready(c))
+		fd_stop_both(c->handle.fd);
 }
 
 static inline void conn_xprt_want_recv(struct connection *c)
 {
 	__conn_xprt_want_recv(c);
-	conn_cond_update_xprt_polling(c);
 }
 
 static inline void conn_xprt_stop_recv(struct connection *c)
 {
 	__conn_xprt_stop_recv(c);
-	conn_cond_update_xprt_polling(c);
 }
 
 static inline void conn_xprt_want_send(struct connection *c)
 {
 	__conn_xprt_want_send(c);
-	conn_cond_update_xprt_polling(c);
 }
 
 static inline void conn_xprt_stop_send(struct connection *c)
 {
 	__conn_xprt_stop_send(c);
-	conn_cond_update_xprt_polling(c);
 }
 
 static inline void conn_xprt_stop_both(struct connection *c)
 {
 	__conn_xprt_stop_both(c);
-	conn_cond_update_xprt_polling(c);
 }
 
 /* read shutdown, called from the rcv_buf/rcv_pipe handlers when
@@ -290,7 +262,6 @@ static inline void conn_sock_shutw(struct connection *c, int clean)
 {
 	c->flags |= CO_FL_SOCK_WR_SH;
 	__conn_xprt_stop_send(c);
-	conn_cond_update_xprt_polling(c);
 
 	/* don't perform a clean shutdown if we're going to reset or
 	 * if the shutr was already received.

include/types/connection.h

@@ -141,18 +141,18 @@ enum {
 
 	/* Do not change these values without updating conn_*_poll_changes() ! */
 	/* unused : 0x00000001 */
-	CO_FL_XPRT_RD_ENA   = 0x00000002,  /* receiving data is allowed */
+	/* unused : 0x00000002 */
 	/* unused : 0x00000004, 0x00000008 */
 
 	/* unused : 0x00000010 */
-	CO_FL_XPRT_WR_ENA   = 0x00000020,  /* sending data is desired */
+	/* unused : 0x00000020 */
 	/* unused : 0x00000040, 0x00000080 */
 
 	/* These flags indicate whether the Control and Transport layers are initialized */
 	CO_FL_CTRL_READY    = 0x00000100, /* FD was registered, fd_delete() needed */
 	CO_FL_XPRT_READY    = 0x00000200, /* xprt_init() done, xprt_close() needed */
 
-	CO_FL_WILL_UPDATE   = 0x00000400, /* the conn handler will take care of updating the polling */
+	/* unused : 0x00000400 */
 
 	/* This flag is used by data layers to indicate they had to stop
 	 * receiving data because a buffer was full. The connection handler

src/connection.c

@@ -80,8 +80,6 @@ void conn_fd_handler(int fd)
 		return;
 	}
 
-	conn->flags |= CO_FL_WILL_UPDATE;
-
 	flags = conn->flags & ~CO_FL_ERROR; /* ensure to call the wake handler upon error */
 
 	if (unlikely(conn->flags & CO_FL_WAIT_L4_CONN) &&
@@ -161,36 +159,10 @@ void conn_fd_handler(int fd)
 		return;
 
 	/* commit polling changes */
-	conn->flags &= ~CO_FL_WILL_UPDATE;
 	conn_cond_update_polling(conn);
 	return;
 }
 
-/* Update polling on connection <c>'s file descriptor depending on its current
- * state as reported in the connection's CO_FL_XPRT_* flags. The connection
- * flags are updated with the new flags at the end of the operation. Polling
- * is totally disabled if an error was reported.
- */
-void conn_update_xprt_polling(struct connection *c)
-{
-	unsigned int f = c->flags;
-
-	if (!conn_ctrl_ready(c))
-		return;
-
-	/* update read status if needed */
-	if (f & CO_FL_XPRT_RD_ENA)
-		fd_want_recv(c->handle.fd);
-	else
-		fd_stop_recv(c->handle.fd);
-
-	/* update write status if needed */
-	if (f & CO_FL_XPRT_WR_ENA)
-		fd_want_send(c->handle.fd);
-	else
-		fd_stop_send(c->handle.fd);
-}
-
 /* This is the callback which is set when a connection establishment is pending
  * and we have nothing to send. It may update the FD polling status to indicate
  * !READY. It returns 0 if it fails in a fatal way or needs to poll to go
@@ -358,7 +330,6 @@ int conn_unsubscribe(struct connection *conn, void *xprt_ctx, int event_type, st
 	if (event_type & SUB_RETRY_SEND)
 		__conn_xprt_stop_send(conn);
 
-	conn_update_xprt_polling(conn);
 	return 0;
 }
 
@@ -381,7 +352,6 @@ int conn_subscribe(struct connection *conn, void *xprt_ctx, int event_type, stru
 	if (event_type & SUB_RETRY_SEND)
 		__conn_xprt_want_send(conn);
 
-	conn_update_xprt_polling(conn);
 	return 0;
 }
 
@@ -1036,7 +1006,7 @@ int conn_send_socks4_proxy_request(struct connection *conn)
 				conn,
 				((char *)(&req_line)) + (sizeof(req_line)+conn->send_proxy_ofs),
 				-conn->send_proxy_ofs,
-				(conn->flags & CO_FL_XPRT_WR_ENA) ? MSG_MORE : 0);
+				(conn->subs && conn->subs->events & SUB_RETRY_SEND) ? MSG_MORE : 0);
 
 		DPRINTF(stderr, "SOCKS PROXY HS FD[%04X]: Before send remain is [%d], sent [%d]\n",
 				conn->handle.fd, -conn->send_proxy_ofs, ret);

src/stream_interface.c

@@ -396,7 +396,7 @@ int conn_si_send_proxy(struct connection *conn, unsigned int flag)
 		ret = conn_sock_send(conn,
 				     trash.area + ret + conn->send_proxy_ofs,
 		                     -conn->send_proxy_ofs,
-		                     (conn->flags & CO_FL_XPRT_WR_ENA) ? MSG_MORE : 0);
+		                     (conn->subs && conn->subs->events & SUB_RETRY_SEND) ? MSG_MORE : 0);
 
 		if (ret < 0)
 			goto out_error;