Skip to content

Commit

Permalink
Do not break making headers because of exceeding CWND (SQUASH ME)
Browse files Browse the repository at this point in the history
We should not break making headers because of exceeding CWND, since
there is a chance that some other frames (for example ack to settings
will be sent between sending headers.
  • Loading branch information
EvgeniiMekhanik committed Sep 14, 2023
1 parent 73f28c0 commit 4c7bf82
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 21 deletions.
48 changes: 29 additions & 19 deletions fw/http_frame.c
Original file line number Diff line number Diff line change
Expand Up @@ -785,7 +785,6 @@ tfw_h2_destroy_stream_send_queue(TfwH2Ctx *ctx)
assert_spin_locked(&((TfwConn *)conn)->sk->sk_lock.slock);

__tfw_h2_destroy_stream_send_queue(root);
ctx->cur_xmit_stream = NULL;
}

void
Expand Down Expand Up @@ -2408,7 +2407,6 @@ tfw_h2_insert_frame_header_and_send(TfwH2Ctx *ctx, TfwStream *stream,
case STREAM_FSM_RES_OK:
break;
case STREAM_FSM_RES_TERM_STREAM:
BUG_ON(ctx->cur_xmit_stream);
tfw_h2_stream_purge_send_queue(stream);
return 0;
case STREAM_FSM_RES_TERM_CONN:
Expand Down Expand Up @@ -2444,20 +2442,35 @@ tfw_h2_make_frames_for_stream(TfwH2Ctx *ctx, TfwStream *stream,
{
int r = 0;
TfwFrameType frame_type;
unsigned long tmp_cwnd_awail = ULONG_MAX;
T_FSM_INIT(stream->xmit.state, "HTTP/2 make frames");

#define ADJUST_AVAILABLE_CWND(cwnd, type) \
do { \
if (cwnd <= FRAME_HEADER_SIZE) { \
if (type == HTTP2_HEADERS \
|| type == HTTP2_CONTINUATION) \
ctx->cur_xmit_stream = stream; \
if (cwnd <= FRAME_HEADER_SIZE) \
T_FSM_EXIT(); \
} \
cwnd -= FRAME_HEADER_SIZE; \
frame_type = type; \
} while(0)

/*
* We can't break making headers because of exceeding available cwnd,
* since there is a chance that some other frames will be sent in
* between sending headers. This is prohibited by RFC and leads to
* connection closing by client. So we adjust cwnd in temporary variable
* and recalculate real cwnd when we start making DATA frames or when we
* finish making frames for current stream.
*/
#define ADJUST_TMP_AVAILABLE_CWND(cwnd, tmp_cwnd) \
do { \
if (tmp_cwnd != ULONG_MAX) { \
unsigned long delta = ULONG_MAX - tmp_cwnd; \
*cwnd_awail = (*cwnd_awail > delta ? \
*cwnd_awail - delta : 0); \
tmp_cwnd = ULONG_MAX; \
} \
} while(0)

T_FSM_START(stream->xmit.state) {

T_FSM_STATE(HTTP2_ENCODE_HEADERS) {
Expand Down Expand Up @@ -2501,7 +2514,7 @@ do { \
}

T_FSM_STATE(HTTP2_MAKE_HEADERS_FRAMES) {
ADJUST_AVAILABLE_CWND(*cwnd_awail, HTTP2_HEADERS);
ADJUST_AVAILABLE_CWND(tmp_cwnd_awail, HTTP2_HEADERS);
if (unlikely(ctx->hpack.enc_tbl.ack_sent)) {
r = tfw_hpack_enc_tbl_write_sz(&ctx->hpack.enc_tbl,
stream);
Expand All @@ -2514,7 +2527,7 @@ do { \
}

r = tfw_h2_insert_frame_header_and_send(ctx, stream, frame_type,
mss, cwnd_awail,
mss, &tmp_cwnd_awail,
&stream->xmit.h_len);
if (unlikely(r)) {
T_WARN("Failed to make headers frame %d", r);
Expand All @@ -2528,9 +2541,9 @@ do { \
}

T_FSM_STATE(HTTP2_MAKE_CONTINUATION_FRAMES) {
ADJUST_AVAILABLE_CWND(*cwnd_awail, HTTP2_CONTINUATION);
ADJUST_AVAILABLE_CWND(tmp_cwnd_awail, HTTP2_CONTINUATION);
r = tfw_h2_insert_frame_header_and_send(ctx, stream, frame_type,
mss, cwnd_awail,
mss, &tmp_cwnd_awail,
&stream->xmit.h_len);
if (unlikely(r)) {
T_WARN("Failed to make continuation frame %d", r);
Expand All @@ -2548,6 +2561,8 @@ do { \
stream->xmit.is_blocked = !stream->rem_wnd;
T_FSM_EXIT();
}

ADJUST_TMP_AVAILABLE_CWND(*cwnd_awail, tmp_cwnd_awail);
ADJUST_AVAILABLE_CWND(*cwnd_awail, HTTP2_DATA);
r = tfw_h2_insert_frame_header_and_send(ctx, stream, frame_type,
mss, cwnd_awail,
Expand All @@ -2568,6 +2583,7 @@ do { \
BUG_ON(stream->xmit.resp);

ss_skb_queue_purge(&stream->xmit.skb_head);
ADJUST_TMP_AVAILABLE_CWND(*cwnd_awail, tmp_cwnd_awail);

T_FSM_EXIT();
}
Expand All @@ -2578,6 +2594,7 @@ do { \

return r;

#undef ADJUST_TMP_AVAILABLE_CWND
#undef ADJUST_AVAILABLE_SIZE
}

Expand All @@ -2599,14 +2616,7 @@ tfw_h2_make_frames(TfwH2Ctx *ctx, unsigned long cwnd_awail, unsigned int mss,
while (tfw_h2_stream_sched_is_active(&sched->root)
&& cwnd_awail >= FRAME_HEADER_SIZE && ctx->rem_wnd && !r)
{
if (ctx->cur_xmit_stream) {
stream = ctx->cur_xmit_stream;
parent = stream->sched.parent;
ctx->cur_xmit_stream = NULL;
tfw_h2_stream_sched_remove(stream);
} else {
stream = tfw_h2_sched_stream_dequeue(&sched->root, &parent);
}
stream = tfw_h2_sched_stream_dequeue(&sched->root, &parent);
/*
* If root scheduler is active we always can find
* active stream.
Expand Down
2 changes: 0 additions & 2 deletions fw/http_frame.h
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,6 @@ typedef struct {
* @rem_wnd - remote peer current flow controlled window;
* @hpack - HPACK context, used in processing of
* HEADERS/CONTINUATION frames;
* @cur_xmit_stream - stream for which headers are currently being made;
* @__off - offset to reinitialize processing context;
* @skb_head - collected list of processed skbs containing HTTP/2
* frames;
Expand Down Expand Up @@ -196,7 +195,6 @@ typedef struct {
long int loc_wnd;
long int rem_wnd;
TfwHPack hpack;
TfwStream *cur_xmit_stream;
char __off[0];
struct sk_buff *skb_head;
TfwStream *cur_stream;
Expand Down

0 comments on commit 4c7bf82

Please sign in to comment.