Subject: [rohc] RE: Second WG last-call for SIP/SigComp - dedicated review

From: "Surtees, Abigail" <[email-masked]>
Date: Mon, 12 Feb 2007 17:07:03 -0000

Hi Gonzalo,

Apologies for the delayed response.  Comments in-line: 
> 
> yes, abstracts should not have normative language. However, 
> you are right pointing out that the second 'must' should be 
> normative. I have moved that text from the Introduction to a 
> new section right after the Terminology section and I have 
> made that 'must' a normative MUST.
> 
> Of course, specifications should be followed once they are published. 
> Therefore, legacy implementations will, obviously, not be 
> compliant with this document. That is why the document 
> contains a Backwards Compatibility section explaining the situation.
> 
> Regarding implementations confirming to other specs, the 
> section on Provate Agreements covers that.
> 
OK

> we do not use normative language because support for RFC 3485 
> is already normatively mandated in RFC 3485, and support for 
> RFC 3586 is already normatively mandated in RFC 3486. We 
> point to those RFCs, but do not repeat the normative 
> statements here (this is common practice when writing RFCs).
> 

OK

> > Section 3.1:
> > I know the 'reason' part here but I think it should be written more 
> > clearly (I think if you were new to it you'd have to read 
> it several 
> > times).  Add another sentence to explain the example.  Also, the 
> > SigComp user guide refers to 'circular buffer' not 'ring 
> buffer' so it 
> > may be worth using 'circular buffer' here (ring buffer isn't used 
> > anywhere else).
> 
> I have replaced 'ring buffer' with 'circular buffer'.
> 
OK.

> I will be happy to add the clarifying sentence you request if 
> you provide me with it.
> 
"For example, the
   condition set by [RFC3320] for SigComp over UDP means: C + 2*B + R +
   2*S + 128 < DMS (each term is described below).  On the other hand,
   8KB additional memory should not cause any problem for an endpoint
   that already implements SIP, SigComp, and applications that use SIP
   as DMS is memory only temporarily needed during decompression of a
   SigComp message (the memory can be reclaimed when the message has
   been decompressed)."

Rephrased slightly to:

"For example, the
condition set by [RFC3320] for SigComp over UDP means: C + 2*B + R +
2*S + 128 < DMS (each term is described below), so if DMS is too small
then at least one of C, B, R and S will be severely restricted.  On the
other hand, DMS is memory which is only during decompression of a
SigComp message (after which it can be reclaimed); therefore, a
requirement of 8Kb should not cause any problem for an endpoint that
already implements SIP, SigComp, and applications that use SIP."

> 
> > Section 3.2:
> 
> I have performed the following change:
> 
> OLD:
> Reason: a non-zero SMS allows an endpoint to upload a state in the
> first SIP message sent to a remote endpoint without the uncertainty of
> whether or not it can be created in the remote endpoint.
>          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
> NEW:
> Reason: a non-zero SMS allows an endpoint to upload a state in the
> first SIP message sent to a remote endpoint without the uncertainty of
> whether the remote endpoint will have enough memory to store such a
>          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
> state.
> ^^^^^^
> 
OK

