Re: [CCAMP] WG Last Call: WSON documents - draft-ietf-ccamp-rwa-info

[Lou Berger <[e-mail]>]

Young,

See below.

On 1/27/2014 6:46 PM, Leeyoung wrote:
> Hi Lou,
> 
> Here's my comments to your comments. Please see inline for detail. Enclosed is a working draft that reflects all the changes per your comments.
> Let me know if this is ready to be published.
> 
> Thanks,
> Young
> 
> -----Original Message-----
> From: Lou Berger [mailto:[e-mail]] 
> Sent: Monday, January 20, 2014 1:53 PM
> To: Leeyoung; CCAMP; [e-mail]
> Subject: Re: [CCAMP] WG Last Call: WSON documents - draft-ietf-ccamp-rwa-info
> 
> Young, (all),
> 
> Now that the IPR issues are resolved on the document set, it's time to get these documents published.
> 
> There are few minor items in this document.
> 
> On 11/7/2013 5:32 PM, Leeyoung wrote:
>> Hi Lou,
>>
>> Here's my response to specific comments to draft-ietf-ccamp-rwa-info. 
>>
>> Thanks.
>> Young
>> -----Original Message-----
>> From: Lou Berger [mailto:[e-mail]]
>> Sent: Tuesday, October 29, 2013 1:26 PM
>> To: CCAMP; [e-mail]
>> Subject: Re: [CCAMP] WG Last Call: WSON documents - 
>> draft-ietf-ccamp-rwa-info
>>
>>
> ...
> 
>>
>> YOUNG>> [Switch] moved to normative reference section.
> 
> A normative reference to a a journal paper, are you sure?  I would have just changed the language to make it informative, but it's your call.
> (Unless the RFC editor chimes in on it...)
> 
>>
> ...
> 
> YOUNG>> It was a mistake I think that took place while I was shifting some references around. I will put [Switch] back to informative reference section. 
> 
>>
>> - Section 5: I found it hard to parse the following:
>>    As resources are the smallest identifiable unit of
>>    processing resource, one can group together resources into blocks if
>>    they have similar characteristics relevant to the optical system
>>    being modeled, e.g., processing properties, accessibility, etc.
>>   Do you perhaps mean?
>>    A resource is the smallest identifiable unit of
>>    allocation. One can group together resources into blocks if
>>    they have similar characteristics relevant to the optical system
>>    being modeled, e.g., processing properties, accessibility, etc.
>>
>> YOUNG>> Agreed. Changed.
> 
> I think you have a bad cut and paste.
> 
> s/resource./allocation.
> 
> YOUNG>> Agree. Done. "A resource is the smallest... allocation...."
> 
>> -Section 5.1: States: " Note that except for <ResourcePoolState>
>>   all the other components of <ResourcePool> are relatively static."
>>   But the related definitions are:
>>
>>    <ResourcePool> ::= <ResourceBlockInfo>...
>>    [<ResourceAccessibility>...] [<ResourceWaveConstraints>...]
>>    [<RBPoolState>] (section 5)
>>
>>    <DynamicNodeInfo> ::=  <NodeID> [<ResourcePoolState>] (section 7.2)
>>
>>    What's the intent here?
>>
>> YOUNG>> See the cleaned text in Section 7.2:
>>    Currently the only node information that can be considered dynamic
>>    is the resource pool state and can be isolated into a dynamic node
>>    information element as follows:
>>
>>    <DynamicNodeInfo> ::=  <NodeID> [<ResourcePool>]
>>
>>    Where
>>
>>    <ResourcePool> ::= <ResourceBlockInfo>...[<RBPoolState>]
> 
> sorry, I didn't mean for you to incorporate the <ResourcePool> definition into the document.  this was just for our discussion.  I don't think adding a duplicate/incomplete definition here is the right thing.  So I'd drop it (starting with Where.)
> 
> YOUNG>> OK. "Where..." is dropped. Also changes in Section 5.1: 
> Original: <RBPoolState> ::=(<ResourceBlockID> <NumResourcesInUse> <InAvailableWavelengths> <OutAvailableWavelengths>)...
> Changed:  <RBPoolState> ::=(<ResourceBlockID> <NumResourcesInUse> [<InAvailableWavelengths>] [<OutAvailableWavelengths>])*
> 

As was the case below, an asterisk isn't valid RBNF, so shouldn't be in
this line.

