Re: [Idr] AD Review of draft-ietf-idr-rfc5575bis-17 -> Updated Version -18 and Flowspec v6

[Christoph Loibl <[e-mail]>]

Hi Alvaro and IDR,

> On 12.12.2019, at 20:38, Alvaro Retana <[e-mail]> wrote:

[…]

Thank you very much for your review. I promised to followup in January. As I did this with the previous review, on behalf of the Authors I opened Github issues for all the comments and worked thru them so that changes (if any) can be tracked easier. The repository can be found here (issues #140 and following):

	https://github.com/stoffi92/rfc5575bis

The resulting document can be found on datatracker (-19):

	https://datatracker.ietf.org/doc/draft-ietf-idr-rfc5575bis/


I kindly ask you and IDR to review this new version. The changes from -18 to -19 are rather small. This time I recommend to use rfcdiff for a quick overview. However, I still added comments to all the major issues that you pointed out on Github and in this Email below.

> 
> There is only one significant item remaining that we should resolve
> before moving the document forward: the handling of unknown
> components; take a look at the comments in §11 (of -18).
> 
> Thanks!

This issue has been discussed before Christmas on the mailinglist. The main points in that discussion were that we do not want to propagate any garbage, while allowing a certain degree of extensibility of the NLRI (given the zoo of FS drafts and expected AFI/SAFI pollution out there this seems to be reasonable - extension Authors may still choose how they extend the NLRI this is not specified in this draft) and keep backwards compatibility.

In this context Alvaro pointed out that the behaviour explained in the draft fits closely RFC7606 Section 5.4 (Typed NLRI) and Jeffrey explained that walking NLRIs may lead to synchronisation issues between a buggy FS sender and a receiver. 

The document that we now uploaded clearly references RFC7606 Typed NLRI as the mechanism to treat unknown FS components and to allow the receiver to skip over (and discard) a single NLRI if it hits an unknown component type (previously the very same behaviour was explained in the draft without referencing RFC7606). We also added a paragraph to the security considerations section to explain that RFC7606 Section 5.4 allows to skip (and discard) *unknown* octets and carefully crafted/misbehaving BGP speakers may exploit that fact.

The remaining Email contains the responses and links to the -17 "review-review" and the -18 review. 

Please review the document. I hope that we can proceed with this document. 

Cheers

Christoph

(I just  noticed that something/one (me?) messed up the IANA Considerations tables (some columns look to be centred or right-aligned). Nothing to worry, I will fix this soon.)


********** THE SECTION BELOW RELATES TO THE -17 REVIEW *************

> ...
> 227 2. Definitions of Terms Used in This Memo
> ...
> 233 Loc-RIB - Local RIB.
> 
> [major] This simple definition doesn't match the one in §1.1/rfc4271.
> 
> <-- *** Authors ***
> Tracked via issue #11: https://github.com/stoffi92/rfc5575bis/issues/11
> Commit mention: https://github.com/stoffi92/rfc5575bis/commit/b4950ef76c347a67a8e31e0eff4d1433a00e34b8
> 
> Added the definition to to match RFC4271
> -->
> 
> [minor] Please add a reference to rfc4271 there too.
> 

<-- AUTHORS
Tracked via issue #131: https://github.com/stoffi92/rfc5575bis/issues/131
Commit mention: https://github.com/stoffi92/rfc5575bis/commit/98e2f95f13a39a1fd434be2fe5d70fd5d5e4c28b
-->

> 
> ...
> 281 4. Dissemination of IPv4 FLow Specification Information
> ...
> 287 This NLRI information is encoded using MP_REACH_NLRI and
> 288 MP_UNREACH_NLRI attributes as defined in [RFC4760]. Whenever the
> 289 corresponding application does not require Next-Hop information, this
> 290 shall be encoded as a 0-octet length Next Hop in the MP_REACH_NLRI
> 291 attribute and ignored on receipt.
> 

[..]

> [major] This is the new text from -18:
> 
> 255     This NLRI information is encoded using MP_REACH_NLRI and
> 256     MP_UNREACH_NLRI attributes as defined in [RFC4760].  Whenever the
> 257     corresponding application does not require Next Hop information, this
> 258     shall be encoded as a 0-octet length Next Hop in the MP_REACH_NLRI
> 259     attribute (if a non 0-octet Next Hop is present it should be ignored
> 260     on receipt).
> 
> I think I get it: the Next Hop is not needed -- so it is either not
> present (0-octet length) or ignored.  Is that it?
> 
> What is confusing me is the part about "Whenever the corresponding
> application does not require Next Hop information", because it sounds
> as if there are cases when the Next Hop is needed...but then it is
> ignored anyway.
> 
> I looked in rfc4760/rfc4271, but they both talk about forwarding to
> the Next Hop, and not about cases where it is not needed (or at least
> I didn't find that).  IOW, the use defined here is specific to the FS
> AFI/SAFI pair.  The specification of the use (or not) or the Next Hop
> should then be a lot clearer, and Normative.
> 
> SUGGESTION>
> This NLRI information is encoded using MP_REACH_NLRI and MP_UNREACH_NLRI
> attributes as defined in [RFC4760].  When advertising Flow Specifications,
> the Length of Next Hop Network Address SHOULD be set to 0.  The Network
> Address of Next Hop field MUST be ignored.
> 

<-- AUTHORS
Tracked via issue #132: https://github.com/stoffi92/rfc5575bis/issues/132
Commit mention: https://github.com/stoffi92/rfc5575bis/commit/3fd408f1fd6ac04c4a4cb3e39521e7efc335fe5c
-->

> 
> ...
> 325 4.2. NLRI Value Encoding
> ...
> 338 Flow Specification components must follow strict type ordering by
> 339 increasing numerical order. A given component type may (exactly
> 340 once) or may not be present in the specification. If present, it
> 341 MUST precede any component of higher numeric type value.
> 
> [major] What should happen if a component appears more than once?
> 
> [major] What should happen if the order is not maintained?
> 
> <-- *** Authors ***
> Tracked via issue #19: https://github.com/stoffi92/rfc5575bis/issues/19
> 
> Basically if the NLRI is not encoded to specification we have a malformed
> NLRI (most probably the BGP NLRI parser will throw an exception on the
> implementation). I am not sure if this needs to be made explicit. - But I can
> make this explicit.
> -->
> 
> The question is: what should the implementation do in response to the
> exception?  Should it reset the session?  Should it ignore the Update?
> Should it disable the AFI/SAFI?  Should it only consider the first
> component (if it appears more than once)?  Something else?
> 
> I looked at rfc4760/rfc7606, but didn't find anything that looked to
> me explicitly as corresponding to a Malformed NLRI...but rfc4760 does
> specify "AFI/SAFI disable":
> 
> If a BGP speaker receives from a neighbor an UPDATE message that
> contains the MP_REACH_NLRI or MP_UNREACH_NLRI attribute, and if the
> speaker determines that the attribute is incorrect, the speaker MUST
> delete all the BGP routes received from that neighbor whose AFI/SAFI
> is the same as the one carried in the incorrect MP_REACH_NLRI or
> MP_UNREACH_NLRI attribute. For the duration of the BGP session over
> which the UPDATE message was received, the speaker then SHOULD ignore
> all the subsequent routes with that AFI/SAFI received over that
> session.
> 
> The result is then that FS (including the rules/actions already
> received) would now not be used.  Personal opinion: Disabling the
> AFI/SAFI can be worse than resetting the session because there's no
> indication to the sender that anything is wrong.
> 
> If AFI/SAFI disable is the expected behavior for both out-of-order and
> duplicate components, then nothing else is needed.  A note in §10
> would be nice but not necessary.
> 

<-- AUTHORS
Tracked via issue #133: https://github.com/stoffi92/rfc5575bis/issues/133
Commit mention: https://github.com/stoffi92/rfc5575bis/commit/b268bc69b1afd18fb471cf3bb1ae23216624faae
Related issue mentioned: https://github.com/stoffi92/rfc5575bis/issues/155

If the FS NLRI is not encoded according to specification it is considered Malformed and should be treated according to rfc7606 (malformed in rfc7606 still bounces the session as Alvaro pointed out).

See also the above comment on rfc7606 based typed-nlri.  
-->

> 
> ...
> 542 4.2.12. Type 12 - Fragment
> ...
> 548 0 1 2 3 4 5 6 7
> 549 +---+---+---+---+---+---+---+---+
> 550 | 0 | 0 | 0 | 0 |LF |FF |IsF|DF |
> 551 +---+---+---+---+---+---+---+---+
> ...
> [major] The operation is not specified. Is this also an (operator,bitmask)
> pair, or just 8 bits indicating the values? Can multiple bits be set at the
> same time? What fields in the IP header do these map to?
> 
> <-- *** Authors ***
> Tracked via issue #40: https://github.com/stoffi92/rfc5575bis/issues/40
> Commit mention: https://github.com/stoffi92/rfc5575bis/commit/f078e919842330d670bc195276ead7bdc05a3351
> 
> Added the text to the bit-descriptions and IP-header fields - And an example
> using the bitmask operator.
> -->
> 
> The example is for a "matching packet with DF bit set or First
> Fragments", which answers the question above about whether multiple
> bits can be set -- yes.  What is still not specified is that in the
> case of multiple bits set, an OR operation is expected.
> 

<-- AUTHORS
Tracked via issue #134: https://github.com/stoffi92/rfc5575bis/issues/134

The OR operation that is expected is defined in the bitmask-operator the m-bit.

If set, this is a bitwise match operation
defined as "(data AND value) == value"; if unset, (data AND value) evaluates
to TRUE if any of the bits in the value mask are set in the data

As Jeffrey already pointed out, there are certain combinations that make no sense and will never match a packet. Especially if you have the m-bit set in the operator and some combinations of the "LF FF IsF DF" bits set. For example having all of them set you would require a packet to have the following flag/offset combinations:
(DF=1) AND (Fragment-offset != 0) AND (Fragment-offset == 0 and MF==1) AND (Fragment-offset != 0 and MF==0)
-->

> 
> ...
> 742 6. Validation Procedure
> ...
> 760 A Flow Specification NLRI must be validated such that it is
> 761 considered feasible if and only if all of the below is true:
> 
> [major] There is no Normative language above, but I think there is a
> contradiction of sorts with the new text below ("Rule a) MAY be relaxed...").
> The introductory text to the rules is "must be...considered feasible if and
> only if all of the below is true", which sounds very strict and
> specific...but then the Normative exception comes in ("MAY be relaxed...rules
> b) and c)...MUST be disregarded") saying that it doesn't matter. Please
> reword...perhaps something like: "If a destination is present...a Flow
> Specification MUST be validated this way...otherwise..."
> 
> <-- *** Authors ***
> Tracked via issue #56: https://github.com/stoffi92/rfc5575bis/issues/56
> Commit mention: https://github.com/stoffi92/rfc5575bis/commit/d96a496295021f2aab0332bd2120920547f71c56
> 
> Changed the text to refer to the default behaviour of the implementation
> (MUST validate). However this MAY be relaxed by configuration.
> -->
> 
> I'm still not happy with the contradiction: MUST means "an absolute
> requirement" (no exceptions!), but then we say MAY, which makes the
> MUST optional...even with the "by default"...   Please s/MUST/SHOULD
> The MAY will then explain the "valid reason" to not...
> 

<-- AUTHORS
Tracked via issue #135: https://github.com/stoffi92/rfc5575bis/issues/135

The point to stress out is that the "default" behaviour MUST be "validate" according to the spec. Not the default can be changed but the "validation" MAY be relaxed by configuration.
-->

> 
> ...
> 783 BGP implementations MUST also enforce that the AS_PATH attribute of a
> 784 route received via the External Border Gateway Protocol (eBGP)
> 785 contains the neighboring AS in the left-most position of the AS_PATH
> 786 attribute. While this rule is optional in the BGP specification, it
> 787 becomes necessary to enforce it for security reasons.
> ...
> [major] In the case of receiving Flow Specifications from a neighbor in an
> IXP, it may not be possible to enforce the rule above if a "transparent ASN"
> is being used. Please include some text/guidance about that type of case.
> Include it either here or in the Security Considerations.
> ...
> <-- *** Authors ***
> Tracked via issue #60: https://github.com/stoffi92/rfc5575bis/issues/60
> 
> ...
> In the case of IXP indeed this sometime may break in the cases where RS is
> used. If you even in the IX (which is L2 typically) use direct EBGP session
> it all works ok. If you use session to RS and such RS by design is
> transparent in order to not extend the AS_PATH length that enforce first AS
> should be disabled. I think we could say that use of flow spec when peering
> with RS in IXP env is much less secure as ASN spoofing in the AS-PATH can
> occur. It is advised to apply strict inbound route policy in such scenarios.
> -->
> 
> Please add some text in §13 about it -- something similar to the
> explanation above would be great.
> 

<-- AUTHORS
Tracked via issue #136: https://github.com/stoffi92/rfc5575bis/issues/136
Commit mention: https://github.com/stoffi92/rfc5575bis/commit/96f539d178e301ccbc38c9c29ac6ec59b7774174
-->

> 
> ...
> 904 7.3. Traffic-action (traffic-action) sub-type 0x07
> ...
> 917 o T: Terminal Action (bit 47): When this bit is set, the traffic
> 918 filtering engine will apply any subsequent filtering rules (as
> 919 defined by the ordering procedure). If not set, the evaluation of
> 920 the traffic filter stops when this rule is applied.
> 
> [minor] According to the processing order and the values from Table 2, not
> setting the bit would effectively cause only the traffic-rate-bytes (0x8006)
> to ever be applied. Is that the correct interpretation?
> ...
> <-- *** Authors ***
> Tracked via issue #72: https://github.com/stoffi92/rfc5575bis/issues/72
> 
> Minor1:
> The terminal action applies to filter rules only, not to the filter actions
> (extended communties). Given we have 3 FS filters that match the traffic
> (they are sorted first):
> FS1) The first has T=0 and a traffic-rate-bytes action
> FS2) The second has only a redirect-action
> FS3) The third has only a traffic-rate-packets action
> 
> Evaluation takes place according to the sorted filters FS1 first, then
> FS2(because of T=0 in FS1). However, because FS2 has no T=0 set evaluation
> stops here and the resulting action is: traffic-rate-bytes and redirect.
> 
> Right...rules vs actions...   Please add a reference to §5.1 (Ordering
> of Flow Specifications) to make it clearer for others.
> 
> The example helps, but it is wrong.  The definition says that when the
> "bit is set, the traffic filtering engine will apply any subsequent
> filtering rules"...so the action should result in only
> traffic-rate-bytes.
> 

