Re: [CCAMP] I-D Action: draft-ietf-ccamp-gmpls-ospf-g709v3-04.txt

[Lou Berger <[e-mail]>]

Daniele,

see below.


On 1/8/2013 12:39 PM, Daniele Ceccarelli wrote:
> Hi Lou,
> 
> Please find further comments in line.
> 
> Since the changes from v04 start to be quite a lot we published v05. Please provide further comments (if any) with respect to that version.
> 

Okay.  Note that you have a nit to clean up the next time you update:
http://tools.ietf.org/idnits?url=http://tools.ietf.org/id/draft-ietf-ccamp-gmpls-ospf-g709v3-05.txt


> BR
> Daniele & Sergio 
> 
>> -----Original Message-----
>> From: Lou Berger [mailto:[e-mail]] 
>> Sent: venerdì 21 dicembre 2012 19.32
>> To: Daniele Ceccarelli
>> Cc: [e-mail]; CCAMP
>> Subject: Re: [CCAMP] I-D Action: 
>> draft-ietf-ccamp-gmpls-ospf-g709v3-04.txt
>>
>> Daniele,
>> 	Much thanks -- I do expect that the thread might extend 
>> beyond the end of year holiday, and that many/most are off next week...
>>
>> See below for responses.
>>
>> On 12/21/2012 10:49 AM, Daniele Ceccarelli wrote:
>>> Hi Lou,
>>>
>>> Please see in line.
>>>
>>> We'll upload -05 when all the issues will be solved.
>>>
>>> BR
>>> Daniele & Sergio
>>>
>>> PS. The info model after christmas holidays
>>>
>>>> -----Original Message-----
>>>> From: Lou Berger [mailto:[e-mail]]
>>>> Sent: venerdì 21 dicembre 2012 0.29
>>>> To: Daniele Ceccarelli
>>>> Cc: [e-mail]; CCAMP
>>>> Subject: Re: [CCAMP] I-D Action: 
>>>> draft-ietf-ccamp-gmpls-ospf-g709v3-04.txt
>>>>
>>>> Daniele / Authors,
>>>> 	Thank you for the response.  Please see below for my responses.
>>>>
>>>>
>>>> On 12/20/2012 3:57 AM, Daniele Ceccarelli wrote:
>>>>> Hi Lou,
>>>>>
>>>>> Below you can find the last call comments pasted with
>>>> replies in line.
>>>>>
>>>>> All nits, typos and suggested text changes without any
>>>> comment in line
>>>>> have been accepted/modified accordingly.
>>>>>
>>>>
>>>>> BR
>>>>> Daniele & Sergio
>>>>>
>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: [e-mail] [mailto:[e-mail]]
>>>>>> On Behalf
>>>>>> Of Lou Berger
>>>>>>> Sent: mercoledì 26 ottobre 2011 0.37
>>>>>>> To: [e-mail]
>>>>>>> Cc: CCAMP
>>>>>>> Subject: [CCAMP] some comments on gmpls-ospf-g709v3-00
>>>>>> ...
>>>>>>> 2) SCSI TLV formatting
>>>>>>>
>>>>>>> The field descriptions are missing format related conformance
>>>>>>> (RFC2119) language.
>>>>>>>
>>>>>
>>>>> Done
>>>>
>>>> Thanks.
>>>>
>>>> Some points:
>>>> (Using line numbers from
>>>> http://tools.ietf.org/idnits?url=http://tools.ietf.org/id/draft
>>> -ietf-ccamp-gmpls-ospf-g709v3-04.txt)
>>>>
>>>> I like your solution for the general TLV format definition.
>>>>
>>>> Lines 303/304: "Different sub-TLV MAY be presented in 
>> ascending Type 
>>>> order."
>>>>
>>>> I suspect you mean s/Different sub-TLV/Multiple sub-TLVs
>>>>
>>>
>>> See below
>>>
>>>> By "ascending Type order", are you refering to the TLV type? 
>>>> Are multiple TLVs of the same type allowed? If not, how are errors 
>>>> handled?
>>>
>>> Yes and yes. Multiple TLVs of the same type are often 
>> present as each of them represents a branch of the muxing tree.
>>> What about changing into:
>>>
>>>       Sub-TLV SHOULD be presented
>>> 	in ascending Type order (i.e. type 1 before and type 2 after).
>>
>> Is there any technical reason for such ordering? It doesn't 
>> seem to add any value to me.
> 
> Ok. It was meant to be just a guideline but indeed doesn't add any
> value. Sentence removed.
> 
>>
>> I actually was expecting you to say something referring back 
>> to signal type or number of stages represented in the TLV...
> 
> WRT signal type each TLV is self-consistent, in the sense that there is no need to have them in a given order. In all the example we have ordered them form the root to the leaves of the tree so to make it more "human-readable". We could suggest to follow that orded but like in the case of type 1 and type2 is does not add any value (except being more easily read).
> 
> WRT to number of stage the order is important. Actually the text says that they MUST be put is ascending order and an example (ODU1->ODU2->ODU3) is provided:
> 
> OLD
>       - Stage#1 ... Stage#N : These fields are 8 bits long. Their number is variable and a field is present for 
> 	  each of the indicated number of stages. The last one MUST always indicate the server ODU container (ODUk/OTUk)
> 	  and they MUST be listed in ascending order from the smallest to the biggest one.
> 	  The values of the Stage fields MUST be the same ones defined for the Signal Type field. If
> 	  the number of stages is 0, then the Stage and Padding fields MUST be omitted.
> 	  
> 	  Example: in case the ODU1->ODU2->OD3 hierarchy, the Signal Type field is set to ODU1 and 
> 	  two Stage fields are present, the first indicating ODU2 and the second ODU3 (server layer). 
> 	  
> We added: "from the smallest to the biggest one." at the end of the first sentence:
> 
> NEW:    
>       [CUT]   
> 	The last one MUST always indicate the server ODU container (ODUk/OTUk)
> 	and they MUST be listed in ascending order from the smallest to the biggest one.
> 	[CUT]
> 
>>
>>>
>>>>
>>>> Lines 306-322:
>>>>
>>>> Given that Figures 4 and 5 completely repeat the information in 
>>>> Figure 4, I propose that you drop Figure 4. (and align the 
>> preceding 
>>>> paragraph to match.)
>>>
>>> Figure 4 and 5 represent TLV type1 and TLV type2 
>> respectively, which are quite different from each other. The 
>> first 3 rows are identical but the rest of the TLV is pretty 
>> different. We would prefer to keep both of them.
>>>
>>
>> Ahh.  Sorry, let me try again:
>>
>> Given that Figures 4 and 5 completely repeat the information 
>> in Figure *4*, I propose that you drop Figure *3*. (and align 
>> the preceding paragraph to match.)
> 
> Done
> 
> OLD
> 	The format of the SCSI MUST be as depicted in the following figure, where 
> 	one Type 1 sub-TLV MUST be used for any fixed container and one Type 2 sub-TLV
> 	MUST be used for any variable container. 
> NEW
> 	The SCSI MUST include one Type 1 sub-TLV for any fixed container and one Type 2 sub-TLV
> 	for any variable container. 
> 

