Subject: [core] Pete Resnick's Discuss on draft-ietf-core-coap-15: (with DISCUSS and COMMENT)

From: Barry Leiba <[email-masked]>
Date: Mon, 22 Apr 2013 19:04:47 -0400

> 4.1:
>
>   An empty message has the Code field set to 0.  The Token Length field
>   MUST be set to 0 and no bytes MUST be present after the Message ID
>   field.  If there are any bytes, they MUST be processed as a message
>   format error.
...
> I find the
> combination of MUSTs to be a bit problematic. MUST NOT send data, but
> MUST receive as a format error
...
> If it were me, I'd drop the last sentence.

Indeed.   Actually, I suggest this: "...bytes of data MUST NOT be present;
any data bytes are considered a message format error."

Barry

---

From: "Pete Resnick" <[email-masked]>
Date: Mon, 22 Apr 2013 15:22:15 -0700

Pete Resnick has entered the following ballot position for
draft-ietf-core-coap-15: Discuss

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)

Please refer to http://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.

----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------

The items here are very borderline in my book for DISCUSS items; I'm
happy to be talked out of them. But I would like to hear from the authors
and/or chairs before I give my "YES" (which is my plan once these are
resolved).

4.1:

If you insist on the MUSTs, make the second one "bytes of data MUST NOT
be present". The current construction is ambiguous. That said, I find the
combination of MUSTs to be a bit problematic. MUST NOT send data, but
MUST receive as a format error will lead to some sender saying, "A
conformant receiver MUST reject with an error, so no need for me to
validate on the way out" and for a receiver to say, "A conformant sender
MUST NOT send data, so no need for me to validate on the way in." That's
a recipe for non-interoperability. If it were me, I'd drop the last
sentence.

4.3:

   rejecting a Non-confirmable message MAY involve sending a matching
   Reset message, and apart from the Reset message the rejected message
   MUST be silently ignored.

See comment on 2.1. But if you're going to allow this, I don't understand
the MAY: Doesn't rejecting the message require sending a Reset?
Otherwise, the message has not been rejected; it's simply been ignored.
The second part is either redundant or confusing: What else might I do
with a rejected message other than send the Reset and ignore it? I think
this needs rewriting.

5.2.2: It is probably worth saying somewhere in here: "Once the server
sends back an empty Acknowledgement, it MUST NOT send back the response
in another Acknowledgement, even if the client retransmits another
identical request. If a retransmitted request is received (perhaps
because the original Acknowledgement was delayed), another empty
Acknowledgement is sent and any response MUST be sent as a separate
response."

5.4.2: Cache-Key is undefined, here or in any other document I can find.
It probably needs an explanation somewhere in this document.

5.5: Again, I don't like the combination of MUST NOT include/MUST ignore.
I would drop the MUST ignore part.

5.10.4:

                                                      The server SHOULD
   return the preferred Content-Format if available.  If the preferred
   Content-Format cannot be returned, then a 4.06 "Not Acceptable"
   SHOULD be sent as a response.

What are the exceptions to the above two SHOULDs? If the preferred format
is available, when would a server not return it. If it's not available,
when would the server return other than "Not Acceptable"? Also, since
Accept is not marked as "Critical", why isn't it *always* treated as
elective and therefore ignored if the server can't satisfy the request?
(In other words, shouldn't you also have a "Critical Accept" option
defined?)

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

By section number:

1. The reference to [I-D.ietf-lwig-terminology] worries me, given that it
is not even in LC yet.

2.1:

   When
   a recipient is not able to process a Non-confirmable message, it may
   reply with a Reset message (RST).

Why is this? If a NON message can't be ACKed, why can it be RST? This
seems like additional machinery for the client.

2.2:

                                          the response to a request
   carried in a Confirmable message is carried in the resulting
   Acknowledgement (ACK) message.  This is called a piggy-backed
   response, detailed in Section 5.2.1.
   [...]
   If the server is not able to respond immediately to a request carried
   in a Confirmable message, it simply responds with an empty
   Acknowledgement message so that the client can stop retransmitting
   the request.  When the response is ready, the server sends it in a
   new Confirmable message (which then in turn needs to be acknowledged
   by the client).