<-- AUTHORS
Tracked via issue #137: https://github.com/stoffi92/rfc5575bis/issues/137
Commit mention: https://github.com/stoffi92/rfc5575bis/commit/a096cfa26367bf49b00d6c087e08e0b69dad1e99

You are correct it should be T=1 not T=0. We also added a reference to the Ordering of Flow Specifications section.
—>


[..]

> The reason/history doesn't matter anymore, but the current use does. The
> mechanism described in this document is clearly not experimental. Given that
> changing the Type values is not an option because of the deployed base,
> etc.., then I think we should clean up the Registry and move 0x80-0x82 from
> the Experimental Use range to the FCFS range. This change would mean an
> Update to rfc7153.

[..]

> <-- *** Authors ***
> Tracked via issue #99: https://github.com/stoffi92/rfc5575bis/issues/99
> 
> This is not to be solved within this document. I note that I should propose a
> update to RFC7153 to cleanup the registry independently. - As noticed moving
> the values around is not an option.
> -->
> 
> So, are you working on this already?  ;-)
> 

<-- AUTHORS
Tracked via issue #138: https://github.com/stoffi92/rfc5575bis/issues/138

Seriously. IF the only thing that is needed here is a IANA considerations draft I will look into this later. If this can be done easier. I think after the 5575bis changes have been made in the registry. There is a good chance to have this cleaned up.