I think this new/revised text is ambiguous:

You say "one ... for any" (twice). Is this one for each, or one for all
(containers)?

The flow into the next paragraph is a bit hard to follow.

>>
>> Also, just realized that figures 4 and 5 really should 
>> indicate that priorities are not always included.  And that a 
>> second padding field may be needed too! How about:
>>
>>
>>   0                   1                   2                   3
>>   0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
>>   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>   |        Type = 1 (Unres-fix)   |           Length              |
>>   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>   |  Signal Type  | Num of Stages |T|S| TSG | Res |   Priority    |
>>   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>   |    Stage#1    |      ...      |   Stage#N     |    Padding    |
>>   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>   |  Unreserved ODUj at Prio 0    |             .....             |
>>   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>   |  Unreserved ODUj at Prio 7    |       Unreserved Padding      |
>>   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>
>>                    Figure 4: Bandwidth TLV - Type 1 -
> 
> OK. Wrote padding instead of unreserved Padding
> 
>>
>>
>>   0                   1                   2                   3
>>   0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
>>   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>   |    Type = 2 (Unres/MAX-var)   |           Length              |
>>   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>   |  Signal Type  | Num of Stages |T|S| TSG | Res |   Priority    |
>>   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>   |    Stage#1    |      ...      |   Stage#N     |    Padding    |
>>   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>   |                Unreserved Bandwidth at priority 0             |
>>   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>   |                              ...                              |
>>   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>   |                Unreserved Bandwidth at priority 7             |
>>   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>   |               Maximum LSP Bandwidth at priority 0             |
>>   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>   |                              ...                              |
>>   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>   |               Maximum LSP Bandwidth at priority 7             |
>>   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>
>>                    Figure 5: Bandwidth TLV - Type 2 -
>>
>> (Note some field names have been expanded to match descriptions)
> 
> OK
> 
>>
>>>>
>>>>>
>>>>>>>
>>>>>>> 3) SCSI TLV procedures
>>>>>>>
>>>>>>> You have defined the formats of the TLVs in Section 4.1, but not 
>>>>>>> explicitly how they are to be used. Some RFC2119 language
>>>> really is
>>>>>>> needed to cover how the SCSI is to be encoded and parsed. At a 
>>>>>>> minimum, any TLV inclusion, ordering requirements, and exception 
>>>>>>> handling should be covered. (For example, your examples
>>>>>> always show a
>>>>>>> particular ordering relative to Stage#, is this required,
>>>>>> recommended,
>>>>>>> or just a possibility. You have some informative language,
>>>> which is
>>>>>>> great, but you also need some RFC2119 language.)
>>>>>
>>>>> Done
>>>>
>>>> The length of each TLV type and each field should be defined. 
>>>> (You have it for some fields, but not others).
>>>
>>> Now all of them should have been filled.
>>>
>>
>> great.
>>
>>>>
>>>> 414  With respect to ODUflex, ODUflex Constant Bit Rate (CBR) and
>>>> 415  ODUflex Generig Framing Procedure-Frame mapped (GFP-F) MUST
>>>> 416  always be advertised separately as they use different
>>>> 417  adaptation functions.  In the case both GFP-F resizable and non
>>>> 418  resizable (i.e. 21 and 22) are supported, Signal Type 21
>>>> 419  implicitely supports also signal Signal Type 22, so 
>> only Signal 
>>>> 420  Type 21 MUST be advertised.  Signal Type 22 MUST be used only
>>>> 421  for non resizable resources.
>>>>
>>>> Shouldn't this text be moved to after line 304?
>>>
>>> Agree. Done.
>>
>> great.
>>
>>>
>>>>
>>>> Line 416: By separately do you mean "in separate TLVs"?
>>>
>>> Yes.changed
>>
>> great.
>>
>>>>
>>>> Lines 416/7: Your reference to "different adaptation 
>> functions" lacks 
>>>> context as it is the sole reference in the document to "adaptation 
>>>> functions".  I think you need to either define this 
>> terminology (via 
>>>> reference is fine) or replace it some other terminology.
>>>>
>>>
>>> Reference to [G.805] added
>>
>> okay. Given the signal type definitions are in [OTN-SIG], what 
>> additional information is added by this paragraph? What am I missing?
> 
> Things are quite different between signaling and routing when it comes to "implication". In the case of signaling you either signal type 21 o 22, while in the case of routing if both of them are supported there is no need to advertise both of them and just signal type 21 is enough because it implies also signal type 22 support.
> 
>>
>>>
>>>> Line 419/whole document: Please double check the document for 
>>>> spelling errors (there's one in the above paragraph.)

you still have some errors...

>>>>
>>>> Line 423:
>>>>
>>>> By "number of multiplexing stages level below the indicated signal 
>>>> type", do you mean "number of multiplexing stages represented as 
>>>> transporting the indicated signal type"?
>>>>
>>>> Lines 424-426.  It's best not to define semantics through example.  
>>>> How about replacing 423-426 with:
>>>>
>>>> - Number of stages (8 bits): This field indicates the number of 
>>>> multiplexing stages used to transport the indicated signal type. It 
>>>> MUST be set to the number of stages represented in the TLV.
>>>>
>>> OK
>>>
>>>>
>>>> Line 428: s/Flags:/Flags (8 bits)
>>>
>>> ok
>>>>
>>>> Lines 455-462: should be revised to use 2119 conformance language 
>>>> (and to clarify the malformed text.)
>>>
>>> OK

OLD

409 Value 1 MUST be used on those interfaces where the fallback
410 procedure is enabled and the default value of 1.25 Gbps can be
411 falled back to 2.5 if needed.  Value 2 MUST be used on [RFC4328]
412 interfaces while value 3 MUST be used on [G.709-2012] interfaces
413 where the fallback procedure is unsupported/disabled.  Value 0
414 MUST be used for non multiplexed signal (i.e. non OTN client).

How about:
A value of 1 MUST be used on interfaces which are configured to support
the fall back procedures defined in [G.798-a2].  A value of 2 MUST be
used on interfaces that only support 2.5 Gbps time slots, such as
[RFC4328] interfaces. A value of 3 MUST be used on interfaces that are
configured to only support 1.25 Gbps time slots. A value or 3 MUST be
used for non multiplexed signal types (i.e. a non OTN client).

>>>
>>>>
>>>> The "RES" field isn't defined.
>>>
>>>  <t>- Res (3 bits): reserved bits. MUST be set to 0.</t>
>>
>> "and ignored on receipt."
> 
> Ok
> 
>>
>>>
>>>>
>>>> 464-479: I know what you mean, but I think the text really isn't 
>>>> clear and should be revised.  Suggest you just rewrite rather then 
>>>> tweak (as we tried in this rev.) I'm happy to discuss on 
>> list if that 
>>>> will help.
>>>>
>>>
>>> OLD
>>> 464	      - Priority (8 bits): field with 1 flag for each 
>> priority.  Each
>>> 465	      bit MUST be set when the related priority is 
>> supported and MUST be
>>> 466	      cleared when the related priority is not 
>> supported.  The priority
>>> 467	      0 is related to the most significant bit.  When 
>> no priority is
>>> 468	      supported, priority 0 MUST be advertised.
>>>
>>> 470	      - Stage#1 ...  Stage#N : These fields are 8 bits 
>> long.  Their
>>> 471	      number is variable and a field is present for each of the
>>> 472	      indicated number of stages.  The last one MUST 
>> always indicate the
>>> 473	      server ODU container (ODUk/OTUk) and they MUST be 
>> listed in
>>> 474	      ascending order.  The values of the Stage fields 
>> MUST be the same
>>> 475	      ones defined for the Signal Type field.  If the 
>> number of stages
>>> 476	      is 0, then the Stage and Padding fields MUST be omitted.
>>>
>>> 478	      - Padding: Given that the number of Stages is 
>> variable, padding to
>>> 479	      32 bits field MUST be used when needed.
>>>
>>> NEW
>>>
>>> - Priority (8 bits): bitmap used to state which priorities are being
>> s/state/indicate
>>> advertised. The left most bit represent priority 0 (highest) and the 
>>> right most priority 7 (lowest) And are presented in ascending orded.
>> s/) A/), a
>> s/orded/ordered
>>
>>> Each bit MUST be set when the related priority is supported and MUST
>> "A bit MUST be set (1) for each corresponding priority 
>> represented in the TLV and MUST"
>>
>>> be cleared when the related priority is not supported. 
>> s/be cleared/NOT be set (0)
>> s/supported/represented.
>>
>>> When the
>>> interface does not support priorities, the advertisement MUST be 
>>> compliant with the advertisement of priority 0.
>>>
>> Replace with
>> "At least one priority level MUST be advertised.  A value of 
>> zero (0) MUST be used when not overridden by local policy."
> 
> NEW
> 	  <t>- Priority (8 bits): field with 1 flag for each priority.  A bit MUST be set (1) for each corresponding priority 
> 	  represented in the TLV and MUST NOT be set (0) when the related priority is not represented. 
> 	  At least one priority level MUST be advertised. A value of zero (0) MUST be used when not
> 	  overridden by local policy.</t>

