Re: [core] AD review of draft-ietf-core-new-block-09

[[e-mail]]

Hi Francesca, 

Thank you for the review. 

Changes can be tracked at: https://tinyurl.com/new-block-latest.

Please find our replies inline. 

Cheers,
Jon & Med

> -----Message d'origine-----
> De : core [mailto:[e-mail]] De la part de Francesca
> Palombini
> Envoyé : samedi 3 avril 2021 21:32
> À : [e-mail]
> Cc : [e-mail]
> Objet : [core] AD review of draft-ietf-core-new-block-09
> 
> Thanks for the work on this document.  I have a number of comments
> about things that I think need clarification or rewording in my
> review below, so I’m setting the state on the document to “revised I-
> D needed”, and hope we can get an update solving those before we move
> to IETF last call.  Please keep me in the "to" field to make sure I
> see any reply.
> 
> Please note that the comments are reported while reading the document
> from top to bottom - some comments could be avoided by referencing
> the right sections or restructuring the document. If I notice that
> this is the case, I make a suggestion in the comment - if I do miss
> something that is explained later on, please consider the comment a
> suggestion to add a reference to improve readability and add
> clarifications.
> 
> General:
> 
> 1.
> [FP]: Throughout the text, it seems that "payload" is used as a
> synonym to "CoAP request/response" or "block". This is incorrect and
> should be fixed. (See for example "received payload")
> 

[Med] The definition we are using is called out in the text:

  "The terms "payload" and "body" are defined in [RFC7959].  The term
   "payload" is thus used for the content of a single CoAP message
   (i.e., a single block being transferred)"

> -----
> 
> 2.
> [FP]: Throughout the document, SHOULD is used without explanation of
> why the recommendation is just that - a recommendation - and not a
> requirement. There needs to be more clarification to implementers
> when it is ok and can be expected to deviate from the
> recommendations. I noted those that jumped to my attention, but I
> suspect a general re-read of the document with this comment in mind
> should be done, as there might be more I missed.
> 

[Med] Will double check. Thanks. 

> =====
> 
> Section 1:
> 
> 3.
>    o  They can operate in environments where packet loss is highly
>       asymmetrical.
> 
> [FP]: Please clarify: Block1 and Block2 can also be used in
> environments where packet loss is asymmetrical. What do these new
> options add in that case?

[Med] Please refer to the introduction where this is introduced. See for example: 

   There is a requirement for these blocks of data to be transmitted at
   higher rates under network conditions where there may be asymmetrical
   transient packet loss (i.e., responses may get dropped).  An example
   is when a network is subject to a Distributed Denial of Service
   (DDoS) attack and there is a need for DDoS mitigation agents relying
   upon CoAP to communicate with each other (e.g.,
   [RFC8782][I-D.ietf-dots-telemetry]).  As a reminder, [RFC7959]
   recommends the use of Confirmable (CON) responses to handle potential
   packet loss.  However, such a recommendation does not work with a
   flooded pipe DDoS situation.

> 
> -----
> 
> 4.
>    o  To reduce the transmission times for CON transmission of large
>       bodies, NSTART needs to be increased from 1, but this affects
> 
> [FP]: NSTART appears here for the first time, please add a reference
> to where this is defined (fw reference to a section of this
> specification or to a different document).

[Med] This is already cited in the OLD text: 

   o  To reduce the transmission times for CON transmission of large
      bodies, NSTART needs to be increased from 1, but this affects
      congestion control where other parameters need to be tuned
      (Section 4.7 of [RFC7252]). 
       ^^^^^^^^^^^^^^^^^^^^^^^^
> 
> -----
> 
> 5.
>    Block2 Options when the different transmission properties are
>    required.  If the new options are not supported by a peer, then
> 
> [FP]: I hope these transmission properties are explained into detail
> later on. If so, please add a fw reference. If not, please clarify
> what these properties are.
> 

[Med] Added a pointer to Section 3.1.

> -----
> 
> 6.
>    The No-Response Option was considered but was abandoned as it does
> 
> [FP]: No-Response Option needs a reference.

[Med] Done. 

> 
> =====
> 
> Section 2:
> 
> 7.
> [FP]: Because of the way the document is structured, the subsections
> of section 1 use terminology that is given for granted, and mentioned
> in section 2. This create some confusion in the reader. I suggest to
> re-structure the sections so that Terminology is right after 1.
> Introduction and before 1.1
> 

