MEDIUM: channel: merge back flags CF_WRITE_PARTIAL and CF_WRITE_EVENT

The behaviour of the flag CF_WRITE_PARTIAL was modified by commit
95fad5ba4 ("BUG/MAJOR: stream-int: don't re-arm recv if send fails") due
to a situation where it could trigger an immediate wake up of the other
side, both acting in loops via the FD cache. This loss has caused the
need to introduce CF_WRITE_EVENT as commit c5a9d5bf, to replace it, but
both flags express more or less the same thing and this distinction
creates a lot of confusion and complexity in the code.

Since the FD cache now acts via tasklets, the issue worked around in the
first patch no longer exists, so it's more than time to kill this hack
and to restore CF_WRITE_PARTIAL's semantics (i.e.: there has been some
write activity since we last left process_stream).

This patch mostly reverts the two commits above. Only the part making
use of CF_WROTE_DATA instead of CF_WRITE_PARTIAL to detect the loss of
data upon connection setup was kept because it's more accurate and
better suited.

diff --git a/contrib/debug/flags.c b/contrib/debug/flags.c
index a9e09096a7ea31ed4b8598b86df79b9115e84172..ae885d6d857547a0c6c738c2dfcbe3437cf939df 100644 (file)
--- a/contrib/debug/flags.c
+++ b/contrib/debug/flags.c
@@ -72,7 +72,6 @@ void show_chn_flags(unsigned int f)
 
        SHOW_FLAG(f, CF_ISRESP);
        SHOW_FLAG(f, CF_FLT_ANALYZE);
-       SHOW_FLAG(f, CF_WRITE_EVENT);
        SHOW_FLAG(f, CF_WAKE_ONCE);
        SHOW_FLAG(f, CF_NEVER_WAIT);
        SHOW_FLAG(f, CF_SEND_DONTWAIT);

diff --git a/include/proto/channel.h b/include/proto/channel.h
index bf84a16bbdc8c6d894a654cee2ed3fb52b3db80c..56bf5cf61c56d03767cfd2eae3eb4d218869ea40 100644 (file)
--- a/include/proto/channel.h
+++ b/include/proto/channel.h
@@ -459,7 +459,7 @@ static inline void channel_check_timeouts(struct channel *chn)
            unlikely(tick_is_expired(chn->rex, now_ms)))
                chn->flags |= CF_READ_TIMEOUT;
 
-       if (likely(!(chn->flags & (CF_SHUTW|CF_WRITE_TIMEOUT|CF_WRITE_ACTIVITY|CF_WRITE_EVENT))) &&
+       if (likely(!(chn->flags & (CF_SHUTW|CF_WRITE_TIMEOUT|CF_WRITE_ACTIVITY))) &&
            unlikely(tick_is_expired(chn->wex, now_ms)))
                chn->flags |= CF_WRITE_TIMEOUT;
 
@@ -772,7 +772,7 @@ static inline void co_skip(struct channel *chn, int len)
        c_realign_if_empty(chn);
 
        /* notify that some data was written to the SI from the buffer */
-       chn->flags |= CF_WRITE_PARTIAL | CF_WRITE_EVENT;
+       chn->flags |= CF_WRITE_PARTIAL;
 }
 
 /* Tries to copy chunk <chunk> into the channel's buffer after length controls.

diff --git a/include/types/channel.h b/include/types/channel.h
index 7879b1258baa447124a32230425fd8db2828443a..f7ddd1a5763ed439e5059f0c826becb66113cd2c 100644 (file)
--- a/include/types/channel.h
+++ b/include/types/channel.h
@@ -117,11 +117,11 @@
 
 #define CF_WAKE_ONCE      0x10000000  /* pretend there is activity on this channel (one-shoot) */
 #define CF_FLT_ANALYZE    0x20000000  /* at least one filter is still analyzing this channel */
-#define CF_WRITE_EVENT    0x40000000  /* write activity not processed yet by the stream */
+/* unused: 0x40000000 */
 #define CF_ISRESP         0x80000000  /* 0 = request channel, 1 = response channel */
 
 /* Masks which define input events for stream analysers */
-#define CF_MASK_ANALYSER  (CF_READ_ATTACHED|CF_READ_ACTIVITY|CF_READ_TIMEOUT|CF_ANA_TIMEOUT|CF_WRITE_ACTIVITY|CF_WAKE_ONCE|CF_WRITE_EVENT)
+#define CF_MASK_ANALYSER  (CF_READ_ATTACHED|CF_READ_ACTIVITY|CF_READ_TIMEOUT|CF_ANA_TIMEOUT|CF_WRITE_ACTIVITY|CF_WAKE_ONCE)
 
 /* Mask for static flags which cause analysers to be woken up when they change */
 #define CF_MASK_STATIC    (CF_SHUTR|CF_SHUTW|CF_SHUTR_NOW|CF_SHUTW_NOW)

