Hi Peter,

On Thu, May 21, 2020 at 12:05:39PM +0200, Peter Psenak wrote:
> Benjamin,
> 
> thanks for review, please see inline (##PP):
> 
> On 20/05/2020 00:44, Benjamin Kaduk via Datatracker wrote:
> > Benjamin Kaduk has entered the following ballot position for
> > draft-ietf-isis-mpls-elc-12: Discuss
> > 
> > When responding, please keep the subject line intact and reply to all
> > email addresses included in the To and CC lines. (Feel free to cut this
> > introductory paragraph, however.)
> > 
> > 
> > Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
> > for more information about IESG DISCUSS and COMMENT positions.
> > 
> > 
> > The document, along with other ballot positions, can be found here:
> > https://datatracker.ietf.org/doc/draft-ietf-isis-mpls-elc/
> > 
> > 
> > 
> > ----------------------------------------------------------------------
> > DISCUSS:
> > ----------------------------------------------------------------------
> > 
> > As for other reviewers, many of my comments duplicate those for the OSPF
> > document; I expect that the analogous responses apply and am fine if
> > they only appear for one document's review.
> > 
> > Here, the question I have about normative language applies to the text
> > in Section 3:
> > 
> >     When a router propagates a prefix between ISIS levels ([RFC5302], it
> >     MUST preserve the ELC signaling for this prefix.
> > 
> > The scenario in question is analogous to the OSPF cross-area case: is
> > the router propagating the prefix between ISIS levels required to
> > implement this document; is preservation of the flag value a new
> > requirement from this document vs. a preexisting property; and is this
> > document trying to make normative requirements of devices that don't
> > implement this document?
> 
> ##PP
> this is a new requirement and only applies to the routers that support 
> this document. We are not making normative requirements of devices that 
> don't implement this document, we cannot.
> 
> Maybe we can add that it only applies to the routers that supports this 
> extension:
> 
> "When a router supporting this extension propagates a prefix between 
> ISIS levels ([RFC5302], it MUST preserve the ELC signaling for this prefix."
> 
> Would it work?

That would work, yes.
I think what bothers me about the current text is that it feels like a
global statement that holds universally everywhere (and as such would be
"scope overreach").  Even to say something like "ELC signaling MUST be
preserved when a router propagates a prefix between ISIS levels" (which is
superficially similar) does not bother me very much, since it presumes a
knowledge of what ELC signaling is (and thus, implementation of this
document) in a way that "unknown prefix attribute flag 3" does not.  That
is, we clearly (given your above explanation) don't expect the "unknown
prefix attribute flag 3" to get propagaged, and only when we know that it's
the ELC signalling does the requirement kick in, if that makes sense.

With respect to Alvaro's clarification, your answer for (1) makes sense;
thanks!  I think Alvaro has offered to help work out what (if any)
additional text we might want to be sure that the answer to (2) is clear in
the document.

> 
> > 
> > Likewise, the ASBR case for cross-protocol redistribution seems to
> > rather inherently require understanding the semantics of the flags being
> > translated.
> 
> similarly to the above I can chnage to:
> 
> "When a router supporting this extension redistribute a prefix ..."
> 
> 
> > 
> > 
> > ----------------------------------------------------------------------
> > COMMENT:
> > ----------------------------------------------------------------------
> > 
> > Section 1
> > 
> > Should we add a sentence at the end of the last paragraph about how
> > "this document defines a mechanism to signal the ERLD using IS-IS"?
> 
> not sure I understand, how is described in the body of the document.

I think I maybe put a little more detail on the motivation for this
question in the OSPF document's comments.  Basically, it's about parity
between Abstract and Introduction -- the Introduction says clearly "this
draft defines a mechanism to signal the ELC using IS-IS", but there is not
an analogous statement about the ERLD signaling mechanism.  As such, it's
an editorial matter of style and you should feel free to ignore the comment
or do what you see fit.

> > 
> >     In cases where LSPs are used (e.g., SR-MPLS [RFC8660], it would be
> > 
> > side note(?): I don't know that SR-MPLS is so popular so as to be
> > privileged as the only example given for LSP usage.  If we instead
> > talked about using IGPs to signal labels, this selection would seem less
> > surprising to me.
> 
> this document describes the ELC/ERLD capability signaling for SR MPLS. 
> For non SR MPLS cases, thee are existing mechanisms to learn ELC/ERLD.
> 
> I can replace the text with:
> 
> "In cases where SR is used with the MPLS Data Plane"
> 
> Would it work?

Sure, thanks.

> 
> > 
> > Section 3
> > 
> >     unless all of its interfaces are capable of processing ELs.  If a
> >     router supports ELs on all of its interfaces, it SHOULD set the ELC
> >     for every local host prefix it advertises in IS-IS.
> > 
> > Do we want to say anything about (not) advertising the ELC for other
> > prefixes?
> 
> Do we have to? I can add "MUST NOT set ELC with for any other prefix", 
> but I find it unneeded.

This is the "comment" section of the document, so no, you don't have to :)
I mostly ask because we give guidance on what to do for local host prefixes
but say nothing about the other case.  That could be taken to imply that we
don't know what guidance to give, even though someone skilled in the area
who thinks through what the mechanism is doing will come to the conclusion
that it makes no sense/is actively harmful to advertise the ELC for other
prefixes.

> > 
> > Section 4
> > 
> > I agree with Roman's comment about code 2 vs TBD2.
> 
> that has been fixed already.
> 
> > 
> >     ERLD in the range between 0 to 255.  The scope of the advertisement
> >     depends on the application.  If a router has multiple interfaces with
> > 
> > Just to check: w.r.t. "scope", both this document and the OSPF one only
> > define usage of this MSD type at the scope of a single node, right?  (I
> > don't see a particular reason to preclude using it at a different
> > scope.)
> 
> the scope here means where the information will be flooded - area only 
> or network wide. No such thing as a node scope.

Oops, sorry for misremembering that part, and thanks for the clarification.

> 
> > 
> > Section 6
> > 
> >        - Bit 3 in the Bit Values for Prefix Attribute Flags Sub-TLV
> >        registry has been assigned to the ELC Flag.  IANA is asked to
> > 
> > Is there an "IS-IS" in the name of this registry?
> 
> no the registry name is "Bit Values for Prefix Attribute Flags Sub-TLV".

Okay, thanks for confirming.

> 
> > 
> > Section 7
> > 
> > Should we say anything about considerations for redistributing ELC/ERLD
> > information at the ASBR with respect to exposing "internal information"
> > to external parties?
> 
> why would this be "internal information" and why redistribution would be 
> "external party"? Redistribution between IGPs is predominantly done 
> between IGPs under same administrative domain.

I don't have any scenario in mind; I just wanted to check if there were any
significant considerations to mention.  It sounds like there aren't such
considerations, and thus nothing to mention.
> 
> > 
> >     This document specifies the ability to advertise additional node
> >     capabilities using IS-IS and BGP-LS.  As such, the security
> >     considerations as described in [RFC7981], [RFC7752], [RFC7794],
> >     [RFC8491], [RFC8662], [I-D.ietf-idr-bgp-ls-segment-routing-ext] and
> > 
> > RFC 8662's security considerations have a pretty hard dependency on RFC
> > 6790's security considerations; it might be worth mentioning 6790
> > directly in this list as well.
> 
> would not that be implicit when mentioning RFC 8662?

Yes; this is mostly a question of style.

> 
> > 
> >     [I-D.ietf-idr-bgp-ls-segment-routing-msd] are applicable to this
> >     document.
> > 
> > Could we also have a brief note that the normal IS-IS authentication
> > mechanisms serve to protect the ELC/ERLD information?
> 
> do we need to repeat this every time we add a bit in the TLV?

It's my preference, though not a strict need.

> > 
> >     Incorrectly setting the E flag during origination, propagation or
> >     redistribution may lead to black-holing of the traffic on the egress
> >     node.
> > 
> > This is what happens when the E flag should not be set but is
> > erroneously set.  Should we also say what happens if we should set the E
> > flag but erroneously clear it (e.g., that poor or no load-balancing may
> > occur)?
> 
> yes, there is a text there already:
> 
> "Incorrectly setting of the ERLD value may lead to poor or no 
> load-balancing of the traffic."

I thought that was about the ERLD value, not the ELC indicator, but trust
you to know best.

> > 
> > Section 8
> > 
> > I do see the note in the shepherd writeup about the sixth author (thank
> > you!); if we're already breaking through the 5-author limit, did we
> > consider making those who "should be considered as co-authors" listed as
> > co-authors?
> 
> I'm not fun of this 5 authors rule to be honest.
> 
> > 
> > Section 10.1
> > 
> > Should we reference RFC 7981 from Section 4 as well?  Right now we seem
> > to only use it for the security considerations, which is not necessarily
> > enough to qualify it as a normative reference.
> 
> we reference RFC8491, which references RFC 7981. I don't see a need to 
> reference RFC 7981 directly.

Okay.

Thanks for the clarifications and updates!

-Ben

_______________________________________________
Lsr mailing list
Lsr@ietf.org
https://www.ietf.org/mailman/listinfo/lsr

Reply via email to