Also recall that parentheses implies strict ordering. Perhaps you mean:
<RBPoolState> ::= <ResourceBlockID> <NumResourcesInUse> 		
		[<InAvailableWavelengths>] [<OutAvailableWavelengths>]
		[<RBPoolState>]


> As <InAvailableWavelengths> <OutAvailableWavelengths> are only used in the cases of shared input or output access to the particular block.
> 
> 
> 
>>
>>
>> - Section 5.2: What is the asterisk "*" all about.
>>
>> YOUNG>> That means whatever within () can be repeated. 
>>
> 
> I don't recall this syntax definition in BNF/RBNF. Take a look at http://tools.ietf.org/html/rfc5511#section-2.2.5.
> 
> YOUNG>> Yes, I replaced "*" with "..." (per 2.2.5) which means 'repetition.' 
> 

So you now have:
   <ResourceBlockInfo> ::= ([<ResourceSet>] <InputConstraints>
   [<ProcessingCapabilities>] <OutputConstraints>)...

That's fine, but keep in mind that use of parenthesis implies ordering.
Do you mean:

   <ResourceBlockInfo> ::= [<ResourceSet>] <InputConstraints>
		   [<ProcessingCapabilities>] <OutputConstraints>
		   [<ResourceBlockInfo>]

Actually since the use of <ResourceBlockInfo> already indicates
repetition in section 5, no  repetition is needed here, which yields

   <ResourceBlockInfo> ::= [<ResourceSet>] <InputConstraints>
		   [<ProcessingCapabilities>] <OutputConstraints>

> Also you list [<ResourceSet>] as optional, yet it is mandatory in section 4.1 of
> http://tools.ietf.org/html/draft-ietf-ccamp-rwa-wson-encode-23
> 
> YOUNG>> <ResourceSet> is optional. I don't think section 4.1 of wson-encode-23 treats this as mandatory. Wson-encode-23 document simply provides encoding details irrespective of whether it is mandatory or optional. Let me know if you disagree with this. 
> 

So 4.1 of wson-encode-24 you just distributed says:

       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
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |                     RB Set Field                              |
      :                                                               :
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |I|O|                         Reserved                          |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |                Optical Interface Class List(s) (opt)          |
      :                                                               :
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |               Acceptable Client Signal Type (opt)             |
      :                                                               :
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |                    Input Bit Rate List (opt)                  |
      :                                                               :
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |                Processing Capabilities List (opt)             |
      :                                                               :
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

To me, I'd read this as

   <ResourceBlockInfo> ::= <ResourceBlockSet>
		   [<InputConstraints>] [<OutputConstraints>]
		   [<ProcessingCapabilities>]

Which is correct?  Also note that you have ResourceSet in this document
and "RB Set Field" in wson-encode-24. to align this document:
s/<ResourceSet>/<ResourceBlockSet>

> ...
>>
>> - Section 6.6.  I think you have a BNF problem here.  The BNF says 
>> restriction parameters are always optional, but your text says that 
>> there are requirements based on <RestrictionTypes>.  I think the BNF 
>> needs to be aligned with the text and reflect the requirements.
>>
>> YOUNG>> Made all mandatory element. 
> 
> The new text says:
> 
>    <PortLabelRestriction> ::= <GeneralPortRestrictions>...
>    <MatrixSpecificRestrictions>...
> 
> This now says that a PortLabelRestriction MUST (and always) include
> *both* GeneralPortRestrictions and MatrixSpecificRestrictions.  Is this correct in all cases?
> 
> YOUNG>> No. Actually <PortLabelRestriction> is an optional information. It will be corrected as follows:
> 
>    <PortLabelRestriction> ::= [<GeneralPortRestrictions>...] [<MatrixSpecificRestrictions>...]

A few points here:
1) to get repetition of <PortLabelRestriction> I suggest:
OLD
   <LinkInfo> ::=  <LinkID> [<AdministrativeGroup>]
   [<InterfaceCapDesc>] [<Protection>] [<SRLG>]...
   [<TrafficEngineeringMetric>] [<PortLabelRestriction>]
NEW
   <LinkInfo> ::=  <LinkID> [<AdministrativeGroup>]
   [<InterfaceCapDesc>] [<Protection>] [<SRLG> ...]
   [<TrafficEngineeringMetric>] [<PortLabelRestriction> ...]

