Subject: [Idr] 2 Week WG LC: draft-ietf-idr-error-handling-07.txt

From: "Susan Hares" <[email-masked]>
Date: Wed, 7 May 2014 14:31:15 -0400

John: 

Congratulations on publishing draft of draft-ietf-idr-error-handling-07.txt.
This begins a 2 week WG LC starting today (5/7/14), and ending 5/21/14.  In
your comments please indicate, support/no support for the WG LC. 

Please also indicate if you know if IPR by responding. 

Sue 

-----Original Message-----
From: Idr [mailto:[email-masked]] On Behalf Of John G. Scudder
Sent: Wednesday, May 07, 2014 2:17 PM
To: idr@ietf. org
Subject: Re: [Idr] I-D Action: draft-ietf-idr-error-handling-07.txt

Folks,

Compared to -06, this version lists a couple more NLRI (in)correctness
criteria in S. 3.2 as discussed at the last IETF meeting, adds S. 3.3 on the
handling of typed NLRI (such as for MVPN and EVPN), makes an editorial
clarification regarding when a session must be reset, and removes Appendix
A. The deletion of Appendix A was because it was there for purposes of
explanation in the early versions of the draft, but at this point everyone
seems to get the point and there doesn't seem to be any need to clutter up
the eventual RFC with it. 

With this revision the authors think the document is ready for working group
last call.

Thanks,

--John

On May 7, 2014, at 2:08 PM, [email-masked] wrote:

> 
> A New Internet-Draft is available from the on-line Internet-Drafts
directories.
> This draft is a work item of the Inter-Domain Routing Working Group of the
IETF.
> 
>        Title           : Revised Error Handling for BGP UPDATE Messages
>        Authors         : Enke Chen
>                          Juniper Networks
>                          Pradosh Mohapatra
>                          Keyur Patel
> 	Filename        : draft-ietf-idr-error-handling-07.txt
> 	Pages           : 13
> 	Date            : 2014-05-07
> 
> Abstract:
>   According to the base BGP specification, a BGP speaker that receives
>   an UPDATE message containing a malformed attribute is required to
>   reset the session over which the offending attribute was received.
>   This behavior is undesirable as a session reset would impact not only
>   routes with the offending attribute, but also other valid routes
>   exchanged over the session.  This document partially revises the
>   error handling for UPDATE messages, and provides guidelines for the
>   authors of documents defining new attributes.  Finally, it revises
>   the error handling procedures for a number of existing attributes.
> 
>   This document updates error handling for RFCs 1997, 4271, 4360, 4456,
>   4760, and 5701.
> 
> 
> The IETF datatracker status page for this draft is:
> https://datatracker.ietf.org/doc/draft-ietf-idr-error-handling/
> 
> There's also a htmlized version available at:
> http://tools.ietf.org/html/draft-ietf-idr-error-handling-07
> 
> A diff from the previous version is available at:
> http://www.ietf.org/rfcdiff?url2=draft-ietf-idr-error-handling-07
> 
> 
> Please note that it may take a couple of minutes from the time of 
> submission until the htmlized version and diff are available at
tools.ietf.org.
> 
> Internet-Drafts are also available by anonymous FTP at:
> ftp://ftp.ietf.org/internet-drafts/
> 
> _______________________________________________
> Idr mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/idr

_______________________________________________
Idr mailing list
[email-masked]
https://www.ietf.org/mailman/listinfo/idr

---

From: Jeffrey Haas <[email-masked]>
Date: Wed, 7 May 2014 14:32:47 -0400

Sue,

> Please also indicate if you know if IPR by responding. 

While an implementation report would be more appropriate, might I also
suggest asking in the poll whether you have an implementation covering the
functionality?

-- Jeff (who will be responding a qualified yes)

---

From: "Susan Hares" <[email-masked]>
Date: Wed, 7 May 2014 14:35:37 -0400

IDR folks: 

Please do comment if you know about implementations.   IDR does require 2
implementation for passing a draft to the IESG.   

Sue 

-----Original Message-----
From: Idr [mailto:[email-masked]] On Behalf Of Jeffrey Haas
Sent: Wednesday, May 07, 2014 2:33 PM
To: [email-masked]
Subject: Re: [Idr] 2 Week WG LC: draft-ietf-idr-error-handling-07.txt

Sue,

On Wed, May 07, 2014 at 02:31:15PM -0400, Susan Hares wrote:
> This begins a 2 week WG LC starting today (5/7/14), and ending 
> 5/21/14.  In your comments please indicate, support/no support for the WG
LC.

---

From: "Saikat Ray (sairay)" <[email-masked]>
Date: Wed, 7 May 2014 19:17:49 +0000

Cisco IOS and XR OSes have implemeneted the error handling draft. The
implementations are more than an year old, so not everything may be per
-07, though.

---

From: "John G. Scudder" <[email-masked]>
Date: Wed, 7 May 2014 15:54:47 -0400

On May 7, 2014, at 3:17 PM, Saikat Ray (sairay) <[email-masked]> wrote:

> not everything may be per -07, though.

I suspect many/most [*] will need to update their implementations since -06 introduced one significant change (got rid of "fixing" the attribute flags).