-->

> ...
> 1308 13. Security Considerations
> 

[..]

> 
> All this is moot anyway because ROAs are only defined for the
> IPv4/IPv6 AFs (rfc6482).
> 
> In any case, if I look really close, the text says that "the security
> characteristics of this proposal are equivalent to the existing
> security properties of BGP unicast routing" -- equivalent, not the
> same.  I'll let this go. :-)
> 

<-- AUTHORS
Tracked via issue #139: https://github.com/stoffi92/rfc5575bis/issues/139

Nothing to do.
—>



********** THE SECTION BELOW RELATES TO THE -18 REVIEW *************

> [Part II: Review of -18.]
> 
> [Line numbers from id-nits.]
> 
> ...
> 17   Abstract
> 
> 19      This document obsoletes both RFC5575 and RFC7674.
> 
> [style nit] Usually this statement is placed at the end of the
> Abstract/Introduction.  Personally, I think it makes it easier to
> read...
> 
> [minor] The Abstract shouldn't have references.  In the xml: s/<xref
> target="RFC5575" /> and <xref target="RFC7674" />/RFC 5575 and RFC
> 7674
> 

<-- AUTHORS
Tracked via issue #140: https://github.com/stoffi92/rfc5575bis/issues/140
Commit mention: https://github.com/stoffi92/rfc5575bis/commit/f3db0073a086179cbc7bf82168843206f68a62d0
-->

