]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
MEDIUM: connection: get rid of data->init() which was not for data
authorWilly Tarreau <w@1wt.eu>
Mon, 28 Aug 2017 13:46:01 +0000 (15:46 +0200)
committerWilly Tarreau <w@1wt.eu>
Wed, 30 Aug 2017 05:04:04 +0000 (07:04 +0200)
The ->init() callback of the connection's data layer was only used to
complete the session's initialisation since sessions and streams were
split apart in 1.6. The problem is that it creates a big confusion in
the layers' roles as the session has to register a dummy data layer
when waiting for a handshake to complete, then hand it off to the
stream which will replace it.

The real need is to notify that the transport has finished initializing.
This should enable a better splitting between these layers.

This patch thus introduces a connection-specific callback called
xprt_done_cb() which informs about handshake successes or failures. With
this, data->init() can disappear, CO_FL_INIT_DATA as well, and we don't
need to register a dummy data->wake() callback to be notified of errors.

contrib/debug/flags.c
include/proto/connection.h
include/types/connection.h
src/connection.c
src/session.c

index 2638dd3430ad332c81a2a1f2923d25bbb96d1fab..d45b9684f8ff806af1e9a9f73d2910c27d2ea56b 100644 (file)
@@ -118,7 +118,6 @@ void show_conn_flags(unsigned int f)
        SHOW_FLAG(f, CO_FL_SOCK_RD_SH);
        SHOW_FLAG(f, CO_FL_DATA_WR_SH);
        SHOW_FLAG(f, CO_FL_DATA_RD_SH);
-       SHOW_FLAG(f, CO_FL_INIT_DATA);
        SHOW_FLAG(f, CO_FL_ADDR_TO_SET);
        SHOW_FLAG(f, CO_FL_ADDR_FROM_SET);
        SHOW_FLAG(f, CO_FL_WAIT_ROOM);
index 66887a541376de72d56d8b498a5f8726e4c6f191..db712f58bf281dd6b3e853e765c2ea93670a85f5 100644 (file)
@@ -500,10 +500,29 @@ static inline void conn_init(struct connection *conn)
        conn->handle.fd = DEAD_FD_MAGIC;
        conn->err_code = CO_ER_NONE;
        conn->target = NULL;
+       conn->xprt_done_cb = NULL;
        conn->proxy_netns = NULL;
        LIST_INIT(&conn->list);
 }
 
+/* sets <owner> as the connection's owner */
+static inline void conn_set_owner(struct connection *conn, void *owner)
+{
+       conn->owner = owner;
+}
+
+/* registers <cb> as a callback to notify for transport's readiness or failure */
+static inline void conn_set_xprt_done_cb(struct connection *conn, int (*cb)(struct connection *))
+{
+       conn->xprt_done_cb = cb;
+}
+
+/* unregisters the callback to notify for transport's readiness or failure */
+static inline void conn_clear_xprt_done_cb(struct connection *conn)
+{
+       conn->xprt_done_cb = NULL;
+}
+
 /* Tries to allocate a new connection and initialized its main fields. The
  * connection is returned on success, NULL on failure. The connection must
  * be released using pool_free2() or conn_free().
index 59b8cff8eccc2c491af94b752242c20a8c19eed5..748af8de61dbb07be838445046ccfd53a6780c7c 100644 (file)
@@ -95,8 +95,7 @@ enum {
        CO_FL_ADDR_FROM_SET = 0x00001000,  /* addr.from is set */
        CO_FL_ADDR_TO_SET   = 0x00002000,  /* addr.to is set */
 
-       /* flags indicating what event type the data layer is interested in */
-       CO_FL_INIT_DATA     = 0x00004000,  /* initialize the data layer before using it */
+       /* unused : 0x00004000 */
        /* unused : 0x00008000 */
 
        /* flags used to remember what shutdown have been performed/reported */
