Skip to content

Commit 82bfa5c

Browse files
authored
[!] fix PTO error when HANDSHAKE_DONE is not sent; (#412)
* [!] fix PTO error when HANDSHAKE_DONE is not sent; * [!] fix RETIRE_CID error when suffering severe loss * [~] validate cid after receiving a RC frame
1 parent 3607442 commit 82bfa5c

8 files changed

+67
-8
lines changed

src/transport/xqc_cid.c

+16
Original file line numberDiff line numberDiff line change
@@ -341,3 +341,19 @@ xqc_get_inner_cid_by_seq(xqc_cid_set_t *cid_set, uint64_t seq_num)
341341

342342
return NULL;
343343
}
344+
345+
xqc_bool_t
346+
xqc_validate_retire_cid_frame(xqc_cid_set_t *cid_set, xqc_cid_inner_t *cid)
347+
{
348+
/* maybe retired already */
349+
if (xqc_cid_in_cid_set(cid_set, &cid->cid) == NULL) {
350+
return XQC_FALSE;
351+
}
352+
353+
/* the cid is retired already */
354+
if (cid->state >= XQC_CID_RETIRED) {
355+
return XQC_FALSE;
356+
}
357+
358+
return XQC_TRUE;
359+
}

src/transport/xqc_cid.h

+1
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ xqc_cid_inner_t *xqc_cid_in_cid_set(const xqc_cid_set_t *cid_set, xqc_cid_t *cid
7171
xqc_int_t xqc_cid_switch_to_next_state(xqc_cid_set_t *cid_set, xqc_cid_inner_t *cid, xqc_cid_state_t state);
7272
xqc_int_t xqc_get_unused_cid(xqc_cid_set_t *cid_set, xqc_cid_t *cid);
7373

74+
xqc_bool_t xqc_validate_retire_cid_frame(xqc_cid_set_t *cid_set, xqc_cid_inner_t *cid);
7475

7576
unsigned char *xqc_sr_token_str(const char *sr_token);
7677

src/transport/xqc_conn.c

+9-7
Original file line numberDiff line numberDiff line change
@@ -1095,7 +1095,8 @@ xqc_conn_destroy(xqc_connection_t *xc)
10951095
"first_send_delay:%ui|conn_persist:%ui|keyupdate_cnt:%d|err:0x%xi|close_msg:%s|%s|"
10961096
"hsk_recv:%ui|close_recv:%ui|close_send:%ui|last_recv:%ui|last_send:%ui|"
10971097
"mp_enable:%ud|create:%ud|validated:%ud|active:%ud|path_info:%s|alpn:%*s|rebind_count:%d|"
1098-
"rebind_valid:%d|rtx_pkt:%ud|tlp_pkt:%ud|snd_pkt:%ud|spurious_loss:%ud|detected_loss:%ud|",
1098+
"rebind_valid:%d|rtx_pkt:%ud|tlp_pkt:%ud|snd_pkt:%ud|spurious_loss:%ud|detected_loss:%ud|"
1099+
"max_pto:%ud|finished_streams:%ud|cli_bidi_s:%ud|svr_bidi_s:%ud|",
10991100
xc,
11001101
xc->conn_flag & XQC_CONN_FLAG_HAS_0RTT ? 1:0,
11011102
xc->conn_flag & XQC_CONN_FLAG_0RTT_OK ? 1:0,
@@ -1112,7 +1113,8 @@ xqc_conn_destroy(xqc_connection_t *xc)
11121113
xc->enable_multipath, xc->create_path_count, xc->validated_path_count, xc->active_path_count,
11131114
conn_stats.conn_info, out_alpn_len, out_alpn, conn_stats.total_rebind_count,
11141115
conn_stats.total_rebind_valid, conn_stats.lost_count, conn_stats.tlp_count,
1115-
conn_stats.send_count, conn_stats.spurious_loss_count, xc->detected_loss_cnt);
1116+
conn_stats.send_count, conn_stats.spurious_loss_count, xc->detected_loss_cnt,
1117+
xc->max_pto_cnt, xc->finished_streams, xc->cli_bidi_streams, xc->svr_bidi_streams);
11161118
xqc_log_event(xc->log, CON_CONNECTION_CLOSED, xc);
11171119