> 
> 
> 21      This document defines a Border Gateway Protocol Network Layer
> 22      Reachability Information (BGP NLRI) encoding format, that can be used
> 23      to distribute traffic Flow Specifications.  This allows the routing
> 24      system to propagate information regarding more specific components of
> 25      the traffic aggregate defined by an IP destination prefix.
> 
> [nit] No comma before "that".   s/encoding format, that/encoding format that
> 

<-- AUTHORS
Tracked via issue #141: https://github.com/stoffi92/rfc5575bis/issues/141
Commit mention: https://github.com/stoffi92/rfc5575bis/commit/2bd77ff28d1a401d10c9432a02868897cdd62d85
-->

> 
> ...
> 32      Additionally, it defines two applications of that encoding format:
> 33      one that can be used to automate inter-domain coordination of traffic
> 34      filtering, such as what is required in order to mitigate
> 35      (distributed) denial-of-service attacks, and a second application to
> 36      provide traffic filtering in the context of a BGP/MPLS VPN service.
> 37      Other applications (ie. centralized control of traffic in a SDN or
> 38      NFV context) are also possible.  Other drafts specify IPv6, MPLS
> 39      addresses, L2VPN addresses, and NV03 encapsulation of IP addresses as
> 40      Flow Specification extensions.
> 
> [minor] (Related to the last sentence.) As I mentioned before, the
> Introduction only points at the IPv6 draft, but it could benefit from
> pointers to the other drafts mentioned here.  Given the recent
> discussion about extensibility, it may be better to simply not be
> specific about what other drafts may specify:
> 
> SUGGESTION>
> Other documents may specify Flow Specification extensions.
> 