2) What is unclear is that when <PortLabelRestriction> what it must
contain.  looking at 2.2 of wson-encode-24 I see

      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
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
     |   MatrixID    |RestrictionType| Switching Cap |     Encoding  |
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
     |     Additional Restriction Parameters per RestrictionType     |
     :                                                               :
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

which translates to me to be:
    <PortLabelRestriction> ::= <MatrixID> <RestrictionType>
				<Restriction parameters list>

    <Restriction parameters list> ::=
		<Simple label restriction parameters> |
		<Channel count restriction parameters> |
		<Label range restriction parameters> |
		<Simple+channel restriction parameters> |
		<Exclusive label restriction parameters>

    <Simple label restriction parameters> ::= <LabelSet> ...
	

    <Channel count restriction parameters> ::= <MaxNumChannels>
	

    <Label range restriction parameters> ::=
	<MaxLabelRange> (<LabelSet> ...)
	

    <Simple+channel restriction parameters> ::=
	 <MaxNumChannels> (<LabelSet> ...)

    <Exclusive label restriction parameters> ::= <LabelSet> ...
			

> ----------------------------------------------------------------------------------------------------
> also:
> 
>    <RestrictionParameters> ::= <LabelSet>... <MaxNumChannels>
>    <MaxWaveBandWidth>
> 
> So all parameters MUST be included for all RestrictionTypes?  I
> suspect you mean to say there alternatives based on RestrictionType.
> If correct, syntax is covered in
> http://tools.ietf.org/html/rfc5511#section-2.2.4.
> 
> YOUNG>> No. Indeed these parameters appear only if they match with Restriction Types. So it should be as follows:
> 
> <RestrictionParameters> ::= (<LabelSet>...) | <MaxNumChannels> | <MaxLabelRange> | (<LabelSet>... <MaxNumChannels>) | <LinkSet> (Note: this is arranged to be consistent with draft-ietf-ccamp-general-constraint-encode-13.txt in the naming and order)
> 

Addressed above.

> In Section 6.6, this is a summary of all encoding changes: 
> 
> <PortLabelRestriction> ::= [<GeneralPortRestrictions>...] [<MatrixSpecificRestrictions>...]
> 
> <GeneralPortRestrictions> ::= <RestrictionType> <RestrictionParameters>  (RestictionType and one of the parameters corresponding to the RestrictionType must be there)
> 
> <MatrixSpecificRestriction> ::= <MatrixID> <RestrictionType> <RestrictionParameters> (the same rational apples as above) 
> 
> <RestrictionParameters> ::= 
> (<LabelSet>...) | <MaxNumChannels> | <MaxLabelRange> | (<LabelSet>... <MaxNumChannels>) | <LinkSet> 
> 
> 

Addressed above.

> 
> Also the RestrictionParameters object/field naming doesn't match draft-ietf-ccamp-general-constraint-encode-13.txt, this section should be updated to match.
> 
> YOUNG>> RestrictionParameters are now matching with draft-ietf-ccamp-general-constraint-encode-13.txt.
> 
> s/PortSet/LinkSet
> s/MaxWaveWidth/MaxLabelRange
> 

I think I sense some fatigue. (on my part too!)

OLD
   LABEL_RANGE:   Waveband device with a tunable center frequency and
   passband. This constraint is characterized by the MaxLabelRange
   parameter which indicates the maximum width of the waveband in terms
   of channels. Note that an additional wavelength set can be used to
   indicate the overall tuning range. Specific center frequency tuning
   information can be obtained from dynamic channel in use information.
   It is assumed that both center frequency and bandwidth (Q) tuning
   can be done without causing faults in existing signals.
NEW
   LABEL_RANGE:  Used to indicated a restriction on a range of labels
   that can be switched.  For example a waveband device with a tunable
   center frequency and passband. This constraint is characterized by
   the MaxLabelRange parameter which indicates the maximum range of the
   labels, e.g., which may represent a waveband in terms of channels.
   Note that an additional parameter can be used to
   indicate the overall tuning range. Specific center frequency tuning
   information can be obtained from dynamic channel in use information.
   It is assumed that both center frequency and bandwidth (Q) tuning
   can be done without causing faults in existing signals.

and
OLD
     MaxLabelRange is the maximum width of a tunable waveband switching
     device.
NEW (from general-constraint-encode)
   MaxLabelRange indicates the maximum range of the labels.

That's it, thanks,.
Lou

> ...
> I think this covers all open points on this one.
> 
> Lou
>