BUG/MEDIUM: mux-spop: Properly handle CLOSING state

The CLOSING state was not handled at all by the SPOP multiplexer while it is
mandatory when a DISCONNECT frame was sent and the mux should wait for the
DISCONNECT frame in reply from the agent. Thanks to this patch, it should be
fixed.

In addition, if an error occurres during the AGENT HELLO frame parsing, the
SPOP connection is no longer switched to CLOSED state and remains in ERROR
state instead. It is important to be able to send the DISCONNECT frame to
the agent instead of closing the TCP connection immediately.

This patch depends on following commits:

  * BUG/MEDIUM: mux-spop: Remove frame parsing states from the SPOP connection state
  * MINOR: mux-spop: Don't set SPOP connection state to FRAME_H after ACK parsing
  * BUG/MINOR: mux-spop: Don't open new streams for SPOP connection on error
  * BUG/MINOR: mux-spop: Make the demux stream ID a signed integer

All the series must be backported to 3.1.

(cherry picked from commit ddc5f8d92e3613ec7060b0ee8bdb220866182e86)
Signed-off-by: Willy Tarreau <w@1wt.eu>

diff --git a/src/mux_spop.c b/src/mux_spop.c
index 58350f4..6a30f72 100644 (file)
--- a/src/mux_spop.c
+++ b/src/mux_spop.c
@@ -1536,6 +1536,7 @@ static int spop_conn_send_disconnect(struct spop_conn *spop_conn)
        spop_set_frame_size(outbuf.area, outbuf.data - 4);
        b_add(mbuf, outbuf.data);
        spop_conn->flags |= SPOP_CF_DISCO_SENT;
+       spop_conn->state = SPOP_CS_CLOSING;
        ret = 1;
 
   end:
@@ -1731,8 +1732,6 @@ static int spop_conn_handle_hello(struct spop_conn *spop_conn)
        TRACE_LEAVE(SPOP_EV_RX_FRAME|SPOP_EV_RX_HELLO, spop_conn->conn);
        return 1;
   fail:
-       spop_conn->state = SPOP_CS_CLOSED;
-       TRACE_STATE("switching to CLOSED", SPOP_EV_RX_FRAME|SPOP_EV_RX_HELLO, spop_conn->conn);
        TRACE_DEVEL("leaving on error", SPOP_EV_RX_FRAME|SPOP_EV_RX_HELLO|SPOP_EV_SPOP_CONN_ERR, spop_conn->conn);
        return 0;
 }
@@ -1917,7 +1916,6 @@ static int spop_conn_handle_ack(struct spop_conn *spop_conn, struct spop_strm *s
 
        sent = b_xfer(rxbuf, dbuf, flen);
        BUG_ON(sent != flen);
-       /* b_del(&spop_conn->dbuf, sent); */
        spop_conn->dfl -= sent;
 
        // TODO: may happen or not ?
@@ -2014,27 +2012,34 @@ static void spop_process_demux(struct spop_conn *spop_conn)
 
        TRACE_ENTER(SPOP_EV_SPOP_CONN_WAKE, spop_conn->conn);
 
-       if (spop_conn->state >= SPOP_CS_ERROR)
+       if (spop_conn->state == SPOP_CS_CLOSED)
                goto out;
 
-       if (unlikely(spop_conn->state < SPOP_CS_RUNNING)) {
+       if (unlikely(spop_conn->state < SPOP_CS_RUNNING || spop_conn->state == SPOP_CS_CLOSING)) {
                if (spop_conn->state == SPOP_CS_HA_HELLO) {
                        TRACE_STATE("waiting AGENT HELLO frame to be sent", SPOP_EV_RX_FRAME|SPOP_EV_RX_FHDR|SPOP_EV_RX_HELLO, spop_conn->conn);
                        goto out;
                }
-               if (spop_conn->state == SPOP_CS_AGENT_HELLO) {
-                       /* ensure that what is pending is a valid AGENT HELLO frame. */
-                       TRACE_STATE("receiving AGENT HELLO frame header", SPOP_EV_RX_FRAME|SPOP_EV_RX_FHDR, spop_conn->conn);
+               else { /* AGENT_HELLO OR CLOSING */
+                       /* ensure that what is pending is a valid AGENT HELLO/DISCONNECT frame. */
+                       TRACE_STATE("receiving AGENT HELLO/DISCONNECT frame header", SPOP_EV_RX_FRAME|SPOP_EV_RX_FHDR, spop_conn->conn);
                        if (!spop_get_frame_hdr(&spop_conn->dbuf, &hdr)) {
                                spop_conn->flags |= SPOP_CF_DEM_SHORT_READ;
                                TRACE_ERROR("header frame not available yet", SPOP_EV_RX_FRAME|SPOP_EV_RX_FHDR, spop_conn->conn);
                                goto done;
                        }
 
-                       if (hdr.sid || hdr.fid || hdr.type != SPOP_FRM_T_AGENT_HELLO || !(hdr.flags & SPOP_FRM_FL_FIN)) {
+                       if (spop_conn->state == SPOP_CS_AGENT_HELLO && hdr.type != SPOP_FRM_T_AGENT_HELLO) {
+                               spop_conn_error(spop_conn, SPOP_ERR_INVALID);
+                               spop_conn->state = SPOP_CS_CLOSED;
+                               TRACE_ERROR("unexpected frame type (AGENT HELLO expected)", SPOP_EV_RX_FRAME|SPOP_EV_RX_FHDR|SPOP_EV_RX_HELLO|SPOP_EV_SPOP_CONN_ERR, spop_conn->conn);
+                               TRACE_STATE("switching to CLOSED", SPOP_EV_RX_FRAME|SPOP_EV_RX_FHDR|SPOP_EV_RX_HELLO|SPOP_EV_SPOP_CONN_ERR, spop_conn->conn);
+                               goto done;
+                       }
+                       if ((hdr.type == SPOP_FRM_T_AGENT_HELLO || hdr.type == SPOP_FRM_T_AGENT_DISCON) && (hdr.sid || hdr.fid || !(hdr.flags & SPOP_FRM_FL_FIN))) {
                                spop_conn_error(spop_conn, SPOP_ERR_INVALID);
                                spop_conn->state = SPOP_CS_CLOSED;
-                               TRACE_ERROR("unexpected frame type or flags", SPOP_EV_RX_FRAME|SPOP_EV_RX_FHDR|SPOP_EV_RX_HELLO|SPOP_EV_SPOP_CONN_ERR, spop_conn->conn);
+                               TRACE_ERROR("unexpected frame meta-data", SPOP_EV_RX_FRAME|SPOP_EV_RX_FHDR|SPOP_EV_RX_HELLO|SPOP_EV_SPOP_CONN_ERR, spop_conn->conn);
                                TRACE_STATE("switching to CLOSED", SPOP_EV_RX_FRAME|SPOP_EV_RX_FHDR|SPOP_EV_RX_HELLO|SPOP_EV_SPOP_CONN_ERR, spop_conn->conn);
                                goto done;
                        }
@@ -2061,7 +2066,7 @@ static void spop_process_demux(struct spop_conn *spop_conn)
                        break;
                }
 
-               if (spop_conn->state >= SPOP_CS_ERROR) {
+               if (spop_conn->state == SPOP_CS_CLOSED) {
                        TRACE_STATE("end of connection reported", SPOP_EV_RX_FRAME|SPOP_EV_RX_EOI, spop_conn->conn);
                        break;
                }