Subject: [core] AD review of draft-ietf-core-coap, part 1

From: Barry Leiba <[email-masked]>
Date: Sun, 16 Dec 2012 19:28:30 -0500

I've started my AD review of draft-ietf-core-coap during its working
group last call.  With a lot of other things to review, it's not going
as quickly as I'd like... but I want to stress that this is due to my
workload, and is NOT the fault of the document, which is excellently
written.  Well done, everyone.

So far, I've gotten through the end of Section 5.7.3, and am about to
start on Section 5.8.  I wanted to post what I have so far, so the
authors can have those comments now and anyone who has reactions to
them can reply.

General:
Please check for consistency in capitalization of "Non-Confirmable".
You have a mixture of "Non-Confirmable" and "Non-confirmable", even
within the same paragraph in places, as well as a few instances of
"non-confirmable".  Check for "Confirmable" vs "confirmable" also (and
"Acknowledgment" and "Reset").  Also look for "Message ID" vs
"message-ID" (capitalization and hyphenation).

I think you need quite a few more forward references than you have.
In general, if you talk about something that hasn't been explained and
won't be for some while, there should be a forward reference for it.
For example, in Section 5.7.3 you say that reverse proxies need to be
careful about how they handle the ETag option, and you go into some
bit of detail.  But the ETag option is not explained until 5.10.6.  A
forward reference would be useful.  Look around for these, and figure
that extra references aren't bad, but missing ones can leave readers
wondering.

-- Sec 3.2 --

   string:   A Unicode string which is encoded using UTF-8 [RFC3629] in
             Net-Unicode form [RFC5198].

I don't think you need to mention 3629 at all, and you certainly don't
need it as a normative reference.  5198 covers everything you need,
and can serve as your reference for UTF-8.  It has, itself, a
reference to 3629.

This also applies to Section 5.5.2.  In Section 6.4, you can also take
out the reference to 3629 (3986 also has one, if anyone needs it).

-- Sec 4.3 --

You have four types of messages: Confirmable, Non-Confirmable, Ack,
and Reset.  Here you make it clear what to do with Non-Confirmable
messages: "A Non-confirmable message MUST NOT be acknowledged by the
recipient."  It's clear that Ack and Reset messages are different
message types -- they are neither Confirmable nor Non-Confirmable...
but you require that implementors infer that they, too, are not
acknowledged.

It would probably be useful to make that explicit, probably by adding
a short paragraph to the end of Section 4.3, thus:

   Acknowledgment and Reset messages are different types of messages,
   distinct from both Confirmable and Non-Confirmable messages.  They
   are treated similarly to Non-Confirmable messages in that they
   MUST NOT themselves be acknowledged by the recipient.

-- Sec 4.4 --
The initial variable value should be randomozed... Why?

-- Sec 4.8.1 --
I'm always cautious about using "below", because people aren't always
sure which things "below" are relevant.  For "(see discussion below)"
and "(see below)" in the last two paragraphs, I'd say, "(see Section
4.8.2)".

   ensure the adjusted value is also available to all the endpoints in
   communicating with which these adjusted values are to be used.

This is hyper-silly application of the non-rule rubbish that we
shouldn't oughta end sentences with prepositions
(http://en.wikipedia.org/wiki/List_of_common_English_usage_misconceptions).
 It's terribly hard to follow.  I say throw it out.  Take it away.
What are you waiting for?

That is, just say "to all the endpoints that these adjusted values are
to be used to communicate with."

-- Sec 4.8.2 --
In the formula for EXCHANGE_LIFETIME, would it not be good to
substitute in MAX_TRANSMIT_SPAN instead of the first line, especially
because you say in the text that that value is included? That will
also make it easier to see how it compares with NON_LIFETIME.

-- Sec 5.2 --

   After receiving and interpreting a request, a server responds with a
   CoAP response, which is matched to the request by means of a client-
   generated token.

This is probably a good place to make clear what the difference is
between Message ID and Token: that the Ack of the request is matched
to the request by Message ID, but the response to the request is
matched to the request by the Token.  In cases where the response is
separate from the Ack (see 5.2.2) or where there is no Ack (see
5.2.3), this is an important distinction.  Some text along those
lines.  No?

NEW
   After receiving and interpreting a request, a server responds with a
   CoAP response, which is matched to the request by means of a client-
   generated token, a Message ID (see Section 4.4).
END

   The upper three bits of the 8-bit Response Code number define the
   class of response.  The lower five bits do not have any
   categorization role; they give additional detail to the overall class
   (Figure 9).  There are 3 classes:

I suggest moving the paragraph a couple of paragraphs down -- the one
that begins "As a human readable notation" -- up to here, so the
documentation format is explained before it's used.  I also suggest
that that paragraph is a bit wordier than necessary.  Maybe this?:

NEW
   The upper three bits of the 8-bit Response Code number define the
   class of response.  The lower five bits do not have any
   categorization role; they give additional detail to the overall class
   (Figure 9).  The response code is documented in the format "c.xx",
   where "c" is the class represented as an integer, and "xx" is the
   detail represented as a two-digit integer.  For example, for the
   response code "10000011" (defined as "Forbidden"), "c" is "100"
   and "xx" is "00011", and the code is written as "4.03".

   There are 3 classes:
END

   Responses can be sent in multiple ways, which are defined below.

As with Section 4.8.1 on "below": here I'd say, "Responses can be sent
in multiple ways, which are defined in the following subsections."

-- Sec 5.2.1 --

   The response is returned in the Acknowledgement message independent
   of whether the response indicates success or failure.  In effect, the
   response is piggy-backed on the Acknowledgement message, so no
   separate message is required to both acknowledge that the request was
   received and return the response.

The second sentence feels convoluted, trying to say the same thing too
many times.  Maybe this is better?:

NEW
   The response is returned in the Acknowledgement message independent
   of whether the response indicates success or failure.  In effect, the
   response is piggy-backed on the Acknowledgement message, and no
   separate message is required to return the response.
END

-- Sec 5.2.2 --

Nit:
   Responses to requests
   carried in a Non-Confirmable message are always sent separately (as
   there is no Acknowledgement message).

Match the numbers right:
NEW-1
   Responses to requests
   carried in Non-Confirmable messages are always sent separately (as
   there are no Acknowledgement messages).
...or...
NEW-2
   The response to a request
   carried in a Non-Confirmable message is always sent separately (as
   there is no Acknowledgement message).
END

      there is no underlying transport protocol that could be instructed
      to run a keep-alive mechanism, the requester MAY want to set up a
      timeout that is unrelated to CoAP's retransmission timers in case
      the server is destroyed or otherwise unable to send the response.)