It still seemed worth WGLC'ing since we now have the 'waiting for implementation' state we can park the draft in for the (hopefully not very long) interval until the implementations are updated.

--John

[*] Except Thomas Mangin who probably had his done 24 hours prior to publication of -06 ;-)

---

From: "John G. Scudder" <[email-masked]>
Date: Wed, 7 May 2014 15:55:18 -0400

> Please also indicate if you know if IPR by responding. 

I am not aware of IPR related to this draft.

Thanks,

--John

---

From: Enke Chen <[email-masked]>
Date: Wed, 07 May 2014 13:22:16 -0700

I am not aware of any IPRs related to this draft either.

Thanks.  -- Enke

---

From: "Smith, Donald" <[email-masked]>
Date: Wed, 7 May 2014 20:43:59 +0000

The error handling of the following case described in Section 6.3 of
   [RFC4271] is revised

      If any recognized attribute has Attribute Flags that conflict with
      the Attribute Type Code, then the Error Subcode MUST be set to
      Attribute Flags Error.  The Data field MUST contain the erroneous
      attribute (type, length, and value).

   as follows:

      If any recognized attribute has Attribute Flags that conflict with
      the Attribute Type Code, then the attribute MUST be treated as
      malformed and the treat-as-withdraw approach (see below) used,
      unless the specification for the attribute mandates different
      handling for incorrect Attribute Flags.

Are there specifications for attributes that mandates different behavior or is that a future looking statement?

(coffee != sleep) & (!coffee == sleep)
 [email-masked]

---

From: "Keyur Patel (keyupate)" <[email-masked]>
Date: Wed, 7 May 2014 21:29:39 +0000

+1.

Regards,
Keyur

---

From: Jeff Tantsura <[email-masked]>
Date: Wed, 7 May 2014 22:42:02 +0000

Yes/support

Cheers,
Jeff

---

From: Jeff Tantsura <[email-masked]>
Date: Wed, 7 May 2014 22:43:29 +0000

Hi Jeff,

We are (E///) going thru the last rounds of testing, so count as
implemented.

Cheers,
Jeff

---

From: "John G. Scudder" <[email-masked]>
Date: Thu, 8 May 2014 16:58:02 -0400

Donald,

> Are there specifications for attributes that mandates different behavior or is that a future looking statement?

I'm aware of one, draft-ietf-idr-aigp-18, S. 3.2:

   If a BGP Path Attribute is received that has the AIGP attribute
   codepoint, but also has the transitive bit set, the attribute MUST be
   considered to be a malformed AIGP attribute, and MUST be discarded as
   specified in this section.

Regards,

--John

---

From: Warren Kumari <[email-masked]>
Date: Thu, 8 May 2014 18:38:18 -0400

> Yes/support

As do I.
W

---

From: Jon Mitchell <[email-masked]>
Date: Thu, 8 May 2014 19:58:49 -0400

Support.

-Jon

> On May 7, 2014, at 2:31 PM, "Susan Hares" <[email-masked]> wrote:
> 
> John: 
>

---

From: <[email-masked]>
Date: Fri, 9 May 2014 14:18:32 +0000

Susan, John,

I support.

Please find below some comments/questions:

------
§1 Introduction
"In the case of optional
   transitive attributes, the behavior is especially troublesome and may
   present a potential security vulnerability.  The reason is that such
   attributes may have been propagated without being checked by
   intermediate routers that do not recognize the attributes -- in
   effect the attribute may have been tunneled, and when they do reach a
   router that recognizes and checks them, the session that is reset may
   not be associated with the router that is at fault."

Another reason, and IMO a significant one, is that when the error is detected, this erroneous transitive attribute has reached multiple routers, up to a significant number, network wide. Therefore this create many multiple simultaneous failure in the networks, with a large part of the network been affected. When centralized route reflectors are used (e.g. VPN networks), if the Route Reflector does not recognize the attribute/error, then a significant part of the IBGP sessions (hence the network) is affected.

A third reason, is that a receiver, when detecting an error, will tend to always believe that he is the one to be right and that the error is on the sender side; while this may not be the case. The bug or misinterpretation of the spec could equally be on the receiver or on the sender. Eventually, nobody will ever know (e.g. draft-hares-idr-update-attrib-low-bits-fix). So if the error is in fact on the receiver side, again we have multiple routers detecting a bug at the same time, just like with unknown transitive attributes. So let's be humble when we detect an error, and let's not consider that the speaker is to buggy to talk to and we'd be safer kicking it outside the network.

Bottom line, my main reason is the fear of the multiplier effect (one wrong message triggers multiple BGP sessions failures)
-----
§2 Revision to the base spec.

Editorial: IMHO, page 4 lacks structure which makes it harder to identify the different cases of error. I would propose to add a number (or sub paragraph). e.g. 
OLD :      The error handling of the following case described in Section 6.3 of  [RFC4271] is revised
NEW: 2) The error handling of the following case described in Section 6.3 of  [RFC4271] is revised

----
Editorial: "This document partially revises [...] provides guidelines for the authors of documents defining new attributes."

