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
--
last-call mailing list
last-call@xxxxxxxx
https://www.ietf.org/mailman/listinfo/last-call