11181120
if (xc->conn_flag & XQC_CONN_FLAG_WAIT_WAKEUP) {
@@ -2394,7 +2396,6 @@ xqc_path_send_one_or_two_ack_elicit_pkts(xqc_path_ctx_t *path,
23942396
xqc_list_head_t *sndq;
23952397
xqc_int_t probe_num;
23962398
xqc_bool_t send_hsd;
2397-
xqc_bool_t send_hsd_next;
23982399
int has_reinjection = 0;
23992400

24002401
c = path->parent_conn;
@@ -2404,17 +2405,18 @@ xqc_path_send_one_or_two_ack_elicit_pkts(xqc_path_ctx_t *path,
24042405
shall send HANDSHAKE_DONE on PTO as it has not been acknowledged. */
24052406
probe_num = XQC_CONN_PTO_PKT_CNT_MAX;
24062407
send_hsd = XQC_FALSE;
2407-
send_hsd_next = XQC_FALSE;
24082408

24092409
packet_out_last_sent = NULL;
24102410
packet_out_later_send = NULL;
24112411

24122412
xqc_log(c->log, XQC_LOG_DEBUG, "|send two ack-eliciting pkts"
24132413
"|path:%ui|pns:%d|", path->path_id, pns);
24142414

2415-
/* if server's HANDSHAKE_DONE frame has not been acked, try to send it */
2415+
/* if server's HANDSHAKE_DONE frame was sent and has not been acked, try to
2416+
send it */
24162417
if ((c->conn_type == XQC_CONN_TYPE_SERVER)
2417-
&& !(c->conn_flag & XQC_CONN_FLAG_HANDSHAKE_DONE_ACKED))
2418+
&& !(c->conn_flag & XQC_CONN_FLAG_HANDSHAKE_DONE_ACKED)
2419+
&& c->conn_flag & XQC_CONN_FLAG_HANDSHAKE_DONE_SENT)
24182420
{
24192421
send_hsd = XQC_TRUE;
24202422
}
@@ -4473,7 +4475,7 @@ xqc_conn_set_cid_retired_ts(xqc_connection_t *conn, xqc_cid_inner_t *inner_cid)
44734475

44744476
ret = xqc_cid_switch_to_next_state(&conn->scid_set.cid_set, inner_cid, XQC_CID_RETIRED);
44754477
if (ret != XQC_OK) {
4476-
xqc_log(conn->log, XQC_LOG_ERROR, "|set cid retired error|");
4478+
xqc_log(conn->log, XQC_LOG_ERROR, "|set cid retired error|ret:%d", ret);
44774479
return ret;
44784480
}
44794481

src/transport/xqc_conn.h

+8
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,7 @@ typedef enum {
134134
XQC_CONN_FLAG_MP_WAIT_SCID_SHIFT,
135135
XQC_CONN_FLAG_MP_WAIT_DCID_SHIFT,
136136
XQC_CONN_FLAG_MP_READY_NOTIFY_SHIFT,
137+
XQC_CONN_FLAG_HANDSHAKE_DONE_SENT_SHIFT,
137138
XQC_CONN_FLAG_SHIFT_NUM,
138139
} xqc_conn_flag_shift_t;
139140

@@ -179,6 +180,7 @@ typedef enum {
179180
XQC_CONN_FLAG_MP_WAIT_SCID = 1ULL << XQC_CONN_FLAG_MP_WAIT_SCID_SHIFT,
180181
XQC_CONN_FLAG_MP_WAIT_DCID = 1ULL << XQC_CONN_FLAG_MP_WAIT_DCID_SHIFT,
181182
XQC_CONN_FLAG_MP_READY_NOTIFY = 1ULL << XQC_CONN_FLAG_MP_READY_NOTIFY_SHIFT,
183+
XQC_CONN_FLAG_HANDSHAKE_DONE_SENT = 1ULL << XQC_CONN_FLAG_HANDSHAKE_DONE_SENT_SHIFT,
182184

183185
} xqc_conn_flag_t;
184186

@@ -420,6 +422,12 @@ struct xqc_connection_s {
420422
/* internal loss detection stats */
421423
uint32_t detected_loss_cnt;
422424

425+
/* max consecutive PTO cnt among all paths */
426+
uint16_t max_pto_cnt;
427+
uint32_t finished_streams;
428+
uint32_t cli_bidi_streams;
429+
uint32_t svr_bidi_streams;
430+
423431
/* receved pkts stats */
424432
struct {
425433
xqc_pkt_type_t pkt_types[3];

src/transport/xqc_frame.c

+7
Original file line numberDiff line numberDiff line change
@@ -877,6 +877,13 @@ xqc_process_retire_conn_id_frame(xqc_connection_t *conn, xqc_packet_in_t *packet
877877
return XQC_OK;
878878
}
879879

880+
/* skip if cid not available anymore */
881+
if (!xqc_validate_retire_cid_frame(&conn->scid_set.cid_set, inner_cid)) {
882+
xqc_log(conn->log, XQC_LOG_DEBUG, "|cid not valid any more|seq_num:%ui",
883+
seq_num);
884+
return XQC_OK;
885+
}
886+
880887
if (XQC_OK == xqc_cid_is_equal(&inner_cid->cid, &packet_in->pi_pkt.pkt_dcid)) {
881888
/*
882889
* The sequence number specified in a RETIRE_CONNECTION_ID frame MUST NOT refer to

src/transport/xqc_send_ctl.c

+5-1
Original file line numberDiff line numberDiff line change
@@ -623,7 +623,7 @@ xqc_send_ctl_on_packet_sent(xqc_send_ctl_t *send_ctl, xqc_pn_ctl_t *pn_ctl, xqc_
623623
xqc_conn_state_2_str(send_ctl->ctl_conn->conn_state),
624624
packet_out->po_flag & XQC_POF_IN_FLIGHT ? 1: 0,
625625
packet_out->po_flag & (XQC_POF_LOST | XQC_POF_TLP));
626-
626+
627627
if (packet_out->po_frame_types
628628
& (XQC_FRAME_BIT_DATA_BLOCKED
629629
| XQC_FRAME_BIT_STREAM_DATA_BLOCKED
@@ -744,6 +744,10 @@ xqc_send_ctl_on_packet_sent(xqc_send_ctl_t *send_ctl, xqc_pn_ctl_t *pn_ctl, xqc_
744744
}
745745
}
746746

747+
if (packet_out->po_frame_types & XQC_FRAME_BIT_HANDSHAKE_DONE) {
748+
send_ctl->ctl_conn->conn_flag |= XQC_CONN_FLAG_HANDSHAKE_DONE_SENT;
749+
}
750+
747751
send_ctl->ctl_conn->conn_last_send_time = now;
748752

749753
if (!send_ctl->ctl_recent_stats_timestamp) {

src/transport/xqc_stream.c

+20
Original file line numberDiff line numberDiff line change
@@ -517,6 +517,8 @@ xqc_stream_create(xqc_engine_t *engine, const xqc_cid_t *cid, xqc_stream_setting
517517
return NULL;
518518
}
519519

520+
conn->cli_bidi_streams++;
521+
520522
return stream;
521523
}
522524

@@ -674,6 +676,14 @@ xqc_destroy_stream(xqc_stream_t *stream)
674676
xqc_log(stream->stream_conn->log, XQC_LOG_DEBUG, "|send_state:%d|recv_state:%d|stream_id:%ui|stream_type:%d|",
675677
stream->stream_state_send, stream->stream_state_recv, stream->stream_id, stream->stream_type);
676678

679+
if ((stream->stream_state_send == XQC_SEND_STREAM_ST_DATA_RECVD)
680+
&& (stream->stream_state_recv == XQC_RECV_STREAM_ST_DATA_READ))
681+
{
682+
if(stream->stream_conn) {
683+
stream->stream_conn->finished_streams++;
684+
}
685+
}
686+
677687
if (stream->stream_if->stream_close_notify
678688
&& !(stream->stream_flag & XQC_STREAM_FLAG_DISCARDED))
679689
{
@@ -839,6 +849,16 @@ xqc_passive_create_stream(xqc_connection_t *conn, xqc_stream_id_t stream_id, voi
839849
return NULL;
840850
}
841851

852+
xqc_stream_type_t stream_type;
853+
stream_type = xqc_get_stream_type(stream_id);
854+
855+
if (stream_type == XQC_CLI_BID) {
856+
conn->cli_bidi_streams++;
857+
858+
} else if (stream_type == XQC_SVR_BID) {
859+
conn->svr_bidi_streams++;
860+
}
861+
842862
return stream;
843863
}
844864

src/transport/xqc_timer.c

+1
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ xqc_timer_loss_detection_timeout(xqc_timer_type_t type, xqc_usec_t now, void *us
103103
}
104104

105105
send_ctl->ctl_pto_count++;
106+
conn->max_pto_cnt = xqc_max(send_ctl->ctl_pto_count, conn->max_pto_cnt);
106107
xqc_log(conn->log, XQC_LOG_DEBUG, "|xqc_send_ctl_set_loss_detection_timer|PTO|conn:%p|pto_count:%ud",
107108
conn, send_ctl->ctl_pto_count);
108109
xqc_send_ctl_set_loss_detection_timer(send_ctl);

0 commit comments

Comments
 (0)