<-- AUTHORS
Tracked via issue #142: https://github.com/stoffi92/rfc5575bis/issues/142
Commit mention: https://github.com/stoffi92/rfc5575bis/commit/c77d7729ba25ba1d55580366d51cba246bd3ac3f
-->

> 
> 42      The information is carried via the BGP, thereby reusing protocol
> 43      algorithms, operational experience, and administrative processes such
> 44      as inter-provider peering agreements.
> 
> [nit] s/carried via the BGP/carried via BGP
> 

<-- AUTHORS
Tracked via issue #143: https://github.com/stoffi92/rfc5575bis/issues/143
Commit mention: https://github.com/stoffi92/rfc5575bis/commit/3590435e4eb82e86333156f6831422d5d52c0821
-->

> 
> ...
> 120  1.  Introduction
> ...
> 154     A Flow Specification received from an external autonomous system will
> 155     need to be validated against unicast routing before being accepted
> 156     (Section 6).  The flow specification received from an internal BGP
> 157     peer within the same autonomous system (per [RFC4271]) is assumed to
> 158     have been validated prior to transmission within the iBGP mesh of an
> 159     autonomous system.  If the aggregate traffic flow defined by the
> 160     unicast destination prefix is forwarded to a given BGP peer, then the
> 161     local system can install more specific Flow Specifications that may
> 162     result in different forwarding behavior, as requested by this system.
> 
> [nit] s/flow specification (and other case variations)/Flow Specification/g
> 
> [nit] s/(per [RFC4271])/[RFC4271]
> 

<-- AUTHORS
Tracked via issue #144: https://github.com/stoffi92/rfc5575bis/issues/144
Commits mentions:
   https://github.com/stoffi92/rfc5575bis/commit/39ca5251d300906c519929373ecb568807ceb461
   https://github.com/stoffi92/rfc5575bis/commit/46c337286cdff56a1cf39c1aaa493aefb3a102ee
-->

> 
> ...
> 249  4.  Dissemination of IPv4 FLow Specification Information
> 
> [nit] s/FLow/Flow
> 

<-- AUTHORS
Tracked via issue #145: https://github.com/stoffi92/rfc5575bis/issues/145
Commit mention: https://github.com/stoffi92/rfc5575bis/commit/27479f12bedaad66f23f8df5d961939e052e09bb
-->

