Draft:   draft-pullen-srmp-06.txt
Reviewer: Elwyn Davies
Review Date: Thursday 7/14/2005 9:01 AM
Telechat Date: 7 July 2005  (updated post chat)

Summary: Almost ready.

Review Comments:
This document is close to being ready as an Experimental protocol 
specification.  There are a couple of points which deserve some 
attention before it is published in my opinion:
- Wrap around and comparison of timestamps - 16 bit timestamps with 
millisecond resoution are proposed but there is no discussion of wrap 
around. This is a fairly standard problem but a quick comment would help.
- The trade-off between bit saving in the headers and complexity may 
have come down too heavily on the side of bit saving.  Consequently a 
number of different floating point formats packed into 16 bits are 
specified, which may result in significant implementation complexity... 
but its an experimental protocol, so maybe that is one of the things to 
sort out during experimentation.

> From the point of view of 'potential damage to the Internet', I am not 
familiar with the details of the Widmer/Handley multicast congestion 
control work which is being implemented in a modified form here, but I 
assume that this work is likely to be of good quality which is being 
properly discussed in an IETF WG.  So even though this is a modified 
form it should not cause significant rampant congestion, and good 
experience will be obtained.


This is an updated version of the review with the editorial nits included.
Review:

Editorial nits:

Wrong boilerplate.

Abstract: The Abstract is overly long: guidelines indicate about 10-15 
lines of text.  Suggest reducing 2nd and 3rd paras to: SRMP has two 
sublayers: a bundling sublayer handling message aggregation and 
congestion control, and a selectively reliable (SRT) transport 
sublayer.  Selection between reliable and best-efforts messgaes is 
performed by the application.

s1: A reference for Cohen's work would be helpful
s/predominating flow/large majority/
RFCs 2357, 2887 and 3450-53 should be refs.

s2: s/strategy of SRMP/strategy for SRMP/
s/DataId>/dataId> combination/
Maybe acronyms for Mode 0/1/2 messages might make the types more memorable.
(below diagram) s/any of Mode 0/any number of Mode 0/
s/that provides that models/that provides behavior that approximates/???
s/stream of state update/stream of state updates/
feedback implosion => need to define what is meant by this term.
s/netwrok/network/
s/under applications that needs/by applications that need/
?? s/The UDP port is/A separate UDP port is used for each instances and 
the port is/
s/,thus it/. Accordingly, it/

s3.1: s/fo the/destined for the/
s3.2: Type field is not labelled in header layout diagram, and seems to 
indicate 3 bits of 0's rather than 4 from the text. (Type comment 
applies to s3.3, s3.4, s3.5 and s3.6 also)
timestamps - milliseconds refers to the resolution of the timestamp. 
Need to discuss about wrap round??
s/corresponding with/corresponding to/
Length: in octets presumably?
Should explain that DSN's only apply to Mode 0 messages so count of 
DSN's doesn't tell how many messages in bundle.  Would it be useful to 
tell how many messages are in the bundle?

s3.3: X_r is defined twice (second one is probably authoritative but I 
am not certain they are actually the same field).
Receiver Timestamp:  Millisecond resolution  presumably but needs to be 
specified.

s3.4: Field lengths don't add up to 32 bits.
Length in octets???

s3.5: Length in octets???

s3.6: Length in octets?

s3: discussion of padding vs use of reserved?  Allows fields to be used 
later.

globally:s/MUST not/MUST NOT/, s/SHALL not/SHALL NOT/
s4.2: Should clarify that bundling is optional?  This is why we have 
'should keep a timer'.  If bundling is not doen then don't neeed the 
imer and every message is transmitted individually (I assume).

s5: s/Dataid/dataId/ globally

s5.2.2: Behaviour on receipt of a duplicate segment number?  Should drop 
whole packet?

s5.3.1: Presumably if any more messages arrive to be sent, one shouldn't 
wait until the earlier ones are sent?  And certainly we shouldn't wait 
for the ACK to come back before sending other messages I assume? Some 
words on this would be a good idea.

s 6.1; para 2: s/ constraints delivery/constrains delivery/
s6.2:s/losses emulated losses/losses emulated/

s6.2: para 3: Would it be desirable to explicitly limit the rate of 
acceptance of Mode 2 traffic by SRMP (e.g., by using a token bucket of 
some kind) to enforce the ratio assumptions?