[Med] Fair comment. Rearranged the text. 

> =====
> 
> Section 3:
> 
> 8.
>    The Content-Format Option applies to the body, not to the payload
>    (i.e., it must be the same for all payloads of the same body).
> 
> [FP]: I think this is missing that the previous sentence is true
> "when present together with the Q-Block1 or Q-Block2 Option".

[Med] Sure, this is only if it is present. FWIW, we are using the same wording as in RFC7959: 

   "The Content-Format Option applies to the body, not to the
   payload; in particular, the boundaries between the blocks may be in
   places that are not separating whole units in terms of the structure,
   encoding, or content-coding used by the Content-Format."

We can add a "when present" if you think this is more accurate.

> 
> -----
> 
> 9.
>    include the Q-Block2 Option in a GET or similar request, the Q-
> Block2
>    Option in a PUT or similar request, or the Q-Block1 Option in a
> PUT
>    or similar request so that the server knows that the client
> supports
> 
> [FP]: I am confused by the meaning of "similar request": what is
> considered a similar request to a GET request?

[Med] We meant similar requests that are used to retrieve the representation of a resource. We have FETCH in mind but we also wanted a text that is future proof if such similar requests are defined in the future.

 How is a PUT request
> different from a GET request? Additionally, how does the client
> decides if it needs to include a Q-Block1 or Q-Block2 Option in a PUT
> (or similar) request to indicate support for these two options?
> 

[Med] This can be driven by a local configuration knob, triggered by an application (DOTS, for example), etc. How this is done, is out of the scope of the spec. 

> -----
> 
> 10.
>    Note that if Q-Block1 or Q-Block2 Options are included in a packet
> as
>    Inner options, Block1 or Block2 Options MUST NOT be included as
> Inner
>    options.  Similarly there MUST NOT be a mix of Q-Block and Block
> for
> 
> [FP]: Maybe this is described later on, but - what is the node to do
> if it receives a message with both? Is the message to be rejected
> with an error? This should be clarified.

[Med] Good catch. Added a sentence to indicate that 4.02 (Bad Option) is used in such case.

> 
> -----
> 
> 11.
>    being transferred.  It is also used to identify a particular
> payload
>    of a body that needs to be retransmitted.  The Request-Tag is
> opaque,
> 
> [FP]: If it is the same for all the requests carrying one same body,
> how does it identify one particular payload?

[Med] After reading this, we agree the sentence is confusing. We meant that it is ALSO a required information to identify a missing block. That identification requires additional information (block number). Because this is detailed when describing 4.08, we suggest to delete this sentence. 

 Also I am assuming that
> "a particular payload of a body" means a block of the body - if that
> is not correct, I think the "payload of a body" should be rephrased.

[Med] The use is aligned with the definition: 

 " The term
   "payload" is thus used for the content of a single CoAP message
   (i.e., a single block being transferred), while the term "body" is
   used for the entire resource representation that is being transferred
   in a block-wise fashion."

> 
> -----
> 
> 12.
>    Each individual payload of the body is treated as a new request
>    (Section 5).
> 
> [FP]: Again, I suspect the meaning of this sentence will be clarified
> in section 5, but I think it is incorrect to say that each payload is
> treated as a new request - each payload is treated as a payload.

[Med] As indicated above, the wording assumes the following: 

" "payload" is thus used for the content of a single CoAP message"
                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

 I
> think this needs to be rephrased, but at this point in the text I
> can't say what the intent of this sentence was, so can't suggest a
> rephrasing.
> 

[Med] The main message here is that each individual message is to be treated as a new request, and be thus assigned a new token as per draft-ietf-core-echo-request-tag-12#section-4. 

We used to have token details included here, but given the comments we received from the WG, we discuss token issue in a separate section that is called in the text you quoted: Section 5. 

> -----
> 
> 13.
>       and that the resource was created.  The token used SHOULD be
> from
>       the last received payload.  The client should then release all
> of
> 
> [FP]: I think it would be worth to highlight that the last received
> block is not necessarily the block with the highest block number.
> (note this appears in more than one occurrence)