I would propose to group these guidelines in a specific paragraph. Indeed, such guidelines are expected to be referred to and read for each new IDR spec. (while currently, those guidelines are a bit spread in the document)
----
§ 3.3. Typed NLRI

"   A BGP speaker advertising support for such a typed address family
   MUST handle routes with unrecognized NLRI types within that address
   family by discarding them, unless the relevant specification for that
   address family specifies otherwise."

The latest part seems strange: " unless the relevant specification for that address family specifies otherwise". Indeed, given that the NLRI types are unrecognized by the BGP speaker, it seems strange to mandate him to comply with the spec which is unknown to him.
------
Editorial/nit picking: §5.11/5.12 & 5.5 have chosen a different way of expressing the same thing. §5.5 groups per (type of error (malformed or illegitimate presence) , type of session) while §5.11/5.12 groups per (type of session, type or error). Personally I have a preference for the §5.11/5.12 way, which is more concise.

------
What's next after WGLC ?
Over the last years, the draft has been regularly updated, showing that this subject is a large and difficult. Unless we have a reason to believe that now the spec is final and perfect, what is the plan for the next spec change (in 12 months ;-) )? Are we ready/ok to create a bis draft?
------

Thanks,
Regards,
Bruno

> From: Idr [mailto:[email-masked]] On Behalf Of Susan Hares
> Sent: Wednesday, May 07, 2014 8:31 PM
> To: 'John G. Scudder'; 'idr@ietf. org'
> Compared to -06, this version lists a couple more NLRI (in)correctness criteria
> in S. 3.2 as discussed at the last IETF meeting, adds S. 3.3 on the handling of
> typed NLRI (such as for MVPN and EVPN), makes an editorial clarification
> regarding when a session must be reset, and removes Appendix A. The
> deletion of Appendix A was because it was there for purposes of explanation
> in the early versions of the draft, but at this point everyone seems to get the
> point and there doesn't seem to be any need to clutter up the eventual RFC
> with it.
> 

_________________________________________________________________________________________________________________________

Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.

---

From: <[email-masked]>
Date: Fri, 9 May 2014 14:18:36 +0000

Donald, John,

> I'm aware of one, draft-ietf-idr-aigp-18, S. 3.2:

My understanding of draft-ietf-idr-error-handling-07 is that it seems to define a different behavior (session shutdown) for 2 other attributes: MP_REACH,  MP_UNREACH

- §3.2: "the MP_REACH or MP_UNREACH attribute of an update SHALL be considered to be incorrect if [...]
The attribute flags of the attribute are inconsistent with those specified in [RFC4760]."

-§2: " in order to use the approach of "treat-as-
   withdraw", the entire NLRI field and/or the MP_REACH_NLRI and
   MP_UNREACH_NLRI attributes need to be successfully parsed.  If this
   is not possible, the procedures of [RFC4271] continue to apply,
   meaning that the session MUST be reset and a NOTIFICATION sent.
   Alternatively the error handling procedures specified in [RFC4760]
   for disabling a particular AFI/SAFI MAY be followed.  One notable
   case where it would be not possible to successfully parse the NLRI is
   if the NLRI field is found to be "syntactically incorrect" (see
   Section 3.2)."

Is this a correct understanding? Is this intended (that a flag error in attributes MP_(UN)REACH triggers a session shutdown) ?

Thanks,
Regards,
Bruno

---

From: "John G. Scudder" <[email-masked]>
Date: Fri, 9 May 2014 11:08:26 -0400

Hi Bruno,

Thanks for your comments. Some responses below. I haven't commented on your editorial/nit points but will see if I can find a way to incorporate them and may follow up later.

>   transitive attributes, the behavior is especially troublesome and may
...
> Bottom line, my main reason is the fear of the multiplier effect (one wrong message triggers multiple BGP sessions failures)

I agree with your points, especially regarding the multiplier effect. Are you suggesting we introduce this reasoning into the draft? At this point I think it's not critical to provide extensive rationale -- we have moved on from convincing ourselves of the importance of the problem, to agreeing on the specification of how to fix it. This is what motivated the removal of Appendix A.

> The latest part seems strange: " unless the relevant specification for that address family specifies otherwise". Indeed, given that the NLRI types are unrecognized by the BGP speaker, it seems stra…

Possibly this should be written differently to be clearer. Let me provide an example. Suppose a route reflector advertises MP-BGP support for AFI/SAFI 25/70 (L2VPN/EVPN). EVPN NLRI are structured as TLVs:

                   +-----------------------------------+
                   |    Route Type (1 octet)           |
                   +-----------------------------------+
                   |     Length (1 octet)              |
                   +-----------------------------------+
                   | Route Type specific (variable)    |
                   +-----------------------------------+

However there is no place in the MP-BGP capability for the route reflector to say "I support EVPN route types 1, 2, 3 and 4". Suppose EVPN route type 5 is specified at some point. When the route reflector receives a type-5 EVPN route, what is it supposed to do? The options would seem to be

- Hard error, reset session
- Soft error, discard the route (default action in the draft)
- Something else (the EVPN spec should say what)

An earlier unpublished revision of the text called the section "TLV-structured NLRI" but one of the reviewers pointed out that it applies even to NLRI that might not be formally a TLV, e.g. a structure that just uses a type and a value, where length is implied by type.

