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

From: Lou Berger <[email-masked]>
Date: Tue, 04 Dec 2012 16:34:12 -0500

Authors,
	Please review any changes and how LC comments are addressed.

Thank you,
Lou

On 11/28/2012 2:37 AM, [email-masked] 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-04
> 
> 
> Internet-Drafts are also available by anonymous FTP at:
> ftp://ftp.ietf.org/internet-drafts/
> 
> _______________________________________________
> CCAMP mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/ccamp
> 
> 
> 
>

---

From: Lou Berger <[email-masked]>
Date: Wed, 19 Dec 2012 18:27:36 -0500

Authors?

---

From: Daniele Ceccarelli <[email-masked]>
Date: Thu, 20 Dec 2012 08:57:03 +0000

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: [email-masked] [mailto:[email-masked]] 
>On Behalf
>Of Lou Berger
>> Sent: mercoledì 26 ottobre 2011 0.37
>> To: [email-masked]
>> 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

>>
>> 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

>...
>> 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.

>Detailed editorial and technical comments:
>
>- Please verify that abbreviations are defined before being used .
>There are a number of these.

Done

>
>- Please use a consistent decimal representation (sometimes 
>commas are used other times periods)
>
>- the references [G709-v1] and [G709-v3] each actually refer 
>to multiple documents, each documented needs to have it's own 
>(correct) reference, i.g., [G709-v1] and [G709-v1a1]. The 
>document text will need to be revisited to ensure the proper 
>reference is made.

Updated accordingly with latest proposal: RFC4328 when referencing "old G.709" and G709-2012 when referncing the last one (v4).

>
>-
>http://tools.ietf.org/idnits?url=http://tools.ietf.org/id/draft
-ietf-ccamp-gmpls-ospf-g709v3-03.txt
>shows there are unresolved nits that need to resolved .  I'm 
>using line numbers from this url in my subsequent comments.
>
>- Line 24: drop "The recent revision of "
>
>- Line 138: "ODUk an higher order" --> "ODUk to indicate a 
>higher order"
>
>- Line 145: "iterated" --> "repeated"
>
>- Line 181/2: Drop "Some of the prominent representations are 
>captured below."
>
>- Line 202: perhaps "termed" --> "referred to"?
>
>- Line 226: I'd suggest 110, but don't feel too strongly about it.
>
>- Line 228: Start with "When supporting the extensions defined 
>in this document, the"
>
>- Lines 236-261: Replace all lines with "MUST be interpreted 
>as defined in [RFC4203]".  No need to repeat what's already specified.
>
>- Lines 265-272: Additional information is fine, but don't 
>repeat 4203 unless directly quoting.
>
>- Line 300: Need to specify what "differing characteristics" 
>means using
>RFC2119 language.
>
>- Section 4.1.
>
>I think the combine presentation of the two types is 
>confusing.  I suggest adding a section 4.1.1 right after line 
>311 covering "Switch Capability Specific Information for fixed 
>containers" followed by a section 4.1.2 covering "Switch 
>Capability Specific Information for flexible containers".  
>Field definitions will need to be reorganized as appropriate.

Rearranged as follows:
4.1.  Switch Capability Specific Information . . . . . . . . . .  7  8
       4.1.1.  Switch Capability Specific Information for fixed
               containers . . . . . . . . . . . . . . . . . . . . . .  9
       4.1.2.  Switch Capability Specific Information for
               variable containers  . . . . . . . . . . . . . . . . . 10
       4.1.3.  Switch Capability Specific Information - Field
               values and explanation . . . . . . . . . . . . . . . . 12

>
>Also the common format and rules related to the OTN-TDM SCSI 
>container should be defined before going into the type 
>specific definitions. (at the 4.1) level.
>
>- Lines 405-417: How about replace all with:
>  "Signal Type: Indicates the ODU type being advertised. 
>Values are defined in [OTN-SIG]"
>
>- Line 428: after "level" suggest adding something like "below 
>the indicated signal type"
>
>- Line 442: "00" --> "0"
>
>- Lines 447, 454, 463, 464: I find the "don't care" case a bit 
>odd.  How about make 0 "unused" or "ignored" and use it instead of 4?
>
>- Lines 472/3: Replace "stage of the muxing hierarchy" --> "of 
>the indicated Number of stages"
>
>- Line 476: "then no Stage fields MUST be included." --> "then 
>the Stage and Padding fields MUST be omitted."
>
>- Lines 484/5: "Only Unreserved/MAX LSP BW fields for 
>supported" --> "Unreserved/MAX LSP BW fields for each identified"
>
>- Line 486: After "to 7)" add ", and Unreserved/MAX LSP BW 
>fields for other priority values MUST be omitted."
>
>- Line 497: Drop "Only", "for supported" --> "for each supported"
>
>- As mentioned above need processing rules/procedures, for 
>multiple ISCDs, SCSI information, e.g., use and ordering of 
>multiple containers.
>
>- Line 503: "infer" -> "imply"
>
>- Fill in all example field values
>
>- 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).

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.
>
>That's it on this document.
>
>Lou
> 