It took me a bit to figure out why things worked this way, and I think a
sentence or two of explanation would be useful. A piggy-backed response
to a Confirmable request doesn't itself need to be confirmable because if
the ACK gets lost, the client will  re-transmit the request until it gets
the answer. When the response to a Confirmable request is not
piggy-backed, the response should itself be Confirmable, since a
Confirmable request will normally want a guaranteed response.

   Likewise, if a request is sent in a Non-confirmable message, then the
   response is usually sent using a new Non-confirmable message,
   although the server may send a Confirmable message.

"Likewise" seems wrong here. A Non-confirmable request can *not* get an
Acknowledgement message, and therefore can *not* get a piggy-backed
response. Additionally, the response MAY be Confirmable or MAY be
Non-confirmable, though certainly Non-Confirmable is the more likely
case. This should probably be reworded.

3:

   Token Length (TKL):  4-bit unsigned integer.  Indicates the length of
      the variable-length Token field (0-8 bytes).  Lengths 9-15 are
      reserved, MUST NOT be sent, and MUST be processed as a message
      format error.

Why not make TKL a 3-bit unsigned integer and have a reserved padding bit
before it? Is this protocol likely to be extended to 9-15 byte Tokens?

   Code:  8-bit unsigned integer.  Indicates if the message carries a
      request (1-31) or a response (64-191), or is empty (0).
      
As above, why not make a 3-bit field for Code Type (000=request,
001=reserved, 010=success response, 100=client error response, 101
=server error response, 110/111=reserved), and then a 6-bit Code? It
would also make the registry much easier to read.

4.2:

                                               A Confirmable message
   always carries either a request or response and MUST NOT be empty,
   unless it is used only to elicit a Reset message.
   
I don't understand the requirement not to be empty, especially when there
is an exception at the end of the sentence. Shouldn't this be "and
contains data bytes unless it is used only to elicit a Reset message."?

                                                      The
   Acknowledgement message MUST echo the Message ID of the Confirmable
   message, and MUST carry a response or be empty (see Section 5.2.1 and
   Section 5.2.2).
   
I don't understand the second MUST: What other choice is there besides
carrying a response or being empty? Aren't those the only two options?
   
                    The Reset message MUST echo the Message ID of the
   Confirmable message, and MUST be empty.

Why MUST a Reset be empty? What harm is there if there is data in there?
   
   Rejecting an Acknowledgement
   or Reset message is effected by silently ignoring it.

I don't understand the above. As far as I can tell, an Acknowledgement
nor a Reset can be rejected; the side that sent them will never know it
is rejected.
   
4.5: All of the MUSTs and MAYs in the section are used rather terribly.
I'm glad to suggest text if you need.

4.6: The phrase "and (by fitting into one UDP payload) obviously MUST fit
within a single IP datagram" is unnecessary, but even if you do use it,
s/MUST/needs to. The MUST is not a requirement on an implementation of
CoAP. That said, I fear this section is really nothing more than an
implementation note: Because it is a layer violation, it's not clear to
me that any implementer has the ability to figure out much of this. (For
example, the idea that an implementer would be willing to -- or even know
how to -- set the Do Not Fragment bit or figure out the Path MTU is a bit
hopeful.) I have no objection to this section, but it might be better as
an implementation note rather than a set of requirements.

5.2: I found "indicating a value of 4*32+3, hexadecimal 0x83 or decimal
131" just adds confusion rather than clarify. Perhaps instead: "even
though if taken as an 8-bit unsigned, the entire Response Code field
would have a value of hexadecimal 0x83 or decimal 131 (4*32+3)". But
personally, I would simply drop it; it doesn't add anything. See also
comments above on section 3 and below on 12.1.

Also:

   The response codes are designed to be extensible: Response Codes in
   the Client Error and Server Error class that are unrecognized by an
   endpoint MUST be treated as being equivalent to the generic Response
   Code of that class (4.00 and 5.00, respectively).  However, there is
   no generic Response Code indicating success, so a Response Code in
   the Success class that is unrecognized by an endpoint can only be
   used to determine that the request was successful without any further
   details.
   
First, I suggest changing the first sentence after the colon: "Response
Code details that are unrecognized by an endpoint when the class is
Client Error or Server Error  are treated as equivalent to the...". Much
clearer, and the MUST is wrong. Second, I don't see why you don't simply
define a 2.00 so that this can be a bunch simpler.

5.2.3: Potential confusion: s/the client may send a Resent message/the
client will send a Resent message

5.4.1: I don't understand the need for the third bullet. Isn't this
already said in the second bullet? The fourth bullet has the same issue
as 2.1 and 4.3.

5.4.4

   If the option is not present, the default
   value MUST be assumed.