> Over the last years, the draft has been regularly updated, showing that this subject is a large and difficult. Unless we have a reason to believe that now the spec is final and perfect, what is the…

IMO, yes. Of course we could continue error-handling as a seemingly eternal draft, but I think there is some value in saying "close enough" and graduating it to RFC, especially for implementors that are less steeped in the WG and don't necessarily have the background to distinguish between Internet Drafts that are really *drafts* and those that are de-facto standards.

--John

---

From: "John G. Scudder" <[email-masked]>
Date: Fri, 9 May 2014 11:14:54 -0400

> Is this a correct understanding? Is this intended (that a flag error in attributes MP_(UN)REACH triggers a session shutdown) ?

Yes, for the reasons you quoted from S. 2.

Thanks for pointing it out,

--John

---

From: <[email-masked]>
Date: Fri, 9 May 2014 15:49:21 +0000

Hi John,

Thanks for your answers.
Please see inline.

> From: John G. Scudder [mailto:[email-masked]]
> 
> Hi Bruno,
> 
> Thanks for your comments. Some responses below. I haven't commented on
> your editorial/nit points but will see if I can find a way to incorporate them
> and may follow up later.
> 
> > Bottom line, my main reason is the fear of the multiplier effect (one wrong
> message triggers multiple BGP sessions failures)
> 
> I agree with your points, especially regarding the multiplier effect. Are you
> suggesting we introduce this reasoning into the draft? 

I was indeed suggesting introducing the "multiplier effect" motivation.
I agree with you that this is not critical in the doc, so it's really up to you. I thought I could be useful for network operator, especially for implementations which do not enable new error handling by default. Hence the operator needs to understand the importance of the feature.

>At this point I think it's
> not critical to provide extensive rationale -- we have moved on from
> convincing ourselves of the importance of the problem, to agreeing on the
> specification of how to fix it. This is what motivated the removal of Appendix
> A.
> 
> > § 3.3. Typed NLRI
> 
> >
> > The latest part seems strange: " unless the relevant specification for that
> address family specifies otherwise". Indeed, given that the NLRI types are
> unrecognized by the BGP speaker, it seems strange to mandate him to
> comply with the spec which is unknown to him.
> 
> Possibly this should be written differently to be clearer. Let me provide an
> example. 

Thanks for the example.
The current text was clear. It's just me who was not familiar with typed NLRI and who read the text too quickly. My mistake.

>Suppose a route reflector advertises MP-BGP support for AFI/SAFI
> 25/70 (L2VPN/EVPN). EVPN NLRI are structured as TLVs:
> 
> However there is no place in the MP-BGP capability for the route reflector to
> say "I support EVPN route types 1, 2, 3 and 4". Suppose EVPN route type 5 is
> specified at some point. When the route reflector receives a type-5 EVPN
> route, what is it supposed to do? The options would seem to be
> 
> An earlier unpublished revision of the text called the section "TLV-structured
> NLRI" but one of the reviewers pointed out that it applies even to NLRI that
> might not be formally a TLV, e.g. a structure that just uses a type and a value,
> where length is implied by type.
> 
> > What's next after WGLC ?
> > Over the last years, the draft has been regularly updated, showing that this
> subject is a large and difficult. Unless we have a reason to believe that now
> the spec is final and perfect, what is the plan for the next spec change (in 12
> months ;-) )? Are we ready/ok to create a bis draft?
> 
> IMO, yes. Of course we could continue error-handling as a seemingly eternal
> draft, but I think there is some value in saying "close enough" and graduating
> it to RFC, especially for implementors that are less steeped in the WG and
> don't necessarily have the background to distinguish between Internet
> Drafts that are really *drafts* and those that are de-facto standards.

Thanks for the clarification. I support moving forward the current draft to RFC. I was just checking whether the authors were ok with continuing the work if needed

Regards,
Bruno

---

From: Ondrej Zajicek <[email-masked]>
Date: Sun, 11 May 2014 12:03:16 +0200

> Congratulations on publishing draft of draft-ietf-idr-error-handling-07.txt.

Hello

Here are my comments to the draft. Maybe some of these issues are not
relevant or were already discussed and settled, in that case i am sorry
for the noise.

5.2. AS_PATH

	The draft says 'The error conditions for the attribute have been
	defined in [RFC4271]'. Unfortunately, RFC 4271 is rather vague
	about error conditions for AS_PATH. It says just:

	"The AS_PATH attribute is checked for syntactic correctness.  If the
	path is syntactically incorrect, then the Error Subcode MUST be set
	to Malformed AS_PATH."

	Without exactly specifying the syntactic correctness of AS_PATH.
	Although most if it is intuitive and self-explanatory, there are
	several issues (AFAIK) not addressed anywhere:

	1) Is incorrect AS_PATH with unknown (undefined in RFC 4271)
	segment types (i.e. different ones than AS_SET or AS_SEQUENCE)?
	I would guess yes unless receiver knows these new types (like
	AS_CONFED_* from RFC 5065), but 'no' can be also viewed as
	reasonable possibility.

	2) Is incorrect AS_PATH that cannot be generated by rules from
	RFC 4271 5.1.2? E.g. path containing two small sequence
	segments? I would guess no, but yes is also reasonable
	possibility (if your view on syntax and grammar is more
	generative than descriptive).

	3) Is incorrect AS_PATH that contains seqments of zero length?
	I would say yes according to strict reading of RFC 4271 4.3,
	although syntactically there is no problem with them.

	Also note that the draft implicitly updates RFC 5065 part 5
	(BGP confederations, error handling), as these error conditions
	(for  AS_CONFED_* fields in unexpected contexts) should be
	handled by treat-as-withdraw instead of session-reset.