diff --git a/src/proto_http.c b/src/proto_http.c
index e0ae82ee907cf18d13b27dd6c4454eee561dc17d..8ac4c018ba24520dfd0269d4fc9a34a9a2d6018c 100644 (file)
--- a/src/proto_http.c
+++ b/src/proto_http.c
@@ -3694,7 +3694,7 @@ void http_end_txn_clean_session(struct stream *s)
        s->si[1].exp       = TICK_ETERNITY;
        s->si[1].flags    &= SI_FL_ISBACK | SI_FL_DONT_WAKE; /* we're in the context of process_stream */
        s->req.flags &= ~(CF_SHUTW|CF_SHUTW_NOW|CF_AUTO_CONNECT|CF_WRITE_ERROR|CF_STREAMER|CF_STREAMER_FAST|CF_NEVER_WAIT|CF_WAKE_CONNECT|CF_WROTE_DATA);
-       s->res.flags &= ~(CF_SHUTR|CF_SHUTR_NOW|CF_READ_ATTACHED|CF_READ_ERROR|CF_READ_NOEXP|CF_STREAMER|CF_STREAMER_FAST|CF_WRITE_PARTIAL|CF_NEVER_WAIT|CF_WROTE_DATA|CF_WRITE_EVENT);
+       s->res.flags &= ~(CF_SHUTR|CF_SHUTR_NOW|CF_READ_ATTACHED|CF_READ_ERROR|CF_READ_NOEXP|CF_STREAMER|CF_STREAMER_FAST|CF_WRITE_PARTIAL|CF_NEVER_WAIT|CF_WROTE_DATA);
        s->flags &= ~(SF_DIRECT|SF_ASSIGNED|SF_ADDR_SET|SF_BE_ASSIGNED|SF_FORCE_PRST|SF_IGNORE_PRST);
        s->flags &= ~(SF_CURR_SESS|SF_REDIRECTABLE|SF_SRV_REUSED);
        s->flags &= ~(SF_ERR_MASK|SF_FINST_MASK|SF_REDISP);

