Skip to content

Commit

Permalink
fix(s2n-quic-transport): revert "improve amplification credits on cli…
Browse files Browse the repository at this point in the history
…ent #1818" (#1906)
  • Loading branch information
WesleyRosenblum authored Aug 11, 2023
1 parent 9110f50 commit a11f5d0
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 26 deletions.
10 changes: 2 additions & 8 deletions quic/s2n-quic-transport/src/connection/transmission.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,14 +175,8 @@ impl<'a, 'sub, Config: endpoint::Config> tx::Message for ConnectionTransmission<
// end, so we check that next. Finally, if there is no ApplicationData or Handshake packet
// to transmit, the Initial packet itself will be padded.
let mut pn_space_to_pad = {
let needs_padding = if Config::ENDPOINT_TYPE.is_client() {
// if we're the client and haven't completed the handshake, we need to pad out packets
// in order to give the server as many amplification credits as possible.
!space_manager.is_handshake_confirmed()
} else {
// if we're the server, we only need to pad while we have the initial space
has_transmission(space_manager.initial(), transmission_constraint)
};
let needs_padding =
has_transmission(space_manager.initial(), transmission_constraint);

if !needs_padding {
// There is no Initial packet, so no padding is needed
Expand Down
19 changes: 1 addition & 18 deletions quic/s2n-quic-transport/src/transmission/early.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::{
ack::AckManager, contexts::WriteContext, endpoint, recovery, space::CryptoStream, transmission,
};
use core::ops::RangeInclusive;
use s2n_quic_core::{frame::Ping, packet::number::PacketNumberSpace};
use s2n_quic_core::packet::number::PacketNumberSpace;

pub struct Payload<'a, Config: endpoint::Config> {
pub ack_manager: &'a mut AckManager,
Expand All @@ -30,9 +30,6 @@ impl<'a, Config: endpoint::Config> super::Payload for Payload<'a, Config> {
"Early transmissions should not be used for MTU probing"
);

// record the starting capacity
let start_capacity = context.remaining_capacity();

let did_send_ack = self.ack_manager.on_transmit(context);

// Payloads can only transmit and retransmit
Expand All @@ -44,20 +41,6 @@ impl<'a, Config: endpoint::Config> super::Payload for Payload<'a, Config> {
// send PINGs last, since they might not actually be needed if there's an ack-eliciting
// frame already present in the payload
self.recovery_manager.on_transmit(context);

// In order to trigger the loss recovery mechanisms during the handshake make all packets
// ack-eliciting. This is especially true for the client in order to give the server more
// amplification credits.
//
// Only send a PING if:
// * We're not congestion limited
// * The packet isn't already ack-eliciting
// * Another frame was written to the context
if !context.ack_elicitation().is_ack_eliciting()
&& start_capacity != context.remaining_capacity()
{
let _ = context.write_frame(&Ping);
}
}

if did_send_ack {
Expand Down

0 comments on commit a11f5d0

Please sign in to comment.