Last Call Review of draft-ietf-p2psip-diagnostics-19
review-ietf-p2psip-diagnostics-19-genart-lc-melnikov-2015-12-15-00

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 wait for direction from your
document shepherd or AD before posting a new version of the draft.

For more information, please see the FAQ at

<

http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.

Document: draft-ietf-p2psip-diagnostics-19
Reviewer: Alexey Melnikov
Review Date: 2015-12-15
IETF LC End Date:
IESG Telechat date: 2015-12-17



Summary: Nearly ready for publication as Proposed Standard

I think this document has a list of issues, but they should be easy to fix:

In Section 5.3:

The dMFlags field described above is a 64 bit field that allows
   initiator nodes to identify up to 62 items of base information to
   request in a request message (the first and last flags being
   reserved).

But the IANA registration section uses flags 1 and doesn't seem to
reserve the highest bit either. If this text is now wrong, it should be
deleted. If the IANA section is wrong, please fix it. If I am wrong,
please tell me :-).

In Section 5.3:

SOFTWARE_VERSION: A single value element containing a US-ASCII
      string that identifies the manufacture, model, operating system
      information and the version of the software.  Given that there are
      very large number of peers in some networks, and no peer is likely
      to know all other peer’s software, this information may be very
      useful to help determine if the cause of certain groups of
      misbehaving peers is related to specific software versions.  While
      the format is peer-defined, a suggested format is as follows:
      "ApplicationProductToken (Platform; OS-or-CPU) VendorProductToken
      (VendorComment)".  For example: "MyReloadApp/1.0 (Unix; Linux
      x86_64) libreload-java/0.7.0 (Stonyfish Inc.)".  The string is a
      C-style string, and MUST be delimited by "\0".

Did you mean "terminated"? I don't see what can be delimited, as this
implies presence of multiple fields.

      "\0" MUST NOT be
      included in the string itself to prevent confusion with the
      delimiter.



EWMA_BYTES_SENT (32 bits): A single value element containing an
      unsigned 32-bit integer representing an exponential weighted
      average of bytes sent per second by this peer. sent = alpha x
      sent_present + (1 - alpha) x sent where sent_present represents
      the bytes sent per second since the last calculation and sent
      represents the last calculation of bytes sent per second.  A
      suitable value for alpha is 0.8.  This value is calculated every
      five seconds.


I don't understand the formula. What is "x"?
Should the formula be on its own line for ease of understanding?

BATTERY_STATUS

This flag doesn't seem to be defined in a useful fashion. You need to at
least provide some guidance here to insure interoperability.

In Sections 9.3-9.5: is RFC-AAAA this document or RFC 6940 (or something
else)?

Thank you,
Alexey