Re: AD review of draft-ietf-httpbis-priority-09

Thanks Lucas!

I followed up in the github issue and all looks good to me, thank you for your answers.

You can go ahead and submit the next version, so people reading for IETF Last Call will read the most updated version.

Francesca

From: Lucas Pardue <lucaspardue.24.7@gmail.com>
Date: Tuesday, 16 November 2021 at 00:22
To: Francesca Palombini <francesca.palombini@ericsson.com>
Cc: draft-ietf-httpbis-priority@ietf.org <draft-ietf-httpbis-priority@ietf.org>, ietf-http-wg@w3.org <ietf-http-wg@w3.org>, draft-ietf-httpbis-http2bis@ietf.org <draft-ietf-httpbis-http2bis@ietf.org>
Subject: Re: AD review of draft-ietf-httpbis-priority-09
Hi Francesca,

Thank you for the review!

On Fri, Nov 12, 2021 at 11:34 PM Francesca Palombini <francesca.palombini@ericsson.com<mailto:francesca.palombini@ericsson.com>> wrote:
Thank you for this document. Here is my review: please handle these  comments together with any future Last Call ones. I have opened an issue with the text below in the github: https://github.com/httpwg/http-extensions/issues/1783<https://protect2.fireeye.com/v1/url?k=1fc93b36-405203e4-1fc97bad-866132fe445e-1055aa965f9d18da&q=1&e=47cb57e5-7475-4e4b-b6f5-e690601f2775&u=https%3A%2F%2Fgithub.com%2Fhttpwg%2Fhttp-extensions%2Fissues%2F1783> . The first one should be fixed (and is easy to fix), the rest are more for my understanding – and apologies in advance if I have missed previous decisions.

Francesca

1. -----

FP: For some reason the IDnit check didn't pick it up, but the boilerplate about BCP 14 (first paragraph of Section 1.1) is outdated and should be updated to:

   The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT",
   "SHOULD", "SHOULD NOT", "RECOMMENDED", "NOT RECOMMENDED", "MAY", and
   "OPTIONAL" in this document are to be interpreted as described in
   BCP 14 [RFC2119] [RFC8174] when, and only when, they appear in all
   capitals, as shown here.

Thanks. See https://github.com/httpwg/http-extensions/pull/1784<https://protect2.fireeye.com/v1/url?k=8bd55f75-d44e67a7-8bd51fee-866132fe445e-0c7fcd22c4581033&q=1&e=47cb57e5-7475-4e4b-b6f5-e690601f2775&u=https%3A%2F%2Fgithub.com%2Fhttpwg%2Fhttp-extensions%2Fpull%2F1784>


2. -----

   However, in order to maintain wire compatibility, HTTP/2 priority
   signals are still mandatory to handle (see Section 5.3.2 of [HTTP2]).

FP: I have re-read this Section of HTTP2 several times, but I still can't be sure that "mandatory to handle" is accurate. In particular, paragraph 1 states that "some of the mandatory handling is retained" (side note: that leaves me confused to which parts are retained - maybe one of the HTTP2 authors in CC can help me here):

   This update to HTTP/2 deprecates the priority signaling defined in
   RFC 7540 [RFC7540].  The bulk of the text related to priority signals
   is not included in this document.  The description of frame fields
   and some of the mandatory handling is retained to ensure that
   implementations of this document remain interoperable with
   implementations that use the priority signaling described in RFC
   7540.

And the last paragraph states:

   priority of requests in the absence of any priority signals.  Servers
   MAY interpret the complete absence of signals as an indication that
   the client has not implemented the feature.  The defaults described

Which to me implies that clients are not required to implement them. But maybe this is me misinterpreting "handling" as processing from either clients or servers, while maybe it is intended to be mandatory to be supported from servers only?

Just to clarify, this might not be an actionable comment on this document, but just want to make sure that [HTTP2] and this document are consistent.

My understanding is that RFC7540bis is trying to say that we can't remove the priority pieces of HTTP/2 HEADERS frames, or the entire PRIORITY frame. Implementations of HTTP/2 will be expected to parse those elements of the protocol and ensure correctness but ignoring the information in valid frames is allowed. My working knowledge has always been that clients don't process priority signals but now I take a look at RFC7540(bis) and can't seem to find a requirement for that.

The Priorities draft is not trying to change HTTP/2, we can probably wordsmith things to make that more clear. I'm happy to take a cue from RFC7540bis authors if they have a suggestion.


3. -----

   SETTINGS_NO_RFC7540_PRIORITIES parameter with value of 1, it SHOULD
   stop sending the HTTP/2 priority signals.  If the value was 0 or if
   the settings parameter was absent, it SHOULD stop sending
   PRIORITY_UPDATE frames (Section 7.1), but MAY continue sending the

FP: For both SHOULDs above: it might be good to give a hint of when it is acceptable to deviate from the RECOMMENDations here. Otherwise, since there are a number of SHOULD throughout the document (I noted those in Section 4.1 as well), it might be good to address them in one sentence explaining that those recommendations have exceptions. (I am pointing this out as I expect it will otherwise come up later in IESG evaluation)

There's not really strong proactive reasons for deviating from this behaviour, our SHOULDs reflect that implementations might have their own reasons or constraints that prevent the guidance from being MUST level. I'll see if there is any text we can do to better elucidate the reason for these SHOULDs, rather than reasons to deviate.


4. -----

Section 16. IANA Considerations

FP: Is there a reason not to request the first available values in the IANA registries: 0x7 for SETTINGS_NO_RFC7540_PRIORITIES and 0xb for PRIORITY_UPDATE instead of 0x9 and 0x10 ?

I can't recall there being initial strong reasons for the chosen HTTP/2 values but changing them now would affect any implementations. I think at this stage the values we have picked are likely already burned.

Cheers,
Lucas

Received on Thursday, 18 November 2021 15:32:55 UTC