Re: [CCAMP] Alia Atlas' Discuss on draft-ietf-ccamp-wson-signal-compatibility-ospf-15: (with DISCUSS and COMMENT)

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

Hi Young, Alia,

>>> I'd like to see clearer descriptions on particularly the error-handling
>>> around multiple TLVs and sub-TLVs.
>>> I see a few points of ambiguity - as described below.  I think these
>>> should be straightforward to clarify but interoperability could be 
>>> affected if this isn't done.   Thanks!
>>>
>>> In Sec 2, it says "Only one Optical Node Property TLV shall be advertised
>>> in each LSA." What is the error-handling if multiple of this TLV are
>>> seen?
>>
>> YOUNG>> This is to allow for fine granularity changes in topology per
>> [RFC3630]. It is not an error per se if there are multiple of this TLV are
>> in each LSA.
>>
>>> Is that a SHALL or a "at most one... MUST be advertised"?  I'd expect
>>> normative language.
>>
>> YOUNG>> "SHALL" seems to be right one, again per [RFC3630].
>>
> [Alia] Ok - SHALL is fine.  Perhaps text like 
> "To allow for fine granularity changes in topology, only one Optical Node
> Property TLV SHALL be advertised in each LSA.  An implementation MUST
> support receiving multiple Optical Node Property TLVs in an LSA."

Sorry, but I think this reads as nonsense :-(
"SHALL" and "MUST" are identical in meaning.
So you have written "only one TLV MUST be advertised". That is ambiguous!
It could mean "no more than one" or it could mean "it is only necessary to advertise one must, but others may be included"
The thrust of the conversation appears to be for the latter, so don't try to do this in clever compact code - write it out in full!

That might give (if it is what you intend to say)...

When using the extensions defined in this document, at least one Optical Node Property TLV MUST be advertised in each LSA. To allow for fine granularity changes in topology, more than one Optical Node Property TLV MAY be advertised in a single LSA. Implementations MUST support receiving multiple Optical Node Property TLVs in an LSA.

[snip]

>> May I suggest the following changes:
>>
>> OLD
>>
>>   The format of the SCSI MUST be as depicted in the following figure:
>>
>> NEW
>>
>>    The format of the SCSI SHALL be as depicted in the following figure:
>>
>> END

These sentences are identical! MUST==SHALL

So Alia's text is nicer...

> [Alia] How about "A SCSI may contain multiple Available Label sub-TLVs and 
> multiple Shared Backup Label sub-TLVs.  The following figure shows the 
> format for a SCSI that contains these sub-TLVs.  The order of the sub-TLVs
> in the SCSI is arbitrary."

[snip]
 
>>> End of Sec 4: "In case where the new sub-TLVs or their attendant
>>> encodings are malformed, the proper action would be to log the problem
>>> and ignore..."  First, please be explicit in what behavior MUST be done.

I think this request got lost in the discussions that follow. Alia is correctly asking for...

"When bad stuff happens you MUST do some things, SHOULD do other things, and MAY do additional things."

>> YOUNG>> malformed sub-TLVs or their attendant encodings refer to that which are
>> not parse-able due to corruption, etc. and cannot be stored in the TED.

Hmmm. The most common cause of unparsable TLVs will be encoding errors, bugs, misinterpretation of specs, up-level implementations, etc. Corruption doesn't happen much.
The upshot is the same - you can't process the TLV.

>> How about adding a new sentence to describe the expected action as follows:
>>
>> OLD
>>
>>   In case where the new sub-TLVs or their attendant encodings are
>>   malformed, the proper action would be to log the problem and ignore
>>   just the sub-TLVs in GMPLS path computations rather than ignoring
>>   the entire LSA.
>>
>> NEW
>>
>>   In case where the new sub-TLVs or their attendant encodings are
>>   malformed, the proper action would be to log the problem and ignore
>>   just the sub-TLVs in GMPLS path computations rather than ignoring
>>   the entire LSA. This should not stop the LSA advertisement process.
>
> [Alia] Usually, one doesn't reflood LSAs with malformed TLVs or sub-TLVs. 
> I'm certainly willing to listen to reasoning on why flooding potentially corrupt
> data would be a good idea, but you'll need really clear reasoning.

Why is this any different to any other TLV in any other LSA? Don't we just call on standard OSPF behavior?

>>> Is this just for malformed sub-TLVs?  Is it safe to assume that sub-TLVs
>>> further on in the TLV (or following TLVs) can be properly parsed?
>>
>> YOUNG>> Yes we are just talking about malformed sub-TLVs. It is safe to assume
>> that other sub-TLVs in the TLV or following TLVs can be properly parsed as they
>> are a distinct sets of info elements with proper sub-TLV types, TLV types.
>
> [Alia]  Are you picturing that a sub-TLV is malformed if its length isn't accurate or just
> if one of the fields inside the sub-TLV is using a bad value or setting a reserved value?
>  In the latter cases, perhaps it might be tolerable to assume that there wasn't data-
> corruption that will affect the rest of the parsing - but I'm dubious.   For the first, if
> the length is wrong, then you can't accurately know where the next sub-TLV starts
> so all parsing after isn't trustable, IMHO. 

I think you have to handle both cases. Furthermore, if the TLV is of variable length, anything unexpected needs to be treated with caution.

>>> Second, please add in some words
>>> about the expected load of logs.
>>
>> YOUNG>> How about adding one sentence followed by "In case..."
>>
>> NEW
>>        The expected load of logs is not expected to be a high volume as malformed
>>        TLVs do not occur frequently in GMPLS.
>>
>> END
>
> [Alia] Uh - of course error conditions aren't expected to be common!  How
> about considering how often the advertising router might send the LSA? 
> How about considering the number of neighbors that will be flooding it?
> How about dampening the number of logs?  Making it a MAY log also makes
> it clear that it's up to the implementation.

The normal way of handling this is...
"Errors of this nature SHOULD be logged for the local operator. Implementations MUST provide a rate limit on such logs, and that rate limit SHOULD be configurable."

Adrian