Don't you really mean, "If an option is present with no value, the value
is the default."? The way you have it written sounds like a completely
missing option should be assumed to be present and have a default value.
I don't think that's what you mean. (The MUST is just superfluous
anyway.)

5.4.5:

   If a message includes an option with more occurrences than the option
   is defined for, the additional option occurrences MUST be treated
   like an unrecognized option (see Section 5.4.1).
   
I think you want to be specific about the order:

   If a message includes an option with more occurrences than the option
   is defined for, any additional option occurrences that appear
   subsequently in the message MUST be treated like an unrecognized
   option (see Section 5.4.1).

That is to say, you can't choose to keep later ones and discard earlier
ones.

5.4.6: If it were me, I would have put the NoCacheKey bits in the high
four bits so that you could simply do a <224 test for cache-matching
options. I suppose this ship has sailed.

5.5.1: The implementation note notwithstanding, I don't understand why
Content-Format is not a SHOULD.

5.7.1:

   If a response is generated out of a cache, it MUST be generated with
   a Max-Age Option that does not extend the max-age originally set by
   the server

The reverse would be clearer:

   If a response is generated out of a cache, the generated Max-Age
   Option MUST NOT extend the max-age originally set by the server
   
Also: "a proxy SHOULD be conservative" s/SHOULD/should. There's nothing
to implement here.

5.10.5 s/MUST/is. The MUST is not meaningful.

8.1:

   A server SHOULD be aware that a request arrived via multicast, e.g.
   by making use of modern APIs such as IPV6_RECVPKTINFO [RFC3542], if
   available.

That SHOULD is not meaningful. It is useful, not required with exceptions
(as SHOULD indicates), for the server to know that it is using
multicast.

This also gives a reason not to allow RST on *any* NON messages.

8.2: "the server MAY always pretend"? We are getting sillier with our
2119 usage later in the document. Did you really mean, "the server MAY
ignore the request"? Isn't that true of any NON request, not just
multicast ones?

(I did not do an significant review of sections 9 or 11.)

12.1: As mentioned regarding section 3 and 5.2 above, I think it would be
much easier to figure these out if you separated out the bit fields of
code type and code, and then had sub-registries for request codes,
success codes, client error codes, and server error codes.

---

From: Carsten Bormann <[email-masked]>
Date: Tue, 23 Apr 2013 08:03:39 +0200

> 1. The reference to [I-D.ietf-lwig-terminology] worries me, given that it

Point of information: This passed WGLC in LWIG just yesterday.
draft-ietf-lwig-terminology-04 has the WGLC changes.
Waiting for write-up.

Grüße, Carsten

---

From: Carsten Bormann <[email-masked]>
Date: Thu, 25 Apr 2013 13:23:50 +0200

Hi Pete,

