Subject: [rohc] draft-ietf-rohc-sigcomp-06

From: "West, Mark (ITN)" <[email-masked]>
Date: Wed, 15 May 2002 14:04:23 +0100

Hi all,

Herewith some review comments on draft-ietf-rohc-sigomp-06...

There are a few nits and a handful of comments and questions, but 
nothing very serious. Overall I think that it looks pretty good.  (And 
especially the section on bit-ordering ;-)

Here goes...

Figure 1 - split across pages 5/6

3.3, page 10
1st para: "an endpoint _generally_ behaves in a predictable manner". In 
what way can it be unpredictable?

3rd para: "locally available state" -> "locally_available_state"

3.3.1, page 10
Is there any upper limit on decompression_memory_size?

Page 11
Does cycles_per_bit refer to compressed or uncompressed bits?
(I think that this is explicit later, but it worth clarifying here, 
even if it is obvious!)

3.3.2, page 12
Regarding backwards compatibility, should this read "then it MUST be"?

4.2.1, page 14
"In general, this alternative is an implementation choice." -> "In 
general, the strategy used is an implementation choice."

6.2, page 20
Point 5 and the 2nd last paragraph seem to duplicate text about the use 
of retention priority 65535

7.1, page 22
Why is the 'T' bit called the 'T' bit?
Why does the length of the feedback exclude the length field?

Figure 4, diagram split across pages 22/23

7.2, page 23
1st para, is the uniqueness criterion the sole basis for the compressor 
to choose how many bytes of state-identifier to send?

3rd para: "in every currently stored state item" -> "in every currently 
stored state item within the compartment"
(Assuming that state is matched 'per compartment')

Page 24
1st para: is it worth making a more general statement somewhere about 
the storage of 16-bit values in the UDVM memory (i.e. MSB precedes LSB)?

Page 25
1st para: There is some repetition of text about the 'no special 
significance' of the 'useful values' area

7.3, page 25
Would it be clearer to have a diagram to indicate the code_len and 
destination address fields?

8, page 26
How would you define 'typical' and 'often' in the context of 'sub 100 
byte algorithms'?

8.1, page 27
Is it worth adding a note about how registers are initialised (i.e. that 
they should be!)?

8.2, page 27
Does the decompressor dispatcher have to buffer the entire message prior 
to invoking the UDVM?  (It states explicitly elsewhere that it doesn't, 
but the 1st paragraph of 8.2 might be slightly misleading in this case).

The 'reserved' bits in the input_bit_order register are only reserved 
for version 0x01 of the UDVM.  (Last para of this section should also 
account for this in checking that the value is in the range 0 - 7).

8.7, page 33
Point 3: "more that the" -> "more than the"

Point 6: What is an 'unknown operand'?

9, page 34
I count 36 instructions, not 33!

9.1.3, page 37
Is the restriction that the sort be natural (i.e. preserving original 
order) a useful/necessary one?

9.2.3, page 39
Description of POP restates the definition of "and then".

9.2.4, page 39
What happens if the start of the source / destination lie outside of the 
range specified in byte_copy_left, byte_copy_right?

9.2.5, page 40
The trailing ", if any" makes it slightly unclear (I think) what happens 
if no data is copied.  Is it worth explicitly stating that destination 
is unchanged in this case?

9.3.5, page 42
last para: "continues at the following" -> "continues instruction 
execution at the following"

9.4.4, page 44
Point 2: reference to 8.1 should be to 8.2

Is there any particular reason why decompression failure is called if 
state_length is 0 but state_begin is non-zero?

The last paragraph describing the address at which execution resumes is 
a little confusing.  It makes sense, but I had to read it a couple of 
times to make sure that I got it...

9.4.6, page 46
"Decompression failure must occur" -> "Decompression failure MUST occur"

9.4.7, page 46
"Decompression failure must occur" -> "Decompression failure MUST occur"

9.4.9, page 47/8
Given the definition of STATE-CREATE, and the fact that multiple states 
can be created, why is it necessary for the END-MESSAGE instruction to 
also be able to create state?

