Subject: [Idr] AD Review of draft-ietf-idr-bgp-extended-messages-20

From: "Alvaro Retana (aretana)" <[email-masked]>
Date: Thu, 23 Feb 2017 19:17:05 +0000

Dear authors:

I just finished reading this document.  I started hoping that this would be a quick and easy read, and it was, but I have some concerns related to the operation of this extension, specifically as it refers to error correction and transition.  Please take a look at the comments below.  I note that these two major concerns were already discussed on the WG list as a result of the RtgDir review [1] and during the WGLC [2], but, while there seemed to be understanding of the points made and good discussion, none of that was reflected in the draft.

I will wait until we resolve what I consider are Major issues before starting the IETF Last Call.

Thanks!

Alvaro.

[1] https://mailarchive.ietf.org/arch/msg/rtg-dir/6EJK3UC7bFnTps0afdpl2Iw3d5Y/?qid=3668a9f4a846476e78c8992fcd3ce2e6
[2] https://mailarchive.ietf.org/arch/msg/idr/mBxUNwF0qsdGUWpqpUkGYAwJkYg/?qid=18c32eeee917280402a62da361902190

Major:

M1. Section 4. (Operation): “An implementation that supports the BGP Extended Messages MUST be prepared to receive an UPDATE message that is larger than 4096 bytes.“  Only UPDATEs?  I know that the most likely case for exceeding the 4k size is an UPDATE, but why are the other messages not considered?  I think that OPEN/NOTIFICATIONs exceeding 4k would be unusual (and maybe even unnecessary or unwanted), but given the extra space I can imagine the benefits of adding more detailed information in NOTIFICATIONs – not that we should, but we could.  Also, what does “prepared to receive” mean, and how can “MUST be prepared to receive” be enforced?  Given the discussion in Section 5 (Error Handling), you might want to add something like “…even if the Capability is not advertised”.

M2. Section 5 (Error Handling).  “A BGP speaker that has the ability to use extended messages but has not advertised the BGP Extended Messages capability, presumably due to configuration, SHOULD NOT accept an extended message.  A speaker MAY implement a more liberal policy and accept extended messages even from a peer that has not advertised the capability.”  This paragraph troubles me a lot because it is in direct contraction with Section 3: “A peer which does not advertise this capability MUST NOT send BGP Extended Messages, and BGP Extended Messages MUST NOT be sent to it.”.  However, I think that John Scudder’s reasoning [3] makes sense ("keep the session up at (almost) all costs", and there’s clear precedence in the WG) for the case where the sender did advertise the Capability, but I’m not convinced on the case where it didn’t – please include something like John’s explanation in the text.

M2.1.  Even with John’s explanation, I find myself thinking that this specification could result in some sloppy implementations: if I need to account for receiving unexpected Extended Messages in my code, then maybe I won’t worry too much about controlling what to send to my peers – specially in cases where it would be easy to just replicate an UPDATE (like in a peer-group) and not worry about possible exceptions.  I know that we can’t avoid bad implementations, no matter what text is added – but I think that recognizing the threat (maybe in the Security Considerations section) of someone receiving an Extended Message when they don’t support it would be good.   I know that there’s text in the document already which talks about what to do if the receiver doesn’t support Extended Messages – I’m just worried about potential issues with memory allocation if the receiver was not ready…

[3] https://mailarchive.ietf.org/arch/msg/idr/-_GW6rftabJLrrQnxoxM5XXDGAM/?qid=5e8521fc3a43a5580a3c1e05961dc084

M3. Section 5 (Error Handling).  “…reset the session with a Bad Message Length NOTIFICATION…”  Please be clear and specific: (something like this would be more precise) “...send a NOTIFICATION message with the Error Code set to Message Header Error and the Error Subcode set to Bad Message Type, and close the session”.  Alternatively, you can just remove the text (after the reference to rfc4271) as that is what rfc4271 already says and there’s no need to repeat it here and risk not being precise…

M4. Section 5 (Error Handling).  “Similarly, any speaker that treats an improper extended message as a fatal error, MUST do likewise.”  It sounds that you’re saying that any fatal error will result in a “Bad Message Length NOTIFICATION”.  I hope that is not what you meant – and that other errors should result in the appropriate action from rfc4271/rfc7606.  IOW, the error checking for the message (besides de length) doesn’t change, right?

M5. Section 5 (Error Handling).  “The inconsistency between the local and remote BGP speakers MUST be reported via syslog and/or SNMP.”   SNMP?  AFAIK, there’s no object that can report this inconsistency since there’s no NOTIFICATION generated.  In the proposed text by Gunter Van De Velde [4], SNMP and syslog were mentioned as examples – I suggest you follow that path (no need for all the “flowery language”) and just reference mechanisms by example to avoid having to point at how it would be done.