How about:
- Priority (8 bits): a bitmap used to indicate which priorities are
being advertised. The bitmap is in ascending order, with the leftmost
bit representing priority level 0 (i.e., the highest) and the rightmost
bit representing priority level 7 (i.e., the lowest).  A bit MUST be
set (1) corresponding to each priority represented in the TLV, and MUST
NOT be set (0) when the corresponding priority is not represented.  At
least one priority level MUST be advertised that, unless overridden
by local policy, SHALL be at priority level 0.

>>
>>> - Stage#1 ...  Stage#N : Each field is 8 bits long.  One 
>> field MUST be 
>>> used in ascending order (from the lowest order ODU to the highest 
>>> order ODU) for each stage of the muxing branch being advertised. The 
>>> last one MUST always indicate the server ODU container (ODUk/OTUk).
>>> The values of the Stage fields MUST be the same ones defined for the 
>>> Signal Type field.  If the number of stages is 0, the Stage 
>> field MUST 
>>> NOT be included.
>>>
>> How about:
>> Stage (8 bits): Each Stage field indicates the signal type 
>> used in the to transport the signal indicated in the Signal 
>> Type field. The number of Stage fields included in a TLV MUST 
>> equal the value of the Number of Stages field.  The Stage 
>> fields MUST be ordered to match the data plane in ascending 
>> order (from the lowest order ODU to the highest order ODU).
> 
> Saying that each stage field indicates the signal type used to
> transport the signal indicated in the signal type field is not
> correct because e.g. In the case of ODU1->ODU2->ODU3 it is not
> correct to say that ODU2 and ODU3 are used to transport the ODU1
> because the ODU2 tranports the ODU1 and the ODU3 tranports the ODU3.
> How about:
> 
> <t>- Stage (8 bits): Each Stage field indicates the signal type
> beloning to the muxing branch used to transport the signal indicated
> in the Signal Type field. The number of Stage fields included in a
> TLV MUST equal the value of the Number of Stages field.  The Stage 
> fields MUST be ordered to match the data plane in ascending order
> (from the lowest order ODU to the highest order ODU). The values of
> the Stage fields MUST be the same ones defined for the Signal Type
> field. If the number of stages is 0, then the Stage and Padding
> fields MUST be omitted.
> 