The processing of STATE-FREE requests states that "upon reaching the 
END-MESSAGE instruction these values are used to byte copy..."
Are the values copied at the point of STATE-FREE execution or at the 
point of calling END-MESSAGE (requiring the state identifiers to be 
preserved in UDVM memory)?

Figure 12, split across pages 48/9

Figure 13, split across pages 50/1

10.1, page 52
"as it always can be circumvented" -> "as it can always be circumvented"

That's all, folks..!

Cheers,

Mark.

-- 
Mark A. West, Consultant Engineer
Roke Manor Research Ltd., Romsey, Hants.  SO51 0ZN
Phone +44 (0)1794 833311   Fax  +44 (0)1794 833433

(Yes, I do know that my disclaimer is in an attachment.  And, no, I 
didn't ask for it to be that way)

---

From: Carsten Bormann <[email-masked]>
Date: Wed, 15 May 2002 23:54:21 +0200

Hi Mark,

thanks for your comments!
Now let me pick them apart :-)

> what way can it be unpredictable?

It might have a bad RAM chip.

No, really: It might want to change state_memory_size when it runs into 
memory shortage.  This is not predictable for the compressor, so it's 
really bad behavior, but it is one strategy for graceful degradation, so 
we didn't want to rule it out.  This unpredictability can be handled 
with feedback (in optimistic or in pessimistic ways).

This discussion MUST go into the UDVM user guide...

> 3rd para: "locally available state" -> "locally_available_state"

I think this is not a parameter with a single value per se, but rather 
was meant to be descriptive for the set of locally available state.
(Note that there is no other mention of "locally_available_state" either.)

> Is there any upper limit on decompression_memory_size?

practically, it's 128K, because we can't announce higher values in 
feedback with the current encoding.  That allows for a 64K compressed 
(!) message and a 64K UDVM, which should be fine (and there's always 
continuous mode if it's not).

> (I think that this is explicit later, but it worth clarifying here, even 
> if it is obvious!)

Definitions says:

SigComp message

A message sent from the compressor dispatcher to the decompressor
dispatcher. In case of a message-based transport such as UDP, a
SigComp message corresponds to exactly one datagram. For a stream-
based transport such as TCP, the SigComp messages are separated by
reserved delimiters.
<<

Then 3.3.1 goes on:

The cycles_per_bit parameter specifies the number of "UDVM cycles"
available to decompress each bit in a SigComp message.
<<

That's explicit enough for me.  Do you have a proposal for better wording?

> Regarding backwards compatibility, should this read "then it MUST be"?

This is a promise by the IETF, not a mandate on the implementers...
(In future versions, it might become one.)

> general, the strategy used is an implementation choice."

Sure. (My bad -- I tend to use the original 1624 meaning of 
"alternative" and forget that this has been pretty much lost in the 
mists of time.)

Even clearer:

In general, which of the strategies is used is an implementation choice.

> of retention priority 65535

Repetitions are good for learning :-)
I think the two sentences address different things, though, so I would 
really like to leave them as is.

> Why is the 'T' bit called the 'T' bit?

Beats me.  (The -extended people probably can remember.)
Why didn't you ask why the F-bit is not a B-bit?

> Why does the length of the feedback exclude the length field?

Because this is good protocol practice.
One less check to code.

(This design rule became painfully obvious quite a while ago: There was 
a bug in 4.2BSD once where a length field of zero caused the 
implementation to advance the pointer by zero bytes and so to examine 
the same field again and again in a tight loop -- in kernel mode. I'm 
not online right now and so can't look it up -- great story for my 
students though...)

Also, it gives you one more byte you can put there :-)

> to choose how many bytes of state-identifier to send?

I believe so (actually, it's a tradeoff between this and the compressed 
size).  Of course, this includes the security considerations.

> (Assuming that state is matched 'per compartment')

