Subject: Gen-ART review of draft-ietf-ccamp-lmp-behavior-negotiation-10

From: Richard Barnes <[email-masked]>
Date: Sun, 3 Feb 2013 14:13:36 -0500

I have been selected as the General Area Review Team (Gen-ART) reviewer for this draft (for background on Gen-ART, please seehttp://www.alvestrand.no/ietf/gen/art/gen-art-FAQ.html).

Please wait for direction from your document shepherd or AD before posting a new version of the draft.

Document: draft-ietf-ccamp-lmp-behavior-negotiation-10
Reviewer: Richard Barnes
Review Date: 2013-02-02
IETF LC End Date: 2012-01-21
IESG Telechat date: 2012-02-07

Summary:  Ready, with a couple of minor questions / clarifications.

Comment:

Overall, this document seems very clear and readable.  Thanks!  The one concern I have is over the use of "likely" in the discussion of backward compatibility; I would like to see more precise language there.

Section 2.1.  Would be helpful to either include the old formats and/or say explicitly what is changing. 

Section 2.2.  
"Nodes which support" -> "nodes that support"  
"Ordering of CONFIG objects" -> "... With different C-type values"

Section 3.1.MBZ. Might help to clarify that this means that the number of bits MUST be a multiple of 32. (I got a little confused between bits and bytes here.)

Section 4. "Likely"
Is it possible for a 4204-compliant implementation to not do one of these?  If so then remove "likely".  If not, then why happens on the exceptional case?

---

From: Lou Berger <[email-masked]>
Date: Tue, 05 Feb 2013 09:24:19 -0500

Dan/Richard,

On 2/4/2013 10:05 PM, Lidan (Dan) wrote:
> Hi Richard,
> 
> Thanks for the review of this draft!
> 

> Added the original format of Config, ConfigAck and ConfigNack
> messages which are defined in RFC4204.
> 

I personally think it's a mistake to repeat definitions in non-bis RFCs.
 I think this increases the possibility of mistakes and confusion (e.g.,
when the text isn't copied properly or when the original document is
replaced).

My original thought was to propose text to follow Richard's suggestion
of explicitly saying what has changed, but I see such text is there at
the start of section 2:

   LMP Config, ConfigNack and ConfigAck messages are modified by this
   document to allow for the inclusion of multiple CONFIG objects. The
   Config and ConfigNack messages were only defined to carry one CONFIG
   object in [RFC4204]. The ConfigAck message, which was defined
   without carrying any CONFIG objects in [RFC4204], is modified to
   enable explicit identification of negotiated configuration
   parameters. The inclusion of CONFIG objects in ConfigAck messages is
   triggered by the use of the BehaviorConfig object (defined below) in
   a received Config message.

Richard,

Is this text sufficient?  Alternatively, this text can be moved to
immediately proceed the BNF.

Much thanks
Lou
(document co-author)

---

From: Lou Berger <[email-masked]>
Date: Tue, 05 Feb 2013 09:53:28 -0500

Richard,
	Thank you for the review.  I have one additional question/response on
your comments:

On 2/3/2013 2:13 PM, Richard Barnes wrote:
> Section 3.1.MBZ. Might help to clarify that this means that the
> number of bits MUST be a multiple of 32. (I got a little confused
> between bits and bytes here.)
> 

I think you're right.  The current text can easily result in confision
between the size of the MBZ field and the setting of the length field
define in RFC4204.  It's probably best to just remove the reference to
the length field. How about this:

OLD
The number of bits present is based on the Length field of the LMP
object header and MUST include enough bits so the Length field MUST be
at least 8, and MUST be a multiple of 4.

NEW
This field MUST be sized to ensure 32 bit alignment of the object.

Lou
(co-author)

---

From: Richard Barnes <[email-masked]>
Date: Tue, 5 Feb 2013 10:08:50 -0500

Hey Lou,

That text looks fine to me!

--Richard

---

From: "Lidan (Dan)" <[email-masked]>
Date: Tue, 5 Feb 2013 03:05:34 +0000

Hi Richard,

> Section 2.1.  Would be helpful to either include the old formats and/or say explicitly what is changing.
Added the original format of Config, ConfigAck and ConfigNack messages which are defined in RFC4204.

> "Ordering of CONFIG objects" -> "... With different C-type values"
Updated the text as suggested.

> Section 3.1.MBZ. Might help to clarify that this means that the number of bits MUST be a multiple of 32. (I got a little confused between bits and bytes here.)
OLD:
The remaining bits in the flags field MUST be set to zero (0). The number of bits present is based on the Length field of the LMP object header and MUST include enough bits so the Length field MUST be at least 8, and MUST be a multiple of 4.
NEW:
The remaining bits in the flags field MUST be set to zero (0). The number of bits present is based on the Length field of the LMP object header and MUST include enough bits so the Length field MUST be at least 8, and the number of bits MUST be a multiple of 32.

> Is it possible for a 4204-compliant implementation to not do one of these?  If so then remove "likely".  If not, then why happens on the exceptional case?
There is no other exception, so I just removed the word "likely".

I will hold the new version until I get the further instruction from my document shepherd or AD.

Best regards,

Dan

-----邮件原件-----
发件人: Richard Barnes [mailto:[email-masked]] 
发送时间: 2013年2月4日 3:14
收件人: [email-masked]; The IESG
抄送: [email-masked]; [email-masked]
主题: Gen-ART review of draft-ietf-ccamp-lmp-behavior-negotiation-10

Comment:

Section 2.2.  

Section 4. "Likely"

---

From: "Lidan (Dan)" <[email-masked]>
Date: Wed, 6 Feb 2013 00:52:14 +0000

Ok, the old formats of Config/ConfigAck/ConfigNack are not necessary to be repeated in this draft. I will clean the text.

Thanks,

Dan

发件人: Richard Barnes [mailto:[email-masked]]
发送时间: 2013年2月5日 23:09
收件人: Lou Berger
抄送: Lidan (Dan); Richard Barnes; [email-masked]; [email-masked]; [email-masked]; The IESG
主题: Re: 答复: Gen-ART review of draft-ietf-ccamp-lmp-behavior-negotiation-10

Hey Lou,

--Richard

On Tue, Feb 5, 2013 at 9:24 AM, Lou Berger <[email-masked]<mailto:[email-masked]>> wrote:
Dan/Richard,

Richard,

Much thanks
Lou
(document co-author)