quicwg/ack-frequency

What is the max of Request Max Ack Delay field in ACK_FREQUENCY frame

Closed this issue · 7 comments

Given that the max_ack_delay TP is limited to 2^14 per https://www.rfc-editor.org/rfc/rfc9000#section-18.2-4.28.1, we probably have to state the maximum value here too.

Or is the intention that this extension permits a larger value than in RFC 9000?

mirjak commented

I added a note in Pr #276 but not more. I guess if a larger value is sent it is simply ignored. But do we need to state that explicitly? Or do we even want to threat that as an error?

Please don't ignore invalid values. This is easily detectable and should be a connection error.

mirjak commented

Not sure if that really is a connection error. Effectively we don't restrict the value you can sent, however, if you sent a too large value the client will anyway use a smaller one (bounded by the definition of max_ack_delay in RFC9000). After all sending this value can not force the client to do anything, therefore it's really more a request or nice ask. However, if we restrict the max value here to align with max_ack_delay that means you also have to change this extension if you ever want to change the limit for max_ack_delay. That seems to tie things more closely together than needed.

mirjak commented

I just merged PR #276 to address this issue at least partially.

@marten-seemann stated on that PR the follwowing:

I think #235 would be better addressed by defining the maximum value for the Requested Max Ack Delay (1000*2^14, if I understand correctly), instead of reiterating what RFC 9000 defines.

Please provide further comments if this issue needs to be addresses more.

mirjak commented

Actually the draft already says to treat this as an error:

"Receipt of an invalid value MUST be treated as a connection error of type TRANSPORT_PARAMETER_ERROR."

mirjak commented

After discussion with @LPardue we recognised that this is actually the wrong error type. the right error type would be FRAME_ENCODING_ERROR.

Also the draft currently says "endpoint MUST update its max_ack_delay" however, we now says in the draft that the ACK_FREQUENCY frame is a request (that can not be enforced). Therefore, a MUST seem not appropriate here.

I created PR #284 to address both. Please review!

mirjak commented

#284 merged. I case this need further wg discussion we can always create another PR on top of this.