How about:
Stage (8 bits): Each Stage field indicates a signal type in the
multiplexing hierarchy used to transport the signal indicated
in the Signal Type field. The number of Stage fields included in a
TLV MUST equal the value of the Number of Stages field.  The Stage
fields MUST be ordered to match the data plane in ascending order
(from the lowest order ODU to the highest order ODU). The values of
the Stage field are the same as those defined for the Signal Type
field. When the Number of stage field carries a 0, then the Stage and
Padding fields MUST be omitted.

>>
>>
>>> - Padding: Padding bytes MUST be inserted so that the
>>>          subsequent field starts at a 4-byte aligned boundary.  It is
>>>          important to note that the Length field includes the padding
>>>          bytes.  Padding SHOULD be using zeros.
>>>
>> How about:
>> Padding (variable): The Padding field is used to ensure the 32 
>> bit alignment of stage fields. The length of the Padding field 
>> is always a multiple of 8 bits (1 byte).  Its length can be 
>> calculated, in bytes, as:
>>      <value of Number of Stages field> % 4 When present, the 
>> Padding field MUST be set to a zero (0) value on transmission 
>> and MUST be ignored on receipt.
>>
> 
> In case of number of stages == 3,  "<value of Number of Stages field> % 4" is 3, correct? While the padding bytes is 1.
>  Wouldn't "4-<value of Number of Stages field>" be more correct?

