[Gen-art] Genart last call review of draft-ietf-wish-whip-09

Dale Worley via Datatracker <noreply@ietf.org> Tue, 08 August 2023 15:45 UTC

Return-Path: <noreply@ietf.org>
X-Original-To: gen-art@ietf.org
Delivered-To: gen-art@ietfa.amsl.com
Received: from ietfa.amsl.com (localhost [IPv6:::1]) by ietfa.amsl.com (Postfix) with ESMTP id D9A92C1519A8; Tue, 8 Aug 2023 08:45:21 -0700 (PDT)
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
From: Dale Worley via Datatracker <noreply@ietf.org>
To: gen-art@ietf.org
Cc: draft-ietf-wish-whip.all@ietf.org, last-call@ietf.org, wish@ietf.org
X-Test-IDTracker: no
X-IETF-IDTracker: 11.5.0
Auto-Submitted: auto-generated
Precedence: bulk
Message-ID: <169150952187.16313.7471460147046530522@ietfa.amsl.com>
Reply-To: Dale Worley <worley@ariadne.com>
Date: Tue, 08 Aug 2023 08:45:21 -0700
Archived-At: <https://mailarchive.ietf.org/arch/msg/gen-art/XLPl2-fyOplDQDlgDPOuY7nZIuY>
Subject: [Gen-art] Genart last call review of draft-ietf-wish-whip-09
X-BeenThere: gen-art@ietf.org
X-Mailman-Version: 2.1.39
List-Id: "GEN-ART: General Area Review Team" <gen-art.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/gen-art>, <mailto:gen-art-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/gen-art/>
List-Post: <mailto:gen-art@ietf.org>
List-Help: <mailto:gen-art-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/gen-art>, <mailto:gen-art-request@ietf.org?subject=subscribe>
X-List-Received-Date: Tue, 08 Aug 2023 15:45:22 -0000

Reviewer: Dale Worley
Review result: Ready with Issues

I am the assigned Gen-ART reviewer for this draft. The General Area
Review Team (Gen-ART) reviews all IETF documents being processed
by the IESG for the IETF Chair.  Please treat these comments just
like any other last call comments.

For more information, please see the FAQ at

<https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.

Document:  draft-ietf-wish-whip-09
Reviewer:  Dale R. Worley
Review Date:  2023-08-08
IETF LC End Date:  2023-08-08
IESG Telechat date:  [not known]

Summary:

    This draft is on the right track but has open issues, described in
    the review.

The exposition could be clarified in several places.  A few of the
clarifications could be considered technical issues.  I list the
issues in textual order, with an initial summary of the most important
issues.

* Summary of the major issue:

A very important aspect of this document is that it defines the usage
of "ICE via SDP via HTTP", equivalently, "supporting ICE offer/answer
over HTTP" (as opposed to the original "ICE via SDP via SIP").  That
definition likely will have broader usage than just WHIP.  But the
document suffers from the "expert's disease", in that the authors
clearly have deep knowledge of ICE and consequently only document the
details of the ICE processing without providing any of the framework.
This leaves naive readers to reconstruct the big picture.  I was going
to add "As far as I can tell, all of the needed details are listed
somewhere in section 4", but as you can see below, once I wrote an
outline of what is needed, there are a few points that don't seem to
be specified, at least not to the point that I recognized it.)

I suggest that section 4 be organized by:

- stating that it is defining "supporting ICE offer/answer
  over HTTP" (so that it can be clearly referenced by later
  specifications)

- specifying the abstract operations involved (with the mapping to
  specific operations listed either in this list or a later list):

- - starting the ICE negotiation (via POST carrying
    application/sdp)

- - updating the ICE when trickling (via PATCH carrying
    application/trickle-ice-sdpfrag)

