RE: final follow-up review of last-call

Hi,

further discussion inline.

Jani Juvan <mailto:[email protected]> wrote on den 25 januari 2008
13:49 :

> Hi,
> 
> We agree and accept to all your changes with these exceptions
> (rest of the text is cut out to limit the size):
> 
> Kristofer Sandlund wrote:
> 
> 		====================
> 		Errors
> 		====================
> 
> 		Section 5.2.1:
> 		There are transition arrows missing in the
> state machine, shouldn't
> 		FC state have transistion !CRC3 !CRC7, like RC has?
> 
> 
> 
> 	NACK. It is perfectly legal to fail decompression of
> CRC3 or CRC7
> 	packets without going to a lower state. The only reason
> for dropping
> 	back is when "context damage detected".
> 
> 
> 
> We were looking for an arrow from FC to FC for !CRC3 and
> !CRC7. But feel free to omit it anyway.
> 

We will omit it. We feel the diagram is ok as it is.

> 
> 		Section 6.5:
> 		  "The format for the irregular chain item for
> the outer IP header is
> 		  also determined using one flag for TTL/Hop
> Limit and one
> 		for TOS/TC."
> 		not true any more. replace by:
> 		  "using one flag for TTL/Hop Limit and TOS/TC."
> 
> 
> 
> 	ACK
> 
> 
> The sentence following that correction still has a reference
> to "these flags" in the draft of the new draft.
> We also noticed that the description of the Dynamic chain in
> the same section incorrectly states: "The dynamic chain is
> only used in the IR header format."
> 

ACK, fixed the text.

> 
> 
> 		The formal notation part
> 		====================
> 
> 		MSN is not initialized in FN for profiles
> 0x0102, 0x0104 and 0x0108
> 		(6.3.1 says it should be initialized to a random value).
> 
> 
> 	NACK, the MSN is handled differently than other fields,
> how it is
> 	set for these profiles is handled by the english text,
> the fact that
> 	it is incremented by one is neither included in the FN code.
> 
> 
> Does this really follow formal notation? English text can be
> used to describe encoding methods and parameters, but just
> change values of control fields, really? Shouldn't it have
> been a parameter? Why not do this for more control fields then?
> 

In section 6.3, there is normative text for non-RTP profiles about
how the MSN behaves. For RTP profiles, teh FN is clear about 
behavior. We feel that the specification is then formally and 
normatively complete and correct with respect to the handling of 
the MSN. So our preferred resolution here would be to do nothing,
it is also the simplest alternative for the specification itself.

> 
> 		All global control fields should have static encoding by
default.
> 		This to ensure that they can keep their previous
> values between packets.
> 
> 
> 
> 	NACK, all the global control fields (except MSN) are such that:
> 	a) They are "user-selected" for each input packet
> 	b) They are calculated from another field
> 
> 	So there is no case that I can see where a default
> would have any
> 	impact in the spec
> 
> 
> As an example: At the decompressor the reorder_ratio_value
> parameter for the baseheader is undefined, the control field
> is also undefined and thus msn_lsb fails for some compressed
> header formats (not co_repair). In our understanding you
> don't remember values between packets unless they are static encoded.
> 

Not sure this is necessary, but we can add a DEFAULT
section on the global scope. However, this is not supported by the 
FN in RFC 4997, so we would have to add a definition that explains 
this additional FN feature. Doing anything else would make the FN 
code become incredibly ugly, and we think that this is unnecessary.

> 
> 
> 		There is a problem with how DF is handled in
> IPv6: Since DF has
> 		ULENGTH==0 for ipv6 but methods like dont_fragment and
> 		profile_1_7_flags_enc has a DF with ULENGTH==1
> in its UNCOMPRESSED
> 		format the binding will not be awesome. The
> concatenated list of
> 		flags will simply not match as intended to the
> bitpattern in the
> 		method. This problem affects all profiles and several
encoding
> methods. 
> 
> 
> 
> 	Changed dont_fragment to have the following
> UNCOMPRESSED section.
> 
> 	  UNCOMPRESSED {
> 	    df [ 0, 1 ];
> 	  }
> 
> 	Should fix the entire issue.
> 
> 
> The same change needs to be done to "callers" of
> dont_fragment. For instance "profile_1_7_flags1_enc" still
> returns DF as 1 bit... same applies for
> profile_2_3_4_flags_enc and profile_8_flags_enc.

NACK. These encodings only "return" the CLENGTH of 1 bit, which is
what we want, it never re-binds ULENGTH to 1 bit for IPv6.
No change.

> 
> 
> 
> 		udp()
> 		The control field checksum_used should have
> static encoding
> 		by default.
> 
> 
> 
> 	NACK, this field is always bound so a default has no meaning.
> 
> 
> It is not bound for the decompressor for CO headers (except
> co_repair) when parsing the irregular chain.

NACK. The irregular chain binds the UVALUE with the ENFORCE, and the
ULENGTH gets bound in the CONTROL section, so everything is bound
by the current code. No change.

> 
> 
> 		variable_scaled_timestamp()
> 		for the compressed not_present format the
> ENFORCE statement should
> 		be ENFORCE(tsc_flag == 0), the other is unnecessary.
> 
> 
> 
> 	NACK, it is an error to have both tss=1 and tsc=1 and
> therefore no
> 	format makes this a legal binding with the current ENFORCE.
> 	We had to clarify this in 4815 for ROHCv1, and I'm not
> falling into
> 	that trap any more :)
> 
> 
> Little misunderstanding here, we meant that the tss_flag is
> irrelevant as long as tsc_flag is zero. Because that is
> exactly what the current ENFORCE says. Either way works.

No change it is, then.

BR,
    Kristofer & Ghyslain

> 
> 
> Regards,
> Jani and Anders