Skip to content

Commit

Permalink
QUIC RXDP: Make ACK eliciting definition more resilient and centralised
Browse files Browse the repository at this point in the history 
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from #21135)
  • Loading branch information
@hlandau @paulidale
hlandau authored and paulidale committed Jul 16, 2023
1 parent f80e61b commit 6cdb672
Showing 1 changed file with 18 additions and 51 deletions.
69 changes: 18 additions & 51 deletions ssl/quic/quic_rx_depack.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,6 @@ static int depack_do_frame_ping(PACKET *pkt, QUIC_CHANNEL *ch,
return 0;
}

/* This frame makes the packet ACK eliciting */
ackm_data->is_ack_eliciting = 1;
return 1;
}

Expand Down Expand Up @@ -147,9 +145,6 @@ static int depack_do_frame_reset_stream(PACKET *pkt,
return 0;
}

/* This frame makes the packet ACK eliciting */
ackm_data->is_ack_eliciting = 1;

if (!depack_do_implicit_stream_create(ch, frame_data.stream_id,
OSSL_QUIC_FRAME_TYPE_RESET_STREAM,
&stream))
Expand Down Expand Up @@ -225,9 +220,6 @@ static int depack_do_frame_stop_sending(PACKET *pkt,
return 0;
}

/* This frame makes the packet ACK eliciting */
ackm_data->is_ack_eliciting = 1;

if (!depack_do_implicit_stream_create(ch, frame_data.stream_id,
OSSL_QUIC_FRAME_TYPE_STOP_SENDING,
&stream))
Expand Down Expand Up @@ -276,9 +268,6 @@ static int depack_do_frame_crypto(PACKET *pkt, QUIC_CHANNEL *ch,
return 0;
}

/* This frame makes the packet ACK eliciting */
ackm_data->is_ack_eliciting = 1;

rstream = ch->crypto_recv[ackm_data->pkt_space];
if (!ossl_assert(rstream != NULL))
/*
Expand Down Expand Up @@ -311,9 +300,6 @@ static int depack_do_frame_new_token(PACKET *pkt, QUIC_CHANNEL *ch,
return 0;
}

/* This frame makes the packet ACK eliciting */
ackm_data->is_ack_eliciting = 1;