No, it can't -- we don't know the compartment until *after* the message 
is decompressed.  That is the main reason the whole state thing is more 
complex than it looked on the napkin we were designing it on...

Should we add words to make this clearer?

> the storage of 16-bit values in the UDVM memory (i.e. MSB precedes LSB)?

Well, the sequence of things in the document is slightly strange:
7 is about the message format, 8 introduces the UDVM.
For some reason, the "useful values" slipped into 7.
I'm not sure I want to fix this now, though.

> significance' of the 'useful values' area

Here again, I believe repetition is good -- this is a note, anyway.

> destination address fields?

I thought we had that -- in the intro to section 7.

> How would you define 'typical' and 'often' in the context of 'sub 100 

I believe the first two paras of a new section are allowed to be ad copy 
   even in an RFC :-)

> Is it worth adding a note about how registers are initialised (i.e. that 

There are initialized with zeros, as all UDVM core except for the Useful 
Values.
If that is not good for a decompressor, it needs to provide its own 
initialization.

(I gather you want these points made in a note?)

> but the 1st paragraph of 8.2 might be slightly misleading in this case).

Hmm. Fix how?

> account for this in checking that the value is in the range 0 - 7).

Isn't that true for all "reserved" fields?
The whole spec is about 0x01!
(I.e., is there a change that needs to be done in many places?)

The last para could have a "In UDVM version 0x01, ..." added at the front.

> Point 3: "more that the" -> "more than the"

Fixed.

> Point 6: What is an 'unknown operand'?

An operand that cannot be decoded according to the current bit 
assignments. (Hmm, is that less clear than "unknown instruction"?)

> I count 36 instructions, not 33!

Oops.  These parts of the text should be automatically generated, but 
trying to respect the RFC-editor's minimal usage of nroff I didn't. 
Next version will be run through m4 before nroffing :-)

> order) a useful/necessary one?

Yes, we discussed this.  If you don't fully specify the behavior, the 
compressor cannot use the result as a decompression table.  We don't 
really care for "stable" sorting (this is the term I learned for this), 
but it is the simplest behavior to specify (and there are good 
algorithms that do this).

> Description of POP restates the definition of "and then".

Repetition is good...

> range specified in byte_copy_left, byte_copy_right?

Not much that would be surprising from 8.4:

Set k := m + 1 (modulo 2^16)
If k = byte_copy_right then set n := byte_copy_left, else set n := k

If byte_copy_right is not hit, there is no effect.

> is unchanged in this case?

append "(i.e., if the value of the length operand is zero, the value of 
the destination operand is not changed.)"?

> execution at the following"

Sounds good to me.

> Point 2: reference to 8.1 should be to 8.2

Good catch.  Applies to 9.4.3, too.

> state_length is 0 but state_begin is non-zero?

Hmm.  Must have made sense to me once, but does not make sense to me now 
-- but it's been a 20-hour working day...

> times to make sure that I got it...

The text could probably be improved considerably by introducing 
derived_state_length, derived_state_instruction and 
derived_state_address and referring to those where these are meant 
instead of the operands.

> "Decompression failure must occur" -> "Decompression failure MUST occur"

Good points.

> also be able to create state?

I don't like that either, but then, it's been the choice of the majority 
of the authors, and it does not really make a difference.

> preserved in UDVM memory)?

The values of the operands are stashed away into some hidden UDVM 
storage during STATE-FREE, the things they point to are not.

"the UDVM
simply buffers the two supplied operands until the END-MESSAGE
instruction is reached"

Idea for a wording change?

> Figure 13, split across pages 50/1

Sorry I didn't bother to fix these -- RFC editor will impose a new page 
layout anyway.

> "as it always can be circumvented" -> "as it can always be circumvented"

Wouldn't that become a "split infinitive", a bane of those English 
teachers I never had :-)?

Again -- thanks for the thorough review and the good questions, Mark!

I have made a few of the most obvious editorial changes in the CVS 
version; let's have some more discussion of the others, and I'll do the 
rest of the edits.

Gruesse, Carsten

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