Subject: [sip-clf] draft-ietf-sipclf-format-00 comments

From: Anders Nygren <[email-masked]>
Date: Fri, 11 Mar 2011 12:01:49 -0600

Hi
I am not sure what is the correct way for commenting on the SIP-CLF spec,
so I will do it here until someone tells me that there is a better
place to do it.

I am trying to implement a log reader and writer according to
draft-ietf-sipclf-format-00 and have found a few small errors.

page 8:
   "Record Length (6 bytes):  Hexadecimal encoded total length of this
    log record, including "Flags" and "Record Length" fields, and
    terminating line-feed."

Should that include the version field as well?

page 12:
   "Length Field (2 bytes):  Indicates the length of the value coded in
    this TLV, hexadecimal encoded.  This length does NOT include the
    TLV header."

The length field is shown as 4 bytes in figure 5.

/Anders Nygren

---

From: Gonzalo Salgueiro <[email-masked]>
Date: Fri, 11 Mar 2011 14:38:53 -0500

Anders - 

Thanks for providing your feedback, it is much appreciated as we work through developing this format.  

I'm very glad to hear you are implementing this already and would be willing to follow up with you offline to get some data points on how you decided to do this, how difficult it was, and how useful you found it, what you would change, etc.

Comments inline...

> so I will do it here until someone tells me that there is a better
[GS] This is indeed the place.

> Should that include the version field as well?

[GS] Yes this does include the Version field. It is the length of the ENTIRE record including all of <IndexPointers>, <MandatoryFields>, <OptionalFields>
The list of items in the text you have selected is not meant to be  complete listing of all fields, rather a representative list indicating that everything is included. If you think there is value in specifically mentioning the Version field, then I will modify the text as follows:

Hexadecimal encoded total length of this log record, including "Version", "Record Length", "Flags" fields, and terminating line-feed.

> The length field is shown as 4 bytes in figure 5.

Thanks. Great catch.  This is indeed wrong. The figure is correct. I will change the text to 4 bytes.

Warm Regards,

Gonzalo
> 
> /Anders Nygren
> _______________________________________________
> sip-clf mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/sip-clf

---

From: Anders Nygren <[email-masked]>
Date: Fri, 11 Mar 2011 16:43:44 -0600

One more thing,
page 9,
The text says that fractional seconds is 6 bytes but figure 4 shows 3 bytes.

/Anders

---

From: Gonzalo Salgueiro <[email-masked]>
Date: Fri, 11 Mar 2011 18:07:45 -0500

Thanks Anders. This reflected an earlier intent for microsecond accuracy that has since been rescinded. I have amended the text.

Regards,

Gonzalo

---

From: Anders Nygren <[email-masked]>
Date: Fri, 11 Mar 2011 17:54:23 -0600

Hi
In chapter 4.1, the text
  "The <IndexPointers> portion of the SIP CLF record is a 64-byte header
   that indicates meta-data about the record."
is repeated before and after the figure 3.

chapter 4.3 figure 5 shows a 0x0A after the TLVs. But I think that
0x0A is not part of the
optional parameters portion but is the termination of the record, and
should be included
even if there are no optional parameters in the record.

/Anders

On Fri, Mar 11, 2011 at 5:07 PM, Gonzalo Salgueiro <[email-masked]> wrote:
> Thanks Anders. This reflected an earlier intent for microsecond accuracy
> that has since been rescinded. I have amended the text.
> Regards,
> Gonzalo
> I'm very glad to hear you are implementing this already and would be willing
> to follow up with you offline to get some data points on how you decided to
> do this, how difficult it was, and how useful you found it, what you would
> change, etc.
>
> Comments inline...
>
> [GS] Yes this does include the Version field. It is the length of the ENTIRE
> record including all of <IndexPointers>, <MandatoryFields>, <OptionalFields>
>
> The list of items in the text you have selected is not meant to be  complete
> listing of all fields, rather a representative list indicating that
> everything is included. If you think there is value in specifically
> mentioning the Version field, then I will modify the text as follows:
>
> Hexadecimal encoded total length of this log record, including "Version",
> "Record Length", "Flags" fields, and terminating line-feed.
>
>
>
> page 12:
>

---

From: Gonzalo Salgueiro <[email-masked]>
Date: Fri, 11 Mar 2011 19:31:12 -0500

Anders - 

Inline..

> is repeated before and after the figure 3.

[GS] Fixed.
> even if there are no optional parameters in the record.

[GS] In principle I agree with you. For the sake of graphical continuity, I'll need to get creative so that I don't need to represent the logical structure of the SIP CLF record as:

        <IndexPointers>
        <MandatoryFields>
        <OptionalFields>
	0x0A

I'll kick it around in my head as to how best represent that.  Great catch.

Regards,

Gonzalo