[Last-Call] Re: Rtgdir last call review of draft-ietf-lsr-igp-flex-algo-reverse-affinity-04

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.


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.

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.

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.

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


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."


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.

Section 6,

[Cheng]Same comments like section 5

Section 7,8,9, similar comments like the IS-IS ones.

##PP

Same response as above.

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.

thanks,
Peter





--
last-call mailing list -- last-call@xxxxxxxx
To unsubscribe send an email to last-call-leave@xxxxxxxx




[Index of Archives]     [IETF Annoucements]     [IETF]     [IP Storage]     [Yosemite News]     [Linux SCTP]     [Linux Newbies]     [Mhonarc]     [Fedora Users]

  Powered by Linux