Subject: Re: [Idr] draft-ietf-idr-error-handling-10.txt

From: "John G. Scudder" <[email-masked]>
Date: Tue, 3 Jun 2014 15:55:54 -0400

On May 30, 2014, at 11:06 AM, Deniz Bahadir <[email-masked]> wrote:

> I appreciate the ambiguity about attribute flags being addressed. Just one question: Shouldn't a well-known attribute also be considered to have conflicting flags if the partial bit is set?

The point makes sense. On the other hand, given that the motivation for this document was to reduce operational pain caused by picky BGP implementations, it is not at all obvious that it makes sense to introduce a new error condition. Existing implementations I’m aware of always treated the “must be consistent” rule from RFC 4271 as meaning “optional and transitive must be set the way they were specced” so we just imported that standard practice into the error-handling draft without trying to expand the scope.

I’ll address your other points separately.

—John

---

From: "John G. Scudder" <[email-masked]>
Date: Thu, 5 Jun 2014 13:53:13 -0400

On May 30, 2014, at 11:06 AM, Deniz Bahadir <[email-masked]> wrote:

> 1.
> It occurs to me that the title of section 7 ("Error Handling Procedures for Existing Attributes") implies to address *all* existing attributes. The first sentence, however, seems to mitigate that by saying the following subsections "elaborate on the conditions for error-checking *various* path attributes".
> Nevertheless, for sake of completeness, I would recommend to mention all (non-deprecated) path attributes (as can be found here:http://www.iana.org/assignments/bgp-parameters/bgp-parameters.xhtml) in the following subsections.

Fair point. We originally didn’t set out to do this for every possible attribute, but given we’re most of the way there it may be that we should try to finish the job. That said, by my reading (see below for details) only a few need to be covered:

- 24	Traffic Engineering
- 25	IPv6 Address Specific Extended Community
- 28	BGP Entropy Label Capability Attribute
- 128	ATTR_SET

Barring objections, I will look at adding them to the next version of the draft.

The rest look OK to me. I guess to remove any doubt in the mind of the reader we could put in a section that says “for attributes 17, 18, 22, 23, 26, 27 and 29, error handling is defined in the relevant specification. Attributes 11, 12, 13, 19, 20 and 21 are deprecated.” Then again, that’s just the equivalent of “this page intentionally left blank”. I guess I’ll add something like that unless there are objections.

—John

My take on all the non-deprecated attributes below. This is based on a pretty quick skim; further comments are welcome.

17	AS4_PATH	[RFC6793]
18	AS4_AGGREGATOR	[RFC6793]

RFC 6793 already has an error handling section that’s — by design — consistent with what’s required by error-handling.

22	PMSI_TUNNEL	[RFC6514]

Same for this one — section 5 looks good.

23	Tunnel Encapsulation Attribute	[RFC5512]

The “error handling” section is a bit ambiguous but essentially follows the same model as “treat-as-withdraw". I’d be OK with leaving it alone.

24	Traffic Engineering	[RFC5543]

This does need work.

25	IPv6 Address Specific Extended Community	[RFC5701]

Ditto.

26	AIGP	[RFC-ietf-idr-aigp-18]

Like RFC 6793, the AIGP spec has reasonable error handling section already and explicitly references error-handling.

27	PE Distinguisher Labels	[RFC6514]

This also has a section (S. 5) that follows the draft already.

28	BGP Entropy Label Capability Attribute	[RFC6790]

Looks like this needs some work. 

29	BGP-LS Attribute (TEMPORARY - expires 2015-03-11)	[draft-ietf-idr-ls-distribution]

This is still in draft; I think the draft should be updated to cover the necessary. I’ll make sure the authors know to do so.

128	ATTR_SET	[RFC6368]

The error handling section here references the old draft-ietf-idr-optional-transitive, and should be updated to say that the error handling should be treat-as-withdraw always and not only if the partial bit is set.

---

From: Deniz Bahadir <[email-masked]>
Date: Fri, 06 Jun 2014 09:41:56 +0200

> Barring objections, I will look at adding them to the next version of the draft.

That is fine with me.

> The rest look OK to me. I guess to remove any doubt in the mind of the reader we could put in a section that says “for attributes 17, 18, 22, 23, 26, 27 and 29, error handling is defined in the rel…

I would pretty much appreciate such an additional sentence, to let the 
reader know that this document did not leave out any (currently 
existing) attribute by accident.

> RFC 6793 already has an error handling section that’s — by design — consistent with what’s required by error-handling.

OK, with the additional sentence from the former comment I would retract 
my original comment 2 and am fine with not giving AS4_PATH and 
AS4_AGGREGATOR a subsection for their own. :-)