M5.1. [minor] Gunter had originally suggested that the message that caused the inconsistency be included in the report.  Are you expecting the inconsistency report to just be a “Bob sent me an Extended Message, but I didn’t advertise the Capability to him”-type message, or something more?  It might be useful to provide some guidance indicating what type of information might be useful/interesting.

[4] https://mailarchive.ietf.org/arch/msg/idr/RtcX--cG91_rXZw6Jb3OpSOWI00/?qid=bfc7a8f05a4c86ca9a8c568ec68a9bf5

M6. Updates to rfc4271.  The Abstract/Introduction correctly mention that this document Updates rfc4271.  But I think we need to be more specific, specially where rfc4271 changes and there is normative language involved.  I found two cases:

M6.1. Section 5 doesn’t describe the behavior if the message is longer than 65535.  Please include either an explicit update to rfc4271/Section 6.1 for the use of Extended Messages, or the specific process here.

M6.2. rfc4271: “The value of the Length field MUST always be at least 19 and no greater than 4096”  That needs to be updated to 65535.

M7. What about transition/migration/partial deployment?  What should the behavior be if, for example, an Extended Message UPDATE is received from a peer, but can’t be propagated to others because they don’t support Extended Messages (think route reflectors or simple eBGP -> iBGP)??  There should be some guidance for the general case (i.e. when the total size is >4k due simply to the total amount of information, and not because a single attribute, for example, is really big), and some requirements looking forward to potential new messages/attributes that specifically rely on Extended Messages.

M7.1. This topic was also brought up during WGLC, for example [5] and [6].  There are a couple of suggested actions/text on the list too: [7] and [8] – one of them is: “if the attribute size is such that the message length does get exceeded then the Prefix SHOULD not be announced and an error should be logged (existing case)” [8].  A couple of points to highlight: (1) “SHOULD not be announced”: which goes back to Section 5 (and, in this case, sending Extended Messages without receiving the Capability from the neighbors); (2) the behavior can result in inconsistent routing, etc…please mention the potential risks of partial deployment.

M7.2. What should the default be for this extension?  Should it be enabled by default or not?

[5] https://mailarchive.ietf.org/arch/msg/idr/D-0QACWXZIv_MwfNt31-Z0B21kI/?qid=ab2c93d1df016705a4834b4e2fb0c1bf
[6] https://mailarchive.ietf.org/arch/msg/idr/n3UQqkWf5RTsl2FBRKP2oqjzqok/?qid=a35699c937ccd38d9b9054e04351487d
[7] https://mailarchive.ietf.org/arch/msg/idr/Ugdm6XTnNp5JNLaiCUQwfOZNwyI/?qid=af475de36c9d2d6e64b3c5fbf95d8b1e
[8] https://mailarchive.ietf.org/arch/msg/idr/dQryJqArmA4OZUWAHLA2zpwhSN4/?qid=af475de36c9d2d6e64b3c5fbf95d8b1e

Minor:

P1. Abstract: s/extend its current message size from 4096/extend its current maximum message size from 4096

P2. s/I-D.ietf-sidr-bgpsec-overview/I-D.ietf-sidr-bgpsec-protocol

P3. IANA already assigned Code 6 for the Capability.  Please use that value and remind IANA of the early allocation in the IANA Considerations section.

P4. I don’t understand what this means: “Applications generating messages which might be encapsulated within BGP messages MUST limit the size of their payload to take into account the maximum message size.”   The part that is not clear is “messages…encapsulated within BGP messages…”.  What is that?  I guess you’re talking about any stuff that BGP may be transporting, right?  Maybe s/messages which might be/information to be

P5. Security Considerations: I think it would be good to also reference rfc4272 (BGP Security Vulnerabilities Analysis) in this section.

Nits:

N1. “It does enable large BGPsec BGPSEC_PATHs, see [I-D.ietf-sidr-bgpsec-protocol].”  Nice, but superfluous as it refers to this document.

---

From: Jeffrey Haas <[email-masked]>
Date: Tue, 28 Feb 2017 16:06:27 -0500

Alvaro,

I'm not speaking for the authors.  However, a few comments on your writeup:

On Thu, Feb 23, 2017 at 07:17:05PM +0000, Alvaro Retana (aretana) wrote:
> M1. Section 4. (Operation): “An implementation that supports the BGP
> Extended Messages MUST be prepared to receive an UPDATE message that is
> larger than 4096 bytes.“  Only UPDATEs?  I know that the most likely case
> for exceeding the 4k size is an UPDATE, but why are the other messages not
> considered?  

