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

Make the ECN requirements match the the discussion in #2156. Fixes #2156 #2201

Merged
merged 4 commits into from Dec 18, 2018

Conversation

ekr
Copy link
Collaborator

@ekr ekr commented Dec 17, 2018

  1. Implementations are free to enable sending or not.
    2, Implementations MAY read the ECN code points if they
    can and send ECN ACKs.
  2. Implementations MUST accept the 0x03 ACK type and if they are
    sending ECN SHOULD use the ECN section.

I recognize that some people want to have stronger language here, based
on this discussion I think this is where we are. I'll defer to the
chairs to judge whether more is needed here and can change the PR as
required.

 quicwg#2156

1. Implementations are free to enable sending or not.
2, Implementations MAY read the ECN code points if they
   can and send ECN ACKs.
3. Implementations MUST accept the 0x03 ACK type and if they are
   sending ECN SHOULD use the ECN section.

I recognize that some people want to have stronger language here, based
on this discussion I think this is where we are. I'll defer to the
chairs to judge whether more is needed here and can change the PR as
required.
other peer. Even if ECN is not used on the path to the peer, the endpoint MUST
provide feedback about ECN markings received (if accessible).
other peer. Even if not setting ECN codepoints on packets it transmits, the
endpoint MAY provide feedback about ECN markings received (if accessible).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SHOULD?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SHOULD SGTM.

Also, can you remove the parens around if accessible?

@ekr
Copy link
Collaborator Author

ekr commented Dec 18, 2018 via email

Copy link
Contributor

@ianswett ianswett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG, minus one MAY/SHOULD

other peer. Even if ECN is not used on the path to the peer, the endpoint MUST
provide feedback about ECN markings received (if accessible).
other peer. Even if not setting ECN codepoints on packets it transmits, the
endpoint MAY provide feedback about ECN markings received (if accessible).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SHOULD SGTM.

Also, can you remove the parens around if accessible?

@@ -4133,7 +4135,9 @@ Receivers send ACK frames (types 0x02 and 0x03) to inform senders of packets
they have received and processed. The ACK frame contains one or more ACK Blocks.
ACK Blocks are ranges of acknowledged packets. If the frame type is 0x03, ACK
frames also contain the sum of QUIC packets with associated ECN marks received
on the connection up until this point.
on the connection up until this point. QUIC implementations MUST properly handle
both types 0x02 and 0x03 and if they have enabled ECN for packets they send,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
both types 0x02 and 0x03 and if they have enabled ECN for packets they send,
both types 0x02 and 0x03, and if they have enabled ECN for packets they send,

@martinthomson martinthomson merged commit 4d7ec17 into quicwg:master Dec 18, 2018
@martinthomson martinthomson added editorial An issue that does not affect the design of the protocol; does not require consensus. -transport labels Dec 18, 2018
@martinthomson martinthomson added design An issue that affects the design of the protocol; resolution requires consensus. and removed editorial An issue that does not affect the design of the protocol; does not require consensus. labels Jan 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-transport design An issue that affects the design of the protocol; resolution requires consensus.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants