Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[!] fix PTO error when HANDSHAKE_DONE is not sent; #412

Merged
merged 3 commits into from
Apr 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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