thanks for this comprehensive review.
I have done some of the changes as simple editorial fixes, these are
marked like [1304] below and can be reviewed in
http://trac.tools.ietf.org/wg/core/trac/changeset/1304
(Overview in http://trac.tools.ietf.org/wg/core/trac/timeline).

Some of the replies are marked "-> Ticket": This means that the
authors think that the change is a good idea, but probably needs a bit
more discussion with the WG, so we will handle this as a ticket.

Grüße, Carsten

        4.1:

        If you insist on the MUSTs, make the second one "bytes of data MUST NOT
        be present". The current construction is ambiguous.

(Done the obvious editorial fix as [1304].)

        That said, I find the

There are two different kinds of MUST here.
The "MUST NOT send data" is an extension point MUST -- a later version
of CoAP MIGHT change that.  Since a 1.0 version cannot know what those
bytes might mean in a 1.1, it MUST NOT send them.
The "MUST treat as error" is an anti-soup MUST (the alternative would
be MUST ignore, but that is a recipe for soup, and might make the
extension point unworkable).

        4.3:

I think we are trying to use "reject" as a technical term here (as
used in the draft "rejecting" often does not imply actively sending a
message).  I'm open to inventing another term that is assigned the
semantics that "reject" has in CoAP right now.

Good point!

-> Ticket

The "cache key" is the informal concept that describes all the data
that go into selecting the right cache instance, such as the URI, but
also options such as "Accept".
This concept is important to allow proxies (and other caches) to
handle options they don't understand.
We probably should describe this in 5.6 and do a foward reference
here (there is little point in raising this already in 2.3, I think).

Add at the end of (the introduction to) 5.6:
»The set of options that is used for matching the cache entry is also
collectively referred to as the "Cache-Key".«
-> Ticket

The MUST NOT is an extension point MUST NOT.
The MUST ignore means that we expect such extensions to be "non-critical".

        5.10.4:

                                                             The server SHOULD

We had this discussion a couple of times between the authors and in the WG.
From a protocol point of view, you are of course right -- if the
options is elective, it can always be ignored.
From a quality of implementation point of view, this is a "if you
implement this option, this is the way we expect you to".
Sending the wrong response typically is an efficiency violation, so
this can stay at a less-than-MUST level.
We can turn this into an implementation note if this RFC 2119 usage is
too offensive.

        By section number:

(See previous note: Now in "waiting for write-up" also on the datatracker.)

        2.1:

          When

Because Observe benefits from this capability (here the sender is a
server):  The client might have asked to observe a resource, the
sender might be sending occasional updates, and then the client loses
its mind (reboots).  RST is a quick way to stop the incoming messages.

        2.2:

        the answer.

Yes, we could add that explanation.
This is at an early stage in the exposition, so we have to figure out
how to do this without generating confusion.
-> Ticket

        When the response to a Confirmable request is not
        piggy-backed, the response should itself be Confirmable, since a
        Confirmable request will normally want a guaranteed response.

Likewise -> A similar form of separate response is used...
-> Ticket

        3:

The value of the field can be 0-8, so three bits is not enough.
(The values of 9-15 are reserved and might be used for something
completely different in the future.)

This requires some surgery but is probably a good idea.
-> Ticket

        4.2:

                                                      A Confirmable message

"be empty" is a technical term here for code=0.

                                                             The

It could carry a request.

The code must be 0.

(That is the technical meaning of "reject" again.)

This is a difficult section, because it has to express all the
lenience that is needed to make very constrained implementations work.
Text gladly accepted.
-> Ticket

        s/MUST/needs to.

Done in [1305].

        The MUST is not a requirement on an implementation of

It is a bit more than an implementation note, because we are setting
expectations for message sizes here.  This is a subject that has
received quite a bit of discussion, and it seemed the current text has
just the right level of firmness.  (We really need a 6919bis for this
kind of language.)

        personally, I would simply drop it; it doesn't add anything.

I'm picking up the "8-bit" and the reordering as an editorial comment [1306].
(It is not meant to add anything, it's meant as explanatory.)
[I'd rather write those in RFC4648 base32hex.]

        See also

        Also:

Done the s/MUST be/are/ as [1306].

The non-mention of a 2.00 is intentional: 200 OK is so ingrained that,
if we ever mention a 2.00, the HTTP crowd will always send these when
we want them to send specific codes such as 2.05 etc.

[this is 5.3.2, and about "Reset" messages]
Editorial change in [1307] (added a "likely").

The second bullet is about requests, the third about responses.
(And the fourth about either.)

        5.4.4

        is the default."?

No, that is not possible.  If it is present, it has a value, and that
will be used.

        The way you have it written sounds like a completely
        missing option should be assumed to be present and have a default value.

Yes!

Sometimes redundancy is helpful.
This is a reminder that those options that do have default values
always count as "present", even if they are absent.

        5.4.5:

Good clarification.  Wordsmithed this into [1308].

Option numbers have more than 8 bits...

This is another one of these hard-to-calibrate-the-lenience things.
The "why" is right there, though:
      This is
      not a "SHOULD"-level requirement solely because it is not a
      protocol requirement, and it also would be difficult to outline
      exactly in what cases this expectation can be violated.

        5.7.1:

(This was trying to say that you probably MUST generate the option...)
Wordsmithed this into [1309].

Hmm, I'm not sure I want to get rid of this 2119 abuse here.
[Maybe change it to RFC 6919 OUGHT TO?]
But really, you may want to implement some network-delay--based fudge factor here.

You MAY consider it unfresh at any time, but MUST consider it unfresh
after Max-Age expires.  It sure means something to me.

        8.1:

This really means "A server SHOULD spend the effort needed to be
aware"... (The exception to the SHOULD being that it may not be
possible in in a RFC 3542 challenged host.)

Trade-Off.  We really want those RSTs with Observe.

Yes, but this section is giving a server specific license to discard
incoming multicast at the slightest discomfort.  We don't want to say
the same for every NON.  This certainly can be said in a better way
-> Ticket

(Yes, see ticket on 3 above.)