5.3. NEXT_HOP

	Also a bit vague. RFC 4271 6.3 defines syntactical and
	semantical correctness for NEXT_HOP attribute. A syntactically
	incorrect attribute is obviously malformed, but it is not clear
	whether the attribute should be considered maformed if it is
	just semantically incorrect. Part 2 of the draft suggests it
	shouldn't (as it does not specify a session reset in RFC 4271).

	I would suggest to replace 'The error conditions for the
	NEXT_HOP attribute ...' with 'The attribute is considered
	malformed if it is syntactically incorrect according to RFC
	4271', if that is what was meant.

5.8. Community
5.9. Extended community
5.10. IPv6 Address Specific BGP Extended Community Attribute

	I am a bit confused by the formulation 'malformed if its length
	is nonzero and is not a multiple of 4/8/20'. I would understand
	that the first part is redundant (as zero length is also a
	multiple of 4/8/20 and is therefore OK), but in 5.12 Cluster
	list, the nonzeroness is not explicitly mentioned in the similar
	case. Is this variation significant? I would suppose it is not
	and both communities and cluster lists of zero length are
	valid. In that case, perhaps it would be a good idea to use the
	same wording for both communities and cluster lists.

5.11. ORIGINATOR_ID
5.12. CLUSTER_LIST

	Note that error handling of RFC 4456 also specifies the behavior 
	for 'semantically incorrect' CLUSTER_LIST containing local
	ORIGINATOR_ID (the condition caused by routing loop). This case
	is not mentioned in the discussed draft (perhaps intentionally
	as it is semantic and not syntactic error). Also note that RFC
	4456 specifies that such update should be ignored, although that
	behavior is obviously broken and perhaps should be replaced with
	treat-as-withdraw. The same also holds for received ORIGINATOR_ID
	attribute containing local value.

-- 
Ondrej 'Santiago' Zajicek

---

From: "Gaurav Dawra (gdawra)" <[email-masked]>
Date: Mon, 12 May 2014 14:39:43 +0000

+1

Regards,
-Gaurav

---

From: Jakob Heitz <[email-masked]>
Date: Mon, 12 May 2014 16:34:49 +0000

I support.

-----Original Message-----
From: Idr [mailto:[email-masked]] On Behalf Of Susan Hares
Sent: Wednesday, May 07, 2014 11:31 AM
To: 'John G. Scudder'; 'idr@ietf. org'
Subject: [Idr] 2 Week WG LC: draft-ietf-idr-error-handling-07.txt

John: 

Congratulations on publishing draft of draft-ietf-idr-error-handling-07.txt.
This begins a 2 week WG LC starting today (5/7/14), and ending 5/21/14.  In your comments please indicate, support/no support for the WG LC. 

Sue 

-----Original Message-----
From: Idr [mailto:[email-masked]] On Behalf Of John G. Scudder
Sent: Wednesday, May 07, 2014 2:17 PM
To: idr@ietf. org
Subject: Re: [Idr] I-D Action: draft-ietf-idr-error-handling-07.txt

Folks,

Compared to -06, this version lists a couple more NLRI (in)correctness criteria in S. 3.2 as discussed at the last IETF meeting, adds S. 3.3 on the handling of typed NLRI (such as for MVPN and EVPN), makes an editorial clarification regarding when a session must be reset, and removes Appendix A. The deletion of Appendix A was because it was there for purposes of explanation in the early versions of the draft, but at this point everyone seems to get the point and there doesn't seem to be any need to clutter up the eventual RFC with it. 

With this revision the authors think the document is ready for working group last call.

Thanks,

--John

directories.
IETF.
tools.ietf.org.

---

From: Robert Raszuk <[email-masked]>
Date: Mon, 12 May 2014 21:20:40 +0200

Support.

r.

On Mon, May 12, 2014 at 6:34 PM, Jakob Heitz <[email-masked]>wrote:

> This begins a 2 week WG LC starting today (5/7/14), and ending 5/21/14.
>  In your comments please indicate, support/no support for the WG LC.
>
> criteria in S. 3.2 as discussed at the last IETF meeting, adds S. 3.3 on
> the handling of typed NLRI (such as for MVPN and EVPN), makes an editorial

---

From: <[email-masked]>
Date: Tue, 13 May 2014 08:11:10 +0000

John, all,

In the current version of the draft, attribute flags that conflict with the attribute type code are handled with "treat-as-withdraw". This good IMO.

However, an exception is done for 2 attributes: MP_(UN)REACH_NLRI. For those 2 attributes, such flag error is handled with "Session reset". I don't recall much debate on this point so I'd like to initiate a discussion on this point.

