[CCAMP] AD review of draft-ietf-ccamp-rwa-wson-encode

["Adrian Farrel" <[e-mail]>]

Hello,

I've now done my AD review of this document. I am rather regretting
starting the IETF last call for draft-ietf-ccamp-general-constraint-
encode on which this document depends because it seems that this
document introduces WSON-specific encodings (all of those before
Section 4) that should/could have been made generic. In fact, I
thought the point of the general encodings document was to produce
protocol objects that could be used by technology-specific documents
like this one.

If you can respond to my comments below, we can work out whether the
general encodings document needs to be brought back for more work.

Thanks for your efforts with this.

Adrian

===========

Abstract

   while other parts are fairly specific to WSONs.

They are either specific or they are not. I think you need
s/fairly specific/specific/

---

Shouldn't the Introduction include a discussion of and reference to
[Gen-encode]?

---

The Introduction has


   This document provides efficient encodings

and

   Note that since these encodings are relatively efficient

Which are they? And relative to what?

---

Section 1.1 has

   Refer to Section 5 of [Gen-Encode] for the terminology of Resources,
   Resources Blocks, and Resource Pool.

I think you mean [RWA-Info]. That would also make [RWA-Info] a normative
reference which seems correct anyway.

---

Section 1.1 makes RFC 6163 a normative reference.

---

Section 2.1 seems somewhere between a copy of and a modification of
the Link Set Field in 2.3 of [Gen-Encode].

Some clarification would be useful. If this is different, why is it not
using one of the generic encodings applied in a specific way? If it is
the same, you should probably just reference the encoding in
[Gen-Encode] and describe here how the fields are used, but if you
insist in re-drawing the figure it should be aligned with the figure in
[Gen-Encode].

---

Section 2.1

   The RB identifier represents the ID of the resource block which is a
   32 bit integer.

You might note that the scope of the RB identifier is local to the node
on which it is applied although that node may choose to use a globally
known encoding such as from RFC 6205.

I assume that flexi-grid is out of scope for WSON. If it is not, then
you need to think further about your 32 bit identifiers.

---

Section 3.1

Why isn't the Resource Accessibility Field expressed in terms of the
use of a generic Connectivity Matrix Field from section 2.1 of
[Gen-Encode]? I thought the whole point of [Gen-Encode] was to derive
application agnostic encodings that could be used without modification
(but with applicability notes) by specific technologies.

---

Section 3.2

I looked for the equivalent of the Resource Wavelength Constraints
Field in [Gen-Encode]. I understand that Input Wavelength Constraints
Field and Output Wavelength Constraints Field are encoded using the
generic Label Set of  [Gen-Encode], but I thought that the whole concept
of Resource Constraints would be generic.

---

Section 3.3

As with the previous section I don't see anything that is WSON-specific
in the concept of the 3.3. Resource Block Pool State (RBPoolState) Field
and I wondered why [Gen-Encode] doesn't have anything to cover this.

---

Section 3.3

   Where Action = 0 denotes a list of 16 bit integers and Action = 1
   denotes a bit map. In both cases the elements of the RB Set field
   are in a one-to-one correspondence with the values in the usage RB
   usage state area.

This is not clear. I think the Action field is explaining how the
RB Usage State field is encoded.
- Not sure why you call it "Action"
- Would be good if you could make it clear that "16 bit integers"
  or "bit map" apply to the RB Usage State field.

But...

   RB#i State (16 bits, unsigned integer): indicates Resource Block #i
   is in use or available.

- You might say what value of the 16 bit unsigned integer indicates in
  use and what value indicates available.
- You should explain to me why a list of 16 bit integers to encode a
  set of Booleans is in anyway efficient or appropriate.

It is *really* hard to parse from this section that the state applies
to the whole RB when Action is 0, but applies to the elements of the RBs
when Action is 1. At least, that is what I think I parsed from the text
although I also found text in the section that convinced me you meant
something else.

In short, this section is not clear!

---

Why don't Sections 3.4 and 4 have a B-bit like that in 3.2?

---

Sections 3.4 and 4 should list the allowed settings of I and O
(presumably: 01, 10, 11). Cf. Section 3.2.