This doesn't feel like a proper 2119 MAY (the "may want to"
construction is one clue).  I would lower-case it.

   An exchange is separate by definition when the Acknowledgement to the
   Confirmable request is an empty message.  The Acknowledgement to the
   Confirmable response MUST also be an empty message, i.e. one that
   carries neither a request nor a response.  However, a server MUST
   stop retransmitting its response on any matching Acknowledgement
   (silently ignoring any response code or payload) or Reset message.

I'm a little confused at what this is trying to tell me.  Is this
giving one possible scenario, or is this telling me definitively how
to detect that the server is separating the response from the ack?  I
think it's the latter, but that's not how it seems to read.

If that's what you mean, how about this?:

NEW
   When the server chooses to use a separate response, it sends the
   Acknowledgement to the Confirmable request as an empty message.
   If the server then sends a Confirmable response, the client's
   Acknowledgement to that response MUST also be an empty message,
   (one that carries neither a request nor a response).  The server
   MUST stop retransmitting its response on any matching
   Acknowledgement (silently ignoring any response code or payload)
   or Reset message.
END

I think I might also move this up above the implementation note.

-- Sec 5.2.3 --
Isn't the second sentence really saying something more general, beyond
what this section is for?: that any endpoint at any time MUST be
prepared to receive either a Confirmable or Non-Confirmable response
to either a Confirmable or Non-Confirmable request.  In other words,
whether a response is or is not Confirmable can be unrelated to
whether the request was Confirmable or not.  Should that be said
somewhere above, somewhere in Section 4, as a more general statement?

-- Sec 5.3 --

   An endpoint receiving a token it did not generate MUST treat it as
   opaque and make no assumptions about its format.

I'd say "make no assumptions about its *content*.  This also applies
to Section 5.10.6.

-- Sec 5.3.2 --
Is it true, and is it worth mentioning, that the determination and
matching of endpoint identifiers is specific to the underlying
transport and is out of scope for this specification?  The reason I
ask is that I want it to be clear that, while for UDP this is done by
comparing the IP addresses and ports of the endpoints as given in the
transport headers, it might be done differently in a system that, for
example, identifies an endpoint through means other than IP address.

Along that line:
   In case a message carrying a response is unexpected (i.e. the client
   is not waiting for a response at the endpoint addressed and/or with
   the given token), the response is rejected (Section 4.2,
   Section 4.3).

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

-- Sec 5.4.2 --

   In addition, for options that are marked Safe to Forward, the option
   indicates whether it is intended to be part of the Cache-Key in a
   request (NoCacheKey is not all set) or not (NoCacheKey is set).

"NoCacheKey is not all set" seems a little convoluted, especially
because NoCacheKey hasn't been introduced or explained yet.  Maybe
this is better?:

NEW
   In addition, for options that are marked Safe to Forward, the option
   indicates whether it is intended to be part of the Cache-Key in a
   request (some of the NoCacheKey bits are 0) or not (all NoCacheKey
   bits are 1; see Section 5.4.6).
END

As I look ahead, I see that there is nowhere an explanation of why
there are three bits to represent NoCacheKey.  Why is that?  (Both:
why are there three bits to represent it, *and* why is it not
explained in the document?)

-- Section 5.4.5 --

   The definition of an option MAY specify the option to be repeatable.
   An option that is repeatable MAY be included one or more times in a
   message.  An option that is not repeatable MUST NOT be included more
   than once in a message.

The first MAY should not be a 2119 key word (it's talking about what's
in this document, not about the protocol).  I'd change it to, "The
definitions of some options specify that those options are
repeatable."

-- Section 5.5.1 --

   For responses indicating a client or server error, the payload is
   considered a representation of the result of the requested action
   only if a Content-Format Option is given.  In the absence of this
   option, the payload is a Diagnostic Payload ({{diagnostic-message-
   payload}}).

What's that at the end?

-- Section 5.7.3 --

   In processing the response, a reverse-proxy has to be careful about
   namespacing the ETag option.

"Namespace" as a verb?  Hm.  I'm sure I don't know what "namespacing
the ETag option" means.  Can you try re-wording?

---------------------
That's all for now.  I'll post again, starting with Section 5.8, after
I get another chunk done.

Barry, responsible AD

---

From: Barry Leiba <[email-masked]>
Date: Sun, 16 Dec 2012 22:48:39 -0500

>   generated token, a Message ID (see Section 4.4).

That was spurious junk that got left in there from when I was playing with
some text earlier.  Please trash it, and don't let it confuse anything.

Barry