Re: [sidr] draft-ietf-sidr-bgpsec-protocol

[Keyur Patel <[e-mail]>]

Hi Alvaro,

Comments inlined #Keyur

From: "Alvaro Retana (aretana)" <[e-mail]>
Date: Friday, January 6, 2017 at 3:10 PM
To: "Sriram, Kotikalapudi (Fed)" <[e-mail]>, Keyur Patel <[e-mail]>
Cc: "[e-mail]" <[e-mail]>, sidr <[e-mail]>, "[e-mail]" <[e-mail]>
Subject: Re: draft-ietf-sidr-bgpsec-protocol


On 1/6/17, 12:40 AM, "Sriram, Kotikalapudi (Fed)" <[e-mail]> wrote:



[Cut the distribution list a little.]



Sriram:



Hi!  Happy New Year!



I have some comments on this, please see below.



Thanks!



Alvaro.





…

| | 1)  Section 4.1 “The BGPsec Path attribute and the AS_PATH attribute are mutually

| | exclusive. That is, any update message containing the BGPsec Path attribute MUST NOT

| | contain the AS_PATH attribute”.  For any restarting speakers in a GR mode, where the bgp

| | capability is not exchanged, the existing stale routes won’t have an AS_PATH attribute. We

| | could add some clarifying that helps to indicate that such routes should be considered

| | valid in stale mode (till they get refreshed)?

|

| [Sriram]  As you have clarified for me on the phone, what you are saying here is that the

| two BGPsec peers lost the BGPsec session and now restarting in GR mode, but they have not

| exchanged BGPsec capability this time. Hence, they are now simple BGP (non-BGPsec) peers

| in GR mode. RFC4271 considers update message received without a well-known AS_PATH

| attribute as an error, and unfortunately in this case the cached BGPsec updates do not have

| AS_PATH (albeit they have BGPsec_Path). So you are saying "the router should not panic"

| and instead simply treat each cached update as NOT-IN-ERROR even though it is missing

| AS_PATH attribute. This way the GR can work properly. Of course, shortly the updates will

| have AS_PATH (and not considered in error) when they get refreshed (over the new simple

| BGP session). Per your suggestion, I will include new text in Section 7 to describe this

| required behavior for the GR mode.



I don’t have an objection for this behavior, but I think we should make the WG (and idr!) aware of the change and get their comments (if any) before I approve the publication.



#Keyur: Ack. Though I was only requesting some text clarification so that it is very clear to the implementers.



Regards,

Keyur





…

| | 3)  Section 5 and Section 5.2, 1st paragraph: RFC4271 considers update message received

| | without a well-known AS_PATH attribute as an error.  We need some text to clarify the

| | (error handling if any) behavior when an update message is received without a bgpsec and

| | an aspath attribute. The current draft text seems unclear about generation of bgpsec

| | attribute as well (in a ibgp scenario). Is it a requirement to generate an empty bgpsec

| | attribute?

|

| [Sriram]  As you have clarified for me over the phone, RFC 4271 (page 26) says the

| following :

|

|  "When a BGP speaker originates a route then:

|   b) the originating speaker includes an empty AS_PATH attribute in

|        all UPDATE messages sent to internal peers.  (An empty AS_PATH

|         attribute is one whose length field contains the value zero)."

|

|

| [Sriram]  So what needs to be said in the BGPsec document is the following:  The

| BGPsec_Path attribute is not attached in updates originated inside an AS and propagated to

| BGPsec capable internal peers. However, when a route is originated inside an AS and

| propagated to non-BGPsec internal peers, an empty AS_PATH attribute is included in the

| update (see [RFC 4271], page 26).



The Route Selection Section (9.1.2) in RFC4271 is not explicit about performing loop detection only on eBGP sessions – the criteria is generic to any route, so there is a possibility that a BGPsec-capable router may want to perform loop detection on an iBGP-received Update.  Given this text from Section 5 in the BGPsec spec:



   Whenever the use of AS path information is called for

   (e.g., loop detection, or use of AS path length in best path

   selection) the externally visible behavior of the implementation

   shall be the same as if the implementation had run the algorithm in

   Section 4.4 and used the resulting AS_PATH attribute as it would for

   a non-BGPsec update message.



…how should an iBGP speaker perform loop detection if there’s no BGPsec_Path attribute?  In other words, there is no defined mechanism to run the algorithm in 4.4 without it.



I’m not suggesting that you include an empty attribute, but that you indicate in 4.4 that no BGPsec_Path attribute is equivalent to an empty AS_PATH.



Thanks!



Alvaro.