I believe this got overlooked, since I don't see any effect on the text 


in the -18 version of the document.




Tom Taylor


-------- Original Message --------
Subject: Early review of draft-ietf-idr-error-handling-15
Date: Sun, 26 Oct 2014 20:56:45 -0400
From: Tom Taylor <tom.taylor.stds at gmail.com>


To: Gen Art <gen-art at ietf.org>, 


draft-ietf-idr-error-handling.all at tools.ietf.org.




I am the assigned Gen-ART reviewer for this draft. For background on
Gen-ART, please see the FAQ at

<

http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.

Please resolve these comments along with any other comments
you may receive.

Document: draft-ietf-idr-error-handling-15
Reviewer: Tom Taylor
Review Date: 26/10/2014
IETF LC End Date:
IESG Telechat date: (if known)

Summary: This draft has a number of editorial issues but otherwise looks
good to go. (I am no BGP expert and did not check on the correctness of
the references to the individual attributes.).

Major issues:

Minor issues:

Nits/editorial comments:

1) IdNits has a number of complaints, but the only real one is a m
question regarding the choice of copyright boilerplate.

In the copyright matter: are you using the pre-2009 Trust template
because of the quotations from the older documents that are being
amended? I suppose that is justified, but others on the Gen-Art list may
have an opinion.

2) Spell out Address Family Identifier / Subsequent Address Family
Identifier (AFI/SAFI) on first occurrence. See



http://www.rfc-editor.org/rfc-style-guide/abbrev.expansion.txt



Asterisked items there (like BGP) are well-known abbreviations and do
not have to be spelled out. All others do on first occurrence.

3) For the new text proposed in section 3 a., would you consider a
slight reordering as follows? It highlights the condition (session
reset) more clearly, I'd suggest.

  New Text:

       An error for which a session reset is specified that is
       detected while processing the UPDATE message MUST ...

4) Grammatical and reference problem with the opening part of the new
text proposed in section 3.c. Suggested text:

  New Text:

       If the value of either the Optional or the Transitive bit in
       the Attribute Flags is in conflict with its value as given
       in the attribute specification ...

5) For section 3.e, do you mean:

   e.  "Treat-as-withdraw" MUST be used instead of session reset
       for the cases where [RFC4271] Section 6.3 specifies a
       session reset and the detected error is found in any of
       the attributes ORIGIN, AS_PATH, NEXT_HOP, MULTI_EXIT_DISC,
       or LOCAL_PREF.

6) Similar change for section 3.f:

   f.  "Attribute discard" MUST be used instead of session reset
       for the cases where [RFC4271] Section 6.3 specifies a
       session reset and the detected error is found in either
       of the attributes ATOMIC_AGGREGATE or AGGREGATOR.

7) For section 3.h.:

    s/for the handling of these malformed attributes/
      for the handling of all of these malformed attributes/
                          ^^^^^^

8) In section 3.i., I think you have to spell out Network Layer
Reachability Information (NLRI).

9) In section 5.1, second bullet of the restrictions is ambiguous. Do
you mean:

  o    An UPDATE message MUST NOT contain more than *any* one
       of the following:

or do you mean

   o  An UPDATE message MUST NOT contain more than one of the following:
                            ... MP_REACH_NLRI attribute, *or*
      MP_UNREACH_NLRI attribute.

10) Section 5.3, first paragraph:
   s/following are true/following is true/