>-----Original Message-----
>From: [email-masked] [mailto:[email-masked]] 
>On Behalf Of Lou Berger
>Sent: giovedì 20 dicembre 2012 0.28
>To: [email-masked]; CCAMP
>Subject: Re: [CCAMP] I-D Action: 
>draft-ietf-ccamp-gmpls-ospf-g709v3-04.txt
>
>Authors?
>
>>>  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

---

From: Lou Berger <[email-masked]>
Date: Thu, 20 Dec 2012 18:29:21 -0500

Daniele / Authors,
	Thank you for the response.  Please see below for my responses.

On 12/20/2012 3:57 AM, Daniele Ceccarelli wrote:
> All nits, typos and suggested text changes without any comment in
> line have been accepted/modified accordingly.
> 

>>> The field descriptions are missing format related conformance

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

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?

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.)

>>> great, but you also need some RFC2119 language.)

The length of each TLV type and each field should be defined. (You have
it for some fields, but not others).

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?

Line 416: By separately do you mean "in separate TLVs"?

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.

Line 419/whole document: Please double check the document for spelling
errors (there's one in the above paragraph.)

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.

Line 428: s/Flags:/Flags (8 bits)

Lines 455-462: should be revised to use 2119 conformance language (and
to clarify the malformed text.)

The "RES" field isn't defined.

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.

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.

> 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.

>> Detailed editorial and technical comments:
Thank you!
[...]

>> Section 8.  This section needs some work.  (I'm assuming your 

Hereto, we'll see what the reviewers say...

>    Switching Type           Description                Reference

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.

Lou

---

From: Daniele Ceccarelli <[email-masked]>
Date: Fri, 21 Dec 2012 15:49:12 +0000

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

>From: Lou Berger [mailto:[email-masked]] 
>Sent: venerdì 21 dicembre 2012 0.29
>To: Daniele Ceccarelli
>Cc: [email-masked]; CCAMP
>> All nits, typos and suggested text changes without any 
>comment in line 
>> have been accepted/modified accordingly.
>> 
>
>> BR
>> Daniele & Sergio
>> 
>> 
-ietf-ccamp-gmpls-ospf-g709v3-04.txt)
>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).

>
>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.

>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.

>419  implicitely supports also signal Signal Type 22, so only 
>Signal 420  Type 21 MUST be advertised.  Signal Type 22 MUST 
>be used only

Agree. Done.

>Line 416: By separately do you mean "in separate TLVs"?

Yes.changed
>
>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

>Line 419/whole document: Please double check the document for 
>spelling errors (there's one in the above paragraph.)
>
>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

>The "RES" field isn't defined.

 <t>- Res (3 bits): reserved bits. MUST be set to 0.</t>  

>
>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 advertised.
The left most bit represent priority 0 (highest) and the right most priority 7 (lowest)
And are presented in ascending orded. Each
bit MUST be set when the related priority is supported and MUST be
cleared when the related priority is not supported. When the interface does not 
support priorities, the advertisement MUST be compliant with the advertisement of priority 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.

- 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.

>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.

- 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.

- 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:

>>> - 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.
>>>
>> 
>> 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)

>>> implications / added risk of additional information 
>provided in this 
>>> document.
>>> 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-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. Each
   TLV includes a 16-bit type identifier (the T-field).  Two
   T-field values are applicable to the new sub-TLV.

   The IANA has created a new registry and will manage TLV type
   identifiers as follows:

   - TLV Type (T-field value)
   - TLV Name
   - Whether allowed on LSP_ATTRIBUTES object
   - Whether allowed on LSP_REQUIRED_ATTRIBUTES object.

   This document defines new TLV types as follows:

   - TLV Type = 1
   - TLV Name = Unreserved Bandwidth for fixed containers
   - allowed on ISCD sub-TLV
   
   - TLV Type = 2
   - TLV Name = Unreserved Bandwidth for fixed containers
   - allowed on ISCD sub-TLV
   
   New TLV type values may be allocated only by an IETF Consensus
   action.

>>> From: [email-masked] [mailto:[email-masked]] On 
>>> Behalf Of Lou Berger
>>> OSPF for Generalized MPLS (GMPLS) Control of Evolving G.709 OTN 
>>> Networks

---

From: Lou Berger <[email-masked]>
Date: Fri, 21 Dec 2012 13:31:42 -0500

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.

> 	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.

I actually was expecting you to say something referring back to signal
type or number of stages represented in the TLV...

> 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…

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.)

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 -

   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)

> Now all of them should have been filled.

great.

>> Shouldn't this text be moved to after line 304?

great.

>> Line 416: By separately do you mean "in separate TLVs"?

great.

> 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?

>  <t>- Res (3 bits): reserved bits. MUST be set to 0.</t>  

"and ignored on receipt."

> - 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."

> - 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).

>          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.

> - 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 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.

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.

> - 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.

> - 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.

> 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?

>    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
s/Two/The same
>    IANA
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-traffic-eng-tlvs.xml,

>    - TLV Name = Unreserved Bandwidth for fixed containers

The request Registration Procedures are Standards Action.

Lou