Re: [Gen-art] Genart last call review of draft-ietf-ospf-mpls-elc-13

[Peter Psenak <[e-mail]>]

Hi Elwyn,

please see inline (##PP)


On 11/05/2020 19:37, Elwyn Davies wrote:
> Hi, Peter.
> 
> In the light of some of your responses here, I would just like to
> clarify that one of the reasons for gen-art reviews is to try and make
> extremely complicated technical documents more accessible for those who
> are not as deeply embedded in the jargon and technicalities of the
> subject as well as making sure that readers can quickly determine
> whether a document is relevant to work that they are doing.
> 
> Please take this into account when considering your further actions.
> 
> Note that the optionality of ERLD-MSD advertisements appears on
> reflection to be a more serious issue than just an editorial nit.
> 
> Regards,
> 
> Elwyn
> 
> On 07/05/2020 08:53, Peter Psenak wrote:
> 
>> Hi Elwyn,
>>
>> please see inline:
>>
>> On 06/05/2020 16:25, Elwyn Davies via Datatracker wrote:
>>> Reviewer: Elwyn Davies
>>> Review result: Ready with Nits
>>>
>>> I am the assigned Gen-ART reviewer for this draft. The General Area
>>> Review Team (Gen-ART) reviews all IETF documents being processed
>>> by the IESG for the IETF Chair.  Please treat these comments just
>>> like any other last call comments.
>>>
>>> For more information, please see the FAQ at
>>>
>>> <https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.
>>>
>>> Document: draft-ietf-ospf-mpls-elc-13
>>> Reviewer: Elwyn Davies
>>> Review Date: 2020-05-06
>>> IETF LC End Date: 2020-05-05
>>> IESG Telechat date: 2020-05-21
>>>
>>> Summary:
>>> Ready with nits.  Aside:  I have to say that the convolutions and
>>> cross-referencing of doing the OSPF and IS-IS  extensions plus BGP-LS
>>> added to
>>> the cross-linking with MPLS is leading to mind-blowing complexity.
>>> Sooner or
>>> later something is going to blow up here!
>>>
>>> Major issues:
>>> None
>>>
>>> Minor issues:
>>> None
>>>
>>> Nits/editorial comments:
>>>
>>> Abstract and title :  The application to BGP-LS (s5) is not mentioned
>>> in the
>>> abstract or the title.  Also the first use of BGP-LS needs to be
>>> expanded.
>>
>> Why would the BGP-LS need to be mentioned in the abstract?
> At present BGP-LS is the subject of a separate document and this
> document extends the BGP add-on called BGP-LS as well as OSPF v2 and
> v3.  It is therefor important that implementers of BGP-LS can easily
> find documents that are relevant to their work.

##PP
not that I necessarily agree, but let me add it.

>>
>> I have expanded the first use of BGP-LS
>>
>>>
>>> Abstract: s/tunnel/LSP/
>>
>> done
>>
>>>
>>> s1: Suggest s/SR-MPLS/Segment Routing with the MPLS Data Plane/
>>>
>>> s1: Query:  As a non-expert in this area, I was wondering if the
>>> signalling
>>> capability is or will be implemented in IS-IS?  A brief comment on
>>> the status
>>> wrt IS-IS would be helpful.  [It turns out that you already reference
>>> the
>>> document that implements this later in this draft.]
>>
>> yes, it is being added to ISIS. Yes, this draft reference the ISIS
>> draft. I see no reason to to include ISIS draft status in this
>> document though.
> As has been mentioned in other reviews of this document and the
> corresponding IS-IS document, having two documents that cover this
> extension is not very desirable.  As a non-expert, but who knows that
> OSPF and IS-IS provide closely related functionality, one gets to this
> point in the document and wonders why OSPF and not IS-IS. If the WG does
> not bite the bullet and combine the drafts, it would be highly desirable
> to point out that the same functionality is being proposed for all three
> protocols

##PP

Alvaro has responded to this already. Please have a look at his response 
to Barry Leiba's comments.

All I would add is that we have had many separate RFCs for OSPF and ISIS 
specifying the same functional extension in the past. I see no reason 
why that would suddenly became an issue.



>>
>>
>>>
>>> s1, last sentence: s/it's/it is/
>>
>> done
>>
>>>
>>> s3: It would be a good idea to expand 'prefix' to 'address prefix
>>> advertisement' on its first occurrence here.  Thereafter 'prefix' is
>>> fine by me.
>>
>> "prefix" is being used in almost all OSPF and ISIS document, we never
>> use address prefix.
> Really? Revisiting RFC 2328 (OSPFv2) we see that prefix is not used at
> all and in RFC 5340 (OSPF for IPv6)  the term prefix is introduced as
> 'IP address prefix'.  In order to make it clear of what this is a
> prefix, and what is going on here, expanding the initial usage is highly
> desirable.


well, one can argue that we have "OSPFv2 Extended Prefix TLV", not the 
"OSPFv2 Extended Address Prefix TLV" and "OSPFv3 PrefixOptions" rather 
than "OSPFv3 AddressPrefixOptions"

>>
>>
>>>
>>> s3, para 3: Why would a router not advertise the ELC with all
>>> prefixes?  Can
>>> you say why this ought not to be a MUST.
>>
>> advertising ELC property with prefix advertisement is optional. We can
>> not mandate it. It would make all routers not advertising this data
>> violating this spec.
> Er, no.  Nothing is said or would be said about routers that do not
> support ELC at all or do not support it on all interfaces.  I am
> assuming that it is not intended to make implementation of this
> capability mandatory in all OSPF deployments. I was trying to understand
> why a router that satisfies the previous condition so that it is
> legitimate for it to announce ELC with any IP address prefix might wish
> to only announce it with some prefixes and not others.  This is an
> interesting question for implementers as the SHOULD implies that an
> implementation has to provide a configuration option on a per prefix basis


##PP
I don't see a problem. We can not mandate the advertisement of something 
that is optional.



>>
>>>
>>> s4, para 3: In that case, what does the absence signify?  Should we
>>> care?
>>
>> the absence of ERLD-MSD advertisements only indicates that a node
>> does not support advertisement of ERLD
>>
>> It can not be interpreted that ERLD is not supported.  Old nodes that do
>> not advertise ERLD-MSD can not be assumed not to support non-zero ERLD.
>>
> There are just too many negatives in this last sentence!  OK.. so if the
> router does not suport ERLD-MSD advertisements what can or must the
> ingress router do to usefully position ELs it is intending to send that
> might pass through this router?  

##PP
What to do is not protocol specific and can not be specified in protocol 
drafts that specify hoe to advertise the ERLD value. Please note that 
ERLD is NOT used by protocols.

> On further reflection, it seems to me
> that either there should be some sort of default value for the label
> stack depth that would otherwise be distributed with ERLD-MSD or routers
> that support ELs MUST advertise the depth they support.

##PP
I don't see how we can suddenly start to mandate this.

> 
> This is looking rather like an issue rather than just a nit.
> 
>>
>>>
>>> s4, para 4:
>>> This needs a correction and a reference to where the Link MSD Sub-TLV is
>>> defined.  As a matter of interest, is there any reason why this
>>> should be sent
>>> in an OSPF context?  If not why not just prohibit sending it? If it
>>> is received
>>> should it provoke an error rather than being ignored? OLD: When the ERLD
>>> MSD-Type is received in the OSPFv2 or OSPFv3 Link MSD Sub-TLV, it
>>> MUST be
>>> ignored. NEW:
>>>    When the ERLD-MSD type is received in the OSPFv2 or OSPFv3 Link MSD
>>> Sub-TLV
>>>    [RFC8476], it MUST be ignored.
>>
>> done.
> Did you address the question of whether should actually provoke an error
> since it appears to be a protocol error?

##PP
I replaced it with your proposed text.

thanks,
Peter

>>
>>> ENDS
>>>
>>> s5:  This section needs to be rewritten to be 'future proof' rather than
>>> referring to the temporary allocations.  A note about the temporary
>>> allocations
>>> can be added with a RFC Editor note requesting its removal on final
>>> publication.
>>
>> I suppose you meant section 6 - IANA Considerations.
> Yes, I did.  Sorry.
>>
>> I have updated the IANA section.
> Good.
>>
>> thanks,
>> Peter
>>>
>>>
>>>
>>>
>>>
>>
> 
>