Re: [core] John Scudder's Discuss on draft-ietf-core-new-block-11: (with DISCUSS and COMMENT)

[[e-mail]]

Hi John, 

Please see inline. 

Cheers,
Jon & Med

> -----Message d'origine-----
> De : John Scudder [mailto:[e-mail]]
> Envoyé : mercredi 19 mai 2021 23:37
> À : BOUCADAIR Mohamed TGI/OLN <[e-mail]>
> Cc : The IESG <[e-mail]>; [e-mail];
> [e-mail]; [e-mail]; [e-mail]
> Objet : Re: John Scudder's Discuss on draft-ietf-core-new-block-11:
> (with DISCUSS and COMMENT)
> 
> Hi Med, Jon,
> 
> My responses in-line.
> 
[...]
> >
> >> -----Message d'origine-----
> >> De : John Scudder via Datatracker [mailto:[e-mail]] Envoyé
> :
> >> jeudi 6 mai 2021 02:42 À : The IESG <[e-mail]> Cc :
> >> [e-mail]; [e-mail];
> >> [e-mail]; [e-mail]; [e-mail] Objet : John
> >> Scudder's Discuss on draft-ietf-core-new-block-11: (with DISCUSS
> and
> >> COMMENT)
> 
> […]
> 
> >> ------------------------------------------------------------------
> ---
> >> -
> >> DISCUSS:
> >> ------------------------------------------------------------------
> ---
> >> -
> >>
> >> For the most part I found this document relatively easy to follow,
> >> considering my complete lack of background in CoAP. However,
> despite
> >> a concerted effort I have not been able to nail down with
> confidence
> >> what the intended semantics of several of your timeouts are,
> notably
> >> NON_RECEIVE_TIMEOUT. Some of the text (for example, §4.4) implies
> >> that the timeout is an upper bound on how long an implementation
> >> should wait before declaring a block to have been lost (“The
> client
> >> SHOULD wait for up to NON_RECEIVE_TIMEOUT”).
> >
> > The text around the timeout values have been made absolute (as per
> your COMMENTS), removing "up to", etc., and once the timeout has
> expired, then the next activity takes place. Does this help clarify
> things?
> 
> Yes, those changes, plus the other rewrites to §7.2, help a lot. This
> DISCUSS question is resolved as far as I’m concerned.
> 
> I would clear the DISCUSS, but I am concerned about my comment #11
> (see below) and I’m going to raise that to a DISCUSS.
> 

Thanks.

