MINOR: quic: Add traces for qc_frm_free()

Useful to diagnose memory leak issues in relation with the QUIC frame objects.
This commit is contained in:
Frédéric Lécaille 2023-07-13 18:40:03 +02:00
parent 6d027c2edb
commit 0645e56a6e
5 changed files with 52 additions and 48 deletions

View File

@ -254,28 +254,9 @@ static inline struct quic_frame *qc_frm_dup(struct quic_frame *origin)
return frm; return frm;
} }
void qc_frm_free(struct quic_conn *qc, struct quic_frame **frm);
void qc_frm_unref(struct quic_frame *frm, struct quic_conn *qc); void qc_frm_unref(struct quic_frame *frm, struct quic_conn *qc);
/* Free a <frm> quic_frame. Remove it from parent element if still attached. */
static inline void qc_frm_free(struct quic_frame **frm)
{
/* Caller must ensure that no other frame points to <frm>. Use
* qc_frm_unref() to handle this properly.
*/
BUG_ON(!LIST_ISEMPTY(&((*frm)->reflist)));
BUG_ON(LIST_INLIST(&((*frm)->ref)));
/* TODO simplify frame deallocation. In some code paths, we must
* manually call this LIST_DEL_INIT before using
* quic_tx_packet_refdec() and freeing the frame.
*/
LIST_DEL_INIT(&((*frm)->list));
pool_free(pool_head_quic_frame, *frm);
*frm = NULL;
}
/* Move forward <strm> STREAM frame by <data> bytes. */ /* Move forward <strm> STREAM frame by <data> bytes. */
static inline void qc_stream_frm_mv_fwd(struct quic_frame *frm, uint64_t data) static inline void qc_stream_frm_mv_fwd(struct quic_frame *frm, uint64_t data)
{ {

View File

@ -481,7 +481,7 @@ static inline void quic_pktns_tx_pkts_release(struct quic_pktns *pktns, struct q
qc_frm_unref(frm, qc); qc_frm_unref(frm, qc);
LIST_DEL_INIT(&frm->list); LIST_DEL_INIT(&frm->list);
quic_tx_packet_refdec(frm->pkt); quic_tx_packet_refdec(frm->pkt);
qc_frm_free(&frm); qc_frm_free(qc, &frm);
} }
eb64_delete(&pkt->pn_node); eb64_delete(&pkt->pn_node);
quic_tx_packet_refdec(pkt); quic_tx_packet_refdec(pkt);

View File

@ -1820,7 +1820,7 @@ static int qcs_send_reset(struct qcs *qcs)
LIST_APPEND(&frms, &frm->list); LIST_APPEND(&frms, &frm->list);
if (qcc_send_frames(qcs->qcc, &frms)) { if (qcc_send_frames(qcs->qcc, &frms)) {
if (!LIST_ISEMPTY(&frms)) if (!LIST_ISEMPTY(&frms))
qc_frm_free(&frm); qc_frm_free(qcs->qcc->conn->handle.qc, &frm);
TRACE_DEVEL("cannot send RESET_STREAM", QMUX_EV_QCS_SEND, qcs->qcc->conn, qcs); TRACE_DEVEL("cannot send RESET_STREAM", QMUX_EV_QCS_SEND, qcs->qcc->conn, qcs);
return 1; return 1;
} }
@ -1871,7 +1871,7 @@ static int qcs_send_stop_sending(struct qcs *qcs)
LIST_APPEND(&frms, &frm->list); LIST_APPEND(&frms, &frm->list);
if (qcc_send_frames(qcs->qcc, &frms)) { if (qcc_send_frames(qcs->qcc, &frms)) {
if (!LIST_ISEMPTY(&frms)) if (!LIST_ISEMPTY(&frms))
qc_frm_free(&frm); qc_frm_free(qcc->conn->handle.qc, &frm);
TRACE_DEVEL("cannot send STOP_SENDING", QMUX_EV_QCS_SEND, qcs->qcc->conn, qcs); TRACE_DEVEL("cannot send STOP_SENDING", QMUX_EV_QCS_SEND, qcs->qcc->conn, qcs);
return 1; return 1;
} }
@ -2121,7 +2121,7 @@ static int qcc_io_send(struct qcc *qcc)
struct quic_frame *frm, *frm2; struct quic_frame *frm, *frm2;
list_for_each_entry_safe(frm, frm2, &frms, list) list_for_each_entry_safe(frm, frm2, &frms, list)
qc_frm_free(&frm); qc_frm_free(qcc->conn->handle.qc, &frm);
} }
/* Re-insert on-error QCS at the end of the send-list. */ /* Re-insert on-error QCS at the end of the send-list. */
@ -2390,7 +2390,7 @@ static void qcc_release(struct qcc *qcc)
while (!LIST_ISEMPTY(&qcc->lfctl.frms)) { while (!LIST_ISEMPTY(&qcc->lfctl.frms)) {
struct quic_frame *frm = LIST_ELEM(qcc->lfctl.frms.n, struct quic_frame *, list); struct quic_frame *frm = LIST_ELEM(qcc->lfctl.frms.n, struct quic_frame *, list);
qc_frm_free(&frm); qc_frm_free(qcc->conn->handle.qc, &frm);
} }
if (qcc->app_ops && qcc->app_ops->release) if (qcc->app_ops && qcc->app_ops->release)