> 
> ...
> 295  4.2.  NLRI Value Encoding
> ...
> 311     All combinations of components within a single Flow Specification are
> 312     allowed.  However, some combinations cannot match any packets (ie.
> 313     "ICMP Type AND Port" will never match any packets), and thus SHOULD
> 314     NOT be propagated by BGP.
> 
> [nit] s/ie./e.g.
> 

<-- AUTHORS
Tracked via issue #146: https://github.com/stoffi92/rfc5575bis/issues/146
Commit mention: https://github.com/stoffi92/rfc5575bis/commit/2c567cb5307d8810161dbc862df24dcc6c67aeb0
-->

> 
> ...
> 607  4.3.1.  Example 1
> ...
> 620            +-------+------------+------------------------------+
> 621            | Value |            |                              |
> 622            +-------+------------+------------------------------+
> 623            |  0x0b | length     | 11 octets (len<240 1-octet)  |
> 624            |  0x01 | type       | Type 1 - Destination Prefix  |
> 625            |  0x18 | length     | 24 bit                       |
> 626            |  0xc0 | prefix     | 192                          |
> 627            |  0x00 | prefix     | 0                            |
> 628            |  0x02 | prefix     | 2                            |
> 629            |  0x03 | type       | Type 3 - IP Protocol         |
> 630            |  0x81 | numeric_op | end-of-list, value size=1, = |
> 631            |  0x06 | value      | IP Protocol 6 = TCP          |
> 632            |  0x04 | type       | Type 4 - Port                |
> 633            |  0x81 | numeric_op | end-of-list, value size=1, = |
> 634            |  0x19 | value      | 25                           |
> 635            +-------+------------+------------------------------+
> 
> [minor] Table 1 defines equal as "== (equal)" -- just to be
> consistent.  s/=/==/g
> 

<-- AUTHORS
Tracked via issue #147: https://github.com/stoffi92/rfc5575bis/issues/147
Commit mention: https://github.com/stoffi92/rfc5575bis/commit/27e2b06bf895e73abbffd7999f53a4b83ad34de7
-->

> 
> ...
> 959  7.3.  Traffic-action (traffic-action) sub-type 0x07
> ...
> 986     o  Traffic Action Field: Other Traffic Action Field (see Section 12)
> 987        bits unused in this specification.
> 
> [major] How should unknown bits be treated?  In the last version, the
> text read: "should always be set to 0 by the originator and not be
> evaluated by the receiving BGP speaker."  Please add something like
> that.
> 

<-- AUTHORS
Tracked via issue #148: https://github.com/stoffi92/rfc5575bis/issues/148
Commit mention: https://github.com/stoffi92/rfc5575bis/commit/1b8029dff11415abb8fd1a3a158caa57481321bd
-->

> 
> ...
> 1047 7.6.  Interaction with other Filtering Mechanisms in Routers
> 
> [minor] Perhaps: s/other Filtering/Custom Filtering
> 

<-- AUTHORS
Tracked via issue #149: https://github.com/stoffi92/rfc5575bis/issues/149

I think it is fine as it is.
-->

> 
> 1049    Implementations SHOULD provide mechanisms that map an arbitrary BGP
> 1050    community value (normal or extended) to Traffic Filtering Actions
> 1051    that require different mappings in different systems in the network.
> 1052    For instance, providing packets with a worse-than-best-effort, per-
> 1053    hop behavior is a functionality that is likely to be implemented
> 1054    differently in different systems and for which no standard behavior
> 1055    is currently known.  Rather than attempting to define it here, this
> 1056    can be accomplished by mapping a user-defined community value to
> 1057    platform-/network-specific behavior via user configuration.
> 
> [] Moving this paragraph to its own section is an improvement.
> 
> [major] However, the Normative specification still bothers me because
> the text is not specific enough for implementations to
> interoperate...which I know is precisely the point (no standard
> behavior), so Normative language should not be used.   Please
> s/SHOULD/should  (to take the text back to what rfc5575 says)
> 

<-- AUTHORS
Tracked via issue #150: https://github.com/stoffi92/rfc5575bis/issues/150
Commit mention: https://github.com/stoffi92/rfc5575bis/commit/38e60a0ee0062c43a0e1d392a5c78b99dc8e82b3

Resolved as suggested. 
-->

