Skip to content

Commit

Permalink
[!] fix PTO error when HANDSHAKE_DONE is not sent; (#412)
Browse files Browse the repository at this point in the history
* [!] fix PTO error when HANDSHAKE_DONE is not sent;

* [!] fix RETIRE_CID error when suffering severe loss

* [~] validate cid after receiving a RC frame
  • Loading branch information
Kulsk authored Apr 24, 2024
1 parent 3607442 commit 82bfa5c
Show file tree
Hide file tree
Showing 8 changed files with 67 additions and 8 deletions.
16 changes: 16 additions & 0 deletions src/transport/xqc_cid.c
Original file line number Diff line number Diff line change
Expand Up @@ -341,3 +341,19 @@ xqc_get_inner_cid_by_seq(xqc_cid_set_t *cid_set, uint64_t seq_num)

return NULL;
}

xqc_bool_t
xqc_validate_retire_cid_frame(xqc_cid_set_t *cid_set, xqc_cid_inner_t *cid)
{
/* maybe retired already */
if (xqc_cid_in_cid_set(cid_set, &cid->cid) == NULL) {
return XQC_FALSE;
}

/* the cid is retired already */
if (cid->state >= XQC_CID_RETIRED) {
return XQC_FALSE;
}

return XQC_TRUE;
}
1 change: 1 addition & 0 deletions src/transport/xqc_cid.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ xqc_cid_inner_t *xqc_cid_in_cid_set(const xqc_cid_set_t *cid_set, xqc_cid_t *cid
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);
xqc_int_t xqc_get_unused_cid(xqc_cid_set_t *cid_set, xqc_cid_t *cid);

xqc_bool_t xqc_validate_retire_cid_frame(xqc_cid_set_t *cid_set, xqc_cid_inner_t *cid);

unsigned char *xqc_sr_token_str(const char *sr_token);

Expand Down
16 changes: 9 additions & 7 deletions src/transport/xqc_conn.c
Original file line number Diff line number Diff line change
Expand Up @@ -1095,7 +1095,8 @@ xqc_conn_destroy(xqc_connection_t *xc)
"first_send_delay:%ui|conn_persist:%ui|keyupdate_cnt:%d|err:0x%xi|close_msg:%s|%s|"
"hsk_recv:%ui|close_recv:%ui|close_send:%ui|last_recv:%ui|last_send:%ui|"
"mp_enable:%ud|create:%ud|validated:%ud|active:%ud|path_info:%s|alpn:%*s|rebind_count:%d|"
"rebind_valid:%d|rtx_pkt:%ud|tlp_pkt:%ud|snd_pkt:%ud|spurious_loss:%ud|detected_loss:%ud|",
"rebind_valid:%d|rtx_pkt:%ud|tlp_pkt:%ud|snd_pkt:%ud|spurious_loss:%ud|detected_loss:%ud|"
"max_pto:%ud|finished_streams:%ud|cli_bidi_s:%ud|svr_bidi_s:%ud|",
xc,
xc->conn_flag & XQC_CONN_FLAG_HAS_0RTT ? 1:0,
xc->conn_flag & XQC_CONN_FLAG_0RTT_OK ? 1:0,
Expand All @@ -1112,7 +1113,8 @@ xqc_conn_destroy(xqc_connection_t *xc)
xc->enable_multipath, xc->create_path_count, xc->validated_path_count, xc->active_path_count,
conn_stats.conn_info, out_alpn_len, out_alpn, conn_stats.total_rebind_count,
conn_stats.total_rebind_valid, conn_stats.lost_count, conn_stats.tlp_count,
conn_stats.send_count, conn_stats.spurious_loss_count, xc->detected_loss_cnt);
conn_stats.send_count, conn_stats.spurious_loss_count, xc->detected_loss_cnt,
xc->max_pto_cnt, xc->finished_streams, xc->cli_bidi_streams, xc->svr_bidi_streams);
xqc_log_event(xc->log, CON_CONNECTION_CLOSED, xc);