diff --git a/src/stream.c b/src/stream.c
index 8df5c75639d20c8b7787420e5f644d4a13fc9bae..ad1ccbee408b1a24c534b843a8a8acb6a05049a2 100644 (file)
--- a/src/stream.c
+++ b/src/stream.c
@@ -628,7 +628,7 @@ static int sess_update_st_con_tcp(struct stream *s)
        if (!(req->flags & CF_WROTE_DATA) &&
            unlikely((rep->flags & CF_SHUTW) ||
                     ((req->flags & CF_SHUTW_NOW) && /* FIXME: this should not prevent a connection from establishing */
-                     ((!(req->flags & (CF_WRITE_ACTIVITY|CF_WRITE_EVENT)) && channel_is_empty(req)) ||
+                     ((!(req->flags & CF_WRITE_ACTIVITY) && channel_is_empty(req)) ||
                       ((s->be->options & PR_O_ABRT_CLOSE) && !(s->si[0].flags & SI_FL_CLEAN_ABRT)))))) {
                /* give up */
                si_shutw(si);
@@ -639,7 +639,7 @@ static int sess_update_st_con_tcp(struct stream *s)
        }
 
        /* we need to wait a bit more if there was no activity either */
-       if (!(req->flags & (CF_WRITE_ACTIVITY|CF_WRITE_EVENT)))
+       if (!(req->flags & CF_WRITE_ACTIVITY))
                return 1;
 
        /* OK, this means that a connection succeeded. The caller will be
@@ -1759,7 +1759,7 @@ redo:
                 */
                if (!((req->flags | res->flags) &
                      (CF_SHUTR|CF_READ_ACTIVITY|CF_READ_TIMEOUT|CF_SHUTW|
-                      CF_WRITE_ACTIVITY|CF_WRITE_EVENT|CF_WRITE_TIMEOUT|CF_ANA_TIMEOUT)) &&
+                      CF_WRITE_ACTIVITY|CF_WRITE_TIMEOUT|CF_ANA_TIMEOUT)) &&
                    !((si_f->flags | si_b->flags) & (SI_FL_EXP|SI_FL_ERR)) &&
                    ((s->pending_events & TASK_WOKEN_ANY) == TASK_WOKEN_TIMER)) {
                        si_f->flags &= ~SI_FL_DONT_WAKE;
@@ -2477,8 +2477,8 @@ redo:
                if (si_b->state == SI_ST_EST)
                        si_update(si_b);
 
-               req->flags &= ~(CF_READ_NULL|CF_READ_PARTIAL|CF_WRITE_NULL|CF_WRITE_PARTIAL|CF_READ_ATTACHED|CF_WRITE_EVENT);
-               res->flags &= ~(CF_READ_NULL|CF_READ_PARTIAL|CF_WRITE_NULL|CF_WRITE_PARTIAL|CF_READ_ATTACHED|CF_WRITE_EVENT);
+               req->flags &= ~(CF_READ_NULL|CF_READ_PARTIAL|CF_WRITE_NULL|CF_WRITE_PARTIAL|CF_READ_ATTACHED);
+               res->flags &= ~(CF_READ_NULL|CF_READ_PARTIAL|CF_WRITE_NULL|CF_WRITE_PARTIAL|CF_READ_ATTACHED);
                si_f->prev_state = si_f->state;
                si_b->prev_state = si_b->state;
                si_f->flags &= ~(SI_FL_ERR|SI_FL_EXP);

diff --git a/src/stream_interface.c b/src/stream_interface.c
index 59827ecadc476ecbf2f8fd6347499a9632f484de..e585b1fe4f77e728214edc4768c7306da466b2e0 100644 (file)
--- a/src/stream_interface.c
+++ b/src/stream_interface.c
@@ -496,10 +496,6 @@ void stream_int_notify(struct stream_interface *si)
         * the buffer is full. We must not stop based on input data alone because
         * an HTTP parser might need more data to complete the parsing.
         */
-
-       /* ensure it's only set if a write attempt has succeeded */
-       ic->flags &= ~CF_WRITE_PARTIAL;
-
        if (!channel_is_empty(ic) &&
            (si_opposite(si)->flags & SI_FL_WAIT_DATA) &&
            (ci_data(ic) == 0 || ic->pipe)) {
@@ -544,7 +540,7 @@ void stream_int_notify(struct stream_interface *si)
 
            /* changes on the consumption side */
            (oc->flags & (CF_WRITE_NULL|CF_WRITE_ERROR)) ||
-           ((oc->flags & (CF_WRITE_ACTIVITY|CF_WRITE_EVENT)) &&
+           ((oc->flags & CF_WRITE_ACTIVITY) &&
             ((oc->flags & CF_SHUTW) ||
              ((oc->flags & CF_WAKE_WRITE) &&
               (si_opposite(si)->state != SI_ST_EST ||
@@ -641,13 +637,10 @@ int si_cs_send(struct conn_stream *cs)
        if (si_oc(si)->flags & CF_SHUTW)
                return 1;
 
-       /* ensure it's only set if a write attempt has succeeded */
-       oc->flags &= ~CF_WRITE_PARTIAL;
-
        if (oc->pipe && conn->xprt->snd_pipe && conn->mux->snd_pipe) {
                ret = conn->mux->snd_pipe(cs, oc->pipe);
                if (ret > 0) {
-                       oc->flags |= CF_WRITE_PARTIAL | CF_WROTE_DATA | CF_WRITE_EVENT;
+                       oc->flags |= CF_WRITE_PARTIAL | CF_WROTE_DATA;
                        did_send = 1;
                }
 
@@ -696,7 +689,7 @@ int si_cs_send(struct conn_stream *cs)
                ret = cs->conn->mux->snd_buf(cs, &oc->buf, co_data(oc), send_flag);
                if (ret > 0) {
                        did_send = 1;
-                       oc->flags |= CF_WRITE_PARTIAL | CF_WROTE_DATA | CF_WRITE_EVENT;
+                       oc->flags |= CF_WRITE_PARTIAL | CF_WROTE_DATA;
 
                        co_set_data(oc, co_data(oc) - ret);
                        c_realign_if_empty(oc);
@@ -965,9 +958,6 @@ static void stream_int_chk_snd_conn(struct stream_interface *si)
        struct channel *oc = si_oc(si);
        struct conn_stream *cs = __objt_cs(si->end);
 
-       /* ensure it's only set if a write attempt has succeeded */
-       oc->flags &= ~CF_WRITE_PARTIAL;
-
        if (unlikely(si->state > SI_ST_EST || (oc->flags & CF_SHUTW)))
                return;