> > Section 3.5:
> > Add a statement that because the static dictionary is 
> mandatory it does
> > not need to be advertised (last point in this email discussion
> > http://www1.ietf.org/mail-archive/web/rohc/current/msg04463.html).
> 
> I have added:
> 
> "Note that, since support for the static SIP/SDP dictionary is
> mandatory, it does not need to be advertised."
> 
OK

> > Section 8.2
> > Re: Use of IP address and port/connection for the remote application
> > identifier for legacy systems.  If a client fails (losing 
> all SIP state
> > including SigComp compartment) and re-REGISTERS with the same IP
> > address, how can the SIP stack tell that the old 
> compartment is invalid.
> > I realise that the answer is probably that this circumstance is
> > relatively unlikely and that NACK would recover the situation, but I
> > think this should be made clearer.
> 
> Implementors are made aware of this type of problem related to legacy 
> systems in the section about Backwards Compatibility.

Hmm, ok.  This one isn't explicitly listed, but the lack of NACK is and
it's when NACK isn't present that the problem would be more major.
> 
> > Section 8.3 
> > I think this section should be combined with section 8.4.  The
> > definition of when compartments should be opened and closed 
> needs to be
> > stronger.  In this section, section 8.4 and section 12, 
> there are strong
> > echoes of the fact that it used to be possible to have different
> > compartment lifetimes.
> > I think there should be standards language (probably 
> SHOULDs) stating
> > when compartments should be opened and closed.  i.e. 
> compartments SHOULD
> > last the lifetime of the registration, but leaving the 
> possibility for
> > early closure and NACK recovery in extreme cases (e.g. overload
> > conditions).
> 
> I have combined Sections 8.3 and 8.4, as you suggest.

(now 9.3) OK

> 
> The document already contains normative language regarding when one 
> should open and close compartments. Regarding the MAY strength on 
> compartment closure, we need to leave the door open for 
> implementations 
> to keep states after the registration expires.
> 
> With respect to your overload example, the document already says that 
> implementations SHOULD NOT close the compartments before the 
> registration expires. Note that we do not use MUST NOT.

OK
> 
> > Section 8.3 (Paragraph about keeping state beyond the end of a
> > compartment):
> > This paragraph still refers to compartments lasting the length of a
> > dialog or transaction!
> > How useful is the technique described in this paragraph, now that a
> > compartment lifetime is a registration? 
> 
> I have removed the text talking about dialogs and 
> transactions. Per my 
> comment above, we do not need to force implementations to close their 
> compartments when the registration expires. If they want to 
> do it, they 
> can do it, but if they don't, they do not harm anyone by not 
> closing the 
> compartment.
> 
OK
> 
> > Section 8.4
> > See comment on section 8.3 about strengthening compartment 
> definition.
> > I also think the security implications of opening a compartment on
> > receipt of a REGISTER should be mentioned.  Compartments 
> and state can
> > only be created if the application says so, but, particularly if the
> > decompressing node is a proxy, the non-2xx response may 
> take a while to
> > be generated.
> 
> Non-2xx responses for REGISTER requests are generated immediately.

But, if the response is not generated at the proxy, then surely it's
going to take some time for the response to come back from the server?

> > Section 8.4
> > "However, applications MAY also choose to keep these
> >    compartments open for a longer period of time, as discussed
> >    previously."
> > Does the 'as discussed previously refer to the 'paragraph 
> about keeping
> > state beyond the end of a compartment'?  If so, then i) I'm not sure
> > it's that useful and ii) the wording implies something 
> different.  If
> > not, then what does it refer to?
> 
> it refers to the text you point to and, per my comments 
> above, I believe 
> it is indeed a useful feature.
> 
OK
> 
> > Section 8.5
> > I was surprised to see this section (but maybe I missed 
> it's discussion
> > on the list).  I can understand softening the 'SHOULD' in 
> RFC3486, but I
> > don't think it should be a 'SHOULD NOT' - it depends on the 
> stateless
> > algorithm in use and the size of the message being 
> compressed.  With a
> > stateless algorithm that has short bytecode and a large 
> message (e.g.
> > INVITE) it is possible to still achieve non-trivial 
> compression gain.
> 
> Theoretically, you are right. Practically, no one intends to do that. 
> Note that revision 03 of this draft covered every single scenario you 
> could eventually implement. The conclusion was that the spec needs to 
> focus on realistic scenarios only; this way, it becomes a 
> simpler spec. 

In general, regarding the simpler spec, I agree with you.  However, in
this particular instance, I don't see the need to restrict the
implementer in this way.  Granted, it is a SHOULD NOT rather than a MUST
NOT, but if the implementer can implement a stateless algorithm that
achieves compression (at least for some messages) why should we
recommend that they shouldn't do that?  Also, how do you know that in
practice no-one intends to do that?

Having looked at where the quoted text appears in RFC 3486, it is not
related to stateless compression or compression without a compartment.
So I'm still not sure why this paragraph is in this document.

If there is really a need for a statement (which I don't think there
is), then I think it should be no more than:

"RFC 3486 [RFC 3486] states that:

      "If the next-hop URI contains the parameter comp=sigcomp, the
      client SHOULD compress the request using SigComp"

Although this states that messages SHOULD be compressed, it should be
noted that in some cases compressing a message may actually result in
message expansion.  Therefore where comp=sigcomp is specified, it is up
to the implementation to work out whether or not to compress a message.
This decision may use available information such as compartment
availability, and uncompressed and compressed message lengths.

Note that it is always possible to use the 'well-known shim header'
mentioned in section 5."

> That is why revision 04 is much simpler than 03.
> 
> > Section 12:
> > I think section 12 should either be part of section 8 or 
> become section
> > 9 (preferably the former) because it is an example of compartment
> > creation and deletion.
> 
> This is of course a matter of taste, but, as it is the case in many 
> other RFCs, we have chosen to place the examples at the end 
> of the document.

OK, fair enough.
> 
> > As mentioned above several sentences have echoes of the 
> possibility of
> > having variable compartment lifetime:
> > 
> > "This compartment will be valid, at least, for the
> >    duration of the registration."
> > Remove the 'at least'
> > 
> > "The compartment opened by the outbound proxy will be valid,
> >    at least, for the duration of the registration."
> > Remove the 'at least'
> 
> The 'at least' is there because implementations may choose to 
> extend the 
> lifetime of the compartment, per my comments above.
> 
> > "Since this URI's SIP/SigComp identifier 'Outbound-id' 
> matches that of
> >    the compartment created before, this compartment is used 
> to compress
> >    the INVITE request."
> > What other compartment could be used?
> 
> The node may have SigComp relationships with other nodes at the same 
> time (e.g., an outbound proxy from other operator for reliability).
> 
> > Nits:
> > 
> 
> > Section 4: final note:
> > 
> > Refers to 'a SigComp structured TCP connection' - I think the word
> > structured is unnecessary and mildly confusing in this note.
> > Alternatively make it 'SigComp-structured'
> 
> Fixed.

Two occurences, only one is fixed.
> 
> > 
> > Section 7: 
> > Phraseology 'failure occurred to' and 'loss occurred to' 
> feel slightly
> > clumsy.
> > Rephrase to:
> > 'failure occurred on' and 'If the response was lost, ...'
> 
> Fixed.
There's a second 'to' relating back to the failure occurring which
should also be an 'on'.

"or on the response to that message"

> 
> > 
> > "Outgoing responses: the remote application identifier is 
> the same as
> > the previously-received request that initiated the transaction the
> > response belongs to."
> > ->
> > "Outgoing responses: the remote application identifier is 
> the same as
> > that of the previously-received request that initiated the 
> transaction
> > to which the response belongs."
> 
> Fixed.

Cut and paste error - during the fix the '>' have crept into the draft!

> 
> > comparment -> compartment (1 instance in section 8.3 and 1 
> in section
> > 11)
> 
> Fixed.

Still one instance of comparment in section 12.
 
> > 
> > Section 8.4: last paragraph
> > "If following the previous rules" -> "If, following the 
> rules above, "
> 
> Fixed.

Missing 'the'

Section 13: first paragrpaph under figure 1:

"'outbound's proxy" -> "outbound proxy's"

Section 13: page 15, paragraph 4:

"   The registrar also constructs the route future outgoing requests
   (from the user agent) and places it in a Service-Route header field
   in a 200 (OK) response (3). "

->

"   The registrar also constructs the route future outgoing requests
   (from the user agent) will follow and places it in a Service-Route
header field
                         ^^^^^^^^^^^^
   in a 200 (OK) response (3). "

> 
> I uploaded a working version of revision 05, which includes all the 
> changes above, to:
> http://users.piuha.net/gonzalo/temp/draft-ietf-rohc-sigcomp-sip-05.txt
> 
> Let me know if you are OK with this version so that I can submit it.
> 
> Cheers,
> 
> Gonzalo
> 

Best regards,

Abbie

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