> 
> ...
> 1080 8.  Dissemination of Traffic Filtering in BGP/MPLS VPN Networks
> ...
> 1092    The NLRI format for this address family consists of a fixed-length
> 1093    Route Distinguisher field (8 bytes) followed by the Flow
> 1094    Specification NLRI value Section 4.2.  The NLRI length field shall
> 1095    include both the 8 bytes of the Route Distinguisher as well as the
> 1096    subsequent Flow Specification NLRI value.  The resulting encoding is
> 1097    shown in Figure 7.
> 
> [minor] s/Flow Specification NLRI value Section 4.2./Flow
> Specification NLRI value (Section 4.2).
> 

<-- AUTHORS
Tracked via issue #151: https://github.com/stoffi92/rfc5575bis/issues/151
Commit mention: https://github.com/stoffi92/rfc5575bis/commit/2acf66c58590fd7ecb525f9d6bb1fd21d249cfc7
-->

> 
> ...
> 1137 10.  Error-Handling
> 
> [nit] s/Error-Handling/Error Handling
> 

<-- AUTHORS
Tracked via issue #152: https://github.com/stoffi92/rfc5575bis/issues/152
Commit mention: https://github.com/stoffi92/rfc5575bis/commit/526d5dfe8ad05f58001335dfdf70e16e893cee8d
-->

> 
> 1139    Error handling according to [RFC7606] SHOULD apply to this
> 1140    specification.
> 
> [major] rfc7606 already applies...there's no need to Normatively say
> it here.  And, BTW, then wound it not apply?  IOW, why is that
> "SHOULD" not a "MUST"?
> 
> NEW (suggestion): Error handling according to [RFC7606] and [RFC4760]
> applies to this specification.
> 

<-- AUTHORS
Tracked via issue #153: https://github.com/stoffi92/rfc5575bis/issues/153
Commit mention: https://github.com/stoffi92/rfc5575bis/commit/dc00addc1ae9349a4f07a5136646c13797005bf4

Changed according to suggestion. However the text may need additional changes later.
-->

> 
> 1142    This document introduces Traffic Filtering Action Extended
> 1143    Communities.  Malformed Traffic Filtering Action Extended Communities
> 1144    in the sense of [RFC7606] Section 7.14. are Extended Community values
> 1145    that cannot be decoded according to Section 7 of this document.
> 
> [] Please see my comment (above) in reply to your comments related to
> §4.2 (NLRI Value Encoding).
> 

<-- AUTHORS
Tracked via issue #154: https://github.com/stoffi92/rfc5575bis/issues/154
Related issues mentioned:
   https://github.com/stoffi92/rfc5575bis/issues/133
   https://github.com/stoffi92/rfc5575bis/issues/155
   https://github.com/stoffi92/rfc5575bis/issues/153

This seems to be resolved in #133 and #155, #153
-->

> 
> 1147 11.  Future NLRI Extensions
> 
> 1149    Future Flow Specification extensions may introduce new Flow
> 1150    Specification components.  In order to facilitate such extensions of
> 1151    the Flow Specification NLRI, in addition to the cases described in
> 1152    [RFC7606], if BGP encounters an unknown Flow Specification component
> 1153    in an UPDATE message, it SHOULD also treat this message as Treat-as-
> 1154    withdraw as specified in [RFC7606] Section 2.
> 
> [minor] This paragraph describes an error condition, so maybe it belongs in §10.
> 
> [major] rfc7606/§3 reads:
> 
> j.  Finally, we observe that 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 -- what
> this entails is discussed in more detail in Section 5.  If this
> is not possible, the procedures of [RFC4271] and/or [RFC4760]
> continue to apply, meaning that the "session reset" approach (or
> the "AFI/SAFI disable" approach) MUST be followed.
> 
> If an unknown Flow Specification component exists, then the entire
> NLRI cannot be "successfully parsed"...which results in not being able
> to use treat-as-withdraw.  The text above leaves us with AFI/SAFI
> disable, which is not extension-friendly.
> 
> 
> rfc7606/§5.4 talks about Typed NLRIs, such as in mVPN/EVPN, and
> concludes by saying:
> 
> 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 FS NLRI is not a Typed NLRI in the same sense that the examples
> from rfc7606 are: the NLRI encoding starts with "Route Type".  But,
> because the NLRI is basically a set of components, *with Types*, I
> think we could make the case that the FS NLRI is a Typed NLRI and that
> the paragraph above applies to it...which would result in being able
> to ignore/discard an UPDATE with an unknown component (vs AFI/SAFI
> disable).  Note that treat-as-withdraw is still not an option.
> 
> To be clear: all this is my personal interpretation/proposal.  If you
> (authors) want to go with this approach (resulting in the ability to
> discard), then we should explicitly bounce it by the WG.  If not, then
> we can only reset the session or disable FS...
> 
> NEW (suggestion)>
> An advertisement containing an unknown Flow Specification component
> should be discarded as specified in Section 5.4 of [RFC7606].
> 
> Additional text would be needed in §4.2 identifying the FS NLRI as Typed.
> 

