[Idr] Re: Review of draft-chen-idr-bgp-ls-sr-policy-nrp

[[e-mail]]

Hi Adrian，

I'm really sorry, I didn't notice this email and sorry for late reply. 
Please see inline...

Best Regards,
Ran


Original


From: AdrianFarrel <[e-mail]>
To: [e-mail] <[e-mail]>;
Cc: [e-mail] <[e-mail]>;
Date: 2024年06月10日 04:48
Subject: [Idr] Review of draft-chen-idr-bgp-ls-sr-policy-nrp

Hi,
 
There's a lot of healthy discussion of this draft as it works towards
possible Working Group adoption. I confess that I haven't caught up on that
thread yet, so I predict some overlap.
 
I thought now might be a good time to contribute a review with a TEAS slant,
as editor of RFC 9543, and as one of the DEs for BGP-LS.
 
Hope this is helpful and can move us forward for rapid adoption.
Ran:  This is very helpful. Thank you very much.


Cheers,
Adrian
 
===
 
Abstract
 
Need to expand "SR" and "BGP-LS".
 
The first two sentences seem to overlap:
- attribute associated with
- attribute of
 
The statement of what an NRP is seems to be different from what 9543
says. Perhaps steal some brief words from the RFC (cf. your text in the
Introduction) before saying that an identifier of the underlying NRP is
an important attribute.
 
s/enable/enables/
 
Final sentence is ambiguous. I think:
   This document defines a new TLV which enables the headend to report
   (using BGP-LS) the configuration and the states of an SR policy and
   any associated NRP information.
 Ran: Agreed.
---
 
1.
 
s/Segment Routing Policy/Segment Routing (SR) Policy/
 Ran: Agreed.
---


1. Final paragraph
 
Per the change to the Abstract, I think it should be that it is the
identifier of the NRP (not the NRP itself) that is the important
attribute.
 Ran: Agreed.
---


1. Final paragraph
 
   This document defines a new
   TLV which enable the headend to report the configuration and the
   states of an SR policy carrying the NRP information by using BGP-LS.
 
While this statement may be true, the document gives no indication of  
how any information beyond the NRP ID is reported. I know this is an
early version of the work, and the WG can be expected to refine it and
add details, but I think we need some hints.
 
Possibly the thing here is some simple addition like...
 
   Information that describes the NRP and provides information about the
   state of the NRP may be carried in sub-TPVs of the new TLV. These  
   sub-TLVs will be defined in a future version of this document.
 
Something similar to that would also need to be added to section 2.
 
Alternatively, I am misunderstanding the intention here and what you are
doing is just indicating which NRP the SR policy uses: no more
information would be carried and there is no additional status to  
report.
 Ran：The purpose of this draft is to inform only which NRP the SR policy uses:  no moreinformation would be carried and there is no additional status to report. NRP status information 
is reported through other methods and is not considered in this draft.

---
 
2. First paragraph
 
Need to mention the name of the TLV. Something like...
 
   In order to collect configuration and states of the NRP SR policy,
   this document defines a new SR Policy state TLV called the NRP TLV
   which enables the headend to report the state at the SR Policy CP  
   level.Should also expand CP.
Ran:  Agreed.
---


2.
 
s/[RFC9552]associated/[RFC9552]  associated/
s/only one this TLV/only one these TLVs/
 Ran:  Agreed
---


2.
 
You can give a value of 8 for Length.
 
I think the field called 'Flag' should be called 'Flags'.
 
You need some more discussion about 'NRP ID'. How is the value encoded?
What is its scope? Where does the value come from? This might simply be
a reference to another document that contains this information.
 Ran:  Will be added in the next version.
---


3.
 
OLD
   The mechanism specified in this document defines the headend to
   report configuration and states of an SR policy carrying the NRP
   information by using BPG-LS.
NEW
   The mechanism specified in this document defines a mechanism for the
   headend to report configuration and states of an SR policy carrying
   the NRP information by using BPG-LS.
END:
 Ran:  Thanks. Will update it.
---


Thank you for including section 3.
 
I understand how the control plane impact of scaling the number of NRPs
is limited because it is only the headends that report NRP status. The
load is shared across the headends.
 
There are several general issues that arise:
 
1. What is the headend of an NRP? I can understand the headend of a path
   or a traffic flow. But not the headend of an NRP. Probably you are
   talking about the headend of the SR policy.
 Ran:  Yes. Refer to the handed of the SR policy.

2. An NRP may support many SR policies. In this case, do all headends of

   all SR policies that use the NRP report the status of the NRP?
 Ran: Although the proposed mechanism allows an NRP may support many SR policies, 
in normal case the association between an SR Policy and NRP is considered to be 1:1.
In an NRP support many SR policies scenarios, all headends of all SR policies that use the NRP report the 
association between an SR Policy and NRP-ID.



3. The scaling issue is more complex with BGP-LS. The scaling has to be
   considered in terms of the BGP-LS servers and the control plane  
   network around the servers.
 
Some discussion is needed of how often the updated status is reported,
and how many SR policies are reporting the same NRP status information.
 Ran: The purpose of this draft is to inform the association between an SR Policy and NRP-ID, and 
generally, the relationship is relatively stable. 
---


6.
 
It would be worthwhile this section explaining an attack vector that is
introduced as follows.
 
An NRP ID indicates which resources are used by an SR policy and that
allows those resources to be attacked. So, reporting the NRP ID in
association with the SR policy is a risk.
 
Of course, securing BGP-LS and the BGP-LS servers protects against that.
 Ran: Agreed.
_______________________________________________

Idr mailing list -- [e-mail]
To unsubscribe send an email to [e-mail]