> […]
> 
> >> ------------------------------------------------------------------
> ---
> >> -
> >> COMMENT:
> >> ——————————————————————————————————
> 
> […]
> 
> >> 2. Section 4.1
> >>
> >>   Q-Block2 Option is useful with GET, POST, PUT, FETCH, PATCH, and
> >>   iPATCH requests and their payload-bearing responses (2.01, 2.02,
> >>   2.03, 2.04, and 2.05) (Section 5.5 of [RFC7252]).
> >>
> >> I found the list of codes incomprehensible on first encountering
> it,
> >> since the concept of response codes hadn’t been introduced yet. I
> do
> >> understand that the document assumes familiarity with CoAP;
> >> nonetheless for basic clarity I think this should say “(response
> >> codes 2.01, 2.02…”. Additionally, the reference to RFC 7252 §5.5
> >> doesn’t seem to be especially germane?
> >
> > The key element in the text you quoted is "payload-bearing
> responses". Hence the pointer to 5.5 of RFC7252.
> 
> OK. I still think the text would be better if you inserted “response
> codes” within the parentheses, as in “(response codes 2.01, 2.01, …”
> instead of the current “(2.01, 2.02, …”.
> 

OK. Fixed.

> […]
> 
> >> 4. Section 4.1
> >>
> >>   The Q-Block1 and Q-Block2 Options are unsafe to forward.  That
> is,
> >> a
> >>   CoAP proxy that does not understand the Q-Block1 (or Q-Block2)
> >> Option
> >>   MUST reject the request or response that uses either option.
> >>
> >> Presumably (hopefully) this is simply describing the behavior of
> >> existing spec-compliant proxies when processing the new messages.
> As
> >> such, is the MUST appropriate? I would think not.
> >
> > This is only for those that are tagged as unsafe to forward. The
> normative language is OK.
> 
> My point here is that the second sentence (the one that begins “that
> is”) is simply stating the behavior you would expect an existing
> proxy to exhibit if it were presented with the “unsafe to forward”
> options. Correct?
> 
> If so, then I think the MUST is inappropriate, since you are not
> specifying new behavior here.
> 

Changed the text and added a pointer to Section 5.7.1 of RFC7252.

> […]
> 
> >> 11. General
> >>
> >> By the way, none of the timers specify jitter (and indeed, if read
> >> literally, jitter would be forbidden). Is this intentional?
> >
> > No +/- tolerances have been defined. When a timer expires, then the
> next action takes place.
> 
> I notice that RFC 7252 jitters its timers, for example:
> 
>    counter.  For a new Confirmable message, the initial timeout is
> set
>    to a random duration (often not an integral number of seconds)
>    between ACK_TIMEOUT and (ACK_TIMEOUT * ACK_RANDOM_FACTOR) (see
>    Section 4.8)
> …
>    ACK_RANDOM_FACTOR MUST NOT be decreased below 1.0, and it SHOULD
> have
>    a value that is sufficiently different from 1.0 to provide some
>    protection from synchronization effects.
> 
> MAX_TRANSMIT_SPAN and MAX_TRANSMIT_WAIT are similarly jittered. A
> number of your introduced parameters
> 
>    This document introduces new parameters MAX_PAYLOADS, NON_TIMEOUT,
>    NON_RECEIVE_TIMEOUT, NON_MAX_RETRANSMIT, NON_PROBING_WAIT, and
>    NON_PARTIAL_TIMEOUT primarily for use with NON (Table 3).
> 
> appear at least superficially similar to the timers the authors of
> RFC 7252 deemed important to jitter to prevent synchronization
> effects. Did you specifically consider jittering them, and decide
> that jitter was unnecessary? If so, can you explain what is different
> about your specification, compared to the base spec, that eliminates
> the concern?

RFC7252 introduces ACK_RANDOM_FACTOR jitter and separately jitter for multicast responses (which is not relevant here).

The ACK_RANDOM_FACTOR is there for when re-transmitting a packet that has not been acknowledged for some reason by its peer. NON_TIMEOUT is for when the next MAX_PAYLOADS_SET can start transmission (not re-transmission) assuming a 'Continue' has not arrived in the interim, and so was not thought necessary to add in ACK_RANDOM_FACTOR style jitter here.

For NON_RECEIVE_TIMEOUT, what is important is that NON_RECEIVE_TIMEOUT is greater than NON_TIMEOUT (We say in the spec a minimum of one second) so that a peer does not fire off a re-transmission request before the local agent has a chance to start to transmit the next MAX_PAYLOADS_SET.  NON_RECEIVE_TIMEOUT is exponentially scaled for each retry to make sure that stability is preserved. So, again, ACK_RANDOM_FACTOR jitter was not thought to be necessary here.

NON_MAX_RETRANSMIT is a fixed count.

NON_PROBING_WAIT is used to put a limit on the potential delay that could incur when obeying PROBING_WAIT when there is no peer response. If the implementation goes with the default EXCHANGE_LIFETIME computation, then NON_PROBING_WAIT includes ACK_RANDOM_FACTOR in the math.

NON_PARTIAL_TIMEOUT if computed using the default EXCHANGE_LIFETIME includes ACK_RANDOM_FACTOR.

> 
> >> 12. Section 7.2
> >>
> >>   If the CoAP peer reports at least one payload has not arrived
> for
> >>   each body for at least a 24 hour period and it is known that
> there
> >>   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)
> and
> >>   the situation re-evaluated for another 24 hour period until
> there
> >> is
> >>   no report of missing payloads under normal operating conditions.
> >> The
> >>   newly derived value for MAX_PAYLOADS should be used for both
> ends
> >> of
> >>   this particular CoAP peer link.  Note that the CoAP peer will
> not
> >>   know about the MAX_PAYLOADS change until it is reconfigured.  As
> a
> >>   consequence of the two peers having different MAX_PAYLOADS
> values,
> >> a
> >>   peer may continue indicate that there are some missing payloads
> as
> >>   all of its MAX_PAYLOADS set may not have arrived.  How the two
> peer
> >>   values for MAX_PAYLOADS are synchronized is out of the scope.
> >>
> >> I take it this is just thrown in here as an operational
> suggestion?
> >> It’s not specifying protocol, right? It seems a little misplaced,
> if
> >> so.
> >
> > This is a guidance to adjust the max_payloads to avoid losses under
> normal conditions.
> 
> OK, but you haven’t said whether this is operational guidance (i.e.,
> you expect this to be done by humans adjusting configuration
> parameters) or if it’s potentially to be done automatically the
> implementation. Given that "How the two peer values for MAX_PAYLOADS
> are synchronized is out of the scope” I would think it’s strictly
> operational guidance. If that’s correct and it’s operational guidance
> exclusively, I think it should not be part of §7.2. Section 7.2
> specifies protocol; mixing in operational advice potentially confuses
> the reader. You could add an “Operational Advice for Tuning
> Parameters” major section, or even a subsection below 7.2, and the
> concern would be resolved.

Please note that we removed this text as per the discussion with Ben.

> 
> By the way, nit:
> 
>    not have arrived.  How the two peer values for MAX_PAYLOADS are
>    synchronized is out of the scope.
> 
> “Out of the scope” -> “out of scope” or “out of the scope of this
> document"
> 
> >> 13. Section 10.1.3
> >>
> >> (This comment relates to the aside in my DISCUSS. You may want to
> >> skip over it until we’ve resolved the DISCUSS, after which this
> may,
> >> or may not, be
> >> relevant.)
> >>
> >> Why doesn’t the server request 1,9,10 in one go? Since its rxmt
> >> request is triggered by rx of 11, one would think it could infer
> 10
> >> had been lost.
> >
> > Because only the missing blocks of the previous set are included
> and there may have been some packet arrival re-ordering for 10 and
> 11.
> 
> I see. I guess I was thinking of this in terms of a sliding-window
> protocol, but your protocol is really still almost stop-and-wait, but
> with much larger blocks. Your rewrite helped a lot with this, thanks.
> 
> […]
> 
> >> 15. Section 10.2.3
> >>
> >> (This comment relates to my DISCUSS. You may want to skip over it
> >> until we’ve resolved the DISCUSS, after which this may, or may
> not,
> >> be relevant.)
> >>
> >> Why doesn’t reception of QB2:10/0/1024 trigger the client to
> request
> >> retransmission? Why does it have to wait for timeout?
> >
> > This is fixed as per a similar comment from Ben:
> >
> > OLD:
> >       [[NON_TIMEOUT (server) delay expires]]
> >          |     [[Server sends next set of payloads]]
> >          |<---------+ NON 2.05 M:0xab T:0xf0 O:1236 ET=23
> QB2:10/0/1024
> >          |   ...    |
> >       [[NON_RECEIVE_TIMEOUT (client) delay expires]]
> >          |     [[Client realizes blocks are missing and asks for
> the
> >          |       missing ones in one go]]
> >          +--------->| NON GET /path M:0x04 T:0xf3 QB2:1/0/1024\
> >          |          |                             QB2:9/0/1024
> >          |     X<---+ NON 2.05 M:0xac T:0xf3 ET=23 QB2:1/1/1024
> >
> > NEW:
> >       [[NON_TIMEOUT (server) delay expires]]
> >          |     [[Server sends next set of payloads]]
> >          |<---------+ NON 2.05 M:0xab T:0xf0 O:1236 ET=23
> QB2:10/0/1024
> >          |     [[On seeing a payload from the next set of payloads,
> >          |      Client realizes blocks are missing and asks for the
> >          |       missing ones in one go]]
> >          +--------->| NON GET /path M:0x04 T:0xf3 QB2:1/0/1024\
> >          |          |                             QB2:9/0/1024
> >          |     X<---+ NON 2.05 M:0xac T:0xf3 ET=23 QB2:1/1/1024
> 
> 
> I presume you’re not concerned about re-ordering between payload 9
> and 10 because NON_TIMEOUT is always guaranteed to be inserted
> between flights of payloads (unless the other side has sent a 2.31
> Continue, which means nothing was lost), therefore there’s a delay
> inserted on the server side between 9 and 10.
> 
> One concern related to that: waiting NON_TIMEOUT isn’t actually
> required, it’s only RECOMMENDED, therefore this isn’t actually a
> guarantee. From §7.2:
> 
>    As the sending of many payloads of a single body may itself cause
>    congestion, it is RECOMMENDED that after transmission of every
>    MAX_PAYLOADS_SET of a single body, a delay is introduced of
>    NON_TIMEOUT before sending the next MAX_PAYLOADS_SET to manage
>    potential congestion issues.
> 
> I am curious why you made this a RECOMMENDED instead of a MUST. In a
> situation like this it would be preferable for you to explain to the
> implementor what situation they can ignore the RECOMMENDED in and
> what they should do instead, or of course to make it into a MUST.

Because a continue signal may be received from the peer and then continue without waiting for the timeout to expire. 

This is to be linked with this text: 

      A response using this Response Code SHOULD NOT be generated for
      every received Q-Block1 Option request (Section 7.2).  It SHOULD
      only be generated when all the payload requests are Non-
      confirmable and a MAX_PAYLOADS_SET has been received by the
       server.  More details about the motivations for this optimization
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 
      are discussed in Section 7.2.
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

We could use **MUST unless a 'Continue' is received**, e.g.,

OLD:
   As the sending of many payloads of a single body may itself cause
   congestion, it is RECOMMENDED that after transmission of every
   MAX_PAYLOADS_SET of a single body, a delay is introduced of
   NON_TIMEOUT before sending the next MAX_PAYLOADS_SET to manage
   potential congestion issues.

NEW:
   As the sending of many payloads of a single body may itself cause
   congestion, after transmission of every MAX_PAYLOADS_SET of a single
   body, a delay MUST be introduced of NON_TIMEOUT before sending the
   next MAX_PAYLOADS_SET to manage potential congestion issues.
   However, if a 'Continue' is received from the peer for the current
   MAX_PAYLOADS_SET, then the next MAX_PAYLOADS_SET can start
   transmission immediately.

... but I know that many would argue this is a SHOULD. 

> 
> Now that I’m looking at RECOMMENDED and SHOULD again, I notice that
> the specification for use of 2.31 in §4.3 has a similar potential
> issue:
> 
>       A response using this Response Code SHOULD NOT be generated for
>       every received Q-Block1 Option request (Section 7.2).  It
> SHOULD
>       only be generated when all the payload requests are Non-
>       confirmable and a MAX_PAYLOADS_SET has been received by the
>       server.  More details about the motivations for this
> optimization
>       are discussed in Section 7.2.
> 
> I can imagine when an implementation might ignore the first SHOULD
> NOT: for example if it’s a very small implementation that doesn’t
> want to keep extra state.

FYI, we changed the first "SHOULD NOT" to "MUST NOT" as we removed auto-scaling (down to 1).

 But how about the SHOULD? Under what
> circumstances is it OK for an implementation to use Response Code
> 2.31 when not all the payloads in a set have been received?

An example would be: If the peer is overloaded/can't process more blocks, it won't send the continue signal.  

 If there
> are no such circumstances, this ought to be a MUST (or turn it around
> and say “MUST NOT be generated unless…”).

The procedure does not mandate the continue signal to be received, but it allows for it as an optimization to shorten the delay between the sets. Hence the current wording.

> 
> A similar concern exists for the specification of response 4.08.
> 
> I quickly skimmed the rest of the document for SHOULD/RECOMMENDED and
> didn’t find others that seemed equally problematic; however I’m of
> the school of thought that says you should ask yourself about every
> SHOULD, “why isn’t this a MUST?” and if you don’t have a good answer,
> change it.
> 
> […]
> 
> >> 16. Section 10.2.4
> >>
> >> Since MAX_PAYLOADS is 10, why does the example say “MAX_PAYLOADS
> has
> >> been reached” after payloads 2-9 have been retransmitted? That’s
> only
> >> 8 payloads.
> >>
> >
> > MAX_PAYLOADS is not a sliding window. It is a specific set of
> blocks, blocks #0 - #9 (payloads 1 to 10) in this case, then blocks
> #10 - #19 and so on.
> 
> More or less a meta-block, as I mention in my comment on #13, above.
> OK.
> 
> I have some new questions and comments related to your new revision:
> 
> 17. Section 1:
> 
>    This document introduces the CoAP Q-Block1 and Q-Block2 Options
> which
>    allow block-wise transfer to work with series of Non-confirmable
>    messages, instead of lock-stepping using Confirmable messages
>    (Section 3).  In other words, this document provides a missing
> piece
>    of [RFC7959], namely the support of block-wise transfer using Non-
>    confirmable where an entire body of data can be transmitted
> without
>    the requirement for an acknowledgement (but recovery is available
>    should it be needed).
> 
> As far as I can tell the spec does not really remove the requirement
> for acknowledgement,

These are not required. They were added as an optimization to avoid the non-timeout if the peer decides to use it.


 it just amortizes the acknowledgements by only
> sending them every MAX_PAYLOADS_SET. Response Code 2.31 is
> essentially an acknowledgement, and it gets sent that frequently,
> right? There’s also (if I recall correctly) some flavor of
> acknowledgement that is sent when the entire body has been
> transferred. So, I think the new paragraph isn’t accurate.
> 
> This observation also applies to this claimed benefit in §3:
> 
>    o  They support sending an entire body using NON messages without
>       requiring an intermediate response from the peer.
> 
> Response Code 2.31 is exactly an intermediate response. I guess maybe
> your focus is that if the intermediate response isn’t received,
> transmission continues, albeit more slowly than it would otherwise,
> and unreliably too, so in that sense the responses aren’t “required”.
> I think this requires awfully close parsing of the word “required”,
> though.
> 
> 18. Section 2:
> 
>    MAX_PAYLOADS_SET is the set of blocks identified by block numbers
>    that, when divided by MAX_PAYLOADS, they have the same numeric
> 
> Remove “they”

Fixed. Thanks. 

> 
>    result.  For example, if MAX_PAYLOADS is set to '10', a
>    MAX_PAYLOADS_SET could be blocks #0 to #9, #10 to #19, etc.
>    Depending on the data size, the MAX_PAYLOADS_SET may not comprise
> all
>    the MAX_PAYLOADS blocks.
> 
> I don’t understand the last sentence ("Depending on the data size,
> the MAX_PAYLOADS_SET may not comprise all the MAX_PAYLOADS blocks.”)
> Are you trying to say that if the body size isn’t evenly divisible by
> MAX_PAYLOADS then the final MAX_PAYLOADS_SET will have fewer than
> MAX_PAYLOADS blocks in it?

We meant that the last set may include fewer blocks than MAX_PAYLOADS. Changed to:

 " Depending on the overall data
                    ^^^^^^^^ 
   size, the final MAX_PAYLOADS_SET may not comprise all the
             ^^^^^
   MAX_PAYLOADS blocks. "

Better?

> 
> (I do think this change, to introduce the term MAX_PAYLOADS_SET, is
> generally helpful; thanks.)
> 
> Thanks,
> 
> —John

_________________________________________________________________________________________________________________________

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.