if (token_len == 0) {
/*
* RFC 9000 s. 19.7: "A client MUST treat receipt of a NEW_TOKEN frame
Expand Down Expand Up @@ -488,9 +474,6 @@ static int depack_do_frame_stream(PACKET *pkt, QUIC_CHANNEL *ch,
return 0;
}

/* This frame makes the packet ACK eliciting */
ackm_data->is_ack_eliciting = 1;

if (!depack_do_implicit_stream_create(ch, frame_data.stream_id,
frame_type, &stream))
return 0; /* protocol error raised by above call */
Expand Down Expand Up @@ -644,9 +627,6 @@ static int depack_do_frame_max_data(PACKET *pkt, QUIC_CHANNEL *ch,
return 0;
}

/* This frame makes the packet ACK eliciting */
ackm_data->is_ack_eliciting = 1;

ossl_quic_txfc_bump_cwm(&ch->conn_txfc, max_data);
ossl_quic_stream_map_visit(&ch->qsm, update_streams, ch);
return 1;
Expand All @@ -669,9 +649,6 @@ static int depack_do_frame_max_stream_data(PACKET *pkt,
return 0;
}

/* This frame makes the packet ACK eliciting */
ackm_data->is_ack_eliciting = 1;

if (!depack_do_implicit_stream_create(ch, stream_id,
OSSL_QUIC_FRAME_TYPE_MAX_STREAM_DATA,
&stream))
Expand Down Expand Up @@ -709,9 +686,6 @@ static int depack_do_frame_max_streams(PACKET *pkt,
return 0;
}

/* This frame makes the packet ACK eliciting */
ackm_data->is_ack_eliciting = 1;

if (max_streams > (((uint64_t)1) << 60)) {
ossl_quic_channel_raise_protocol_error(ch,
QUIC_ERR_FRAME_ENCODING_ERROR,
Expand Down Expand Up @@ -760,9 +734,6 @@ static int depack_do_frame_data_blocked(PACKET *pkt,
return 0;
}

/* This frame makes the packet ACK eliciting */
ackm_data->is_ack_eliciting = 1;

/* No-op - informative/debugging frame. */
return 1;
}
Expand All @@ -784,9 +755,6 @@ static int depack_do_frame_stream_data_blocked(PACKET *pkt,
return 0;
}

/* This frame makes the packet ACK eliciting */
ackm_data->is_ack_eliciting = 1;

/*
* This is an informative/debugging frame, so we don't have to do anything,
* but it does trigger stream creation.
Expand Down Expand Up @@ -832,9 +800,6 @@ static int depack_do_frame_streams_blocked(PACKET *pkt,
return 0;
}

/* This frame makes the packet ACK eliciting */
ackm_data->is_ack_eliciting = 1;

if (max_data > (((uint64_t)1) << 60)) {
/*
* RFC 9000 s. 19.14: "This value cannot exceed 2**60, as it is not
Expand Down Expand Up @@ -867,9 +832,6 @@ static int depack_do_frame_new_conn_id(PACKET *pkt,
return 0;
}

/* This frame makes the packet ACK eliciting */
ackm_data->is_ack_eliciting = 1;

ossl_quic_channel_on_new_conn_id(ch, &frame_data);

return 1;
Expand All @@ -889,9 +851,6 @@ static int depack_do_frame_retire_conn_id(PACKET *pkt,
return 0;
}

/* This frame makes the packet ACK eliciting */
ackm_data->is_ack_eliciting = 1;

/* TODO(QUIC): Post MVP ADD CODE to send |seq_num| to the ch manager */
return 1;
}
Expand All @@ -910,9 +869,6 @@ static int depack_do_frame_path_challenge(PACKET *pkt,
return 0;
}

/* This frame makes the packet ACK eliciting */
ackm_data->is_ack_eliciting = 1;

/* TODO(QUIC): ADD CODE to send |frame_data| to the ch manager */

return 1;
Expand All @@ -932,9 +888,6 @@ static int depack_do_frame_path_response(PACKET *pkt,
return 0;
}

/* This frame makes the packet ACK eliciting */
ackm_data->is_ack_eliciting = 1;

/* TODO(QUIC): ADD CODE to send |frame_data| to the ch manager */

return 1;
Expand All @@ -958,9 +911,6 @@ static int depack_do_frame_handshake_done(PACKET *pkt,
if (!ossl_quic_wire_decode_frame_handshake_done(pkt))
return 0;

/* This frame makes the packet ACK eliciting */
ackm_data->is_ack_eliciting = 1;

ossl_quic_channel_on_handshake_confirmed(ch);
return 1;
}
Expand Down Expand Up @@ -1011,6 +961,24 @@ static int depack_process_frames(QUIC_CHANNEL *ch, PACKET *pkt,
return 0;
}

/*
* There are only a few frame types which are not ACK-eliciting. Handle
* these centrally to make error handling cases more resilient, as we
* should tell the ACKM about an ACK-eliciting frame even if it was not
* successfully handled.
*/
switch (frame_type) {
case OSSL_QUIC_FRAME_TYPE_PADDING:
case OSSL_QUIC_FRAME_TYPE_ACK_WITHOUT_ECN:
case OSSL_QUIC_FRAME_TYPE_ACK_WITH_ECN:
case OSSL_QUIC_FRAME_TYPE_CONN_CLOSE_TRANSPORT:
case OSSL_QUIC_FRAME_TYPE_CONN_CLOSE_APP:
break;
default:
ackm_data->is_ack_eliciting = 1;
break;
}

switch (frame_type) {
case OSSL_QUIC_FRAME_TYPE_PING:
/* Allowed in all packet types */
Expand Down Expand Up @@ -1282,7 +1250,6 @@ static int depack_process_frames(QUIC_CHANNEL *ch, PACKET *pkt,

default:
/* Unknown frame type */
ackm_data->is_ack_eliciting = 1;
ossl_quic_channel_raise_protocol_error(ch,
QUIC_ERR_FRAME_ENCODING_ERROR,
frame_type,
Expand Down

0 comments on commit 6cdb672

Please sign in to comment.