Woops.  4-(<value of Number of Stages field> % 4), right?


> 
> How about:
>  <t>- Padding (variable): The Padding field is used to ensure the 32 bit alignment of stage fields.
> 	  The length of the Padding field is always a multiple of 8 bits (1 byte).  Its length can be 
> 	  calculated, in bytes, as: 4- "value of Number of Stages field". 
see above.
> The Padding field MUST
> 	  be set to a zero (0) value on transmission and MUST be ignored on receipt.</t>

s/The/When present, the


> 
>>>
>>>> 481-493: I still find this text is really confusing.  I think it 
>>>> would cleaner to separate out the fixed container and variable 
>>>> container field definitions (3 definitions: Unres ODUj at Prio N, 
>>>> Unreserved Bandwidth at priority N, and MAX LSP Bandwidth 
>> at priority 
>>>> N). Again happy to discuss further on list.
>>>>
>>>
>>> OLD
>>> 481	      - Unreserved Bandwidth/Max LSP BW : In case of 
>> fixed containers
>>> 482	      (Type=1) the Unreserved Bandwidth field MUST be 
>> 16 bits long and
>>> 483	      indicates the Unreserved Bandwidth in number of available
>>> 484	      containers.  Unreserved/MAX LSP BW fields for 
>> each identified
>>> 485	      priority MUST be included, in order of increasing 
>> prioritiy (0 to
>>> 486	      7) and Unreserved/MAX LSP BW fields for other 
>> priority values MUST
>>> 487	      be omitted.  In case the number of supported 
>> priorities is odd, a
>>> 488	      16 bits all zeros padding field MUST be added.  
>> On the other hand,
>>> 489	      in case of variable containers (Type 2) the 
>> Unreserved/MAX LSP
>>> 490	      Bandwidth fields MUST be 32 bits long and 
>> expressed in IEEE
>>> 491	      floating point format.  The advertisement of the 
>> MAX LSP bandwidth
>>> 492	      MUST take into account HO OPUk bit rate tolerance and be
>>> 493	      calculated according to the following formula:
>>>
>>> NEW
>>> - Unreserved ODUj at Prio N (16 bits): This field is used 
>> only in case 
>>> of fixed containers (Type=1). It MUST be 16 bits long and indicates 
>>> the Unreserved Bandwidth in number of available containers.
>>> A different field for each supported priority MUST be used. In case 
>>> the number of supported priorities is odd, a 16 bits all 
>> zeros padding 
>>> field MUST be added.
>>>
>> How about:
>> - Unreserved ODUj (16 bits): This field indicates the 
>> Unreserved Bandwidth at a particular priority level.  This 
>> field MUST be set to the number of ODUs at the indicated the 
>> Signal Type for a particular priority level.  One field MUST 
>> be present for each bit set in the Priority field, and is 
>> ordered to match the Priority field. Fields MUST not be 

s/MUST not/MUST NOT

>> present for priority levels that are not indicated in the 
>> Priority field.

>> This field is REQUIRED for Type 1 (fixed 
>> container) TLVs, and MUST NOT be used for Type 2 TLVs.
>>

Actually, these lines are redundant with the format definition and can
be dropped.

