From 7b3d38a633e4c5975c1b60afceb1e8fcb2111b76 Mon Sep 17 00:00:00 2001 From: Christopher Faulet Date: Fri, 5 May 2023 11:28:45 +0200 Subject: [PATCH] MEDIUM: tree-wide: Change sc API to specify required free space to progress sc_need_room() now takes the required free space to receive more data as parameter. All calls to this function are updated accordingly. For now, this value is set but not used. When we are waiting for a buffer, 0 is used. So we expect to be unblocked ASAP. However this must be reviewed because SC_FL_NEED_BUF is probably enough in this case and this flag is already set if the input buffer allocation fails. --- addons/promex/service-prometheus.c | 8 ++++---- include/haproxy/applet.h | 8 ++++---- include/haproxy/stconn.h | 7 ++++++- src/cache.c | 4 ++-- src/cli.c | 4 ++-- src/dns.c | 7 +++---- src/flt_spoe.c | 4 ++-- src/hlua.c | 12 ++++++------ src/http_client.c | 17 ++++++++--------- src/peers.c | 6 +++--- src/proxy.c | 5 +++-- src/resolvers.c | 4 +++- src/stats.c | 17 ++++++++++------- src/stconn.c | 8 ++++---- 14 files changed, 60 insertions(+), 51 deletions(-) diff --git a/addons/promex/service-prometheus.c b/addons/promex/service-prometheus.c index d03399c08..e02e8c0c7 100644 --- a/addons/promex/service-prometheus.c +++ b/addons/promex/service-prometheus.c @@ -1365,7 +1365,7 @@ static int promex_dump_metrics(struct appctx *appctx, struct stconn *sc, struct return 1; full: - sc_need_room(sc); + sc_need_room(sc, channel_htx_recv_max(sc_ic(appctx_sc(appctx)), htx) + 1); return 0; error: /* unrecoverable error */ @@ -1512,7 +1512,7 @@ static int promex_send_headers(struct appctx *appctx, struct stconn *sc, struct return 1; full: htx_reset(htx); - sc_need_room(sc); + sc_need_room(sc, 0); return 0; } @@ -1544,7 +1544,7 @@ static void promex_appctx_handle_io(struct appctx *appctx) /* Check if the input buffer is available. */ if (!b_size(&res->buf)) { - sc_need_room(sc); + sc_need_room(sc, 0); goto out; } @@ -1585,7 +1585,7 @@ static void promex_appctx_handle_io(struct appctx *appctx) */ if (htx_is_empty(res_htx)) { if (!htx_add_endof(res_htx, HTX_BLK_EOT)) { - sc_need_room(sc); + sc_need_room(sc, sizeof(struct htx_blk) + 1); goto out; } channel_add_input(res, 1); diff --git a/include/haproxy/applet.h b/include/haproxy/applet.h index b805440e6..62df5c833 100644 --- a/include/haproxy/applet.h +++ b/include/haproxy/applet.h @@ -178,7 +178,7 @@ static inline int applet_putchk(struct appctx *appctx, struct buffer *chunk) ret = ci_putchk(sc_ic(se->sc), chunk); if (ret == -1) - sc_need_room(se->sc); + sc_need_room(se->sc, chunk->data); return ret; } @@ -194,7 +194,7 @@ static inline int applet_putblk(struct appctx *appctx, const char *blk, int len) ret = ci_putblk(sc_ic(se->sc), blk, len); if (ret == -1) - sc_need_room(se->sc); + sc_need_room(se->sc, len); return ret; } @@ -211,7 +211,7 @@ static inline int applet_putstr(struct appctx *appctx, const char *str) ret = ci_putstr(sc_ic(se->sc), str); if (ret == -1) - sc_need_room(se->sc); + sc_need_room(se->sc, strlen(str)); return ret; } @@ -227,7 +227,7 @@ static inline int applet_putchr(struct appctx *appctx, char chr) ret = ci_putchr(sc_ic(se->sc), chr); if (ret == -1) - sc_need_room(se->sc); + sc_need_room(se->sc, 1); return ret; } diff --git a/include/haproxy/stconn.h b/include/haproxy/stconn.h index 5ee73d0f2..acc22977b 100644 --- a/include/haproxy/stconn.h +++ b/include/haproxy/stconn.h @@ -433,6 +433,7 @@ static inline void sc_have_room(struct stconn *sc) { if (sc->flags & SC_FL_NEED_ROOM) { sc->flags &= ~SC_FL_NEED_ROOM; + sc->room_needed = 0; sc_ep_report_read_activity(sc); } } @@ -441,10 +442,14 @@ static inline void sc_have_room(struct stconn *sc) * by lack of room. Since it indicates a willingness to deliver data to the * buffer that will have to be retried. Usually the caller will also clear * SE_FL_HAVE_NO_DATA to be called again as soon as SC_FL_NEED_ROOM is cleared. + * + * The caller is responsible to specified the amount of free space required to + * progress. */ -static inline void sc_need_room(struct stconn *sc) +static inline void sc_need_room(struct stconn *sc, ssize_t room_needed) { sc->flags |= SC_FL_NEED_ROOM; + sc->room_needed = room_needed; } /* The stream endpoint indicates that it's ready to consume data from the diff --git a/src/cache.c b/src/cache.c index 4deb34ea8..cebd17e3d 100644 --- a/src/cache.c +++ b/src/cache.c @@ -1471,7 +1471,7 @@ static void http_cache_io_handler(struct appctx *appctx) /* Check if the input buffer is available. */ if (!b_size(&res->buf)) { - sc_need_room(sc); + sc_need_room(sc, 0); goto out; } @@ -1513,7 +1513,7 @@ static void http_cache_io_handler(struct appctx *appctx) if (len) { ret = htx_cache_dump_msg(appctx, res_htx, len, HTX_BLK_UNUSED); if (ret < len) { - sc_need_room(sc); + sc_need_room(sc, len - ret); goto out; } } diff --git a/src/cli.c b/src/cli.c index c26840a46..977e22869 100644 --- a/src/cli.c +++ b/src/cli.c @@ -900,7 +900,7 @@ static void cli_io_handler(struct appctx *appctx) /* Check if the input buffer is available. */ if (!b_size(&res->buf)) { - sc_need_room(sc); + sc_need_room(sc, 0); goto out; } @@ -937,7 +937,7 @@ static void cli_io_handler(struct appctx *appctx) * would want to return some info right after parsing. */ if (buffer_almost_full(sc_ib(sc))) { - sc_need_room(sc); + sc_need_room(sc, b_size(&res->buf) / 2); break; } diff --git a/src/dns.c b/src/dns.c index e5ddee71f..23e9d9de5 100644 --- a/src/dns.c +++ b/src/dns.c @@ -525,7 +525,7 @@ static void dns_session_io_handler(struct appctx *appctx) /* check if there is enough room to put message len and query id */ if (available_room < sizeof(slen) + sizeof(new_qid)) { - sc_need_room(sc); + sc_need_room(sc, sizeof(slen) + sizeof(new_qid)); ret = 0; break; } @@ -583,7 +583,7 @@ static void dns_session_io_handler(struct appctx *appctx) /* check if it remains available room on output chan */ if (unlikely(!available_room)) { - sc_need_room(sc); + sc_need_room(sc, 1); ret = 0; break; } @@ -617,8 +617,7 @@ static void dns_session_io_handler(struct appctx *appctx) if (ds->tx_msg_offset) { /* msg was not fully processed, we must be awake to drain pending data */ - - sc_need_room(sc); + sc_need_room(sc, 0); ret = 0; break; } diff --git a/src/flt_spoe.c b/src/flt_spoe.c index ba35f01f9..6ab1cfb3e 100644 --- a/src/flt_spoe.c +++ b/src/flt_spoe.c @@ -1147,9 +1147,9 @@ spoe_send_frame(struct appctx *appctx, char *buf, size_t framesz) memcpy(buf, (char *)&netint, 4); ret = applet_putblk(appctx, buf, framesz+4); if (ret <= 0) { - if ((ret == -3 && b_is_null(&sc_ic(sc)->buf)) || ret == -1) { + if (ret == -3 && b_is_null(&sc_ic(sc)->buf)) { /* WT: is this still needed for the case ret==-3 ? */ - sc_need_room(sc); + sc_need_room(sc, 0); return 1; /* retry */ } SPOE_APPCTX(appctx)->status_code = SPOE_FRM_ERR_IO; diff --git a/src/hlua.c b/src/hlua.c index 8eb8e32df..973e27726 100644 --- a/src/hlua.c +++ b/src/hlua.c @@ -4929,7 +4929,7 @@ __LJMP static int hlua_applet_tcp_send_yield(lua_State *L, int status, lua_KCont * applet, and returns a yield. */ if (l < len) { - sc_need_room(sc); + sc_need_room(sc, channel_recv_max(chn) + 1); MAY_LJMP(hlua_yieldk(L, 0, 0, hlua_applet_tcp_send_yield, TICK_ETERNITY, 0)); } @@ -5472,7 +5472,7 @@ __LJMP static int hlua_applet_http_send_yield(lua_State *L, int status, lua_KCon if (l < len) { snd_yield: htx_to_buf(htx, &res->buf); - sc_need_room(sc); + sc_need_room(sc, channel_recv_max(res) + 1); MAY_LJMP(hlua_yieldk(L, 0, 0, hlua_applet_http_send_yield, TICK_ETERNITY, 0)); } @@ -5776,7 +5776,7 @@ __LJMP static int hlua_applet_http_start_response_yield(lua_State *L, int status struct channel *res = sc_ic(sc); if (co_data(res)) { - sc_need_room(sc); + sc_need_room(sc, -1); MAY_LJMP(hlua_yieldk(L, 0, 0, hlua_applet_http_start_response_yield, TICK_ETERNITY, 0)); } return MAY_LJMP(hlua_applet_http_send_response(L)); @@ -10390,7 +10390,7 @@ void hlua_applet_http_fct(struct appctx *ctx) /* Check if the input buffer is available. */ if (!b_size(&res->buf)) { - sc_need_room(sc); + sc_need_room(sc, 0); goto out; } @@ -10463,7 +10463,7 @@ void hlua_applet_http_fct(struct appctx *ctx) */ if (htx_is_empty(res_htx) && (strm->txn->rsp.flags & (HTTP_MSGF_XFER_LEN|HTTP_MSGF_CNT_LEN)) == HTTP_MSGF_XFER_LEN) { if (!htx_add_endof(res_htx, HTX_BLK_EOT)) { - sc_need_room(sc); + sc_need_room(sc, sizeof(struct htx_blk)+1); goto out; } channel_add_input(res, 1); @@ -11029,7 +11029,7 @@ static int hlua_cli_io_handler_fct(struct appctx *appctx) case HLUA_E_AGAIN: /* We want write. */ if (HLUA_IS_WAKERESWR(hlua)) - sc_need_room(sc); + sc_need_room(sc, -1); /* Set the timeout. */ if (hlua->wake_time != TICK_ETERNITY) task_schedule(hlua->task, hlua->wake_time); diff --git a/src/http_client.c b/src/http_client.c index c2dec8bbe..af30c8e58 100644 --- a/src/http_client.c +++ b/src/http_client.c @@ -726,15 +726,19 @@ static void httpclient_applet_io_handler(struct appctx *appctx) * it's the first call, we can freely copy the * request from the httpclient buffer */ ret = b_xfer(&req->buf, &hc->req.buf, b_data(&hc->req.buf)); - if (!ret) - goto full; + if (!ret) { + sc_need_room(sc, 0); + goto out; + } if (!b_data(&hc->req.buf)) b_free(&hc->req.buf); htx = htx_from_buf(&req->buf); - if (!htx) - goto full; + if (!htx) { + sc_need_room(sc, 0); + goto out; + } channel_add_input(req, htx->data); @@ -983,11 +987,6 @@ static void httpclient_applet_io_handler(struct appctx *appctx) sc_will_read(sc); goto out; -full: - /* There was not enough room in the response channel */ - sc_need_room(sc); - goto out; - error: se_fl_set(appctx->sedesc, SE_FL_ERROR); goto out; diff --git a/src/peers.c b/src/peers.c index 503a71fcc..1bbc1ca05 100644 --- a/src/peers.c +++ b/src/peers.c @@ -1623,7 +1623,7 @@ static inline int peer_send_teachmsgs(struct appctx *appctx, struct peer *p, /* pretend we're full so that we get back ASAP */ struct stconn *sc = appctx_sc(appctx); - sc_need_room(sc); + sc_need_room(sc, 0); ret = -1; break; } @@ -2652,7 +2652,7 @@ static inline int peer_send_msgs(struct appctx *appctx, /* pretend we're full so that we get back ASAP */ struct stconn *sc = appctx_sc(appctx); - sc_need_room(sc); + sc_need_room(sc, 0); return -1; } } @@ -2925,7 +2925,7 @@ static void peer_io_handler(struct appctx *appctx) /* Check if the input buffer is available. */ if (sc_ib(sc)->size == 0) { - sc_need_room(sc); + sc_need_room(sc, 0); goto out; } diff --git a/src/proxy.c b/src/proxy.c index 55e9f2037..2b79dea47 100644 --- a/src/proxy.c +++ b/src/proxy.c @@ -3386,8 +3386,10 @@ static int cli_io_handler_show_errors(struct appctx *appctx) newline = ctx->bol; newptr = dump_text_line(&trash, es->buf, global.tune.bufsize, es->buf_len, &newline, ctx->ptr); - if (newptr == ctx->ptr) + if (newptr == ctx->ptr) { + sc_need_room(sc, 0); goto cant_send_unlock; + } if (applet_putchk(appctx, &trash) == -1) goto cant_send_unlock; @@ -3410,7 +3412,6 @@ static int cli_io_handler_show_errors(struct appctx *appctx) cant_send_unlock: HA_RWLOCK_RDUNLOCK(PROXY_LOCK, &ctx->px->lock); cant_send: - sc_need_room(sc); return 0; } diff --git a/src/resolvers.c b/src/resolvers.c index 4a9209ebb..add9a25ab 100644 --- a/src/resolvers.c +++ b/src/resolvers.c @@ -2722,8 +2722,10 @@ int stats_dump_resolvers(struct stconn *sc, list_for_each_entry_from(ns, &resolver->nameservers, list) { ctx->obj2 = ns; - if (buffer_almost_full(&rep->buf)) + if (buffer_almost_full(&rep->buf)) { + sc_need_room(sc, b_size(&rep->buf) / 2); goto full; + } if (!stats_dump_resolv_to_buffer(sc, ns, stats, stats_count, diff --git a/src/stats.c b/src/stats.c index 56f62d884..0f2e027c1 100644 --- a/src/stats.c +++ b/src/stats.c @@ -3853,12 +3853,16 @@ static int stats_dump_proxies(struct stconn *sc, /* dump proxies */ while (ctx->obj1) { if (htx) { - if (htx_almost_full(htx)) + if (htx_almost_full(htx)) { + sc_need_room(sc, htx->size / 2); goto full; + } } else { - if (buffer_almost_full(&rep->buf)) + if (buffer_almost_full(&rep->buf)) { + sc_need_room(sc, b_size(&rep->buf) / 2); goto full; + } } px = ctx->obj1; @@ -3880,7 +3884,6 @@ static int stats_dump_proxies(struct stconn *sc, return 1; full: - sc_need_room(sc); return 0; } @@ -4378,7 +4381,7 @@ static int stats_send_http_headers(struct stconn *sc, struct htx *htx) full: htx_reset(htx); - sc_need_room(sc); + sc_need_room(sc, 0); return 0; } @@ -4438,7 +4441,7 @@ static int stats_send_http_redirect(struct stconn *sc, struct htx *htx) full: htx_reset(htx); - sc_need_room(sc); + sc_need_room(sc, 0); return 0; } @@ -4469,7 +4472,7 @@ static void http_stats_io_handler(struct appctx *appctx) /* Check if the input buffer is available. */ if (!b_size(&res->buf)) { - sc_need_room(sc); + sc_need_room(sc, 0); goto out; } @@ -4513,7 +4516,7 @@ static void http_stats_io_handler(struct appctx *appctx) */ if (htx_is_empty(res_htx)) { if (!htx_add_endof(res_htx, HTX_BLK_EOT)) { - sc_need_room(sc); + sc_need_room(sc, sizeof(struct htx_blk) + 1); goto out; } channel_add_input(res, 1); diff --git a/src/stconn.c b/src/stconn.c index cb3d750a8..615a97cc6 100644 --- a/src/stconn.c +++ b/src/stconn.c @@ -609,7 +609,7 @@ static void sc_app_chk_rcv(struct stconn *sc) if (ic->pipe) { /* stop reading */ - sc_need_room(sc); + sc_need_room(sc, -1); } else { /* (re)start reading */ @@ -1269,7 +1269,7 @@ static int sc_conn_recv(struct stconn *sc) /* the pipe is full or we have read enough data that it * could soon be full. Let's stop before needing to poll. */ - sc_need_room(sc); + sc_need_room(sc, 0); goto done_recv; } @@ -1339,7 +1339,7 @@ static int sc_conn_recv(struct stconn *sc) */ BUG_ON(c_empty(ic)); - sc_need_room(sc); + sc_need_room(sc, channel_recv_max(ic) + 1); /* Add READ_PARTIAL because some data are pending but * cannot be xferred to the channel */ @@ -1353,7 +1353,7 @@ static int sc_conn_recv(struct stconn *sc) * here to proceed. */ if (flags & CO_RFL_BUF_FLUSH) - sc_need_room(sc); + sc_need_room(sc, -1); break; }