- - restarting ICE (via PATCH carrying (via PATCH carrying
    application/trickle-ice-sdpfrag, but it's not clear how this is
    distinguished, RFC 8840 does not specify it; perhaps because it
    carries ice-ufrag/ice-pwd attributes (see section 4.1)?)

- - termination (via DELETE from the client; not clear how from the
    server)

- specifying how WHIP constrains the ICE abstraction (Media server
  MUST not do trickle ICE updates, not clear if Media server MAY do
  ICE restarts, Media server MAY not support trickle ICE or ICE
  restart, etc.)

- discussion of how out-of-order requests may arise (which isn't at
  all clear to me, as requests seem to be generated only by the
  server, and carried by TCP, so they seem to be inherently
  serialized)

- discussion of how ETags MAY/MUST be handled, as that is
  comprehensible only when one looks at the serialization needs of all
  of the operations together

- specifying the particular features of each of the operations,
  including any particulars of request and response fields and what
  response codes are to be used for what error situations

* Summary of minor issues that might have technical content:

At various points in section 4 the text refers to a device sending a
request or generating a response but in many of them, the text doesn't
state whether the device is on the client or server end, or might be
both.  I assume that the nature of the operation implies what devices
might perform it based on text elsewhere, but it's hard to fully
understand on the first reading pass.  If possible, each sentence
should make this clear. -- Then again, if section 4 is reorganized to
describe generic ICE/SDP/HTTP usage, attention should be paid that the
generic usage may define how e.g. the server does an operation, even
if WHIP servers may not do that operation.

Section 4 lists a collection of 4xx and 5xx response codes to be used
in certain situations.  Are these set due to the specific semantics of
the codes or just to ensure that the recipient can tell what the cause
of the error was?  (HTTP suffers from having a large number of error
responses but all of them have vague semantics.  Unfortunately, there
doesn't seem to be an HTTP response header that can carry codes
defined for ICE usage specifically.)

Section 4 requires the WHIP server to reject certain specific request
methods, but no others.  Verify that you intend this specific
limitation.

Section 4.1 references sections 6.6.2, 2.3, and 3.1 of RFC 9110.  All
of these references appear to be incorrect, in that those sections do
not discuss the topics at hand.  Section 4.3 references section 6.4.7
of RFC 9110, which does not exist.  Some of these appear to be
referencing sections in RFC 7231, which is obsoleted by RFC 9110.

Section 4.1 talks about out-of-order PATCH requests but is unclear
about what may cause them, as the immediate implication is that
somehow the client is sending overlapping PATCH requests.  Probably
the authors fully understand the allowed sequences of operations that
cause this but the text needs to clarify what is being described.

Section 4.1 has inconsistent statements about a WHIP resource's
response code if the client requests an ICE restart but the resource
does not support ICE restart.

Section 4.2 extends RFC 8842, and this document should be marked as
doing so.

In Section 4.4, I notice that there's no explicit differentiation
between the STUN and the TURN server information in returned Link
header fields.  Please verify that either (1) the presence of username
and credential-type differentiates the two cases, or (2) the Media
client/server do not need to differentiate the two cases.

Section 4.7 requires POST responses to include Link headers for any
WHIP extensions supported by the server.  Perhaps OPTIONS responses
should be required to do so also.

The designators of WHIP extensions are described as URIs.  In fact,
they are URNs, and it would be more precise to refer to them as URNs
in the text.

* All issues (most of them exposition/editorial), in text order:

1.  Introduction

   It also describes how to negotiate media flows
   using the Offer/Answer Model with the Session Description Protocol
   (SDP) [RFC3264] as well as the formats for data sent over the wire
   (e.g., media types, codec parameters, and encryption).

I think "as well as" should be "including".

   RTSP [RFC7826], which is based on RTP, is
   not compatible with the SDP offer/answer model [RFC3264].

This statement might be clarified, as the naive reader (me) considers
offer/answer to also be based on RTP.  I suspect that the critical
meaning is "RTSP, unlike SDP offer/answer, has no provisions for
negotiating the characteristics of the media session."

   This document proposes a simple protocol for supporting WebRTC as
   media ingestion method which:

I would amend to "... a simple protocol, WHIP, for ..." to have a
clear assertion in the introduction of what WHIP is.  I would also add
"HTTP", as the preceding text describes what WHIP *isn't* based on:

   This document proposes a simple protocol, WHIP, based on HTTP, for
   supporting WebRTC as media ingestion method which:

--
   *  Allows for ingest both in traditional media platforms and in
      WebRTC end-to-end platforms with the lowest possible latency.

Verify that "ingest" is used in the field as shorthand for "ingestion"
(rather than being a typo).  The text seems to use both words with the
same meaning; it should probably use only one.

   *  Is usable both in web browsers and in native encoders.

I would say "standalone encoders", but perhaps this is the industry
terminology.

2.  Terminology

For clarity, I would move Figure 1 to after paragraph 1 of section 2
and amend it to something like this.  Or perhaps better, relocate the
current section 3 before section 2, so it can provide the context for
the glossary.

 +--------------+  +-------------+    +---------------+            +--------------+
 | Media client |  | WHIP client |    | WHIP server   |            | Media server |
 +------+-------+  +--+----------+    +---------+---+-+            +------+-------+
        |             |                         |   |                     |
        |             |                         | +-+-------------+       |
        |             |                         | | WHIP resource |       |
        |             |                         | +--------|------+       |
        |             |                         |          |              |
        |             |HTTP POST (SDP Offer)    |          |              |
        |             +------------------------>+          |              |
        |             |201 Created (SDP answer) |          |              |
        |             +<------------------------+          |              |
        |             |          ICE REQUEST               |              |
        +---------------------------------------------------------------->+
        |             |          ICE RESPONSE              |              |
        |<----------------------------------------------------------------+
        |             |          DTLS SETUP                |              |
        |<===============================================================>|
        |             |          RTP/RTCP FLOW             |              |
        +<--------------------------------------------------------------->+
                      | HTTP DELETE                        |
                      +----------------------------------->|
                      | 200 OK                             |
                      <------------------------------------+

The reason to move it before the definitions is that definitions would
be a lot clearer with the diagram already in mind.

The "WHIP client" is separated into two components, one dealing with
HTTP and SDP and one dealing with RTP, with both being involved in
ICE, in the same way that the server is divided into two components
with the same division of responsibilities.

I suspect that historically, the server-side is conceptualized that
the "Media Server" exists a priori with the "WHIP endpoint" being
added to it as an extra unit, but the client-side is conceptualized as
being a unitary system.  That structure makes the specification harder
to understand, though, and I think showing the client and server with
more parallel structures would clarify the specification.

The "WHIP resource" is shown as being a dependent of the "WHIP
server".

The "WHIP endpoint" is renamed "WHIP server".  At least in the SIP
world, "endpoint" is used to refer to *both* ends of one connection,
and so the term "WHIP endpoint" would be expected to include the WHIP
client as well.

The capitalization of "Media server" is made consistent with the other
terms.  (See also the entry in the glossary.)

Although conceptually the left two items (Media client and WHIP
client) and the right three items (WHIP server, WHIP resource, and
Media server) collectively form the two end-systems of the
client/server relationship, I have not tried to invent names for the
two collections because there doesn't seem to be a need to reference
them as wholes.

These changes do require adjusting the terminology in the rest of the
document.

3.  Overview

See comments on section 2.

4.  Protocol Operation

   The SDP offer SHOULD use the "sendonly" attribute and the SDP answer
   MUST use the "recvonly" attribute in any case.

You may want to extend this to "SHOULD use the "sendonly" attribute
but MAY use the "sendrecv" attribute", since the other possible values
are useless.

"in any case" seems redundant given the presence of "MUST".

POST /whip/endpoint HTTP/1.1

I recommend preceding each of the HTTP messages with a description of
what the endpoints are.  E.g. "POST request from WHIP client to WHIP
server" and "201 Created response from WHIP server to WHIP client".

              Figure 2: HTTP POST doing SDP O/A example

Perhaps more informative as "Example WHIP POST executing SDP
offer/answer".

   Once a session is setup, ICE consent freshness [RFC7675] SHALL be
   used to detect non graceful disconnection and DTLS teardown for
   session termination by either side.

I think the meaning is "Once a session is set up, ICE consent
freshness [RFC7675] MUST be maintained.  Both endpoints MUST monitor
freshness and MUST tear down the connection if freshness is lost."
(Note I don't know how ICE consent freshness is done, so I am assuming
that no further details need be specified to ensure interoperability.)
Probably the 2nd following paragraph ("A Media Server terminating...")
can be combined with this paragraph.  Also, expand it to place the
same requirement on the Media client.

   The WHIP endpoints MUST return an "405 Method Not Allowed" response
   for any HTTP GET, HEAD or PUT requests on the endpoint URL in order
   to reserve its usage for future versions of this protocol
   specification.

This and the 2nd following paragraph only forbid a few methods, and
allow an implementation to implement any others as extensions.  Please
verify that these are exactly what you need.  Also, the wording should
be "reserve their usage" because the referent is "any ... requests".

   The WHIP endpoints MUST support OPTIONS requests for Cross-Origin
   Resource Sharing (CORS) as defined in [FETCH] and it SHOULD include
   an "Accept-Post" header with a mime type value of "application/sdp"
   on the "200 OK" response to any OPTIONS request received as per
   [W3C.REC-ldp-20150226].

I think this would be less awkward as

   The WHIP endpoints MUST support OPTIONS requests for Cross-Origin
   Resource Sharing (CORS) as defined in [FETCH].  The "200 OK"
   response to any OPTIONS request SHOULD include an "Accept-Post"
   header with a mime type value of "application/sdp" as per
   [W3C.REC-ldp-20150226].

4.1.  ICE and NAT support

Text might be added to the beginning of this section similar to the
text at the beginning of section 4.2, as WHIP puts specific
restrictions on ICE usage for simplicity's sake.  It seems like "NAT"
should be replaced by something else, as "NAT" is not mentioned
elsewhere in the document.  "STUN/TURN" seems to be natural, but there
is a section for that (4.4).  Perhaps this section should be titled
"ICE usage".

Also, several subsections of section 4 seem to be about usage and
restrictions of various protocols; perhaps they should have parallel
names e.g. "ICE usage", "WebRTC usage", "STUN/TURN usage".

   In order to simplify the protocol, there is no support for exchanging
   gathered trickle candidates from Media Server ICE candidates once the
   SDP answer is sent.

I think this would be clearer as follows, since that moves the fact we
are talking about the Media server earlier in the sentence:

   In order to simplify the protocol, there is no support for the
   Media server sending further trickle candidates once the SDP answer
   is sent.

--

   The WHIP client MAY perform trickle ICE or ICE restarts as per
   [RFC8838] by sending an HTTP PATCH request ...

PATCH is an unusual method, defined in RFC 5789, and it seems like
there should be a reference for it here.

Also, you probably want to insert here a statement whether the WHIP
resource MAY *initiate* ICE restarts; as far as I can tell, the text
does not address this.

Also, you want to directly state that the semantics (meaning) of an
ICE update/restart request and thus how it is to be processed, is
determined by the body, as specified in RFC 8840.  The current text
leaves that silently implied.

   If the WHIP resource supports either Trickle ICE or ICE restarts, but
   not both, it MUST return a "405 Not Implemented" response for the
   HTTP PATCH requests that are not supported.

   If the WHIP resource does not support the PATCH method for any
   purpose, it MUST return a "501 Not Implemented" response, as
   described in [RFC9110] Section 6.6.2.

The canonical description for "405" is "Method not allowed".  But I
would reorganize and combine these paragraphs along these lines:

   If the WHIP resource supports either Trickle ICE or ICE restarts,
   but not both, it MUST return a "405 Method not allowed" response
   for HTTP PATCH requests for the feature that is not supported.  If
   neither feature is supported, it MUST return a "501 Not
   Implemented" response for such HTTP PATCH requests.

This allows PATCH requests for other purposes, if any of those are
ever defined.

BTW, the reference "[RFC9110] Section 6.6.2" ("Trailer") seems to be
unrelated to the text; please verify it.

   As the HTTP PATCH request sent by a WHIP client may be received out-
   of-order by the WHIP resource, ...

To clarify this, I would expand it to:

   The WHIP client MAY send overlapping HTTP PATCH requests to one
   WHIP resource.  As the HTTP PATCH request sent by a WHIP client may
   be received out-of-order by the WHIP resource, ...

However it is unclear to me why the client would want to do this.
Perhaps the concept is that the server may also be sending PATCH
requests for trickle ICE etc.?  In that case, you would want to phrase
it more like:

   As the WHIP client and the WHIP resource may be both be sending
   HTTP PATCH requests without coordination, an HTTP PATCH request
   sent by a WHIP client may be received out-of-order by the WHIP
   resource, ...

In any case, you should ensure you understand the possible
complications and explain them here to the reader.  Also, the final
sentence of the paragraph is:

   Note that including the ETag
   in the original "201 Created" response is only REQUIRED if the WHIP
   resource supports ICE restarts and OPTIONAL otherwise.

I *think* this means that the entirety of the preceding paragraph is
conditional "If the WHIP resource supports ICE restarts, the following
complications are possible ... and so the WHIP resource must return
ETag header fields ...".  Whatever the causation/conditions are, they
should be put at the start of the paragraph.

   A WHIP client sending a PATCH request for performing trickle ICE MUST
   include an "If-Match" header field with the latest known entity-tag
   as per [RFC9110] Section 3.1.  When the PATCH request is received by
   the WHIP resource, it MUST compare the indicated entity-tag value
   with the current entity-tag of the resource as per [RFC9110]
   Section 3.1 and return a "412 Precondition Failed" response if they
   do not match.

Beware that this paragraph appears to be a continuation of the
preceding paragraph, in that the sentence "Note ..." appears to be a
condition on it also.  If that is true, when you relocate the
condition to before the first paragraph, the condition should itself
be paragraph-level, so it is clear that its scope is not ended by the
end of the first paragraph.

   A WHIP resource receiving a PATCH request with new ICE candidates,
   but which does not perform an ICE restart, MUST return a "204 No
   Content" response without body.

According to the previous text, if a WHIP resource receives a PATCH
requesting an ICE restart but it does not implement ICE restart, it
MUST return either 405 or 501.  This needs to be clarified; perhaps
there is a difference between "does not support" and "does not
perform"?

   If the Media Server does not support
   a candidate transport or is not able to resolve the connection
   address, it MUST accept the HTTP request with the "204 No Content"
   response and silently discard the candidate.

Note that the Media server does not generate HTTP responses in any
case.  I assume the meaning is "the WHIP resource must accept ...".  I
think the intended meaning is

   If a WHIP resource receives a PATCH request containing a new ICE
   candidate with a transport that the Media Server does not support
   or is unable to resolve the address, the resource must MUST
   silently discard the candidate and process the remainder of the
   request.  Typically, it will accept the HTTP request with the "204
   No Content" response.

The last sentence is written as it is because the resource may have
some other reason for rejecting the PATCH.

   A WHIP client sending a PATCH request for performing ICE restart MUST
   contain an "If-Match" header field with a field-value "*" as per
   [RFC9110] Section 3.1.

Naively, this seems questionable, as presumably all ICE operations
need to be serialized.  Or is it that ICE restart causes all preceding
ICE updates to become irrelevant?  Again, some explanation of the
allowed sequencing of ICE operations and the necessary
serialization/dependencies would be good introduction.

   Also, the "200 OK" response for a successful ICE restart MUST contain
   the new entity-tag corresponding to the new ICE session in an ETag
   response header field and MAY contain a new set of ICE candidates for
   the Media Server.

This reads oddly because it combines "new ICE session" with the
possibility that the triggering PATCH request may not include new ICE
candidates.  What is the implied processing if there are no
candidates?  There are two possibilities, one that the new ICE session
starts with no candidates (in that direction), the other is that the
previously specified set of candidates is retained.  The latter
possibility implies that the new session depends on the previous
session and introduces serialization requirements.

   If the ICE request cannot be satisfied by the WHIP resource, the
   resource MUST return an appropriate HTTP error code and MUST NOT
   terminate the session immediately.

You probably mean "If the ICE update or restart request ...".  Also,
you probably mean "... and MUST continue the previous ICE session."

   In either case, the session MUST be
   terminated if the ICE consent expires as a consequence of the failed
   ICE restart as per [RFC7675] Section 5.1.

Probably intensify to "In any case, ...".

   Because the WHIP client needs to know the entity-tag associated with
   the ICE session in order to send new ICE candidates, it MUST buffer
   any gathered candidates before it receives the HTTP response to the
   initial POST request or the PATCH request with the new entity-tag
   value.

I think this could be clarified by putting more of the sentence in
time order:

   Because the WHIP client needs to know the entity-tag associated
   with the ICE session in order to send a PATCH containing new ICE
   candidates, it MUST wait until it receives the HTTP response to the
   initial POST request or the previous PATCH request before it can
   send any candidates that were gathered after the previous request
   was sent.

--

   ... the STUN requests will contain invalid ICE information and will
   be rejected by the server.  When this situation is detected by the
   WHIP Client, it MUST ...

Given that this is a MUST, we need a more rigid specification of the
specific responses that compel this behavior in the client.  Also,
"the server" is used, but apparently in the sense "the STUN server",
not "the Media server" as elsewhere in the text.  Also, it seems to be
implied that the Media server will not do ICE restart if it receives
these errors from the STUN server.  Is that true?

4.2.  WebRTC constraints

   In the specific case of media ingestion into a streaming service,

Given that the abstract says "ingestion of content into streaming
services and/or CDNs", this appears to confine the paragraph to
discussing a subset of WHIP implementations.  This clause should
probably be revised to clarify that it covers all WHIP usages.

   Both the WHIP client and the WHIP endpoint SHALL use SDP bundle
   [RFC9143].  ...

Please verify that all proper specifications are both "MUST support"
and "MUST use".  E.g., there is "... will use RTP/RTCP multiplexing
..." which should say "SHALL"/"MUST".

   ... bundled "m=" sections as per [RFC8858] i.

The ending "i" seems like a typo, but please verify it isn't the
truncation of intended text.

   ... in a single MediaStream as defined in [[!RFC8830]] ...

It appears that RFC 8830 is an intended reference but it is not listed
in section 8.

   When a WHIP client sends an SDP offer, it SHOULD insert an SDP
   "setup" attribute with an "actpass" attribute value, as defined in
   [RFC8842].  However, if the WHIP client only implements the DTLS
   client role, it MAY use an SDP "setup" attribute with an "active"
   attribute value.  If the WHIP endpoint does not support an SDP offer
   with an SDP "setup" attribute with an "active" attribute value, it
   SHOULD reject the request with a "422 Unprocessable Entity" response.

   NOTE: [RFC8842] defines that the offerer must insert an SDP "setup"
   attribute with an "actpass" attribute value.  However, the WHIP
   client will always communicate with a Media Server that is expected
   to support the DTLS server role, in which case the client might
   choose to only implement support for the DTLS client role.

Note this means that this document updates RFC 8842, and should be
marked as such.

This part seems to be incomplete or inconsistent.  Clearly, a WHIP
server MUST support the DTLS server role, and so it MUST be able to
accept "a=setup:active".  This means that the described 422 error
response should never occur.

4.3.  Load balancing and redirections

   WHIP endpoints and Media Servers might not be colocated on the same
   server, so it is possible to load balance incoming requests to
   different Media Servers.

In stead of the passive "it is possible ...", it would be clearer to
say "the WHIP server may load balance incoming requests to multiple
Media servers."

   WHIP clients SHALL support HTTP redirection
   via the "307 Temporary Redirect" response as described in [RFC9110]
   Section 6.4.7.

RFC 9110 does not contain a section 6.4.7.

4.4.  STUN/TURN server configuration

   A reference to each STUN/TURN server will be returned using the
   "Link" header field [RFC8288] with a "rel" attribute value of "ice-
   server".

I notice that there's no explicit differentiation between the STUN and
the TURN server information.  Please verify that either (1) the
presence of username and credential-type differentiates the two cases,
or (2) the Media client/server do not need to differentiate the two
cases.

              Figure 5: Example ICE server configuration

It seems the caption should be "Example STUN/TURN server
configuration".

   The generation of the TURN server credentials may require performing
   a request to an external provider, which can both add latency to the
   OPTIONS request processing and increase the processing required to
   handle that request.  In order to prevent this, the WHIP Endpoint
   SHOULD NOT return the STUN/TURN server configuration if the OPTIONS
   request is a preflight request for CORS, that is, if The OPTIONS
   request does not contain an Access-Control-Request-Method with "POST"
   value and the the Access-Control-Request-Headers HTTP header does not
   contain the "Link" value.

I think the normative structure can be made clearer as:

   In order to avoid adding latency and processing to an OPTIONS
   request, the WHIP server SHOULD NOT return return the STUN/TURN
   server configuration in OPTIONS responses unless either (1) the
   configuration can be determined without extra processing or latency
   (e.g. by performing a request to an external provider), or (2) if
   the OPTIONS request is an XXX, and contains an
   Access-Control-Request-Method with "POST" value and the the
   Access-Control-Request-Headers HTTP header contains the "Link"
   value.

"is an XXX" optional and is to clarify the situation that exception
(2) supports.  From the current text, I think it should be "is not a
preflight request for CORS".  If "preflight request for CORS" is
retained, please add a reference to "[FETCH]" here, as the only other
mention of CORS in the document which has the reference to "[FETCH]"
is a long way away.

   It might be also possible to configure the STUN/TURN server URIs with
   long term credentials provided by either the broadcasting service or
   an external TURN provider on the WHIP client, overriding the values
   provided by the WHIP endpoint.

This should start with that it is the WHIP client that is being
configured:

   The WHIP client MAY be configured with the STUN/TURN server URIs
   and long term credentials provided by either the broadcasting
   service or an external TURN provider, overriding the values
   provided by the WHIP endpoint.

4.7.  Protocol extensions

   The WHIP endpoint MUST
   return one "Link" header field for each extension, with the extension
   "rel" type attribute and the URI for the HTTP resource that will be
   available for receiving requests related to that extension.

This seems awkward to me.  Perhaps

   The WHIP endpoint MUST return one "Link" header field for each
   extension that it supports, with the "rel" attribute having the
   extension URN as its value and the URI being suitable for that
   extension.

The part "the URI being suitable for that extension" recognizes that
we can't predict what the semantics of the URI will be for a future
extension, but the extension will define the semantics.

   Protocol extensions supported by the WHIP server MUST be advertised
   to the WHIP client in the "201 Created" response to the initial HTTP
   POST request sent to the WHIP endpoint.

It seems like a good idea to require protocol extension support
declaration in OPTIONS responses as well.

5.  Security Considerations

The writers should do a spelling-check pass over section 5, as there
are enough misspellings to inconvenience the Editor.

   On top of that, the WHIP protocol exposes a thin new attack surface
   expecific [sic] of the REST API methods used within it:

It seems like the following three items could be condensed, as most of
the text enumerates problems that are well-known or clear from the
context.  Perhaps something like:

   * HTTP flooding and resource exhaustion:  Both the WHIP server and
     the WHIP resources SHOULD implement a rate limit and avalanche
     control mechanism for incoming requests.
					      
   * Security of WHIP resource URLs:  Servers for WHIP resource URLs
     SHOULD either implement authentication and authorization
     similarly to WHIP servers (see section 4.5) or use URLs that are
     capabilities, in that they are unguessable and the possession of
     the URL shows the client has the right to access the resource.

6.1.  Link Relation Type: ice-server

   The link relation type below has been registered by IANA per
   Section 4.2 of [RFC8288].

I do not see "ice-server" in
https://www.iana.org/assignments/link-relations/link-relations.xhtml,
so it appears the wording should be "IANA is asked to register ...".

6.3.1.  Specification Template

   *  The designated contact shall be responsible for reviewing and
      enforcing uniqueness.

Should this be "designated expert"?  Compare with sections 6.4.1 and
6.4.2.

6.4.1.  Registration Procedure

   The RFC SHOULD include any attributes defined.

If this means what I think it does, it should be expanded to something
like

   The RFC MUST include the syntax and semantics of any
   extension-specific attributes that may be provided in a Link header
   field advertising the extension.

[END]