>> Also:
>> Unreserved Padding (variable): The Padding field is used to ensure the
>> 32 bit alignment of Unreserved ODUj fields. The length of the 
>> Unreserved Padding field is always a multiple of 16 bits (2 
>> byte).  Its length can be calculated, in bytes, as:
>>      <number of priorities indicated in Priorities field> % 2 
>> When present, the Unreserved Padding field MUST be set to a 
>> zero (0) value on transmission and MUST be ignored on receipt.
>>
> 
> Ok, but shouldn't it be: 
>       "Its length can be calculated, in multiple of 2 bytes,
> 	as: "number of priorities indicated in Priorities field" % 2." ?
> 
> When the number of priorities is odd, the unreserved padding field is 2 byte, when the number of priorities is even, the padding field is not there.
> 
How about:

- Unreserved Padding (variable): The Padding field is used to ensure
the 32 bit alignment of Unreserved ODUj fields.  When present the
Unreserved Padding field is 16 bits (2 byte) long.  When the number of
priorities is odd, the unreserved Unreserved Padding field MUST be
included. When the number of priorities is even, the Unreserved Padding
MUST be omitted.

> 
>>> - Unreserved Bandwidth at priority N (32 bits): This field is used 
>>> only in case of variable containers (Type=2). It MUST be 32 
>> bits long 
>>> and indicates the Unreserved Bandwidth in bits/s in IEEE floating 
>>> point format. A different field for each supported priority MUST be 
>>> used.
>>>
>> How about:
>> Unreserved Bandwidth (32 bits): This field indicates the 
>> Unreserved Bandwidth at a particular priority level.  This 
>> field MUST be set to the bandwidth,  in bits/s in IEEE 
>> floating point format, available at the indicated the Signal 
>> Type for a particular priority level.  One field MUST be 
>> present for each bit set in the Priority field, and is ordered 
>> to match the Priority field. Fields MUST not be present for 
>> priority levels that are not indicated in the Priority 
>> field.This field is REQUIRED for Type 2 (variable container) 
>> TLVs, and MUST NOT be used for Type 1 TLVs.
>>

The Unreserved Bandwidth field remains undefined in the current text.

>>> - MAX LSP Bandwidth at priority N (32 bit): This field is 
>> used only in 
>>> case of variable containers (Type=2). It MUST be 32 bits long and 
>>> expressed in bits/s in IEEE floating point format. The advertisement 
>>> of the MAX LSP bandwidth MUST take into account HO OPUk bit rate 
>>> tolerance and be calculated according to the following formula:
>>>
>> How about:
>> Maximum LSP Bandwidth (32 bit): This field indicates the 
>> maximum bandwidth that can be allocated for a single LSP at a 
>> particular priority level. This field MUST be set to the 
>> maximum bandwidth,  in bits/s in IEEE floating point format, 
>> available to a single LSP at the indicated the Signal Type for 
>> a particular priority level.  One field MUST be present for 
>> each bit set in the Priority field, and is ordered to match 
>> the Priority field. Fields MUST not be present for priority 
>> levels that are not indicated in the Priority field.

>> This field 
>> is REQUIRED for Type 2 (variable container) TLVs, and MUST NOT 
>> be used for Type 1 TLVs.
>>

Actually, these lines are redundant with the format definition and can
be dropped.