if (xc->conn_flag & XQC_CONN_FLAG_WAIT_WAKEUP) {
Expand Down Expand Up @@ -2394,7 +2396,6 @@ xqc_path_send_one_or_two_ack_elicit_pkts(xqc_path_ctx_t *path,
xqc_list_head_t *sndq;
xqc_int_t probe_num;
xqc_bool_t send_hsd;
xqc_bool_t send_hsd_next;
int has_reinjection = 0;

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

packet_out_last_sent = NULL;
packet_out_later_send = NULL;

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

/* if server's HANDSHAKE_DONE frame has not been acked, try to send it */
/* if server's HANDSHAKE_DONE frame was sent and has not been acked, try to
send it */
if ((c->conn_type == XQC_CONN_TYPE_SERVER)
&& !(c->conn_flag & XQC_CONN_FLAG_HANDSHAKE_DONE_ACKED))
&& !(c->conn_flag & XQC_CONN_FLAG_HANDSHAKE_DONE_ACKED)
&& c->conn_flag & XQC_CONN_FLAG_HANDSHAKE_DONE_SENT)
{
send_hsd = XQC_TRUE;
}
Expand Down Expand Up @@ -4473,7 +4475,7 @@ xqc_conn_set_cid_retired_ts(xqc_connection_t *conn, xqc_cid_inner_t *inner_cid)

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

Expand Down
8 changes: 8 additions & 0 deletions src/transport/xqc_conn.h
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ typedef enum {
XQC_CONN_FLAG_MP_WAIT_SCID_SHIFT,
XQC_CONN_FLAG_MP_WAIT_DCID_SHIFT,
XQC_CONN_FLAG_MP_READY_NOTIFY_SHIFT,
XQC_CONN_FLAG_HANDSHAKE_DONE_SENT_SHIFT,
XQC_CONN_FLAG_SHIFT_NUM,
} xqc_conn_flag_shift_t;

Expand Down Expand Up @@ -179,6 +180,7 @@ typedef enum {
XQC_CONN_FLAG_MP_WAIT_SCID = 1ULL << XQC_CONN_FLAG_MP_WAIT_SCID_SHIFT,
XQC_CONN_FLAG_MP_WAIT_DCID = 1ULL << XQC_CONN_FLAG_MP_WAIT_DCID_SHIFT,
XQC_CONN_FLAG_MP_READY_NOTIFY = 1ULL << XQC_CONN_FLAG_MP_READY_NOTIFY_SHIFT,
XQC_CONN_FLAG_HANDSHAKE_DONE_SENT = 1ULL << XQC_CONN_FLAG_HANDSHAKE_DONE_SENT_SHIFT,

} xqc_conn_flag_t;

Expand Down Expand Up @@ -420,6 +422,12 @@ struct xqc_connection_s {
/* internal loss detection stats */
uint32_t detected_loss_cnt;

/* max consecutive PTO cnt among all paths */
uint16_t max_pto_cnt;
uint32_t finished_streams;
uint32_t cli_bidi_streams;
uint32_t svr_bidi_streams;

/* receved pkts stats */
struct {
xqc_pkt_type_t pkt_types[3];
Expand Down
7 changes: 7 additions & 0 deletions src/transport/xqc_frame.c
Original file line number Diff line number Diff line change
Expand Up @@ -877,6 +877,13 @@ xqc_process_retire_conn_id_frame(xqc_connection_t *conn, xqc_packet_in_t *packet
return XQC_OK;
}

/* skip if cid not available anymore */
if (!xqc_validate_retire_cid_frame(&conn->scid_set.cid_set, inner_cid)) {
xqc_log(conn->log, XQC_LOG_DEBUG, "|cid not valid any more|seq_num:%ui",
seq_num);
return XQC_OK;
}

if (XQC_OK == xqc_cid_is_equal(&inner_cid->cid, &packet_in->pi_pkt.pkt_dcid)) {
/*
* The sequence number specified in a RETIRE_CONNECTION_ID frame MUST NOT refer to
Expand Down
6 changes: 5 additions & 1 deletion src/transport/xqc_send_ctl.c
Original file line number Diff line number Diff line change
Expand Up @@ -623,7 +623,7 @@ xqc_send_ctl_on_packet_sent(xqc_send_ctl_t *send_ctl, xqc_pn_ctl_t *pn_ctl, xqc_
xqc_conn_state_2_str(send_ctl->ctl_conn->conn_state),
packet_out->po_flag & XQC_POF_IN_FLIGHT ? 1: 0,
packet_out->po_flag & (XQC_POF_LOST | XQC_POF_TLP));

if (packet_out->po_frame_types
& (XQC_FRAME_BIT_DATA_BLOCKED
| XQC_FRAME_BIT_STREAM_DATA_BLOCKED
Expand Down Expand Up @@ -744,6 +744,10 @@ xqc_send_ctl_on_packet_sent(xqc_send_ctl_t *send_ctl, xqc_pn_ctl_t *pn_ctl, xqc_
}
}

if (packet_out->po_frame_types & XQC_FRAME_BIT_HANDSHAKE_DONE) {
send_ctl->ctl_conn->conn_flag |= XQC_CONN_FLAG_HANDSHAKE_DONE_SENT;
}

send_ctl->ctl_conn->conn_last_send_time = now;

if (!send_ctl->ctl_recent_stats_timestamp) {
Expand Down
20 changes: 20 additions & 0 deletions src/transport/xqc_stream.c
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,8 @@ xqc_stream_create(xqc_engine_t *engine, const xqc_cid_t *cid, xqc_stream_setting
return NULL;
}

conn->cli_bidi_streams++;

return stream;
}

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

if ((stream->stream_state_send == XQC_SEND_STREAM_ST_DATA_RECVD)
&& (stream->stream_state_recv == XQC_RECV_STREAM_ST_DATA_READ))
{
if(stream->stream_conn) {
stream->stream_conn->finished_streams++;
}
}

if (stream->stream_if->stream_close_notify
&& !(stream->stream_flag & XQC_STREAM_FLAG_DISCARDED))
{
Expand Down Expand Up @@ -839,6 +849,16 @@ xqc_passive_create_stream(xqc_connection_t *conn, xqc_stream_id_t stream_id, voi
return NULL;
}

xqc_stream_type_t stream_type;
stream_type = xqc_get_stream_type(stream_id);

if (stream_type == XQC_CLI_BID) {
conn->cli_bidi_streams++;

} else if (stream_type == XQC_SVR_BID) {
conn->svr_bidi_streams++;
}

return stream;
}

Expand Down
1 change: 1 addition & 0 deletions src/transport/xqc_timer.c
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ xqc_timer_loss_detection_timeout(xqc_timer_type_t type, xqc_usec_t now, void *us
}

send_ctl->ctl_pto_count++;
conn->max_pto_cnt = xqc_max(send_ctl->ctl_pto_count, conn->max_pto_cnt);
xqc_log(conn->log, XQC_LOG_DEBUG, "|xqc_send_ctl_set_loss_detection_timer|PTO|conn:%p|pto_count:%ud",
conn, send_ctl->ctl_pto_count);
xqc_send_ctl_set_loss_detection_timer(send_ctl);
Expand Down

0 comments on commit 82bfa5c

Please sign in to comment.