From c73925b6372d59c4133848af40a0b868b802683f Mon Sep 17 00:00:00 2001 From: Kazuho Oku Date: Sun, 14 Feb 2021 14:19:11 +0900 Subject: [PATCH] callback-based API to let the application buffer arbitrary amount of DATAGRAM frames / drop them at any given timing --- include/quicly.h | 21 +++++++++++++----- include/quicly/frame.h | 17 -------------- lib/quicly.c | 50 ++++++++++++++++++++++++++---------------- src/cli.c | 41 +++++++++++++++++++++------------- 4 files changed, 72 insertions(+), 57 deletions(-) diff --git a/include/quicly.h b/include/quicly.h index 98190645c..02977c953 100644 --- a/include/quicly.h +++ b/include/quicly.h @@ -127,6 +127,16 @@ typedef struct st_quicly_stream_scheduler_t { * called when stream is being open. Application is expected to create it's corresponding state and tie it to stream->data. */ QUICLY_CALLBACK_TYPE(int, stream_open, quicly_stream_t *stream); +/** + * Called when building a DATAGRAM frame to be sent. Quicly first calls `get_length` to obtain the size of the datagram frame to be + * sent, then calls `flatten` to serialize the payload. The return value of `flatten` is a boolean indicating if there are more + * DATAGRAME frames to be sent. When there is not enough space to serialize the payload, the `flatten` callback is called with `dst` + * set to NULL. In such case, the application must discard that payload. + */ +typedef struct st_quicly_send_datagram_frame_t { + size_t (*get_length)(struct st_quicly_send_datagram_frame_t *sender, quicly_conn_t *conn); + int (*flatten)(struct st_quicly_send_datagram_frame_t *sender, quicly_conn_t *conn, void *dst); +} quicly_send_datagram_frame_t; /** * */ @@ -418,8 +428,8 @@ struct st_quicly_conn_streamgroup_state_t { struct { \ uint64_t padding, ping, ack, reset_stream, stop_sending, crypto, new_token, stream, max_data, max_stream_data, \ max_streams_bidi, max_streams_uni, data_blocked, stream_data_blocked, streams_blocked, new_connection_id, \ - retire_connection_id, path_challenge, path_response, transport_close, application_close, handshake_done, \ - datagram, ack_frequency; \ + retire_connection_id, path_challenge, path_response, transport_close, application_close, handshake_done, datagram, \ + ack_frequency; \ } num_frames_sent, num_frames_received; \ /** \ * Total number of PTOs observed during the connection. \ @@ -1052,11 +1062,10 @@ static int quicly_stream_has_receive_side(int is_client, quicly_stream_id_t stre */ static int quicly_stream_is_self_initiated(quicly_stream_t *stream); /** - * Registers a datagram frame payload to be sent. When the applications calls `quicly_send` the first time after registering the - * datagram frame payload, the payload is either sent or the reference is discarded. Until then, it is the caller's responsibility - * to retain the memory pointed to by `payload`. At the moment, DATAFRAM frames are not congestion controlled. + * Registers a callback to send DATAGRAM frames. When registered, `quicly_get_first_timeout` returns 0, and `quicly_send` will call + * the registered callback to generate UDP datagram payload. */ -void quicly_set_datagram_frame(quicly_conn_t *conn, ptls_iovec_t payload); +void quicly_set_datagram_frame_sender(quicly_conn_t *conn, quicly_send_datagram_frame_t *sender); /** * */ diff --git a/include/quicly/frame.h b/include/quicly/frame.h index c0ffc4653..10bd1661d 100644 --- a/include/quicly/frame.h +++ b/include/quicly/frame.h @@ -256,9 +256,6 @@ typedef struct st_quicly_new_token_frame_t { static int quicly_decode_new_token_frame(const uint8_t **src, const uint8_t *end, quicly_new_token_frame_t *frame); -static size_t quicly_datagram_frame_capacity(ptls_iovec_t payload); -static uint8_t *quicly_encode_datagram_frame(uint8_t *dst, ptls_iovec_t payload); - typedef struct st_quicly_datagram_frame_t { ptls_iovec_t payload; } quicly_datagram_frame_t; @@ -754,20 +751,6 @@ inline int quicly_decode_new_token_frame(const uint8_t **src, const uint8_t *end return QUICLY_TRANSPORT_ERROR_FRAME_ENCODING; } -inline size_t quicly_datagram_frame_capacity(ptls_iovec_t payload) -{ - return quicly_encodev_capacity(QUICLY_FRAME_TYPE_DATAGRAM_WITHLEN) + quicly_encodev_capacity(payload.len) + payload.len; -} - -inline uint8_t *quicly_encode_datagram_frame(uint8_t *dst, ptls_iovec_t payload) -{ - dst = quicly_encodev(dst, QUICLY_FRAME_TYPE_DATAGRAM_WITHLEN); - dst = quicly_encodev(dst, payload.len); - memcpy(dst, payload.base, payload.len); - dst += payload.len; - return dst; -} - inline int quicly_decode_datagram_frame(uint64_t frame_type, const uint8_t **src, const uint8_t *end, quicly_datagram_frame_t *frame) { diff --git a/lib/quicly.c b/lib/quicly.c index c67fed035..f7beab2ce 100644 --- a/lib/quicly.c +++ b/lib/quicly.c @@ -337,9 +337,9 @@ struct st_quicly_conn_t { */ quicly_retire_cid_set_t retire_cid; /** - * DATAGRAM frame payload to be sent + * payload of DATAGRAM frames to be sent */ - ptls_iovec_t datagram_frame_payload; + quicly_send_datagram_frame_t *datagram_frame_sender; } egress; /** * crypto data @@ -2758,14 +2758,12 @@ static int on_ack_retire_connection_id(quicly_sentmap_t *map, const quicly_sent_ static int should_send_datagram_frame(quicly_conn_t *conn) { - if (conn->egress.datagram_frame_payload.base == NULL) + if (conn->egress.datagram_frame_sender == NULL) return 0; if (conn->application == NULL) return 0; if (conn->application->cipher.egress.key.aead == NULL) return 0; - if (conn->super.remote.transport_params.max_datagram_frame_size < conn->egress.datagram_frame_payload.len) - return 0; return 1; } @@ -4190,18 +4188,33 @@ static int do_send(quicly_conn_t *conn, quicly_send_context_t *s) /* DATAGRAM frame. Notes regarding current implementation: * * Not limited by CC, nor the bytes counted by CC. * * When given payload is too large and does not fit into a QUIC packet, a packet containing only PADDING frames is sent. - * This is because we do not have a way to retract the generation of a QUIC packet. - * * Does not notify the application that the frame was dropped internally. */ + * This is because we do not have a way to retract the generation of a QUIC packet. */ if (should_send_datagram_frame(conn)) { - size_t required_space = quicly_datagram_frame_capacity(conn->egress.datagram_frame_payload); - if ((ret = _do_allocate_frame(conn, s, required_space, 1)) != 0) - goto Exit; - if (s->dst_end - s->dst >= required_space) { - s->dst = quicly_encode_datagram_frame(s->dst, conn->egress.datagram_frame_payload); - QUICLY_PROBE(DATAGRAM_SEND, conn, conn->stash.now, conn->egress.datagram_frame_payload.base, - conn->egress.datagram_frame_payload.len); - conn->egress.datagram_frame_payload = ptls_iovec_init(NULL, 0); - ++conn->super.stats.num_frames_sent.datagram; + while (1) { + int has_more = 0; + size_t payload_len = conn->egress.datagram_frame_sender->get_length(conn->egress.datagram_frame_sender, conn); + if (payload_len != 0) { + size_t required_space = payload_len + 3; + if (required_space <= conn->super.remote.transport_params.max_datagram_frame_size) { + if ((ret = _do_allocate_frame(conn, s, required_space, 3)) != 0) + goto Exit; + if (s->dst_end - s->dst >= required_space) { + *s->dst++ = QUICLY_FRAME_TYPE_DATAGRAM_WITHLEN; + s->dst = quicly_encodev(s->dst, payload_len); + has_more = + conn->egress.datagram_frame_sender->flatten(conn->egress.datagram_frame_sender, conn, s->dst); + s->dst += payload_len; + QUICLY_PROBE(DATAGRAM_SEND, conn, conn->stash.now, s->dst - payload_len, payload_len); + ++conn->super.stats.num_frames_sent.datagram; + } else { + has_more = conn->egress.datagram_frame_sender->flatten(conn->egress.datagram_frame_sender, conn, NULL); + } + } + } + if (!has_more) { + conn->egress.datagram_frame_sender = NULL; + break; + } } } if (!ack_only) { @@ -4332,9 +4345,9 @@ static int do_send(quicly_conn_t *conn, quicly_send_context_t *s) return ret; } -void quicly_set_datagram_frame(quicly_conn_t *conn, ptls_iovec_t payload) +void quicly_set_datagram_frame_sender(quicly_conn_t *conn, quicly_send_datagram_frame_t *sender) { - conn->egress.datagram_frame_payload = payload; + conn->egress.datagram_frame_sender = sender; } int quicly_send(quicly_conn_t *conn, quicly_address_t *dest, quicly_address_t *src, struct iovec *datagrams, size_t *num_datagrams, @@ -4392,7 +4405,6 @@ int quicly_send(quicly_conn_t *conn, quicly_address_t *dest, quicly_address_t *s assert_consistency(conn, 1); Exit: - conn->egress.datagram_frame_payload = ptls_iovec_init(NULL, 0); if (s.num_datagrams != 0) { *dest = conn->super.remote.address; *src = conn->super.local.address; diff --git a/src/cli.c b/src/cli.c index a87ffc676..d098eb485 100644 --- a/src/cli.c +++ b/src/cli.c @@ -45,7 +45,6 @@ FILE *quicly_trace_fp = NULL; static unsigned verbosity = 0; static int suppress_output = 0, send_datagram_frame = 0; static int64_t enqueue_requests_at = 0, request_interval = 0; -static void *datagram_frame_payload_buf; static void hexdump(const char *title, const uint8_t *p, size_t l) { @@ -489,26 +488,38 @@ static int send_pending(int fd, quicly_conn_t *conn) if ((ret = quicly_send(conn, &dest, &src, packets, &num_packets, buf, sizeof(buf))) == 0 && num_packets != 0) send_packets(fd, &dest.sa, packets, num_packets); - if (datagram_frame_payload_buf != NULL) { - free(datagram_frame_payload_buf); - datagram_frame_payload_buf = NULL; - } - return ret; } -static void set_datagram_frame(quicly_conn_t *conn, ptls_iovec_t payload) +static size_t send_datagram_frame_get_length(quicly_send_datagram_frame_t *self, quicly_conn_t *conn); +static int send_datagram_frame_flatten(quicly_send_datagram_frame_t *self, quicly_conn_t *conn, void *dst); + +static struct { + quicly_send_datagram_frame_t sender; + char payload[1500]; + size_t len; +} datagram_frame_sender = {{send_datagram_frame_get_length, send_datagram_frame_flatten}}; + +size_t send_datagram_frame_get_length(quicly_send_datagram_frame_t *self, quicly_conn_t *conn) { - if (datagram_frame_payload_buf != NULL) - free(datagram_frame_payload_buf); + return datagram_frame_sender.len; +} - /* replace payload.base with an allocated buffer */ - datagram_frame_payload_buf = malloc(payload.len); - memcpy(datagram_frame_payload_buf, payload.base, payload.len); - payload.base = datagram_frame_payload_buf; +int send_datagram_frame_flatten(quicly_send_datagram_frame_t *self, quicly_conn_t *conn, void *dst) +{ + if (dst != NULL) + memcpy(dst, datagram_frame_sender.payload, datagram_frame_sender.len); + return 0; +} + +static void set_datagram_frame(quicly_conn_t *conn, ptls_iovec_t payload) +{ + if (payload.len > sizeof(datagram_frame_sender.payload)) + return; - /* set data to be sent. The buffer is being freed in `send_pending` after `quicly_send` is being called. */ - quicly_set_datagram_frame(conn, payload); + memcpy(datagram_frame_sender.payload, payload.base, payload.len); + datagram_frame_sender.len = payload.len; + quicly_set_datagram_frame_sender(conn, &datagram_frame_sender.sender); } static void on_receive_datagram_frame(quicly_receive_datagram_frame_t *self, quicly_conn_t *conn, ptls_iovec_t payload)