Hi Aijun, Thanks for making these changes.
I did a review of https://author-tools.ietf.org/iddiff?url1=draft-ietf-pce-pcep-extension-native-ip-30&url2=draft-ietf-pce-pcep-extension-native-ip-32&difftype=--html A few things to consider alongside any other IETF LC comments - - Section 3, we have added references for a few but not for others. Consider adding references for terms that are already defined in other RFCs to keep it consistent. - Section 12, capitalize "if" in the last paragraph. - Section 14.6, 14.7 and 14.8, please change "standard action" to "IETF review". Note that, for section 14.2 allocation, we need to wait for draft-dhody-pce-iana-update to make progress. Thanks! Dhruv On Mon, Jul 29, 2024 at 2:36 AM Aijun Wang <wangai...@tsinghua.org.cn> wrote: > Hi, John: > > > > I have uploaded the updated version at > https://datatracker.ietf.org/doc/html/draft-ietf-pce-pcep-extension-native-ip-32 > and > wish it address all your concerns. > > If there is no more comments, we can put forward it to the IESG LC review > then. > > Thanks in advance for your efforts! > > > > Some detail replies can see inline below. > > > > > > Best Regards > > > > Aijun Wang > > China Telecom > > > > *发件人:* forwardingalgori...@ietf.org [mailto:forwardingalgori...@ietf.org > <forwardingalgori...@ietf.org>] *代表 *【外部账号】 John Scudder > *发送时间:* 2024年7月26日 23:59 > *收件人:* Aijun Wang <wangai...@tsinghua.org.cn> > *抄送:* draft-ietf-pce-pcep-extension-native...@ietf.org; pce@ietf.org; > bhassa...@yahoo.com; fsh...@huawei.com; tan...@huawei.com; > zhu.ch...@zte.com.cn > *主题:* Re: AD review of draft-ietf-pce-pcep-extension-native-ip-30 > > > > Hi Aijun, > > > > Thanks for the update. I have a few more comments, below. I have trimmed > for brevity, indicated by […], anything trimmed is agreed or anyway doesn’t > need more discussion. > > > > If you can update to reflect these comments I think we can send it for > IETF last call. > > > > On Jul 22, 2024, at 5:37 AM, Aijun Wang <wangai...@tsinghua.org.cn> wrote: > > > Hi, John: > > I have updated draft according to your suggestions at > https://urldefense.com/v3/__https://datatracker.ietf.org/doc/html/draft-ietf-pce-pcep-extension-native-ip__;!!NEt6yMaO-gk!B09SQ9s42Y1hsFo8BANfznJ4lysLpngvzlzwp9ULLAbS_FzVRmaT6Jx-RsCqbWt6uHiW1t3973FwFYcuh7li0A$ > <https://urldefense.com/v3/__https:/datatracker.ietf.org/doc/html/draft-ietf-pce-pcep-extension-native-ip__;!!NEt6yMaO-gk!B09SQ9s42Y1hsFo8BANfznJ4lysLpngvzlzwp9ULLAbS_FzVRmaT6Jx-RsCqbWt6uHiW1t3973FwFYcuh7li0A$> > For the document track, I think it is OK to move it forward as the > experimental track, we will try to update it later to the standard track > after its experimental deployment. > The detail responses are inline below【WAJ】 > > If there is more suggestions, please let us know. Or else, can we schedule > the IESG Last Call then? > > > Best Regards > > Aijun Wang > China Telecom > > > > […] > > > > During the establishment procedure, PCC should report to the PCE the > status of the BGP session via the PCRpt message, with the status @@ > -453,7 +547,16 @@ > When PCC receives this message with the R bit set to 1 in SRP object > in PCInitiate message, the PCC should clear the BGP session that is > indicated by the BPI object. > ++--- > +jgs: The term "clear the BGP session" isn't standard (although it is > +common colloquial usage). Looking at RFC 4271, in one place (Section 3) > +it does talk about "resetting any BGP connections" so I think you would > +be on firm ground if you want to say "reset". Or you might consider > +referencing the AutomaticStop event (RFC 4271, Section 8.1.2, Event 8). > > 【WAJ】： Here, “clear the BGP session” is just want to express the deletion > of the BGP configuration on PCC, not reset the BGP connection. > Should it be more clearer, if we instead say "clear the BGP session > configuration"?------Have updated the descriptions accordingly. > > > > Yes, that’s an improvement. This leaves it ambiguous whether the session > should be torn down immediately or not, do you intend that? E.g. in some > implementations, a configuration change is not reflected in the operational > state until a commit (or equivalent) is performed. > > 【WAJ】How about “the PCC should clear the BGP configuration and tear down > the BGP session that is indicated by the BPI object.”? I think it is more > clear. I have updated the contents according to the above statements. > > […] > > Such explicit routes operate the same as static routes installed by > network management protocols (Network Configuration Protocol > (NETCONF)/YANG). The procedures of such explicit route addition and @@ > -582,6 +689,9 @@ > > The PCInitiate message should be sent to the on-path routers > respectively. In the example, for explicit route from R1 to R7, the > ++--- > +jgs: What does “respectively” mean here? Can it be removed? > 【WAJ】Here, we just want to describe such message should be sent every > router on the path, each may has different content, for example, the > different next hop information. > > > > OK thanks. In that case, while removing the word “respectively” would be > enough, I suggest you revise it to use the language you’ve used above. > That is, > > > > OLD: > > The PCInitiate message should be sent to the on-path routers > > respectively. > > > > NEW: > > The PCInitiate message should be sent to every router on the > > path. > > > > 【WAJ】Done > > > > […] > > > > BGP Peer Info Object-Class is 46 > > BGP Peer Info Object-Type is 1 for IPv4 and 2 for IPv6 @@ -1071,6 > +1233,10 @@ > - 2: Peer IP can't be reached, BGP Session Failure > > - 3-255: Reserved > ++--- > +jgs: Shouldn't you have a generic error code as well, e.g. "0: Generic > +error", to catch cases other than the ones described by 1 and 2? > ++--- > 【WAJ】 What we thought is the following, that if there is some new error > that lets to the BGP Session Failure, we should define exactly the code > from the reserved range. > The "generic" error gives no more detail indication of the error reason. > Is it right? > > > > Right. In my experience, it’s hard to enumerate every possible error that > could lead to a session failure. It’s probably not the best use of your > efforts to try to come up with a comprehensive list, even the BGP document > set itself doesn’t contain one, consider the list of OPEN error subcodes ( > https://www.iana.org/assignments/bgp-parameters/bgp-parameters.xhtml#bgp-parameters-6). > The first one is “unspecific”. In my view, when an implementor encounters > a case where the error isn’t one that has a specific mapping in the error > codes, it’s better for them to still be able to send an error, and it’s > better for the code to be “unspecific” or “generic” than it is for them > to choose an error code that is *wrong*. > > > > I agree that the ideal is to never need to use the generic code, but if > the need does arise, it’s better to use the generic code than a wrong > code. > > 【WAJ】Thanks for your advice. I have changed the code 0 as “unspecific” > > > > > Flag: 1 Byte. > > @@ -1104,6 +1270,12 @@ > establish the BGP session with the peer in AFI/SAFI=2/1. > > 7.3. Explicit Peer Route Object > ++--- > +jgs: you describe this object as "peer route", but isn't it just a > +generic host route, in your architecture you happen to use for BGP > +peers? It seems to me it would be a more accurate description if you > +replaced the word "Peer" with "Host" throughout this section. > ++--- > 【WAJ】: Here, the "Peer Route" just wants to emphasize the route is to the > peer, although actually is a host route( to the peer). > Describe it solely only with "Host" can't have such refer and maybe > mislead to the reader? We would like to keep it in this way? > > > > The problem with this is that although in your implementation you might > only plan to use it for installing a route to the peer, some other > implementation might choose to use it for something different. History > shows that when a generic mechanism is introduced, it is often used in ways > the inventors didn’t intend. So, my preference is you use an accurate > description. Maybe a happy medium would be to make the suggested change but > also add descriptive text clarifying the intended use? Something like, > > > > OLD: > > The Explicit Peer Route object is defined to specify the explicit > > peer route to the corresponding peer address on each device that is > > on the E2E Native-IP TE path. This Object should be sent to all the > > devices on the path that is calculated by the PCE. > > > > NEW: > > The Host Route object is defined to install a host route to the > > corresponding peer address on each device that is > > on the E2E Native-IP TE path. This Object should be sent to all the > > devices on the path that is calculated by the PCE. Although this > > object could be used to install host routes for other purposes, > > any such use is outside the scope of this specification. > > > > The other way would be to retain the name but add a different explanation, > as in, > > > > NEW: > > Although the object is named “Explicit Peer Route”, it can be seen > > that the routes it installs are simply host routes. The use of this > > object to install host routes for any purpose other than reaching the > > corresponding peer address on each device that is on the E2E > > Native-IP TE path is outside the scope of this specification. > > > > My preference is the first approach but either one works for me. If one of > them is ok for you, just pick it and proceed forward. > > 【WAJ】Considering to limit the change in smaller scope(other parts within > the document, the IANA temporarily allocation etc.) and reduce the risk of > mismatch, I select the second approach.------And, there is another > consideration, that is, to accomplish the overall task that described in > this document, we must install the route to the peer on every router on the > path, not arbitrary host route that name “Host Route Object” which may be > mishandled. > > > > The Explicit Peer Route object is defined to specify the explicit > peer route to the corresponding peer address on each device that is @@ > -1189,7 +1361,18 @@ > establishment. No TLVs are currently defined. > > 7.4. Peer Prefix Advertisement Object > ++--- > +jgs: It appears there is an assumption that IPv4 routes will be sent > +over an IPv4 peering, and IPv6 routes will be sent over an IPv6 peering. > +This might be problematic especially for IPv4 routes, if the provider > +network doesn’t use IPv4 in the underlay and uses tunnel mode to carry > +IPv4 traffic across an IPv6 backbone. > > +Well this restriction doesn't seem necessary to me, I think it would be > +OK to flag it without correcting, if you switch to the Experimental > +track. > ++--- > 【WAJ】It is possible to mix the carrier's transport address family with > different address family of the actual traffic. > > > > Are you saying that this is possible with the protocol you specify here? > Can you outline how? As far as I could see, if my BGP session is between > IPv6 loopbacks, only IPv6 prefixes can go in the Prefix Advertisement > Object, and simiilarly for IPv4. > > > > On the other hand if you’re just saying that this is possible in real > networks, even though your protocol can’t support it, then we agree. > > > > And, again, we want to simplify the parameter negotiations between the > PCCs(and PCE), then solidify the encoding as IPv4 traffic is carried by > IPv4 transport, and the same as for IPv6 address family. Anyway, the > devices within the operator network all support such behavior, but not all > of them support the hybrid comination. > > > > As I said in my earlier comment, I’m ok with the restriction given the > Experimental track. I do think you need to flag it though. For example, > > > > NEW: > > If in the future, a requirement is identified to advertise IPv4 > > prefixes towards an IPv6 peering address, or IPv6 prefixes towards an > > IPv4 peering address, then new Peer Prefix Advertisement Object-Types > > can be defined for these purposes. > > 【WAJ】This is what my exact meanings. I have updated the content > accordingly as your suggestions. > > > > […] > > > > @@ -1637,7 +1827,48 @@ > validity of the PCE and ensure a secure communication channel between > them. Thus, the mechanisms described in [RFC8253] and [RFC9050] > should be used. > ++--- > +jgs: I appreciate that you are trying to provide bare-bones BGP session > +establishment here, and I see the text in Section 9 that says, > > + This document defines the procedures and objects to create the BGP > + sessions and advertise the associated prefixes dynamically. Only the > + key information, for example peer IP addresses, peer AS number are > + exchanged via the PCEP protocol. Other parameters that are needed > + for the BGP session setup should be derived from their default > + values. > + > +but your design makes it impossible to provide transport security, such > +as TCP-AO, because although there is a way to tell the PCC what its BGP > +peer is, there is no way to tell the PCC what key to use in > +communicating with that peer. In the case of session keying, it's not > +reasonable to suggest the key should be "derived from their default > +values". Even if the practice of using a single default key for all > +internal sessions is used (and I'm not saying that would be a best > +practice!), this simply can't work for EBGP, and you do propose to > +cover EBGP. > + > +If you do switch to the Experimental track, in my opinion, something > +like the following would be adequate: > + > +NEW: > + Because this specification does not provide a way to communicate > + properties beyond peer address and AS number for the BGP sessions > + that are established, it will not always be possible to follow best > + practices as described in [BCP194], if suitable default values as > + discussed in [Section 9] cannot be used. An example would be keying > + for use with TCP-AO [RFC5925]. > + > + If such functionality is required in the future, it can be provided > + through the addition of optional TLVs to the BGP Peer Info object, > + that convey the necessary additional information (for example, a key > + chain [RFC8177] name). > + > +Note, it's just my opinion that this would be good enough, other > +reviewers might have their own thoughts (notably SECDIR and the SEC > +ADs). > ++--- > 【WAJ】：Yes, we plan to add additional TLV to convey the information about > the secure of the BGP session. I think your recommendation is appropriate > for the security considerations. I have adopted part of your recommendation > text as the followings: > If suitable default values as discussed in section 9 isn't enough and > securing the BGP transport is required(for example, the TCP-AO(RFC5925), it > can be provided through the addition of optional TLV to the BGP Peer Info > object that convey the necessary additional information(for example, a key > chain(RFC8177) name > > > > Cool. I think it is OK to make RFC 8177 an informative reference, since it > ’s only a “for example”. > > 【WAJ】Done > > > > > _______________________________________________ > Pce mailing list -- pce@ietf.org > To unsubscribe send an email to pce-le...@ietf.org >
_______________________________________________ Pce mailing list -- pce@ietf.org To unsubscribe send an email to pce-le...@ietf.org
