Re: final follow-up review of last-call

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.

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."

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?

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.

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.

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.

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.

Regards,

Jani and Anders

_______________________________________________
Rohc mailing list
[email protected]
https://www1.ietf.org/mailman/listinfo/rohc