Hi Alvaro,
please see inline (##PP3):
On 30/03/2021 19:44, Alvaro Retana wrote:
On March 25, 2021 at 6:03:53 AM, Peter Psenak wrote:
Peter:
Hi!
I have some comments (see below) -- nothing major. I look forward to -12.
Thanks!
Alvaro.
...
Just one high-level comment: It is not clear to me why all the
behaviors from rfc8986 are not covered in this document. If some are
not applicable, or are covered elsewhere, please explain in the text.
##PP
not all behaviors from rfc8986 are applicable to IGPs. Section 10
("Advertising Endpoint Behaviors") lists the ones that are applicable to
ISIS.
I understand that -- other readers may not.
##PP2
we defined all behaviors that rfc8986 mentions should be advertised by
IGP, except the END.T. The END.T was originally defined, but during the
WGLC it was removed based on WG discussion:
Mailing list discussion:
Thread1:
https://mailarchive.ietf.org/arch/msg/lsr/0Bp5DJrRJPvRyzZMZS0P_OE8Y7Q/
Thread2:
https://mailarchive.ietf.org/arch/msg/lsr/nKJbY5f6SHEwVCqqfoXSPidGKXQ/
Please just say somewhere that END.T is not included -- no need to
justify in the document.
##PP3
done
...
342 A prefix/SRv6 Locator that is advertised by a single node and without
343 an A-Flag SHOULD be interpreted as a node specific locator.
[major] "advertised by a single node and without an A-Flag" This is
equivalent to the current behavior of a prefix being "advertised by a
single node and without an A-Flag". IOW, you seem to be specifying
behavior that a node that doesn't implement (or even know about) this
document is expected to follow.
##PP2
if I remove the "prefix" and only keep the SRv6 Locator, would you be
fine with it? We are defining SRv6 Locator in this document.
No. The description throughout this section talks about both prefixes
and locators. It reads as a general specification for all prefixes
(including locators).
Suggestion (taking onto account the responses below)>
A prefix/SRv6 Locator that is advertised by a single node and without
an A-Flag is considered node specific.
##PP3
done
[major] Related... What is a "node specific locator"? The A-flag
functionality could be used in a network that otherwise doesn't
implement SRv6, so calling it a "locator" doesn't seem right.
##PP2
please see my previous response.
[major] "SHOULD be interpreted" Interpreting is not really an
interoperability-requiring action. Is there anything here resulting
from the interpretation that requires normative language?
##PP2
what about:
"An SRv6 Locator that is advertised by a single node and without
an A-Flag is considered as a node specific locator."
...
349 The Prefix Attribute Flags Sub-TLV can be carried in the SRv6 Locator
350 TLV as well as the Prefix Reachability TLVs. When a router
351 originates both the Prefix Reachability TLV and the SRv6 Locator TLV
352 for a given prefix, and the router is originating the Prefix
353 Attribute Flags Sub-TLV in one of the TLVs, the router SHOULD
354 advertise identical versions of the Prefix Attribute Flags Sub-TLV in
355 both TLVs.
[minor] This paragraph doesn't seem necessary given this text in §5:
In cases where a locator advertisement is received in both a Prefix
Reachability TLV and an SRv6 Locator TLV, the Prefix Reachability
advertisement MUST be preferred when installing entries in the
forwarding plane.
##PP2
above mentioned paragraph does not say anything about the Prefix
Attribute Flags Sub-TLV present in the advertisement of the same prefix
in the Locator TLV and Prefix Reachability TLV. So we need to keep it.
[major] If you decide to keep it... "SHOULD advertise
identical...Prefix Attribute Flags Sub-TLV" When is it ok to not do
so? Again, given that the Prefix Reachability TLVs are preferred,
this statement doesn't seem to matter, or carry interoperability
weight. s/SHOULD/should
##PP
well, not really. The Prefix Attribute Flags Sub-TLV should be the same
to guarantee the same treatment of both locator and legacy prefix
advertisements. The fact that the legacy prefix advertisement is
preferred when installing reachability of the prefix to forwarding does
not mean the Prefix Attribute Flags Sub-TLV advertised with Locator TLV
is useless - it can still be used when using Locator for other things -
e.g. derive SID for TILFA protection, etc.
Does this mean that having the same sub-TLV is also a consideration
when comparing the locator and legacy advertisements? If so, then
that should also be mentioned in §5.
[] This point is related to Gunter's recent e-mail [1].
##PP2
the text has been updated to address Gunter's comment as follows:
"The Prefix Attribute Flags Sub-TLV can be carried in the SRv6
Locator TLV as well as the Prefix Reachability TLVs. When a router
originates both the Prefix Reachability TLV and the SRv6 Locator TLV for
a given prefix, and the router is originating the Prefix Attribute Flags
Sub-TLV in one of the TLVs, the router SHOULD advertise same flags in
the Prefix Attribute Flags Sub-TLV in both TLVs. However, unlike TLVs
236/237 the X-flag in the Prefix Attributes Flags sub-TLV is valid when
sent in the SRv6 Locator TLV. The state of the X-flag in the Prefix
Attributes Flags sub-TLV when included in the Locator TLV MUST match the
setting of the embedded X-flag in any advertisement of the same prefix
in TLVs 236/237."
[nit] s/advertise same flags/advertise the same flags
##PP3
done
[nit] s/ 236/237 / 236 and 237
##PP3
done
[major] "MUST match the setting of the embedded X-flag"
What if the setting doesn't match? What should the receiver do?
##PP3
Added following:
"In case of any inconsistency between the Prefix Attribute
Flags advertised in the Locator TLV and in the Prefix Reachability TLV,
the ones advertised in Prefix Reachability TLV MUST be preferred".
By "embedded", you are referring to the X bit inside 236/237, right?
##PP3
yes
To differentiate, please call it "X bit" and add references to
rfc5120/5308.
##PP3
done
...
[minor] Please add Figure numbers/names for all packet formats.
##PP2
s that really required? I have never done it in any of RFCs I was editor
for.
Required, no...but it is good practice.
...
666 9. SRv6 SID Structure Sub-Sub-TLV
[] I don't understand what this sub-sub-TV is used for. Can you
please explain? Is there a relationship between it and the SID that
is advertised in the sub-TLVs? For example, I would assume that the
SID would have the bits that correspond to the argument set to 0 --
what if they're not? What is the purpose of this information? [Of
course, none of the supported behaviors take an ARG...]
##PP2
The SRv6 SID Structure Sub-Sub-TLV indicates the structure of the SID
associated with it. It can be used by implementation for validation of
the SID for consistency (e.g. if there is no ARG but there is something
in the ARG bits, then it can be ignored). They can be signalled via
BGP-LS to controller/apps that can verify the consistency in the block
and SID addressing in the domain. Details are outside the scope of this
draft.
Can you please add something like that to the draft? I think that for
now (unless other people ask as well) simply saying that the use is
outside the scope should be enough.
##PP3
done
...
741 11. Implementation Status
...
752 Types of SID supported: End, End.X, LAN End.X, END.OP
[] "END.OP" is not defined. Also, the others are not types of SIDs,
but sub-TLVs.
##PP2
removed END.OP. This section is going to be removed anyway.
That fact doesn't mean that the information in it can be wrong. If
inaccurate, I would prefer the section not be there at all.
##PP3
I removed the "Implementation Status" section, it would have to be
removed later anyway.
...
834 12.1.2. Revised sub-TLV table
...
839 Type 27 135 235 236 237
841 1 y y y y y
842 2 y y y y y
843 3 n y y y y
844 4 y y y y y
845 5 y n n n n
846 6 n y y y y
847 11 y y y y y
848 12 y y y y y
849 32 n y y y y
[major] Because the structure of the registry is changed, this
document should formally Update rfc7370 (where the current registry
was defined).
##PP2
I added following text:
" "Sub-TLVs for TLVs 135, 235,
236, and 237 (Extended IP reachability, MT IP. Reach, IPv6 IP. Reach,
and MT IPv6 IP. Reach TLVs)" registry defined in [RFC7370] to section
§12.1.1."
Would that be sufficient?
We need the header to include the Updates tag,
##PP3
sorry, I'm not sure I get it, what header and what "Updates tag"?
and something in the
Abstract and Introduction.
In the Abstract>
This documents updates rfc7370 by modifying an existing registry.
##PP3
done
The text in Introduction can be the same, with a reference to §12.1.2.
##PP3
done
This Section should also ask IANA to add this document as a reference
in the regsitry.
##PP3
done
...
906 12.5. Sub-Sub-TLVs for SID Sub-TLVs
908 This document requests a new IANA registry be created under the IS-IS
909 TLV Codepoints Registry to control the assignment of sub-TLV types
910 for the SID Sub-TLVs specified in this document - Section 7.2,
911 Section 8.1, Section 8.2. The suggested name of the new registry is
912 "Sub-Sub-TLVs for SID Sub-TLVs". The registration procedure is
913 "Expert Review" as defined in [RFC8126]. The following assignments
914 are made by this document:
[minor] In line with the name of other registries; suggestion:
"Sub-sub-TLVs for sub-TLVs 5, 43 and 44 (SRv6 End SID , SRv6 End.X SID
and SRv6 LAN End.X SID)".
##PP2
I find that confusing as the sub-TLVs 5 is a locator Sub-TLV, while
Sub-TLVs 43 and 44 are IS reachability sub-TLVs.
What about:
"Sub-Sub-TLVs for SID Sub-TLVs (SRv6 End SID, SRv6 End.X SID
and SRv6 LAN End.X SID)"
Most of the other registries include the number of the TLV. So I
think it would be good to remain consistent. Maybe we should ask the
current DEs: Chris, Hannes and Les.
##PP3
I have updated this based on the discussion with Les.
thanks,
Peter
[End]
_______________________________________________
Lsr mailing list
Lsr@ietf.org
https://www.ietf.org/mailman/listinfo/lsr