Likewise for the other attributes mentioned in that sentence.

Deniz

---

From: John G.Scudder <[email-masked]>
Date: Wed, 11 Jun 2014 19:06:53 -0400

On May 30, 2014, at 11:06 AM, Deniz Bahadir <[email-masked]> wrote:

> Hi guys,
> 
> I have some more (minor) comments (or one might call it nit-picking) concerning the current draft:
> 
> 1.
> Nevertheless, for sake of completeness, I would recommend to mention all (non-deprecated) path attributes (as can be found here: http://www.iana.org/assignments/bgp-parameters/bgp-parameters.xhtml) in the following subsections.

Covered in an earlier message.

> 2.
> To be honest, I do not know all these path-attributes myself. However, I know AS4_PATH and AS4_AGGREGATOR, and would at least recommend to add them to the subsections of section 7. Especially, as the information given in the subsections for AS_PATH and AS_AGGREGATOR pretty much resembles the information for AS4_PATH and AS4_AGGREGATOR in section 6 of RFC 6793 (except maybe for the action taken).

I covered this in my reply to #1 — IMO RFC 6793 is sufficient. 

> 3.
> I would also recommend to repeat (and explain) the definition of "syntactical incorrectness" for NEXT_HOP in subsection 7.3. (This seems to be the only subsection which just refers to another RFC and does not repeat the important part.)

> 
> Some more nit-picking:
> 
> 4.
> Likewise, I was always wondering, what exactly is meant by saying, "Syntactic correctness means that the NEXT_HOP attribute represents a valid IP host address." [RFC 4271, page 32]?
> * Does it mean, the NEXT_HOP value must be a machine that should be up and running as well as reachable (and one must explicitly check for that)? -- Probably not.
> * Does it mean, the NEXT_HOP must be a unicast address (neither 0.0.0.0 nor greater than 223.255.255.255)? -- Maybe.
> * Does it mean, the NEXT_HOP value must be a unicast address but not 127.0.0.1 or some other special IP? -- Possibly.
> * …
> 

> 5.
> Similar, I briefly looked through RFC 4760 but could not find a section telling me, if and when the value of the Next Hop field of MP_REACH_NLRI is considered (syntactically) incorrect. (At least for AFI/SAFI of IPv4/Unicast I would have expected it.)
> Is suppose, the current draft should address this question.
> (Or it shouls at least mention that MP_REACH_NLRI's Next Hop value need not be checked for (syntactical) correctness... although this does not sound right.)

See -12 for an attempt at addressing the three foregoing points. 

> 6.

Covered in an earlier message.

—John

> 
> 
> Thanks,
> Deniz
> 
> 
> Am 30.05.2014 15:03, schrieb John G. Scudder:
>> As reading the rfcdiff will tell you :-) this version:
>> 
>> - Calls out AFI/SAFI disable distinctly.
>> - Corrects the ambiguity about attribute flags that conflict.
>> - Removes some redundancy in 3(j).
>> - Reworks the “Parsing of NLRI” section quite a bit, including:
>>   - Extract the attribute length stuff to its own section, it didn’t make sense where it was.
>>   - Puts back the “MP_ must go first” rule.
>>   - Adds a subsection on “Missing NLRI”.
>> 
>> Thanks for all the discussion.
>> 
>> Regards,
>> 
>> —John
>> 
>> On May 29, 2014, at 7:49 PM, <[email-masked]> <[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
>>>                          John G. Scudder
>>>                          Pradosh Mohapatra
>>>                          Keyur Patel
>>> 	Filename        : draft-ietf-idr-error-handling-10.txt
>>> 	Pages           : 15
>>> 	Date            : 2014-05-29
>>> 
>>> 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-10
>>> 
>>> A diff from the previous version is available at:
>>> http://www.ietf.org/rfcdiff?url2=draft-ietf-idr-error-handling-10
>>> 
>>> 
>>> 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
>> 
> 
> _______________________________________________
> Idr mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/idr