On my side, I would rather also handle this with "treat-as-withdraw" for the following reasons:
- as a simplification to avoid an exception
- I'm not convinced that an error in the container of the attribute is a significant indicator that the value of the attribute has more chance to be wrong. i.e. why should we treat as erroneous a syntaxely correct attribute value? Especially since the consequence will be a session reset. AFAIK we don't have data to support that, for an attribute having a valid value field, there is more chance to have an undetectable error in the value field of the attribute when these 2 flags have an error.
- in the specific case of this 2 attributes, these 2 flags seems to have limited benefit. Indeed, their purpose is to define what the receiver should do if it does not understand the attribute. But here, the speaker knows (through BGP capability) that the receiver do understand that attribute.

So killing a BGP session for 1 or 2 erroneous bit which serves little purpose seems a bit strict to me and I would rather propose to remove that exception. More specifically remove "The attribute flags of the attribute are inconsistent with those specified in [RFC4760]" as an error conditions for MP_(UN)REACH_NLRI attributes.

Thanks,
Bruno

> From: Idr [mailto:[email-masked]] On Behalf Of
> [email-masked]
> Sent: Friday, May 09, 2014 4:19 PM
> To: Susan Hares; 'John G. Scudder'
> Cc: 'idr@ietf. org'
> Another reason, and IMO a significant one, is that when the error is
> detected, this erroneous transitive attribute has reached multiple routers, up
> to a significant number, network wide. Therefore this create many multiple
> simultaneous failure in the networks, with a large part of the network been
> affected. When centralized route reflectors are used (e.g. VPN networks), if
> the Route Reflector does not recognize the attribute/error, then a significant
> part of the IBGP sessions (hence the network) is affected.
> 
> A third reason, is that a receiver, when detecting an error, will tend to always
> believe that he is the one to be right and that the error is on the sender side;
> while this may not be the case. The bug or misinterpretation of the spec
> could equally be on the receiver or on the sender. Eventually, nobody will
> ever know (e.g. draft-hares-idr-update-attrib-low-bits-fix). So if the error is
> in fact on the receiver side, again we have multiple routers detecting a bug at
> the same time, just like with unknown transitive attributes. So let's be
> humble when we detect an error, and let's not consider that the speaker is
> to buggy to talk to and we'd be safer kicking it outside the network.
> 
> Editorial: IMHO, page 4 lacks structure which makes it harder to identify the
> different cases of error. I would propose to add a number (or sub
> paragraph). e.g.
> Editorial: "This document partially revises [...] provides guidelines for the
> authors of documents defining new attributes."
> 
> I would propose to group these guidelines in a specific paragraph. Indeed,
> such guidelines are expected to be referred to and read for each new IDR
> spec. (while currently, those guidelines are a bit spread in the document)
> ----
> § 3.3. Typed NLRI
> 
> Editorial/nit picking: §5.11/5.12 & 5.5 have chosen a different way of
> expressing the same thing. §5.5 groups per (type of error (malformed or
> illegitimate presence) , type of session) while §5.11/5.12 groups per (type of
> session, type or error). Personally I have a preference for the §5.11/5.12
> way, which is more concise.
> 
> ------
> What's next after WGLC ?
> > 5/21/14.  In your comments please indicate, support/no support for the
> WG LC.
> >
> > criteria in S. 3.2 as discussed at the last IETF meeting, adds S. 3.3
> > on the handling of typed NLRI (such as for MVPN and EVPN), makes an
> > editorial clarification regarding when a session must be reset, and
> > removes Appendix A. The deletion of Appendix A was because it was
> > there for purposes of explanation in the early versions of the draft,
> > but at this point everyone seems to get the point and there doesn't
> > seem to be any need to clutter up the eventual RFC with it.
> >
> __________________________________________________________
> __________________________________________________________
> _____
> 
> Ce message et ses pieces jointes peuvent contenir des informations
> confidentielles ou privilegiees et ne doivent donc pas etre diffuses, exploites
> ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez
> le signaler a l'expediteur et le detruire ainsi que les pieces jointes. Les
> messages electroniques etant susceptibles d'alteration, Orange decline toute
> responsabilite si ce message a ete altere, deforme ou falsifie. Merci.
> 
> This message and its attachments may contain confidential or privileged
> information that may be protected by law; they should not be distributed,
> used or copied without authorisation.

---

From: Jakob Heitz <[email-masked]>
Date: Tue, 13 May 2014 15:01:53 +0000

In my understanding, the reason to reset a session is if the NLRI cannot be parsed. An error in those bits does not seem to hinder the parsability of the NLRI any more than any other error in the update message. Therefore, I agree with Bruno.

--
Jakob Heitz.

> On May 13, 2014, at 1:11 AM, "[email-masked]" <[email-masked]> wrote:
> 
> John, all,
>

---

From: "Chris Hall" <[email-masked]>
Date: Tue, 13 May 2014 17:36:14 +0100

Bruno Decraene wrote (on Tue 13-May-2014 at 09:11 +0100):
....
> In the current version of the draft, attribute flags that conflict
> with the attribute type code are handled with "treat-as-withdraw".
> This good IMO.