The fundamental issue is with regards to not only the basic behavior of RFC 4271 
and its earlier RFC 1771 but also to BGP Versioning.

Those RFCs set the expectation that version 4 of BGP will have at most a 4k
packet size.  Capability advertisement, an extension on top of BGP, isn't a
required feature - although it's certainly very common these days.

Thus, for backward compatibility reasons, OPEN messages must be no bigger
than 4k if we're still BGP-4.

I don't think there's an argument for larger KEEPALIVE messages. :-)

Similarly, the NOTIFICATION with no restrictions for version compatibility
reasons.  *However*, once BGP has reached the Established state, it is
possible for it to send longer NOTIFICATION messages, if we permitted it and
the feature was duly negotiated.  This is probably not the best idea because
it will still be necessary to be able to deal with an old speaker in such
cases.  

Also, most implementations don't dump that much information in the
NOTIFICATION Data section.  Putting sufficient semantics on the content
would start to get messy.  We don't want XML or backtrace over that message.

The case for UPDATE is clear.

The remaining message with some potential motivation is ROUTE-REFRESH.  The
basic refresh message is likely to be able to hold AFI/SAFI sets for some
time.  The ORF feature that is built upon refresh already deals with sending
its filters over the course of multiple messages.  So, there's not a lot of
motivation.

And that's the messages we have so far.  While I can see new messages being
defined that may want it, I don't think there's a strong argument for it
today.

> Also, what does “prepared to receive”
> mean, and how can “MUST be prepared to receive” be enforced?  Given the
> discussion in Section 5 (Error Handling), you might want to add something
> like “…even if the Capability is not advertised”.

If the capability hasn't been negotiated, then the expectation is that it's
a PDU error and the session should be dropped.  See prior comments about BGP
versioning.

> M2. Section 5 (Error Handling).  “A BGP speaker that has the ability to
> use extended messages but has not advertised the BGP Extended Messages
> capability, presumably due to configuration, SHOULD NOT accept an extended
> message.  A speaker MAY implement a more liberal policy and accept
> extended messages even from a peer that has not advertised the
> capability.”  This paragraph troubles me a lot because it is in direct
> contraction with Section 3: “A peer which does not advertise this
> capability MUST NOT send BGP Extended Messages, and BGP Extended Messages
> MUST NOT be sent to it.”.  However, I think that John Scudder’s reasoning
> [3] makes sense ("keep the session up at (almost) all costs", and there’s
> clear precedence in the WG) for the case where the sender did advertise
> the Capability, but I’m not convinced on the case where it didn’t – please
> include something like John’s explanation in the text.

John can own this one.  I find the case a little weaker.  It vaguely makes
me want a probe message to verify that I can indeed get a full-sized PDU
through.

> M5. Section 5 (Error Handling).  “The inconsistency between the local and
> remote BGP speakers MUST be reported via syslog and/or SNMP.”   SNMP?
> AFAIK, there’s no object that can report this inconsistency since there’s
> no NOTIFICATION generated.  In the proposed text by Gunter Van De Velde
> [4], SNMP and syslog were mentioned as examples – I suggest you follow
> that path (no need for all the “flowery language”) and just reference
> mechanisms by example to avoid having to point at how it would be done.

IMO, "brought to the attention of the operator.  Example mechanisms might
include syslog or SNMP notifications."

While you're correct that no IETF standard MIB object covers such a
scenario, we shouldn't be proscriptive about some vendor deciding they want
it in their enterprise implementation.

At some point, we need similar boilerplate language for yang notifications.

> M7. What about transition/migration/partial deployment?  What should the
> behavior be if, for example, an Extended Message UPDATE is received from a
> peer, but can’t be propagated to others because they don’t support
> Extended Messages (think route reflectors or simple eBGP -> iBGP)??  There
> should be some guidance for the general case (i.e. when the total size is
> >4k due simply to the total amount of information, and not because a
> single attribute, for example, is really big), and some requirements
> looking forward to potential new messages/attributes that specifically
> rely on Extended Messages.

I'm not sure such guidance belongs in this document.  We already have
scenarios wherein normal protocol machinery can result in messages that are
too large.  The expected behavior is "treat as withdraw" to the next
downstream, similar to the BGP Error handling RFC.

Examples of this include AS_PATH or CLUSTER_LIST attributes needing to add a
new entry on a full PDU.

> M7.2. What should the default be for this extension?  Should it be enabled by default or not?

IMO, there are good reasons to not turn it on for existing BGP deployments
but alternatively good reasons once the extension is widely enough present
in a given topology.  The latter example includes bgpsec.

-- Jeff