Subject: Last Call re-review of draft-ietf-idr-error-handling

From: Tom Taylor <[email-masked]>
Date: Fri, 06 Mar 2015 11:19:31 -0500

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 <[email-masked]>
To: Gen Art <[email-masked]>, 
[email-masked].

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/