[Med] No problem to mention it: "Note that the last received payload may not be the one with the highest block number."

We don't think it is worth to restate it for each code, though.

> 
> -----
> 
> 14.
>       M bit set) have been successfully received.  The token used
> SHOULD
>       be from the last received payload.
> 
> [FP]: "SHOULD" must be motivated: why is this not a MUST? What are
> the conditions where it is acceptable that the token is different
> from the one in the request with the last received payload? (note:
> several occurrences)
> 

[Med] We do already have this text to explain the rationale for SHOULD:   

   "Note that the use of any received token with the same
   Request-Tag would work, but providing the one used in the last
   received payload will aid any troubleshooting.  The client will use
   the token to determine what was the previously sent request to obtain
   the Request-Tag value to be used."

The same applies for the other occurrences you pointer to. 

> -----
> 
> 15.
>       A response using this Response Code SHOULD NOT be generated for
>       every received Q-Block1 Option request.  It SHOULD only be
>       generated when all the payload requests are Non-confirmable and
> a
>       set of MAX_PAYLOADS (Section 6.2) payloads have been received
> by
>       the server.
> 
>       It SHOULD NOT be generated for CON.
> 
> [FP]: Same remark as previous comment - please motivate why this is
> SHOULD and is not MUST. When using SHOULD, guidance should be given
> to implementers about in which case it is ok to deviate from the
> recommendations.

[Med] The text does not use "MUST NOT" for the first part of the text you quoted because there might be situations where MAX_PAYLOADS is set to 1. This is discussed in in 6.2:

   are no other network issues over that period, then the value of
   MAX_PAYLOADS can be reduced by 1 at a time (to a minimum of 1)

The second SHOULD is not a MUST because sending a "continue" signal is an optimization and that the peer endpoint will send the next set of blocks after a timer is reached. This is discussed in 6.2:

   For the server receiving NON Q-Block1 requests, it SHOULD send back a
   2.31 (Continue) Response Code on receipt of all of the MAX_PAYLOADS
   payloads to prevent the client unnecessarily delaying.  Otherwise the
   server SHOULD delay for NON_RECEIVE_TIMEOUT (exponentially scaled
   based on the repeat request count for a payload), before sending the
   4.08 (Request Entity Incomplete) Response Code for the missing
   payload(s).

> 
> -----
> 
> 16.
>    SHOULD "forget" all tracked tokens associated with the body's
> 
>    token in the 4.08 (Request Entity Incomplete) response.  The
> server
>    on receipt of the reset message SHOULD delete the partial body.
> 
>    it SHOULD ignore the payload of the packet, but MUST still respond
> as
> 
>    A server SHOULD only maintain a partial body (missing payloads)
> for
> 
> [FP]: Same remark for the use of SHOULD. (I stopped nothing this down
> at this point in the review, but noted this in the general comments
> above)

[Med] Point taken. Will double check and tweak the text as appropriate. 

> 
> -----
> 
> 17.
>    unset.  It is permissible to set the M bit to request this and
>    missing blocks from this MAX_PAYLOADS set.  Further considerations
> 
> [FP]: "this and missing blocks" is unclear to me. Could you clarify?
> 

[Med] We meant **this block and later blocks from** the current set. This is discussed in the first part of 3.4. 

Reworded as it seems this is confusing for you. 

> -----
> 
> 18.
>    The requested missing block numbers MUST have an increasing block
>    number in each additional Q-Block2 Option with no duplicates.  The
> 
> [FP]: What is the reason for mandating that the blocks requested must
> be in order (increasing block number)?
> 

[Med] This is a way to force the client to check for duplicates and remove them. This also helps with troubleshooting.

> -----
> 
> 19.
>    packet.  The server acknowledges the initial request using an ACK
>    with the payload, and then sends the subsequent payloads as CON
> 
> [FP]: I suspect that this means that the first response (containing
> the first block) is piggybacked on an ACK, see 5.2.1 of RFC 7252.
> That is though not always the case, see 5.2.2. I think this option of
> piggybacked should be kept, but from the text it sounds like it is
> the only option, so I believe some rephrasing to clarify that might
> be necessary.
> 

[Med] Changed to: "Typically, the server acknowledges ..."