<-- AUTHORS
Tracked via issue #155: https://github.com/stoffi92/rfc5575bis/issues/155
Commit mention: https://github.com/stoffi92/rfc5575bis/commit/b268bc69b1afd18fb471cf3bb1ae23216624faae

Defining the FS NLRI as "Typed NLRI" according to rfc7606. When decoding an unknown component type the implementation should discard the NLRI and skip over to the next NLRI according to section 5.4 rfc7606. Everything else is Malformed and treated as such. Explained in the beginning of the Email.

-->

> 
> 1156    The specification of a new Flow Specification Component Type SHOULD
> 1157    clearly identify what the criteria used to match packets forwarded by
> 1158    the router is.  This criteria should be meaningful across router hops
> 1159    and not depend on values that change hop-by-hop such as TTL or Layer
> 1160    2 encapsulation.
> 
> [major] Only "SHOULD"?  I can't imagine a case where it would be ok to
> not identify the criteria clearly.  s/SHOULD/MUST
> 

<-- AUTHORS
Tracked via issue #156: https://github.com/stoffi92/rfc5575bis/issues/156
Commit mention: https://github.com/stoffi92/rfc5575bis/commit/69fb9dfad2280195395f740344262c58e755e090

Resolved as suggested.
-->

> 
> ...
> 1192 12.2.  Flow Component Definitions
> ...
> 1222    In order to manage the limited number space and accommodate several
> 1223    usages, the following policies defined by [RFC8126] are used:
> 
> 1225              +--------------+-------------------------------+
> 1226              | Type Values  | Policy                        |
> 1227              +--------------+-------------------------------+
> 1228              | 0            | Specification required        |
> 1229              | [1 .. 12]    | Defined by this specification |
> 1230              | [13 .. 127]  | Specification required        |
> 1231              | [128 .. 255] | First Come First Served       |
> 1232              +--------------+-------------------------------+
> 
> [minor] Consider simply reserving 0.
> 

<-- AUTHORS
Tracked via issue #157: https://github.com/stoffi92/rfc5575bis/issues/157
Commit mention: https://github.com/stoffi92/rfc5575bis/commit/7b4a941c9db69b247c4243126cbafcb8019c7307

Reserved is fine as well. 
-->

> 
> ...
> 1343 13.  Security Considerations
> ...
> 1357    Deployment of specific relaxations of the validation within an
> 1358    administrative boundary of a network, defined by an AS or an AS-
> 1359    Confederation boundary, may be useful in some networks for quickly
> 1360    distributing filters to prevent denial-of-service attacks.  For a
> 1361    network to utilize this relaxation, the BGP policies must support
> 1362    additional filtering since the origin AS field is empty.
> 1363    Specifications relaxing the validation restrictions SHOULD contain
> 1364    security considerations that provide details on the required
> 1365    additional filtering.  For example, the use of [RFC6811] to enhance
> 1366    filtering within an AS confederation.
> 
> [major] When would it be ok for a specification to not provide
> details?  IOW, why is "SHOULD" used and not "MUST"?
> 

<-- AUTHORS
Tracked via issue #158: https://github.com/stoffi92/rfc5575bis/issues/158
Commit mention: https://github.com/stoffi92/rfc5575bis/commit/6df85c513787fba664549a47be93922ce4a36237

Resolved as suggested.
-->

> 
> ...
> 1418 14.  Contributors
> 
> 1420    Barry Greene, Pedro Marques, Jared Mauch, Danny McPherson, and
> 1421    Nischal Sheth were authors on [RFC5575], and therefore are
> 1422    contributing authors on this document.
> 
> [minor] Danny is listed both as a Contributor and as an author of this document.
> 

<-- AUTHORS
Tracked via issue #159: https://github.com/stoffi92/rfc5575bis/issues/159
Commit mention: https://github.com/stoffi92/rfc5575bis/commit/77b1f01477a1609b3f99ccd2943bb1c28e54a511
-->