> 
> OK
> 
>>
>>>>>
>>>>>> ...
>>>>>>> 6) Finally, some nits:
>>>>>>> s/[OTN-INFO], the OSPF-TE/[OTN-INFO], OSPF-TE s/list of them/list
>>>>>> s/Priority :8 bits/Priority (8 bits):
>>>>>>> s/infer/imply
>>>>>>>
>>>>>>>
>>>>>>
>>>>>> - You have some very nice examples, but are inconsistent 
>> in filling 
>>>>>> in field values.  I think all values that can possibly be 
>> filled in 
>>>>>> in the examples should be.
>>>>>>
>>>>>
>>>>> All the relevant ones have been introduces. Some non 
>> relevant fields 
>>>>> have been left with just the field name in. E.g. In an
>>>> example showing
>>>>> priorities management the T, S and TSG fields have not been filled 
>>>>> with 1 or 0 but just T,S and TSG have been left to make 
>> the reading 
>>>>> easier.
>>>>>
>>>>
>>>> I think this will confuse some readers.  I think it would 
>> be better  
>>>> to fill in as much as possible, and if not, indicate that 
>> the fields 
>>>> are not important to the case (or can carry a specific set of 
>>>> values).  For example why not show that reserved&padding fields are 
>>>> 0, that the SWCaps=OTN-TDM, etc.
>>>
>>> Done (only T, S ans TSG left indicated as T, S and TSG when non 
>>> relevant for the example)
>>>
>>
>> Will you add text to that effect to each of those examples?
> 
> OK
> 
>>
>>>>
>>>>>> Detailed editorial and technical comments:
>>>>>>
>>>> Thank you!
>>>> [...]
>>>>
>>>>
>>>>>> - Section 7 -- update to reference 4203 and 5920.  Discuss 
>>>>>> implications / added risk of additional information
>>>> provided in this
>>>>>> document.
>>>>> Reference to 4203 added and this piece of text added at the end:
>>>>>
>>>>>    For security threats, defensive techniques, 
>> monitoring/detection/
>>>>>    reporting of security attacks and requirements please refer to
>>>>>    [RFC5920] .
>>>>>
>>>>>>
>>>>>> Section 8.  This section needs some work.  (I'm assuming your 
>>>>>> familiar with rfc5226).
>>>>>
>>>>
>>>> Hereto, we'll see what the reviewers say...
>>>>
>>>>> What about:
>>>>>
>>>>> 8.  IANA Considerations
>>>>>
>>>>>    Upon approval of this document, IANA will make the 
>> assignment of a
>>>>>    new registry, the "OTN-TDM Container Registry" under a new GMPLS
>>>>>    Routing Parameters" with two new types (1 and 2)
>>>>>
>>>>>
>>>>>    Switching Type           Description                Reference
>>>>>    ----------------------  --------------------------  ----------
>>>>>    110 (suggested)          OTN-TDM capable (OTN-TDM)  [This.I-D]
>>>>>
>>>>>    This document defines the following sub-TLVs of the ISCD TLV:
>>>>>
>>>>>    Value  Sub-TLV
>>>>>    -----  -------------------------------------------------
>>>>>      1      Unreserved Bandwidth for fixed containers
>>>>>      2      Unreserved/MAX LSP bandwidth for flexible containers
>>>>>
>>>>>>
>>>>>> - Switching types are assigned in
>>>>>> http://www.iana.org/assignments/gmpls-sig-parameters/gmpls-sig-
>>>>>> parameters.xml#gmpls-sig-parameters-3
>>>>>> (Again I suggest 110, not 101, but this isn't a big deal)
>>>>>>
>>>>>> - I think you are actually asking for IANA to establish a
>>>> new registry.
>>>>>> Perhaps something like "OTN-TDM Container Registry" under a new 
>>>>>> "GMPLS Routing Parameters" with two new types.
>>>>
>>>> Sorry, I wasn't clear in my prior mail.  I didn't mean for the text 
>>>> to end up in the draft unmodified.  Take a look at
>>>>
>> http://tools.ietf.org/html/draft-ietf-ccamp-gmpls-dcsc-channel-ext-04
>>>> for an example of how to ask for a new Switching Type, and
>>>> http://tools.ietf.org/html/draft-ietf-ccamp-rfc4420bis-03 for an 
>>>> example of how to ask for a new TLV registry.
>>>
>>>
>>>    Upon approval of this document, IANA will make the 
>> assignment in the
>>>    "Switching Types" section of the "GMPLS Signaling Parameters"
>>>    registry located at 
>> http://www.iana.org/assignments/gmpls-sig-parameters: 
>>> Value      Type                          Reference
>>> ---------  --------------------------    ----------
>>> 110 (*)     OTN-TDM capable (OTN-TDM)    [This.I-D]
>>>
>>> (*) Suggested value
>>>    This document defines a new sub-TLV for the ISCD sub-TLV.
>> How about
>> This document defines 2 new TLVs that are carried in Interface 
>> Switching Capability Descriptors [RFC4203] with Signal Type OTN-TDM.
>>
>>> Each
>>>    TLV includes a 16-bit type identifier (the T-field).  Two
>> s/Two/The same
>>>    T-field values are applicable to the new sub-TLV.
> ok
> 
>>>
>>>    IANA
>>>    The IANA has created a new registry and will manage TLV type
>>>    identifiers as follows:
>> How about:
>> Upon approval of this document, IANA will create and maintain 
>> a new registry, the "sub-TLVs of the OTN-TDM Interface 
>> Switching Capability Descriptor TLV" registry under the "Open 
>> Shortest Path First
>> (OSPF) Traffic Engineering TLVs" registry, see 
>> http://www.iana.org/assignments/ospf-traffic-eng-tlvs/ospf-traf
> fic-eng-tlvs.xml,
>> with the TLV types as follows:
>>
>>>
>>>    - TLV Type = 1
>>>    - TLV Name = Unreserved Bandwidth for fixed containers
>>>    
>>>    - TLV Type = 2
>>>    - TLV Name = Unreserved Bandwidth for fixed containers
>>
>> The request Registration Procedures are Standards Action.
>>

What case does "Whether allowed on ISCD sub-TLV" cover? The scope of the
registry is the OTN-TDM Interface Switching Capability Descriptor TLV.

Thanks,
Lou


>> Lou
>>
>>>
>>>>
>>>> Lou
>>>>
>>>>>>
>>>>>> That's it on this document.
>>>>>>
>>>>>> Lou
>>>>>>
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: [e-mail] [mailto:[e-mail]] On 
>>>>>> Behalf Of Lou Berger
>>>>>> Sent: giovedì 20 dicembre 2012 0.28
>>>>>> To: [e-mail]; CCAMP
>>>>>> Subject: Re: [CCAMP] I-D Action: 
>>>>>> draft-ietf-ccamp-gmpls-ospf-g709v3-04.txt
>>>>>>
>>>>>> Authors?
>>>>>>
>>>>>> On 12/4/2012 4:34 PM, Lou Berger wrote:
>>>>>>> Authors,
>>>>>>> 	Please review any changes and how LC comments are addressed.
>>>>>>>
>>>>>>> Thank you,
>>>>>>> Lou
>>>>>>>
>>>>>>> On 11/28/2012 2:37 AM, [e-mail] wrote:
>>>>>>>>
>>>>>>>> A New Internet-Draft is available from the on-line
>>>>>> Internet-Drafts directories.
>>>>>>>>  This draft is a work item of the Common Control and
>>>>>> Measurement Plane Working Group of the IETF.
>>>>>>>>
>>>>>>>> 	Title           : Traffic Engineering Extensions to 
>>>>>> OSPF for Generalized MPLS (GMPLS) Control of Evolving G.709 OTN 
>>>>>> Networks
>>>>>>>> 	Author(s)       : Daniele Ceccarelli
>>>>>>>>                           Diego Caviglia
>>>>>>>>                           Fatai Zhang
>>>>>>>>                           Dan Li
>>>>>>>>                           Sergio Belotti
>>>>>>>>                           Pietro Vittorio Grandi
>>>>>>>>                           Rajan Rao
>>>>>>>>                           Khuzema Pithewan
>>>>>>>>                           John E Drake
>>>>>>>> 	Filename        : 
>> draft-ietf-ccamp-gmpls-ospf-g709v3-04.txt
>>>>>>>> 	Pages           : 33
>>>>>>>> 	Date            : 2012-11-27
>>>>>>>>
>>>>>>>> Abstract:
>>>>>>>>    ITU-T Recommendation G.709 [G.709-2012] has introduced
>>>>>> new fixed and
>>>>>>>>    flexible Optical Data Unit (ODU) containers, 
>> enabling optimized
>>>>>>>>    support for an increasingly abundant service mix.
>>>>>>>>
>>>>>>>>    This document describes Open Shortest Path First - Traffic
>>>>>>>>    Engineering (OSPF-TE) routing protocol extensions to support
>>>>>>>>    Generalized MPLS (GMPLS) control of all currently defined ODU
>>>>>>>>    containers, in support of both sub-lambda and lambda
>>>>>> level routing
>>>>>>>>    granularity.
>>>>>>>>
>>>>>>>>
>>>>>>>> The IETF datatracker status page for this draft is:
>>>>>>>>
>>>> https://datatracker.ietf.org/doc/draft-ietf-ccamp-gmpls-ospf-g709v3
>>>>>>>>
>>>>>>>> There's also a htmlized version available at:
>>>>>>>> http://tools.ietf.org/html/draft-ietf-ccamp-gmpls-ospf-g709v3-04
>>>>>>>>
>>>>>>>> A diff from the previous version is available at:
>>>>>>>>
>>>>>>
>>>>
>> http://www.ietf.org/rfcdiff?url2=draft-ietf-ccamp-gmpls-ospf-g709v3-0
>>>>>>>> 4
>>>>>>>>
>>>>>>>>
>>>>>>>> Internet-Drafts are also available by anonymous FTP at:
>>>>>>>> ftp://ftp.ietf.org/internet-drafts/
>>>>>>>>
>>>>>>>> _______________________________________________
>>>>>>>> CCAMP mailing list
>>>>>>>> [e-mail]
>>>>>>>> https://www.ietf.org/mailman/listinfo/ccamp
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> CCAMP mailing list
>>>>>>> [e-mail]
>>>>>>> https://www.ietf.org/mailman/listinfo/ccamp
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>> _______________________________________________
>>>>>> CCAMP mailing list
>>>>>> [e-mail]
>>>>>> https://www.ietf.org/mailman/listinfo/ccamp
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>
>>>
>>>
>>>
>>
> 
> 
>