> -----
> 
> 20.
>    The ETag Option should not be used in the request for missing
> blocks
> 
>    client should treat the partial body with the old ETag as no
> longer
> 
>    response, the client should remove any partially received body
> held
> 
> [FP]: Why not replace the should (resp should not) with MUST (resp
> MUST NOT)?
> 

[Med] We used to have "MUST NOT" but removed it to address this valid comment received during the WGLC:

==
* "The ETag Option MUST NOT be used": This is more a factural than a
  normative statement; it *can* not be used there as the server would
  respond thusly. It may be used, but then that indicates that the
  client is trying to verify a freshness. (However, the client should
  not *start* sending an ETag once it learned the current resource's
  ETag when still attempting to pull out more blocks, but that's also not
  a normative requirement but a consequence of those two requests not
  being matchable any more.)
==

> =====
> 
> 21.
> Section 4:
> 
>    encoded as a CBOR Sequence [RFC8742].  It comprises of one or more
>    CBOR encoded [RFC8949] missing block numbers.  The missing block
> 
> [FP]: I suggest to change to " missing block numbers encoded as CBOR
> unsigned integers [RFC8949]."
> 

[Med] OK. 

> -----
> 
> 22.
>    response when created by the server; the client SHOULD drop any
>    duplicates in the same 4.08 (Request Entity Incomplete) response.
> 
> [FP]: I think that "drop" here might be confusing, since the client
> is not dropping a packet - suggestion to change to "ignore".
> 

[Med] Fixed, thanks. 

> -----
> 
> 23.
>    The Content-Format Option (Section 5.10.3 of [RFC7252]) MUST be
> used
>    in the 4.08 (Request Entity Incomplete) response.  It MUST be set
> to
>    "application/missing-blocks+cbor-seq" (Section 11.3).
> 
> [FP]: This is in direct contradiction with the following paragraph
> from a previous section:
> 
>       This Response Code returned without Content-Type "application/
>       missing-blocks+cbor-seq" (Section 11.3) is handled as in
>       Section 2.9.2 [RFC7959].
> 

[Med] That text is a reminder of the base 4.08 behavior that we are updating in this spec.  

> -----
> 
> 24.
>    Request-Tag would work, but providing the one used in the last
> 
> [FP]: Suggestion to rephrase "would work" to "would allow to identify
> the correct body" (or something similar).
> 

[Med] Changed to "would be acceptable".

> -----
> 
> 25.
>    single packet.  If this is the case, then the server can send
>    subsequent 4.08 (Request Entity Incomplete) responses containing
> the
>    missing other blocks on receipt of a new request providing a
> missing
>    payload with the same Request-Tag.
> 
> [FP]: Just another check: is it specified anywhere that reception of
> an error response does not mean the client should stop sending
> requests when Q-Block options are used? If not, it would be good to
> explicitly state that.
> 

[Med] There is text in 3.3 to indicate the behavior when a given error is received. We updated 4.08 text with this NEW sentence: 

"Such message must not treated by the client as a fatal error."  

> -----
> 
> 26.
>    The missing blocks MUST be reported in ascending order without any
>    duplicates.  The client SHOULD silently drop 4.08 (Request Entity
> 
> [FP]: Same question: why does order matter?
> 

[Med] Same answer as above: ease troubleshooting and have a way to force the server to detect/remove duplicates. 

> =====
> 
> Section 5:
> 
> 27.
>    Implementation Note:  To minimize on the number of tokens that
> have
>       to be tracked by clients, it is suggested that the bottom 32
> bits
>       is kept the same for the same body and the upper 32 bits
> contains
>       the current body's request number (incrementing every request,
>       including every re-transmit).  This allows the client to be
> 
> [FP]: This seems to imply that tokens are always 8 bytes, which is
> not necessarily the case. I suggest to add "in the case the token is
> 8 bytes".
> 

[Med] OK. Update the text to mention that the use of 8-byte tokens allows to minimize the number of tokens to be tracked by clients.  

> =====
> 
> Section 6:
> 
> 28.
>    control parameters will need to be tuned to cover this change.
> This
>    tuning is out of scope of this document as it is expected that all
>    requests and responses using Q-Block1 and Q-Block2 will be Non-
>    confirmable.
> 
> [FP]: "it is expected ... Non-confirmable" Where does this come from?
> Maybe I misunderstand this sentence, could you please expand?
> 

