MINOR: xprt_qstrm: handle connection errors

This patch implements proper connection error handling for xprt_qstrm
layer. Basically, processing is interrupted if CO_FL_ERROR is
encountered after either rcv_buf or snd_buf operations. Connectionn
error is set to the newly defined value CO_ER_QSTRM.

diff --git a/include/haproxy/connection-t.h b/include/haproxy/connection-t.h
index bc69ab3532d093d82505ff85727b48535c360a05..a769ca0f1c4ee9fa9538f697cac7b5a9da95a60b 100644 (file)
--- a/include/haproxy/connection-t.h
+++ b/include/haproxy/connection-t.h
@@ -285,6 +285,8 @@ enum {
 
        CO_ER_SSL_FATAL,         /* SSL fatal error during a SSL_read or SSL_write */
 
+       CO_ER_QSTRM,             /* QMux transport parameter exchange failure */
+
        CO_ER_REVERSE,           /* Error during reverse connect */
 
        CO_ER_POLLERR,           /* we only noticed POLLERR */

diff --git a/src/connection.c b/src/connection.c
index e3b8915286344d99a01a8f5ca9a1ad18b1ca4a6e..f75895a76df9b4156e2147efd98a642b32ccb149 100644 (file)
--- a/src/connection.c
+++ b/src/connection.c
@@ -863,6 +863,8 @@ const char *conn_err_code_str(struct connection *c)
 
        case CO_ER_SSL_FATAL:      return "SSL fatal error";
 
+       case CO_ER_QSTRM:          return "Error during QMux transport parameters initial exchange";
+
        case CO_ER_REVERSE:        return "Reverse connect failure";
 
        case CO_ER_POLLERR:        return "Poller reported POLLERR";

diff --git a/src/session.c b/src/session.c
index e4b819617422aa62a8b5b5d2415acd0bedf19d8b..f53cceb64b1bf76f5e0cad81d043b7d47b2c7209 100644 (file)
--- a/src/session.c
+++ b/src/session.c
@@ -502,7 +502,8 @@ static void session_kill_embryonic(struct session *sess, unsigned int state)
                if (!conn->err_code ||
                    conn->err_code == CO_ER_PRX_EMPTY || conn->err_code == CO_ER_PRX_ABORT ||
                    conn->err_code == CO_ER_CIP_EMPTY || conn->err_code == CO_ER_CIP_ABORT ||
-                   conn->err_code == CO_ER_SSL_EMPTY || conn->err_code == CO_ER_SSL_ABORT)
+                   conn->err_code == CO_ER_SSL_EMPTY || conn->err_code == CO_ER_SSL_ABORT ||
+                   conn->err_code == CO_ER_QSTRM)
                        log = 0;
        }
 
@@ -514,6 +515,8 @@ static void session_kill_embryonic(struct session *sess, unsigned int state)
                                conn->err_code = CO_ER_CIP_TIMEOUT;
                        else if (conn->flags & CO_FL_SSL_WAIT_HS)
                                conn->err_code = CO_ER_SSL_TIMEOUT;
+                       else if (conn->flags & CO_FL_QSTRM_RECV)
+                               conn->err_code = CO_ER_QSTRM;
                }
 
                sess_log_embryonic(sess);

diff --git a/src/xprt_qstrm.c b/src/xprt_qstrm.c
index ce7a50f74a97ba73366998f5b5291961a5e4cc31..01a9265fcb7f64c77e9e1e7ad0967ddd1dcd75bd 100644 (file)
--- a/src/xprt_qstrm.c
+++ b/src/xprt_qstrm.c
@@ -65,7 +65,8 @@ int conn_recv_qstrm(struct connection *conn, struct xprt_qstrm_ctx *ctx, int fla
 
        do {
                ret = ctx->ops_lower->rcv_buf(conn, ctx->ctx_lower, buf, b_room(buf), NULL, 0, 0);
-               BUG_ON(conn->flags & CO_FL_ERROR);
+               if (conn->flags & CO_FL_ERROR)
+                       goto fail;
        } while (ret);
 
        if (!b_data(buf))
@@ -91,6 +92,7 @@ int conn_recv_qstrm(struct connection *conn, struct xprt_qstrm_ctx *ctx, int fla
        return 0;
 
  fail:
+       conn->err_code = CO_ER_QSTRM;
        conn->flags |= CO_FL_ERROR;
        return 0;
 }
@@ -123,6 +125,9 @@ int conn_send_qstrm(struct connection *conn, struct xprt_qstrm_ctx *ctx, int fla
 
        sent = ctx->ops_lower->snd_buf(conn, ctx->ctx_lower, buf, b_data(buf),
                                       NULL, 0, 0);
+       if (conn->flags & CO_FL_ERROR)
+               goto fail;
+
        b_del(buf, sent);
        if (b_data(buf))
                goto retry;
@@ -135,6 +140,7 @@ int conn_send_qstrm(struct connection *conn, struct xprt_qstrm_ctx *ctx, int fla
        return 0;
 
  fail:
+       conn->err_code = CO_ER_QSTRM;
        conn->flags |= CO_FL_ERROR;
        return 0;
 }
@@ -143,20 +149,23 @@ struct task *xprt_qstrm_io_cb(struct task *t, void *context, unsigned int state)
 {
        struct xprt_qstrm_ctx *ctx = context;
        struct connection *conn = ctx->conn;
-       int ret;
 
        if (conn->flags & CO_FL_QSTRM_SEND) {
                if (!conn_send_qstrm(conn, ctx, CO_FL_QSTRM_SEND)) {
-                       ctx->ops_lower->subscribe(conn, ctx->ctx_lower,
-                                                 SUB_RETRY_SEND, &ctx->wait_event);
+                       if (!(conn->flags & CO_FL_ERROR)) {
+                               ctx->ops_lower->subscribe(conn, ctx->ctx_lower,
+                                                         SUB_RETRY_SEND, &ctx->wait_event);
+                       }
                        goto out;
                }
        }
 
        if (conn->flags & CO_FL_QSTRM_RECV) {
                if (!conn_recv_qstrm(conn, ctx, CO_FL_QSTRM_RECV)) {
-                       ctx->ops_lower->subscribe(conn, ctx->ctx_lower,
-                                                 SUB_RETRY_RECV, &ctx->wait_event);
+                       if (!(conn->flags & CO_FL_ERROR)) {
+                               ctx->ops_lower->subscribe(conn, ctx->ctx_lower,
+                                                         SUB_RETRY_RECV, &ctx->wait_event);
+                       }
                        goto out;
                }
        }
@@ -164,15 +173,17 @@ struct task *xprt_qstrm_io_cb(struct task *t, void *context, unsigned int state)
  out:
        if ((conn->flags & CO_FL_ERROR) ||
            !(conn->flags & (CO_FL_QSTRM_RECV|CO_FL_QSTRM_SEND))) {
+               /* XPRT should be unsubscribed when transfer done or on error. */
+               BUG_ON(ctx->wait_event.events);
+
                /* MUX will access members from xprt_ctx on init, so create
                 * operation should be called before any members are resetted.
                 */
-               ret = conn_create_mux(conn, NULL);
-               BUG_ON(ret);
+               if (conn_create_mux(conn, NULL) == 0)
+                       conn->mux->wake(conn);
 
                conn->xprt_ctx = ctx->ctx_lower;
                conn->xprt = ctx->ops_lower;
-               conn->mux->wake(conn);
 
                b_free(&ctx->rxbuf);
                b_free(&ctx->txbuf);