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

Leeyoung <leeyoung@huawei.com> Wed, 14 January 2015 23:20 UTC

Return-Path: <leeyoung@huawei.com>
X-Original-To: ccamp@ietfa.amsl.com
Delivered-To: ccamp@ietfa.amsl.com
Received: from localhost (ietfa.amsl.com [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id B15111ACF1D for <ccamp@ietfa.amsl.com>; Wed, 14 Jan 2015 15:20:40 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -0.088
X-Spam-Level:
X-Spam-Status: No, score=-0.088 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, FRT_SLUT=2.522, HK_RANDOM_ENVFROM=0.001, HK_RANDOM_FROM=1, J_CHICKENPOX_21=0.6, RCVD_IN_DNSWL_MED=-2.3, SPF_PASS=-0.001, T_RP_MATCHES_RCVD=-0.01] autolearn=no
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id iYn-sZBzjoDU for <ccamp@ietfa.amsl.com>; Wed, 14 Jan 2015 15:20:37 -0800 (PST)
Received: from lhrrgout.huawei.com (lhrrgout.huawei.com [194.213.3.17]) (using TLSv1 with cipher RC4-SHA (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 147441ACEA7 for <ccamp@ietf.org>; Wed, 14 Jan 2015 15:20:34 -0800 (PST)
Received: from 172.18.7.190 (EHLO lhreml404-hub.china.huawei.com) ([172.18.7.190]) by lhrrg02-dlp.huawei.com (MOS 4.3.7-GA FastPath queued) with ESMTP id BOB02565; Wed, 14 Jan 2015 23:20:33 +0000 (GMT)
Received: from DFWEML701-CHM.china.huawei.com (10.193.5.50) by lhreml404-hub.china.huawei.com (10.201.5.218) with Microsoft SMTP Server (TLS) id 14.3.158.1; Wed, 14 Jan 2015 23:20:32 +0000
Received: from DFWEML706-CHM.china.huawei.com ([10.193.5.225]) by dfweml701-chm ([10.193.5.50]) with mapi id 14.03.0158.001; Wed, 14 Jan 2015 15:20:21 -0800
From: Leeyoung <leeyoung@huawei.com>
To: "adrian@olddog.co.uk" <adrian@olddog.co.uk>, "draft-ietf-ccamp-rwa-wson-encode.all@tools.ietf.org" <draft-ietf-ccamp-rwa-wson-encode.all@tools.ietf.org>
Thread-Topic: [CCAMP] AD review of draft-ietf-ccamp-rwa-wson-encode
Thread-Index: AdAmyKqM3E2i5eP+S/KXayUoWZed4QFfHtyw
Date: Wed, 14 Jan 2015 23:20:20 +0000
Message-ID: <7AEB3D6833318045B4AE71C2C87E8E1729C71AC5@dfweml706-chm>
References: <00dd01d026c8$c3bd9280$4b38b780$@olddog.co.uk>
In-Reply-To: <00dd01d026c8$c3bd9280$4b38b780$@olddog.co.uk>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
x-originating-ip: [10.192.11.120]
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-CFilter-Loop: Reflected
Archived-At: <http://mailarchive.ietf.org/arch/msg/ccamp/_HNL2rIKvYxdjmgDkPSLrtdgmuM>
Cc: "ccamp@ietf.org" <ccamp@ietf.org>, "ccamp-chairs@tools.ietf.org" <ccamp-chairs@tools.ietf.org>
Subject: Re: [CCAMP] AD review of draft-ietf-ccamp-rwa-wson-encode
X-BeenThere: ccamp@ietf.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: Discussion list for the CCAMP working group <ccamp.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/ccamp>, <mailto:ccamp-request@ietf.org?subject=unsubscribe>
List-Archive: <http://www.ietf.org/mail-archive/web/ccamp/>
List-Post: <mailto:ccamp@ietf.org>
List-Help: <mailto:ccamp-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/ccamp>, <mailto:ccamp-request@ietf.org?subject=subscribe>
X-List-Received-Date: Wed, 14 Jan 2015 23:20:40 -0000

Hi Adrian,

Thanks. 

My comment for general vs. WSON-specific issue is that
the design philosophy of general encoding was to generalize a common element that can be applied
to different technologies. For instance, connectivity matrix definitely can be applied to WSON, OTN and other
Switching technology as one encoding can fit to all. You seemed to desire a sharing of the same encoding to describe 
different entities where possible. This is fine for label vs. wavelength as there is one-to-one mapping for this.
However, Resource Block encoding cannot properly share with the connectivity matrix encoding with two reasons:

1. They refer to different elements in the node.
2. They require different sets of fields with different number of fields. 

Please also see my comment in-line for details.

Thanks,
Young
-----Original Message-----
From: CCAMP [mailto:ccamp-bounces@ietf.org] On Behalf Of Adrian Farrel
Sent: Friday, January 02, 2015 2:15 PM
To: draft-ietf-ccamp-rwa-wson-encode.all@tools.ietf.org
Cc: ccamp@ietf.org; ccamp-chairs@tools.ietf.org
Subject: [CCAMP] AD review of draft-ietf-ccamp-rwa-wson-encode

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/

[YOUNG] YES. 

---

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

[YOUNG] A good point. Will add this. 

---


The Introduction has


   This document provides efficient encodings

and

   Note that since these encodings are relatively efficient

Which are they? And relative to what?

[YOUNG] will remove 'relatively'.

---

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.

[YOUNG] Yes. Will change. 
---

Section 1.1 makes RFC 6163 a normative reference.

[YOUNG] Yes. Will correct. 

---

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

[YOUNG] not quite. Link Set and Label Set are two different things with different fields. 
Link Set has directionality and identifier format while Label Set does not have these fields but has connectivity field.
Not sure how we can point to each other. 
---

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.

[YOUNG] Flex-grid is out of scope here. In addition, RB identifier is a resource block ID
Which is different from wavelength identifier. I am sure how a globally known encoding for
Wavelength id can be used for RB identifier. They seemed to refer two different entities. 
 
---

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.

[YOUNG] Not quite. Connectivity Matrix encoding cannot be used for Resource Accessibility Field as the latter
has additional entity (namely, RB set field) to describe. We generalized a single-stage connectivity matrix in [Gen-Encode] that can 
be applicable to any switching technology. Here in WSON, we have a need to model RB constraints (for the pool of Wavelength Converters)
from/to input/output link sets. Putting two different entities into one coding was not the choice of the authors. 

---

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.

[YOUNG] Again, I think the design of generalization is not intended to 
share the same encoding to describe two different entities. Besides, I do
not see the need to generalize encoding of an entity that has no particular
use in other technologies than WSON. 

---

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.

[YOUNG] RB is wavelength converters, which is a unique WSON element. 
---

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!

[YOUNG] Indeed it is not clear. For this, let me get back to you shortly. 

---

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

[YOUNG] I think the reason for B bit is not included in Section 3.4 is the context where it won't be very useful (even if we have a B bit) as input wavelengths available will hardly match with output wavelength available in most cases. Compared to this, wavelength constraints can more often be same for input and output. 

---

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

[YOUNG] OK. How adding:

Currently the only valid combinations of (I,O) are (0,1), (1,0), or (1,1). 

---

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.

[YOUNG] Yes, you're right. It is not completely consistent. How about the following
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
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |I|O|  Reserved                 |        Length                 |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |                          RB Set Field                         |
      :                                                               :
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |                        Optional subfield 1                    |
      :                              ...                              :
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      :                               :                               :
      :                               :                               :
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |                        Optional subfield N                    |
      :                                                               :
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      
Length: 16 bits

   The total length of this field in bytes.

---

Section 4.1

   The following I and E combination are defined:

s/E/O/

[YOUNG] Agree. Thanks.
---

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.

[YOUNG] Yes, for the reference format. The right references are 698.1 and 698.2 --- will correct with the right references. Citations will be added in each of the subsections. 
---

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

[YOUNG] Not sure if I understood this question. What is "OUI"? 

---

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.

[YOUNG] Thanks. It is a good idea to send a liaison to Q6/15. 

---

4.1.1 

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

Please remove the parentheses from this sentence.

[YOUNG] OK. 
---

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?

[YOUNG] I guess 0 will do it. 

---

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

[YOUNG] OK. Will change RFC 6205 as a normative reference and point with more precision to Fig. 2 in Section 3.2 of RFC 6205. Since we are not adding values to the channel spacing here, I am not sure if it is appropriate to discuss this in this document.
---

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.

[YOUNG] I would change: 1024 -> 1023.
---

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.

[YOUNG] Yes, how about adding right after the encoding and before, "when ..." the following: 

   The processing capability ID field defines the following processing capabilities:

     0: Reserved

     1: Regeneration capability

     2: Fault and performance monitoring

     3: Vendor Specific capability

[YOUNG] OK, will remove the note on the further study and add: 

"Fault and performance monitoring" and "Vendor Specific
  capability" have no additional capability parameters. 

---

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.

[YOUNG] Thanks. Agree. 

---

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.

[YOUNG] Unfortunately it is only intended for Routing. Will add this is the case in the text. 

---

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.

[YOUNG] OK, will reference and do some cosmetic surgery on this. 

_______________________________________________
CCAMP mailing list
CCAMP@ietf.org
https://www.ietf.org/mailman/listinfo/ccamp