@@ -107,6 +106,7 @@ enum {
 
        /* flags used to report connection errors or other closing conditions */
        CO_FL_ERROR         = 0x00100000,  /* a fatal error was reported     */
+       CO_FL_NOTIFY_DONE   = 0x001C0000,  /* any xprt shut/error flags above needs to be reported */
        CO_FL_NOTIFY_DATA   = 0x001F0000,  /* any shut/error flags above needs to be reported */
 
        /* flags used to report connection status updates */
@@ -248,16 +248,12 @@ struct xprt_ops {
  * callbacks are supposed to make use of the xprt_ops above to exchange data
  * from/to buffers and pipes. The <wake> callback is used to report activity
  * at the transport layer, which can be a connection opening/close, or any
- * data movement. The <init> callback may be called by the connection handler
- * at the end of a transport handshake, when it is about to transfer data and
- * the data layer is not ready yet. Both <wake> and <init> may abort a connection
- * by returning < 0.
+ * data movement. It may abort a connection by returning < 0.
  */
 struct data_cb {
        void (*recv)(struct connection *conn);  /* data-layer recv callback */
        void (*send)(struct connection *conn);  /* data-layer send callback */
        int  (*wake)(struct connection *conn);  /* data-layer callback to report activity */
-       int  (*init)(struct connection *conn);  /* data-layer initialization */
        char name[8];                           /* data layer name, zero-terminated */
 };
 
@@ -289,7 +285,10 @@ struct conn_src {
  * socket, and can also be made to an internal applet. It can support
  * several transport schemes (raw, ssl, ...). It can support several
  * connection control schemes, generally a protocol for socket-oriented
- * connections, but other methods for applets.
+ * connections, but other methods for applets. The xprt_done_cb() callback
+ * is called once the transport layer initialization is done (success or
+ * failure). It may return < 0 to report an error and require an abort of the
+ * connection being instanciated. It must be removed once done.
  */
 struct connection {
        enum obj_type obj_type;       /* differentiates connection from applet context */
@@ -305,6 +304,7 @@ struct connection {
        union conn_handle handle;     /* connection handle at the socket layer */
        enum obj_type *target;        /* the target to connect to (server, proxy, applet, ...) */
        struct list list;             /* attach point to various connection lists (idle, ...) */
+       int (*xprt_done_cb)(struct connection *conn);  /* callback to notify of end of handshake */
        const struct netns_entry *proxy_netns;
        struct {
                struct sockaddr_storage from;   /* client address, or address to spoof when connecting to the server */
index 564c9761b2e61ddb5cca835512e1f3238a4938c5..54bbc0737b62b4e7ee1e1fa7fdc37d4316fd4a12 100644 (file)
@@ -86,12 +86,14 @@ void conn_fd_handler(int fd)
        if (!(conn->flags & CO_FL_POLL_SOCK))
                __conn_sock_stop_both(conn);
 
-       /* The data layer might not be ready yet (eg: when using embryonic
-        * sessions). If we're about to move data, we must initialize it first.
-        * The function may fail and cause the connection to be destroyed, thus
-        * we must not use it anymore and should immediately leave instead.
+       /* The connection owner might want to be notified about an end of
+        * handshake indicating the connection is ready, before we proceed with
+        * any data exchange. The callback may fail and cause the connection to
+        * be destroyed, thus we must not use it anymore and should immediately
+        * leave instead. The caller must immediately unregister itself once
+        * called.
         */
-       if ((conn->flags & CO_FL_INIT_DATA) && conn->data->init(conn) < 0)
+       if (conn->xprt_done_cb && conn->xprt_done_cb(conn) < 0)
                return;
 
        if (conn->xprt && fd_send_ready(fd) &&
@@ -137,6 +139,16 @@ void conn_fd_handler(int fd)
        if (unlikely(!(conn->flags & (CO_FL_WAIT_L4_CONN | CO_FL_WAIT_L6_CONN | CO_FL_CONNECTED))))
                conn->flags |= CO_FL_CONNECTED;
 
+       /* The connection owner might want to be notified about failures to
+        * complete the handshake. The callback may fail and cause the
+        * connection to be destroyed, thus we must not use it anymore and
+        * should immediately leave instead. The caller must immediately
+        * unregister itself once called.
+        */
+       if (((conn->flags ^ flags) & CO_FL_NOTIFY_DONE) &&
+           conn->xprt_done_cb && conn->xprt_done_cb(conn) < 0)
+               return;
+
        /* The wake callback is normally used to notify the data layer about
         * data layer activity (successful send/recv), connection establishment,
         * shutdown and fatal errors. We need to consider the following
index 62713abce5142f2e0362a52b8821760d44a69b17..cc2a0b87a74ed7b5305191d94d54bea2e0206dd7 100644 (file)
 struct pool_head *pool2_session;
 
 static int conn_complete_session(struct connection *conn);
-static int conn_update_session(struct connection *conn);
 static struct task *session_expire_embryonic(struct task *t);
 
-/* data layer callbacks for an embryonic stream */
-struct data_cb sess_conn_cb = {
-       .recv = NULL,
-       .send = NULL,
-       .wake = conn_update_session,
-       .init = conn_complete_session,
-       .name = "SESS",
-};
-
 /* Create a a new session and assign it to frontend <fe>, listener <li>,
  * origin <origin>, set the current date and clear the stick counters pointers.
  * Returns the session upon success or NULL. The session may be released using
@@ -251,11 +241,11 @@ int session_accept_fd(struct listener *l, int cfd, struct sockaddr_storage *addr
         *          conn -- owner ---> task
         */
        if (cli_conn->flags & CO_FL_HANDSHAKE) {
-               conn_attach(cli_conn, t, &sess_conn_cb);
+               conn_set_owner(cli_conn, t);
+               conn_set_xprt_done_cb(cli_conn, conn_complete_session);
                t->process = session_expire_embryonic;
                t->expire = tick_add_ifset(now_ms, p->timeout.client);
                task_queue(t);
-               cli_conn->flags |= CO_FL_INIT_DATA;
                return 1;
        }
 
@@ -423,9 +413,14 @@ static int conn_complete_session(struct connection *conn)
        struct task *task = conn->owner;
        struct session *sess = task->context;
 
+       conn_clear_xprt_done_cb(conn);
+
        if (conn->flags & CO_FL_ERROR)
                goto fail;
 
+       if (conn->flags & CO_FL_HANDSHAKE)
+               return 0; /* wait more */
+
        /* if logs require transport layer information, note it on the connection */
        if (sess->fe->to_log & LW_XPRT)
                conn->flags |= CO_FL_XPRT_TRACKED;
@@ -439,8 +434,6 @@ static int conn_complete_session(struct connection *conn)
        if (!stream_new(sess, task, &conn->obj_type))
                goto fail;
 
-       conn->flags &= ~CO_FL_INIT_DATA;
-
        task_wakeup(task, TASK_WOKEN_INIT);
        return 0;
 
@@ -449,21 +442,6 @@ static int conn_complete_session(struct connection *conn)
        return -1;
 }
 
-/* Update a session status. The connection is killed in case of
- * error, and <0 will be returned. Otherwise it does nothing.
- */
-static int conn_update_session(struct connection *conn)
-{
-       struct task *task = conn->owner;
-       struct session *sess = task->context;
-
-       if (conn->flags & CO_FL_ERROR) {
-               session_kill_embryonic(sess, task);
-               return -1;
-       }
-       return 0;
-}
-
 /*
  * Local variables:
  *  c-indent-level: 8