Subject: [RTG-DIR] Rtgdir telechat review of draft-ietf-idr-rfc5575bis-20

From: Ines Robles via Datatracker <[email-masked]>
Date: Wed, 15 Apr 2020 09:36:05 -0700

Reviewer: Ines Robles
Review result: Has Nits

Review type: rtgdir - Telechat review
Requested version for review: 20
Deadline: 2020-04-15
Reviewer: Ines Robles

Summary:

This document defines a Border Gateway Protocol Network Layer Reachability
Information (BGP NLRI) encoding format that can be used to distribute traffic
Flow Specifications. It also specifies BGP Extended Community encoding formats.
This document obsoletes both RFC5575 and RFC7674.

 I found the document clear and complete. Some minor nits are detailed below.

  Major issues: Not found

  Minor issues: Not found

  Nits:

1- In the Introduction, it would be nice to mention how this draft obsoletes
RFC7674 and point to appendix B when referring obsoleting RFC5575.

For example, This document obsoletes "Dissemination of Flow Specification
Rules" [RFC5575], whose differences can be found in Appendix B. This document
obsoletes also "Clarification of the Flowspec Redirect Extended
Community"[RFC7674] due to this document includes.....

- please expand iBGP --> Internal BGP  (iBGP)

2- Section 4.2.1.1

in "a -  AND bit:..."
2.1 - "the previous term" refers to a component?
2.2 -nit: "...unset and and MUST..." => "...unset and MUST..."

3-nit Section 5: "...this draft specifies..." if it becomes RFC it would be
nice to read "..this document specifies.."

Thanks for this document,

Ines.

---

From: Christoph Loibl <[email-masked]>
Date: Fri, 17 Apr 2020 09:49:32 +0200

Hi Ines,

Thanks for your review. According to your review I made the following changes to the document which is available now as revision -22:

> On 15.04.2020, at 18:36, Ines Robles via Datatracker <[email-masked]> wrote:
> 
> Reviewer: Ines Robles
> Review result: Has Nits
> 
...

> - please expand iBGP --> Internal BGP  (iBGP)

<-- 
Tracked via issue #165: https://github.com/stoffi92/rfc5575bis/issues/165
Commit mention: https://github.com/stoffi92/rfc5575bis/commit/e8e779a38e35bd53111bf3787b5016503431a173

Introduction modified as suggested:
```
   This document obsoletes "Dissemination of Flow Specification Rules"
   [RFC5575], the differences can be found in Appendix B.  This document
   also obsoletes
   "Clarification of the Flowspec Redirect Extended Community" [RFC7674]
   since it incorporates the encoding of the BGP Flow Specification
   Redirect Extended Community in Section 7.4.
```

iBGP - changed as suggested.

-->

> 2.2 -nit: "...unset and and MUST..." => "...unset and MUST..."

<-- 
Tracked via issue #166: https://github.com/stoffi92/rfc5575bis/issues/166
Commit mention: https://github.com/stoffi92/rfc5575bis/commit/5a5b90085b5375828e8b758e6ee1a11bc2a59994

ad 2.1.: Actually it is the result of the previous {op, value} pair (within a component). Usually components (defined in the next section) are a list of muliple {op, value} pairs. The new changed text makes it a little clearer:

```
   a -  AND bit: If unset, the result of the previous {op, value} pair
      is logically ORed with the current one.  If set, the operation is
      a logical AND.  
```
-->

> nice to read "..this document specifies.."

<-- 
Tracked via issue #167: https://github.com/stoffi92/rfc5575bis/issues/167
Commit mention: https://github.com/stoffi92/rfc5575bis/commit/6762310b2f3035b12a82e3bf15f778a15ad969fa

Solved as suggested
-->

Cheers

Christoph

-- 
Christoph Loibl
[email-masked] | CL8-RIPE | PGP-Key-ID: 0x4B2C0055 | http://www.nextlayer.at

---

From: Ines Robles <[email-masked]>
Date: Fri, 17 Apr 2020 11:07:15 +0300

Thank you very much Christoph for addressing my comments.

Have a nice day,

Ines.

On Fri, Apr 17, 2020 at 10:49 AM Christoph Loibl <[email-masked]> wrote:

> Hi Ines,
>
> Thanks for your review. According to your review I made the following
> changes to the document which is available now as revision -22:
>
>
> Commit mention:
> https://github.com/stoffi92/rfc5575bis/commit/e8e779a38e35bd53111bf3787b5016503431a173
>
> Commit mention:
> https://github.com/stoffi92/rfc5575bis/commit/5a5b90085b5375828e8b758e6ee1a11bc2a59994
>
> ad 2.1.: Actually it is the result of the previous {op, value} pair
> (within a component). Usually components (defined in the next section) are
> a list of muliple {op, value} pairs. The new changed text makes it a little
> clearer:
>
> ```
> Commit mention:
> https://github.com/stoffi92/rfc5575bis/commit/6762310b2f3035b12a82e3bf15f778a15ad969fa
>
> Solved as suggested
> -->
>
> Cheers
>
> Christoph
>
> --
> Christoph Loibl