gamzinav/haproxy
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

MEDIUM: channel: merge back flags CF_WRITE_PARTIAL and CF_WRITE_EVENT

Browse Source 
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.
This commit is contained in:
Willy Tarreau 2018-10-24 17:17:56 +02:00
parent 1ff7633dd7
commit ede3d884fc
6 changed files with 13 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
contrib/debug/flags.c
View File

@ -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);

4
include/proto/channel.h
View File

@ -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.

4
include/types/channel.h
View File

@ -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)

2
src/proto_http.c
View File

@ -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);

10
src/stream.c
View File

@ -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);

16
src/stream_interface.c
View File

@ -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;