Likewise.

> However, an exception is done for 2 attributes: MP_(UN)REACH_NLRI.
> For those 2 attributes, such flag error is handled with "Session
> reset". I don't recall much debate on this point so I'd like to
> initiate a discussion on this point.

There is another exception: a second MP_REACH_NLRI or a second
MP_UNREACH_NLRI are cause for session-reset, but the receiver MUST
attribute-discard the second or subsequent instances of all other
attribute types.

The draft states that for treat-as-withdraw to be applicable, the NLRI
"need to be successfully parsed".  Consistent with that, the rules for
NLRI require them to be "perfectly-formed".  This high threshold seems
reasonable: the NLRI are required for treat-as-withdraw to work, so
errors in the NLRI are not the same as errors in other attributes.

Though if the receiver sees two or more perfectly-formed
MP_REACH_NLRI, it could treat-as-withdraw without much difficulty ?
But, perhaps there are some lapses which cannot be forgiven.

If the MP_xx_NLRI are always the first attributes, and are
perfectly-formed, then any error "downstream" can be
treated-as-withdraw with a high degree of confidence that *all* (my
emphasis) NLRI have been "successfully parsed".

If only that was all there is to it.

[In passing: if there is an MP_REACH_NLRI and/or NLRI, the first
attribute after any leading MP_xx_NLRI ought to be the ORIGIN
attribute.  If so, and if it is also perfectly-formed, we may be
(even) more confident that the MP_xx_NLRI are, indeed,
perfectly-formed -- i.e. the agreement between the attribute length
and the sum of the prefix lengths is (even) less likely to be just a
happy coincidence.]

---------------------------------

So far, so good... to qualify for treat-as-withdraw, it is clear that
all NLRI which are found must be perfectly-formed.  BUT, it is less
than clear how to find all the NLRI, and even less clear how to know
that all the NLRI have been found...

...where an MP_xx_NLRI is not found, we may conclude (a) that no such
attribute was sent, or (b) that one might have been sent, but it is
buried under the error(s) we are trying to handle -- "lost-NLRI" --
depending (inter alia) on how badly mangled the rest of the attributes
are.  

The sender may intend to send MP_xx_NLRI first -- as recommended by
the draft -- but the receiver cannot know that.  In any case, a sender
may fail to live up to its intentions.  Hence, the receiver "MUST be
still be prepared to receive these fields in any position".  I assume
being "prepared" includes applying all the new error handling ?

Suppose the receiver sees a LOCAL_PREF attribute of length 2000, which
for iBGP SHALL be treated-as-withdraw (see 3.1 and 5.5).  If the
sender is following the old rule of sending attributes in ascending
type number order, then there may be lost-NLRI -- unless, of course,
the sender has simply "had a moment" and really sent 2000 octets of
LOCAL_PREF.

The draft does not, as far as I can see, address the risk of
lost-NLRI.

Is it the draft's intention to leave this as (an interesting) problem
for the implementer ?

There are any number of heuristics that may be applied to this
problem.  It would be helpful if, in practice, the vast majority of
implementations only ever send one set of NLRI per message.  It helps
a bit if, in practice, MP_xx_REACH attributes only appear in messages
where both the Withdraw Routes and NLRI parts are empty.  Does anybody
know if either of these is the case ?

[FWIW: if the sender sent an MP_REACH_NLRI as the first attribute, and
later a 2000 octet LOCAL_PREF, it is assumed it didn't send further
MP_REACH_NLRI (now lost) which would otherwise be cause for
session-reset.  You might say that sending more than one MP_REACH_NLRI
would be an act of complete madness and may be discounted, while
sending a 2000 octet LOCAL_PREF is merely eccentric... I couldn't
possibly comment.]

Chris
---------------------------------
PS: I note that if the receiver sees a LOCAL_PREF attribute of length
500 in an eBGP session, then it SHALL be treated as attribute-discard.
If the attributes exactly fit in the Total Attribute Length, then
prima facie all their lengths are correct -- looks like a sort of
checksum.  In which case, the draft deems a 500 octet LOCAL_PREF to be
an outsize attribute, even though LOCAL_PREF should not be there in
the first place and quacks (loudly) like an error.  If nothing else
triggers treat-as-withdraw (or session-reset) then the receiver SHALL
proceed to use the routes... complete nonsense notwithstanding; even
though a 500 octet LOCAL_PREF attribute for iBGP would be
treat-as-withdraw.  Really ?

---

From: "John G. Scudder" <[email-masked]>
Date: Tue, 13 May 2014 09:53:29 -0700

Chris,

To address one point from your long and interesting message:

> know if either of these is the case ?

I (strongly) believe these to be common practice. I'm confident of it for the code base I've currently got access to, but can't speak authoritatively for others of course. Maybe other folks will chime in. 

There is nothing to stop us from explicitly mandating this practice in the future. However for some indefinite time to come we'll still need to be prepared to deal with the deprecated behavior, so I'm not sure it makes any practical difference for the error-handling draft.

It's all a bit of a tightwire act, as your examples demonstrate.

--John

---