View File

@ -1699,7 +1699,7 @@ void qc_release_frm(struct quic_conn *qc, struct quic_frame *frm)
TRACE_DEVEL("freeing unsent frame", TRACE_DEVEL("freeing unsent frame",
QUIC_EV_CONN_PRSAFRM, qc, f); QUIC_EV_CONN_PRSAFRM, qc, f);
LIST_DEL_INIT(&f->ref); LIST_DEL_INIT(&f->ref);
qc_frm_free(&f); qc_frm_free(qc, &f);
} }
} }
LIST_DEL_INIT(&frm->list); LIST_DEL_INIT(&frm->list);
@ -1707,7 +1707,7 @@ void qc_release_frm(struct quic_conn *qc, struct quic_frame *frm)
quic_tx_packet_refdec(frm->pkt); quic_tx_packet_refdec(frm->pkt);
TRACE_DEVEL("freeing frame from packet", TRACE_DEVEL("freeing frame from packet",
QUIC_EV_CONN_PRSAFRM, qc, frm, &pn); QUIC_EV_CONN_PRSAFRM, qc, frm, &pn);
qc_frm_free(&frm); qc_frm_free(qc, &frm);
TRACE_LEAVE(QUIC_EV_CONN_PRSAFRM, qc); TRACE_LEAVE(QUIC_EV_CONN_PRSAFRM, qc);
} }
@ -1935,7 +1935,7 @@ static inline int qc_requeue_nacked_pkt_tx_frms(struct quic_conn *qc,
TRACE_DEVEL("released stream", QUIC_EV_CONN_PRSAFRM, qc, frm); TRACE_DEVEL("released stream", QUIC_EV_CONN_PRSAFRM, qc, frm);
TRACE_DEVEL("freeing frame from packet", QUIC_EV_CONN_PRSAFRM, TRACE_DEVEL("freeing frame from packet", QUIC_EV_CONN_PRSAFRM,
qc, frm, &pn); qc, frm, &pn);
qc_frm_free(&frm); qc_frm_free(qc, &frm);
continue; continue;
} }
@ -1944,7 +1944,7 @@ static inline int qc_requeue_nacked_pkt_tx_frms(struct quic_conn *qc,
if (strm_frm->offset.key + strm_frm->len <= stream_desc->ack_offset) { if (strm_frm->offset.key + strm_frm->len <= stream_desc->ack_offset) {
TRACE_DEVEL("ignored frame in already acked range", TRACE_DEVEL("ignored frame in already acked range",
QUIC_EV_CONN_PRSAFRM, qc, frm); QUIC_EV_CONN_PRSAFRM, qc, frm);
qc_frm_free(&frm); qc_frm_free(qc, &frm);
continue; continue;
} }
else if (strm_frm->offset.key < stream_desc->ack_offset) { else if (strm_frm->offset.key < stream_desc->ack_offset) {
@ -1967,7 +1967,7 @@ static inline int qc_requeue_nacked_pkt_tx_frms(struct quic_conn *qc,
qc, frm, &pn); qc, frm, &pn);
if (frm->origin) if (frm->origin)
LIST_DEL_INIT(&frm->ref); LIST_DEL_INIT(&frm->ref);
qc_frm_free(&frm); qc_frm_free(qc, &frm);
continue; continue;
} }
@ -1975,7 +1975,7 @@ static inline int qc_requeue_nacked_pkt_tx_frms(struct quic_conn *qc,
TRACE_DEVEL("already acked frame", QUIC_EV_CONN_PRSAFRM, qc, frm); TRACE_DEVEL("already acked frame", QUIC_EV_CONN_PRSAFRM, qc, frm);
TRACE_DEVEL("freeing frame from packet", QUIC_EV_CONN_PRSAFRM, TRACE_DEVEL("freeing frame from packet", QUIC_EV_CONN_PRSAFRM,
qc, frm, &pn); qc, frm, &pn);
qc_frm_free(&frm); qc_frm_free(qc, &frm);
} }
else { else {
if (++frm->loss_count >= global.tune.quic_max_frame_loss) { if (++frm->loss_count >= global.tune.quic_max_frame_loss) {
@ -2010,7 +2010,7 @@ static inline void free_quic_tx_packet(struct quic_conn *qc,
goto leave; goto leave;
list_for_each_entry_safe(frm, frmbak, &pkt->frms, list) list_for_each_entry_safe(frm, frmbak, &pkt->frms, list)
qc_frm_free(&frm); qc_frm_free(qc, &frm);
pool_free(pool_head_quic_tx_packet, pkt); pool_free(pool_head_quic_tx_packet, pkt);
leave: leave:
@ -2126,7 +2126,7 @@ static inline void qc_release_pktns_frms(struct quic_conn *qc,
TRACE_ENTER(QUIC_EV_CONN_PHPKTS, qc); TRACE_ENTER(QUIC_EV_CONN_PHPKTS, qc);
list_for_each_entry_safe(frm, frmbak, &pktns->tx.frms, list) list_for_each_entry_safe(frm, frmbak, &pktns->tx.frms, list)
qc_frm_free(&frm); qc_frm_free(qc, &frm);
TRACE_LEAVE(QUIC_EV_CONN_PHPKTS, qc); TRACE_LEAVE(QUIC_EV_CONN_PHPKTS, qc);
} }
@ -2871,7 +2871,7 @@ static int qc_h3_request_reject(struct quic_conn *qc, uint64_t id)
rs = qc_frm_alloc(QUIC_FT_RESET_STREAM); rs = qc_frm_alloc(QUIC_FT_RESET_STREAM);
if (!rs) { if (!rs) {
TRACE_ERROR("failed to allocate quic_frame", QUIC_EV_CONN_PRSHPKT, qc); TRACE_ERROR("failed to allocate quic_frame", QUIC_EV_CONN_PRSHPKT, qc);
qc_frm_free(&ss); qc_frm_free(qc, &ss);
goto out; goto out;
} }
@ -3546,30 +3546,31 @@ static int qc_prep_app_pkts(struct quic_conn *qc, struct buffer *buf,
/* Free all frames in <l> list. In addition also remove all these frames /* Free all frames in <l> list. In addition also remove all these frames
* from the original ones if they are the results of duplications. * from the original ones if they are the results of duplications.
*/ */
static inline void qc_free_frm_list(struct list *l) static inline void qc_free_frm_list(struct quic_conn *qc, struct list *l)
{ {
struct quic_frame *frm, *frmbak; struct quic_frame *frm, *frmbak;
list_for_each_entry_safe(frm, frmbak, l, list) { list_for_each_entry_safe(frm, frmbak, l, list) {
LIST_DEL_INIT(&frm->ref); LIST_DEL_INIT(&frm->ref);
qc_frm_free(&frm); qc_frm_free(qc, &frm);
} }
} }
/* Free <pkt> TX packet and all the packets coalesced to it. */ /* Free <pkt> TX packet and all the packets coalesced to it. */
static inline void qc_free_tx_coalesced_pkts(struct quic_tx_packet *p) static inline void qc_free_tx_coalesced_pkts(struct quic_conn *qc,
struct quic_tx_packet *p)
{ {
struct quic_tx_packet *pkt, *nxt_pkt; struct quic_tx_packet *pkt, *nxt_pkt;
for (pkt = p; pkt; pkt = nxt_pkt) { for (pkt = p; pkt; pkt = nxt_pkt) {
qc_free_frm_list(&pkt->frms); qc_free_frm_list(qc, &pkt->frms);
nxt_pkt = pkt->next; nxt_pkt = pkt->next;
pool_free(pool_head_quic_tx_packet, pkt); pool_free(pool_head_quic_tx_packet, pkt);
} }
} }
/* Purge <buf> TX buffer from its prepare packets. */ /* Purge <buf> TX buffer from its prepare packets. */
static void qc_purge_tx_buf(struct buffer *buf) static void qc_purge_tx_buf(struct quic_conn *qc, struct buffer *buf)
{ {
while (b_contig_data(buf, 0)) { while (b_contig_data(buf, 0)) {
uint16_t dglen; uint16_t dglen;
@ -3578,7 +3579,7 @@ static void qc_purge_tx_buf(struct buffer *buf)
dglen = read_u16(b_head(buf)); dglen = read_u16(b_head(buf));
pkt = read_ptr(b_head(buf) + sizeof dglen); pkt = read_ptr(b_head(buf) + sizeof dglen);
qc_free_tx_coalesced_pkts(pkt); qc_free_tx_coalesced_pkts(qc, pkt);
b_del(buf, dglen + headlen); b_del(buf, dglen + headlen);
} }
@ -3638,9 +3639,9 @@ int qc_send_ppkts(struct buffer *buf, struct ssl_sock_ctx *ctx)
if (ret < 0) { if (ret < 0) {
TRACE_ERROR("sendto fatal error", QUIC_EV_CONN_SPPKTS, qc, first_pkt); TRACE_ERROR("sendto fatal error", QUIC_EV_CONN_SPPKTS, qc, first_pkt);
qc_kill_conn(qc); qc_kill_conn(qc);
qc_free_tx_coalesced_pkts(first_pkt); qc_free_tx_coalesced_pkts(qc, first_pkt);
b_del(buf, dglen + headlen); b_del(buf, dglen + headlen);
qc_purge_tx_buf(buf); qc_purge_tx_buf(qc, buf);
goto leave; goto leave;
} }
else if (!ret) { else if (!ret) {
@ -3983,7 +3984,7 @@ static int quic_build_post_handshake_frames(struct quic_conn *qc)
conn_id = new_quic_cid(&qc->cids, qc, NULL, NULL); conn_id = new_quic_cid(&qc->cids, qc, NULL, NULL);
if (!conn_id) { if (!conn_id) {
qc_frm_free(&frm); qc_frm_free(qc, &frm);
TRACE_ERROR("CID allocation error", QUIC_EV_CONN_IO_CB, qc); TRACE_ERROR("CID allocation error", QUIC_EV_CONN_IO_CB, qc);
goto err; goto err;
} }
@ -4008,7 +4009,7 @@ static int quic_build_post_handshake_frames(struct quic_conn *qc)
err: err:
/* free the frames */ /* free the frames */
list_for_each_entry_safe(frm, frmbak, &frm_list, list) list_for_each_entry_safe(frm, frmbak, &frm_list, list)
qc_frm_free(&frm); qc_frm_free(qc, &frm);
/* The first CID sequence number value used to allocated CIDs by this function is 1, /* The first CID sequence number value used to allocated CIDs by this function is 1,
* 0 being the sequence number of the CID for this connection. * 0 being the sequence number of the CID for this connection.
@ -5004,7 +5005,7 @@ static int qc_dgrams_retransmit(struct quic_conn *qc)
if (!LIST_ISEMPTY(&frms1)) { if (!LIST_ISEMPTY(&frms1)) {
apktns->tx.pto_probe = 1; apktns->tx.pto_probe = 1;
if (!qc_send_app_probing(qc, &frms1)) { if (!qc_send_app_probing(qc, &frms1)) {
qc_free_frm_list(&frms2); qc_free_frm_list(qc, &frms2);
goto leave; goto leave;
} }
@ -7575,7 +7576,7 @@ static inline int qc_build_frms(struct list *outlist, struct list *inlist,
node = eb64_lookup(&qc->streams_by_id, strm_frm->id); node = eb64_lookup(&qc->streams_by_id, strm_frm->id);
if (!node) { if (!node) {
TRACE_DEVEL("released stream", QUIC_EV_CONN_PRSAFRM, qc, cf); TRACE_DEVEL("released stream", QUIC_EV_CONN_PRSAFRM, qc, cf);
qc_frm_free(&cf); qc_frm_free(qc, &cf);
continue; continue;
} }
@ -7583,7 +7584,7 @@ static inline int qc_build_frms(struct list *outlist, struct list *inlist,
if (strm_frm->offset.key + strm_frm->len <= stream_desc->ack_offset) { if (strm_frm->offset.key + strm_frm->len <= stream_desc->ack_offset) {
TRACE_DEVEL("ignored frame frame in already acked range", TRACE_DEVEL("ignored frame frame in already acked range",
QUIC_EV_CONN_PRSAFRM, qc, cf); QUIC_EV_CONN_PRSAFRM, qc, cf);
qc_frm_free(&cf); qc_frm_free(qc, &cf);
continue; continue;
} }
else if (strm_frm->offset.key < stream_desc->ack_offset) { else if (strm_frm->offset.key < stream_desc->ack_offset) {

View File

@ -1182,7 +1182,7 @@ void qc_frm_unref(struct quic_frame *frm, struct quic_conn *qc)
{ {
struct quic_frame *f, *tmp; struct quic_frame *f, *tmp;
TRACE_ENTER(QUIC_EV_CONN_PRSAFRM, qc); TRACE_ENTER(QUIC_EV_CONN_PRSAFRM, qc, frm);
list_for_each_entry_safe(f, tmp, &frm->reflist, ref) { list_for_each_entry_safe(f, tmp, &frm->reflist, ref) {
f->origin = NULL; f->origin = NULL;
@ -1199,3 +1199,25 @@ void qc_frm_unref(struct quic_frame *frm, struct quic_conn *qc)
TRACE_LEAVE(QUIC_EV_CONN_PRSAFRM, qc); TRACE_LEAVE(QUIC_EV_CONN_PRSAFRM, qc);
} }
/* Free a <frm> quic_frame. Remove it from parent element if still attached. */
void qc_frm_free(struct quic_conn *qc, struct quic_frame **frm)
{
TRACE_ENTER(QUIC_EV_CONN_PRSAFRM, qc, *frm);
/* Caller must ensure that no other frame points to <frm>. Use
* qc_frm_unref() to handle this properly.
*/
BUG_ON(!LIST_ISEMPTY(&((*frm)->reflist)));
BUG_ON(LIST_INLIST(&((*frm)->ref)));
/* TODO simplify frame deallocation. In some code paths, we must
* manually call this LIST_DEL_INIT before using
* quic_tx_packet_refdec() and freeing the frame.
*/
LIST_DEL_INIT(&((*frm)->list));
pool_free(pool_head_quic_frame, *frm);
*frm = NULL;
TRACE_LEAVE(QUIC_EV_CONN_PRSAFRM, qc);
}