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

opportunistically reduce the number of ack ranges added to sentmap #487

Merged
merged 4 commits into from
Sep 1, 2021
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion include/quicly/sentmap.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,12 +91,30 @@ typedef enum en_quicly_sentmap_event_t {
*/
typedef int (*quicly_sent_acked_cb)(quicly_sentmap_t *map, const quicly_sent_packet_t *packet, int acked, quicly_sent_t *data);

struct st_quicly_sent_ack_additional_t {
uint8_t gap;
uint8_t length;
};

struct st_quicly_sent_t {
quicly_sent_acked_cb acked;
union {
quicly_sent_packet_t packet;
/**
* ACK frame. Represents up to 8 ack ranges. If not full, `additional` list is terminated by .gap = 0.
*/
struct {
quicly_range_t range;
uint64_t start;
union {
struct {
uint64_t start_length;
struct st_quicly_sent_ack_additional_t additional[4];
} ranges64;
struct {
uint8_t start_length;
struct st_quicly_sent_ack_additional_t additional[7];
} ranges8;
};
} ack;
struct {
quicly_stream_id_t stream_id;
Expand Down
124 changes: 89 additions & 35 deletions lib/quicly.c
Original file line number Diff line number Diff line change
Expand Up @@ -2530,45 +2530,72 @@ static int decrypt_packet(ptls_cipher_context_t *header_protection,
return 0;
}

static int on_ack_ack(quicly_sentmap_t *map, const quicly_sent_packet_t *packet, int acked, quicly_sent_t *sent)
static int do_on_ack_ack(quicly_conn_t *conn, const quicly_sent_packet_t *packet, uint64_t start, uint64_t start_length,
struct st_quicly_sent_ack_additional_t *additional, size_t additional_capacity)
{
quicly_conn_t *conn = (quicly_conn_t *)((char *)map - offsetof(quicly_conn_t, egress.loss.sentmap));

/* TODO log */
/* find the pn space */
struct st_quicly_pn_space_t *space;
switch (packet->ack_epoch) {
case QUICLY_EPOCH_INITIAL:
space = &conn->initial->super;
break;
case QUICLY_EPOCH_HANDSHAKE:
space = &conn->handshake->super;
break;
case QUICLY_EPOCH_1RTT:
space = &conn->application->super;
break;
default:
assert(!"FIXME");
return QUICLY_TRANSPORT_ERROR_INTERNAL;
}

if (acked) {
/* find the pn space */
struct st_quicly_pn_space_t *space;
switch (packet->ack_epoch) {
case QUICLY_EPOCH_INITIAL:
space = &conn->initial->super;
break;
case QUICLY_EPOCH_HANDSHAKE:
space = &conn->handshake->super;
break;
case QUICLY_EPOCH_1RTT:
space = &conn->application->super;
break;
default:
assert(!"FIXME");
return QUICLY_TRANSPORT_ERROR_INTERNAL;
}
/* subtract given ACK range, then make adjustments */
int ret;
if ((ret = quicly_ranges_subtract(&space->ack_queue, sent->data.ack.range.start, sent->data.ack.range.end)) != 0)
/* subtract given ACK ranges */
int ret;
uint64_t end = start + start_length;
if ((ret = quicly_ranges_subtract(&space->ack_queue, start, end)) != 0)
return ret;
for (size_t i = 0; i < additional_capacity && additional[i].gap != 0; ++i) {
start = end + additional[i].gap;
end = start + additional[i].length;
if ((ret = quicly_ranges_subtract(&space->ack_queue, start, end)) != 0)
return ret;
if (space->ack_queue.num_ranges == 0) {
space->largest_pn_received_at = INT64_MAX;
space->unacked_count = 0;
} else if (space->ack_queue.num_ranges > QUICLY_MAX_ACK_BLOCKS) {
quicly_ranges_drop_by_range_indices(&space->ack_queue, space->ack_queue.num_ranges - QUICLY_MAX_ACK_BLOCKS,
space->ack_queue.num_ranges);
}
}

/* make adjustments */
if (space->ack_queue.num_ranges == 0) {
space->largest_pn_received_at = INT64_MAX;
space->unacked_count = 0;
} else if (space->ack_queue.num_ranges > QUICLY_MAX_ACK_BLOCKS) {
quicly_ranges_drop_by_range_indices(&space->ack_queue, space->ack_queue.num_ranges - QUICLY_MAX_ACK_BLOCKS,
space->ack_queue.num_ranges);
}

return 0;
}

static int on_ack_ack_ranges64(quicly_sentmap_t *map, const quicly_sent_packet_t *packet, int acked, quicly_sent_t *sent)
{
quicly_conn_t *conn = (quicly_conn_t *)((char *)map - offsetof(quicly_conn_t, egress.loss.sentmap));

/* TODO log */

return acked ? do_on_ack_ack(conn, packet, sent->data.ack.start, sent->data.ack.ranges64.start_length,
sent->data.ack.ranges64.additional, PTLS_ELEMENTSOF(sent->data.ack.ranges64.additional))
: 0;
}

static int on_ack_ack_ranges8(quicly_sentmap_t *map, const quicly_sent_packet_t *packet, int acked, quicly_sent_t *sent)
{
quicly_conn_t *conn = (quicly_conn_t *)((char *)map - offsetof(quicly_conn_t, egress.loss.sentmap));

/* TODO log */

return acked ? do_on_ack_ack(conn, packet, sent->data.ack.start, sent->data.ack.ranges8.start_length,
sent->data.ack.ranges8.additional, PTLS_ELEMENTSOF(sent->data.ack.ranges8.additional))
: 0;
}

static int on_ack_stream_ack_one(quicly_conn_t *conn, quicly_stream_id_t stream_id, quicly_sendstate_sent_t *sent)
{
quicly_stream_t *stream;
Expand Down Expand Up @@ -3312,12 +3339,39 @@ static int send_ack(quicly_conn_t *conn, struct st_quicly_pn_space_t *space, qui
s->dst = dst;

{ /* save what's inflight */
size_t i;
for (i = 0; i != space->ack_queue.num_ranges; ++i) {
size_t range_index = 0;
while (range_index < space->ack_queue.num_ranges) {
quicly_sent_t *sent;
if ((sent = quicly_sentmap_allocate(&conn->egress.loss.sentmap, on_ack_ack)) == NULL)
struct st_quicly_sent_ack_additional_t *additional, *additional_end;
/* allocate */
if ((sent = quicly_sentmap_allocate(&conn->egress.loss.sentmap, on_ack_ack_ranges8)) == NULL)
return PTLS_ERROR_NO_MEMORY;
sent->data.ack.range = space->ack_queue.ranges[i];
/* store the first range, as well as preparing references to the additional slots */
sent->data.ack.start = space->ack_queue.ranges[range_index].start;
uint64_t length = space->ack_queue.ranges[range_index].end - space->ack_queue.ranges[range_index].start;
if (length <= UINT8_MAX) {
sent->data.ack.ranges8.start_length = length;
additional = sent->data.ack.ranges8.additional;
additional_end = additional + PTLS_ELEMENTSOF(sent->data.ack.ranges8.additional);
} else {
sent->acked = on_ack_ack_ranges64;
sent->data.ack.ranges64.start_length = length;
additional = sent->data.ack.ranges64.additional;
additional_end = additional + PTLS_ELEMENTSOF(sent->data.ack.ranges64.additional);
}
/* store additional ranges, if possible */
for (++range_index; range_index < space->ack_queue.num_ranges && additional < additional_end;
++range_index, ++additional) {
uint64_t gap = space->ack_queue.ranges[range_index].start - space->ack_queue.ranges[range_index - 1].end;
uint64_t length = space->ack_queue.ranges[range_index].end - space->ack_queue.ranges[range_index].start;
if (gap > UINT8_MAX || length > UINT8_MAX)
break;
additional->gap = gap;
additional->length = length;
}
/* additional list is zero-terminated, if not full */
if (additional < additional_end)
additional->gap = 0;
}
}

Expand Down