---

Section 4

How do I know the length of the ResourceBlockInfo field? I need to know
this to decide whether to try to parse the next bytes as another
Optional subfield. I *do* when I reach the end of one Optional subfield,
but I don't know whether another follows.

Possibly you intend the object that includes a ResourceBlockInfo field
to provide the length information, but other fields defined in this
document do include lengths or enough information to deduce the lengths.

---

Section 4.1

   The following I and E combination are defined:

s/E/O/

---

Section 4.1

           1: [ITU-G.698.1] application code.

           2: [ITU-G.698.2] application code.

           3: [ITU-G.959.1] application code.

           4: [ITU-G.695] application code.

You should use the same format as in the references section. E.g.,
[G.959.1].

Do you mean 698 or 694? You have references for 694.1 and 694.2, but
not 698.1 or 698.2. But 694.1 does not seem to include any "application
codes" - they're in 698.1 and 698.2.

Each of the subsections 4.1.1-4.1.4 should include a citations.

---

Section 4.1
How do I interpret a Vendor-Specific Application Code? Is there an OUI
I'm missing?

---

I discussed sections 4.1.1-4.1.4 with the authors and the WG chairs and
have asked the chairs to send a liaison to ITU-T Q6/15 asking them to
cast an eye over the text of these sections.

---

4.1.1 

   Where (values between parenthesis refer to ITU defined values as
   reported above):

Please remove the parentheses from this sentence.

---

4.1.1 and 4.1.2

      An Optional F can be added indicating a FEC Encoding.

      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
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |B|  D  |S|   c   |   W   |   y   |   t   |   z   |  v  |   F   |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |                           reserved                            |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

         F (suffix): = 0 reserved, = 1 Fec Encoding

      Values not mentioned here are not allowed in this application
   code

If F is optional but only one value is allowed (viz. 1) how do I opt to
not indicate a FEC Encoding?

---

4.1.1 and 4.1.2

Your definition of parameter c makes RFC 6205 a normative reference.
                                                                              .
I think you could usefully point with more precision to Figure 2 in 
Section 3.2 of RFC 6205.

However, I wonder whether you want to allow new values that may be added
to the IANA registry created by Section 5.2 of RFC 6205

---

4.1.3 and 4.1.4


         n: maximum number of channels (10 bits, up to 1024 channels)

Hmmm, 2^10 is 1024, but 10 bits can only encode 1023 unless you say that
n=0 is not valid and so n is actually max channels minus one.

---

4.4

   The processing capability list field is then given by:

      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
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
     |            Reserved           |        Processing Cap ID      |
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
     |   Possible additional capability parameters depending upon    |
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
     :   the processing ID                                           :
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

   When the processing Cap ID is "regeneration capability", the

I don't believe you have told me how to encode "regeneration
capability" into the Processing Cap ID field. Possibly you mean that the
numbered list above is intended to define the settings of this field.
                                            
If so then:
- say so
- explain that "Fault and performance monitoring" and "Vendor Specific
  capability" have no additional capability parameters
- probably remove the note about "Fault and performance monitoring" and
  "vendor specific capability" because if it isn't here it is, of
  course, for future study.

---

4.4

   Note that when the capability of regenerator is indicated to be
   Selective Regeneration Pools, regeneration pool properties such as
   input and output restrictions and availability need to be specified.
   The code point for this is subject to further study.

I think you mean to replace that final line with...

   These properties will be encoded in the capabilities field starting
   with the bits marked Reserved in the figure.  An additional
   specification describing the encoding of these parameters is required
   before the value C=2 can be used.

---

Section 6

In Section 6 and 6.1 you appear to be creating a new top-level registry
called "GMPLS Routing Parameters for WSON" with a sub-registry called
"Types for subfields of WSON Resource Block Information".

It's a shame to create a whole new top-level registry. I suppose you 
think that this information will only ever be used in routing and
never in signaling. Probably right, in which case you are good to go,
although your text could be clearer.

---

Section A.3 is using some form of BNF to represent information.

This is probably RBNF from RFC 5511. Anyway, you need to give a
reference so people can read it.

It would be nice to lay the BNF out on the page in a more readable way.