Hi Cheng,
please see inline (##PP2):
On 04/02/2025 14:47, Cheng Li wrote:
Hi Peter,
I suddenly find out that, I started the review before the latest revision, and turned to other business, and then came back to this draft after the revision update.
This makes me miss the new revision. Thanks for reminding. Please see my reply below after reading the latest revision, with [Cheng2].
Thanks,
Cheng
-----Original Message-----
From: Peter Psenak <ppsenak@xxxxxxxxx>
Sent: Tuesday, February 4, 2025 12:39 PM
To: Cheng Li <c.l@xxxxxxxxxx>; rtg-dir@xxxxxxxx
Cc: draft-ietf-lsr-igp-flex-algo-reverse-affinity.all@xxxxxxxx; last-call@xxxxxxxx; lsr@xxxxxxxx
Subject: Re: Rtgdir last call review of draft-ietf-lsr-igp-flex-algo-reverse-affinity-04
Hi Cheng,
thanks for the review, please see inline (##PP):
On 30/01/2025 17:50, Cheng Li via Datatracker wrote:
Reviewer: Cheng Li
Review result: Has Nits
I am not an expert of IGP flexalgo and this is my first official
review. Though I understand what is the Flexalgo and how it can be
used, as a routing area engineer. As a new reader of this document,
the comments may be more helpful to reflect the true feeling of most
readers. But I may make some mistakes, so some comments below are clarification questions.
1. Abstract:
[Cheng] Seems too short and simple, recommend to add more text to
explain clearer.
2. Introduction,
[Cheng] Also, too short. Suggest to add more text to describe what is
flexalgo, motivation of why we need this extension. Some text similar
to the introduction section in RFC9350 is ok.
##PP
ok, will add some more text to both Abstract and Introduction.
[Cheng2] Thanks.
3. use case example
3.1
"The Flexible Algorithm definition can specify 'colors' that are used
by the operator to include or exclude links during the Flex-Algorithm
path computation." [Cheng]suggest to have definition of "color" in
term. or, a reference also helps.
3.2
"These link 'colors' are checked in the forwarding direction of the
SPF computation - e.g. in the direction from the parent to the child."
[Cheng]I can understand the meaning. But what is 'checked' in details?
may be good to have some specific words to describe the action? from
the parent to the child, these terms come from graph/tree/SPF. No sure
if we can find better way to describe the direction. It looks normal
to use 'check' in Flex-algo RFCs, like RFC9350 :)
##PP
Above text in section 3 is not from the last version. The text has been updated. Can you please look at the latest version of the draft.
[Cheng2] ok, thanks, it addresses my comments. Anyone else raise the same comment regarding 'color'? good to hear the same comment.
##PP2:
yes, Shraddha made that comment previously.
3.3
"the input errors can only be detected at node B."
[Cheng] I may ask, why the input errors can ONLY be detected at node
B? as a reader who is new to this area(Flexalgo in IGP). better to
have some explaination?
##PP
Input error for traffic A->B can be detected at node B only on Rx side.
I'm not sure what else to say.
[Cheng2]All right, then ignore it.
3.4
"An operator may measure the rate of such input errors and set certain 'color'
on a link locally on node B when the input error rate crosses a
certain threshold."
[Cheng] Sorry I do not understand this fully.
Clarification question: what system you are talking about when you say
measure the rate of such input errors? It seems that we have a system
to input something. Based on the results, the system set certain
'color' on a link? what input errors will replect on a 'color'? what this color means?
##PP
We are talking about nodes running the IS-IS protocol, which would typically be a L3 device. Such device routes a L3 traffic and as such receives L3 traffic from other L3 devices.
It can detect input errors, CRC errors, etc. Operator can set a threshold on these errors over a certain period of time.
We do not use a color in the latest version, we say "set certain Admin Groups", which is a link attribute that is advertised by ISIS protocol about the link.
[Cheng2] Good to learn that. If you can add some text to explain, that will be great.
##PP2
ok, will try to add some more text.
Section 4.
[Cheng] The name of the TLV is a little bit long, even when it becomes
abbreviation, FAERAG, suggest to make it short, or make it readable.
##PP
I feel it is proper and the abbreviation makes it short enough. We use such naming for other TLVs.
Example:
https://www.rfc-editor.org/rfc/rfc9350.html#section-6.1
[Cheng2]All right, just an suggestion. You can ignore.
4.1
[Cheng]Type = 10, do we already got the IANA allocation? if not?
suggest to use TBA for now. similar comment for all the type value in other TLVs.
##PP
Again, you are not looking at the latest version:
Current text is:
"Type (1 octet): An 8-bit field assigned by IANA to uniquely identify the ISIS FAERAG Sub-TLV. Value 10 has been assigned by IANA."
[Cheng2]LGTM
4.2
[Cheng] in RFC7308, it looks like the name of the TLV is Extended
Administrative Groups, but in this document, it is Extended Administrative
Group. Am I missing something?
We are consistent with the base flex-algo spec:
https://www.rfc-editor.org/rfc/rfc9350.html#section-6.1
[Cheng]The rules of ignore more than one FAERAG sub-TLV is clear to me, thanks!
Section 5
"The format of the IS-IS Flexible Algorithm Include-Any Reverse Admin Group
Sub-TLV is identical to the format of the FAERAG Sub-TLV in Section 4."
[Cheng] what do you mean identical? even for the EAG part? meaning there will
be some EAG info in FAIRAG sub-TLV?
##PP:
The two TLVs differ only in the Type value.
[Cheng2]OK
Section 6,
[Cheng]Same comments like section 5
Section 7,8,9, similar comments like the IS-IS ones.
##PP
Same response as above.
[Cheng2]OK
Section 12 Security Considerations.
[Cheng] I like to use simple sentence in security section as well. But
reviewers always require me to add more text to explain. LOL. Therefore, I will
suggest the authors to add more text to in this section, since the security
area reviewers may require to do so as well.
##PP
I feel the security section is exactly saying the truth. There's nothing
more to add. Do you want me to copy the text from the RFC9350? Looks
redundant, but doable.
[Cheng2]Honestly, if this is a PCE WG document, I know I need to add more text in this section. But this is a LSR WG document, so I am not sure about the normal practice. We may leave it to security experts. I am ok with this now.
##PP2
ok
thanks,
Peter
thanks,
Peter
--
last-call mailing list -- last-call@xxxxxxxx
To unsubscribe send an email to last-call-leave@xxxxxxxx
