[Pce] Re: AD review of draft-ietf-pce-pcep-extension-native-ip-30

[John Scudder <[e-mail]>]

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 <[e-mail]> 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$
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.

[…]

   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.

[…]

   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.


      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.

   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.

[…]

@@ -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”.