[Med] This is actually referring to the applicability scope of the document, especially this part:
    
   The mechanism specified in this document provides
   roughly similar features to the Block1/Block2 Options.  It provides
   additional properties that are tailored towards the intended use case
   of Non-Confirmable transmission.  Concretely, this mechanism
   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   primarily targets applications such as DDoS Open Threat Signaling
   (DOTS) that cannot use CON responses to handle potential packet loss^
               ^^^^^^^^^^^^^^^^^^^^^^^^^^
   and that support application-specific mechanisms to assess whether
   the remote peer is able to handle the messages sent by a CoAP
   endpoint (e.g., DOTS heartbeats in Section 4.7 of [RFC8782]).


Added a pointer to that section. 

> =====
> 
> Section 9:
> 
> 29.
>             +--------->| [[Payloads 3 - 9 not detailed]]
> 
> [FP]: Minor: I think it would make more sense to change "3 - 9" to "2
> - 8" (and same for examples below)
> 

[Med] I prefer the current one where we count payloads from 1, not from 0. Of course, payload 1 corresponds to the one with block num=0. 

> -----
> 
> Thank you for the examples! Very clear and easy to follow.
> 
> -----
> 
> 30.
>            +--------->| NON PUT /path M:0x1f T:0xef RT=11
> QB1:12/0/1024
>            |   ...    |
>         [[NON_RECEIVE_TIMEOUT (server) delay expires]]
>            |     [[The server realizes a block is still missing and
> asks
>            |        for the missing one]]
>            |<---------+ NON 4.08 M:0x92 T:0xef [Missing 10]
> 
> [FP]: I am not sure this is already stated somewhere, and if so
> please ignore this comment, but there should be some considerations
> about NON_RECEIVE_TIMEOUT in comparison to the client's CoAP timer to
> deal with the response: what I am thinking about is the scenario from
> the following example, where the client has for some reason discarded
> the token 0xef (for example to free up space) and is unable to
> process the 4.08 response from the server correctly, since it doesn't
> know with which request it associates with. What happens then? I
> assume the server should try again and then eventually stop and
> release the partial body. Is this described?

[Med] If the token is unknown for whatever reason, then the client should be sending a RST for the unknown token. We have the following:

   If the client elects to stop the transmission of a complete body, it
   SHOULD "forget" all tracked tokens associated with the body's
   Request-Tag so that a reset message is generated for the invalid
   token in the 4.08 (Request Entity Incomplete) response.  The server
   on receipt of the reset message SHOULD delete the partial body.

RFC7252, Section 5.3.2 says:

   In case a message carrying a response is unexpected (the client is
   not waiting for a response from the identified endpoint, at the
   endpoint addressed, and/or with the given token), the response is
   rejected (Sections 4.2 and 4.3).

> 
> -----
> 
> 31.
>           |<---------+ [[Payloads 3 - 8 not detailed]]
> 
> [FP]: This is inconsistent and should be either 3 - 9 or 2 - 8 as
> noted above.
> 

[Med] Why? 

Payload 9 is indicated in the example:

            +--------->| [[Payloads 3 - 8 not detailed]]
            +--------->| NON PUT /path M:0x19 T:0xe9 RT=11 QB1:8/1/1024

> -----
> 
> 32.
> [FP]: Figure 16: I am not sure how the server knows which block the
> client is requesting when it sends the following message:
> 
>           +--------->| NON FETCH /path M:0x58 T:0xc8 RT=31
> QB2:1/0/1024
> 

[Med] It is only asking for Block 1.

> [FP]: In fact, if I understand correctly this could either be a
> request for the missing block from the second set of blocks,

[Med] ... which would be QB2:10/0/1024 or anything up to QB2:9/0/1024 for a missing block in the second MAX_PAYLOADS set. 

 or the
> client could have not received block 1 from the previous set of
> blocks and could be requesting the same as in:
> 
>           +--------->| NON FETCH /path M:0x57 T:0xc7 RT=31
> QB2:1/0/1024
> 
> [FP]: If all the following responses from the server would have been
> lost. The two requests contain the same RT. What am I missing?

[Med] Each client request needs to have an unique Token, hence the increment to T:0xc8.


_________________________________________________________________________________________________________________________

Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.