From: Jeffrey Haas <[email-masked]>
Date: Tue, 13 May 2014 14:48:37 -0400

> Thanks for the clarification. I support moving forward the current draft to RFC. I was just checking whether the authors were ok with continuing the work if needed

The WG wiki also provides a good place for documenting "best practices" on
such things.

-- Jeff

---

From: Jeffrey Haas <[email-masked]>
Date: Tue, 13 May 2014 16:05:49 -0400

Ondrej,

> 	AS_CONFED_* from RFC 5065), but 'no' can be also viewed as

A few issues are identified above:
The error-handling draft doesn't refer to to RFC 5065.  If you pedantically
go by the text above, only non-confederation is supported.

Syntactic checks are probably what we want here.

We should have a section for semantic checks on AS_PATH.  Suggested text:

   The error conditions for the attribute have been defined in
   [RFC4271], and have been extended by [RFC5065].

   An UPDATE message with a malformed AS_PATH attribute SHALL be handled
   using the approach of "treat-as-withdraw

   An UPDATE message containing a syntactically valid AS_PATH that is
   semantically invalid (i.e. an unrecognized path segment type) SHALL be
   handled using the approach of "treat-as-withdraw".

It's my reading that RFC 5065, sec. 5 is covered in the error handling
document by referring back to malformed behavior.  (Assuming you presume the
error handling document is a semantic override of RFC 4271.  Otherwise we
need explicit text overriding that behavior.  I leave that interpretation to
the more pedantic of the working group.)

> 	generative than descriptive).

This would be hazardous.  Forgiving in what you receive, strict in what you
send and all that.  Plus, there was an odd idea circulating IDR a while ago
to try to pad AS_SET length in route selection by generating tiny segments a
while ago.  (Probably not going to work.)

> 	4271', if that is what was meant.

MP_NEXTHOP becomes trickier with features that play with the nexthop length.
I believe those are currently covered by capability.

Semantic incorrectness should probably result in treat-as-withdraw, IMO.  We
can parse the packet, it just looks crazy.

-- Jeff

---

From: Jeffrey Haas <[email-masked]>
Date: Tue, 13 May 2014 16:07:21 -0400

> your comments please indicate, support/no support for the WG LC. 

I support that the document is generally ready, although Ondrej has pointed
out some details that probably should be resolved before WGLC completes.

Juniper implements a form of this draft, although as John points out the
most recent changes will require some minor work.

-- Jeff

---

From: "John G. Scudder" <[email-masked]>
Date: Tue, 13 May 2014 16:22:34 -0700

Chris,

> The draft does not, as far as I can see, address the risk of

The draft doesn't provide a perfect solution to every arbitrary scenario where NLRI might be lost while still producing a syntactically well-formed UPDATE.

The draft does try to strike a balance between prudence and avoidance of ever-mounting complexity of error-detection heuristics to attempt to detect ever less-likely problems. 

> Is it the draft's intention to leave this as (an interesting) problem

I guess if the implementor's paranoia is significantly greater than that of the authors of the draft (and the WG members who assent to it, assuming the WGLC concludes successfully), then you could say that. 

We already mandate in the draft that the MP attributes MUST come first. We could also mandate that the MP attributes MUST NOT be mixed with the legacy encoding. Implementations would still have to be prepared to receive updates with mixed MP and legacy NLRI of course, i.e. it wouldn't be a NOTIFY-able offense, but by deprecating the behavior it may help us some years down the road.

Thanks,

--John

---

From: "Chris Hall" <[email-masked]>
Date: Wed, 14 May 2014 00:49:03 +0100

John G. Scudder wrote (on Tue 13-May-2014 at 17:53 +0100):
> On May 13, 2014, at 9:36 AM, Chris Hall wrote:
> 
> > problem.  It would be helpful if, in practice, the vast majority
> > of implementations only ever send one set of NLRI per message.
> > It helps a bit if, in practice, MP_xx_REACH attributes only
> > appear in messages where both the Withdraw Routes and NLRI
> > parts are empty.  Does anybody know if either of these is the
> > case ?

> I (strongly) believe these to be common practice. I'm confident of
> it for the code base I've currently got access to, but can't speak
> authoritatively for others of course. Maybe other folks will chime
> in.
> 
> There is nothing to stop us from explicitly mandating this practice
> in the future. However for some indefinite time to come we'll still
> need to be prepared to deal with the deprecated behavior, so I'm not
> sure it makes any practical difference for the error-handling draft.

I agree that the draft should specify not only (1) that MP_xx_NLRI
MUST be the first attribute, but also (2) that where MP_xx_NLRI is
present it MUST be the only NLRI and (3) that MP_REACH_NLRI and
MP_UNREACH_NLRI MUST NOT appear together.

If the idr WG can testify to (2) and (3) being existing common
practice, then the draft can observe that these nominally new
requirement are already met by most/nearly all/virtually all BGP
deployments.

The joy of this is that the simplest possible approach to finding the
NLRI is already sufficient and cannot be improved upon in most/nearly
all/virtually all cases.

The practical difference is that even with a badly broken message, the
risk of doing an "incomplete" treat-as-withdraw is
limited/small/negligible.

Chris