Dissemination of Flow Specification Rules for IPv6
draft-ietf-idr-flow-spec-v6-22

[[ discuss ]]

[ general ]

* I agree with existing discuss points raised.


[[ comments ]]

[ section 3.8,{1,2} ]

* Per RFC 5952 section 4.3, s/upper hex/lower hex/g for IPv6 address strings.


[[ nits ]]

[ appendix A ]

* s/non of/none of/, I think

Why are the SHOULDs in Section 3.3, 3.4, and 3.5 not MUSTs?  Put another way: When might someone legitimately do other than what it says?

Thank you to Vincent Roca for the SECDIR review.

Thanks for modernizing Flowspec.

Nits in Appendix A:
s/a array/an array/
s/accorinding/according/

Thanks to Qin Wu for the OpsDir review. if Qin hadn't asked the "why isn't this folded into the upcoming -bis" (and Christoph's answer) I would have had to ballot DISCUSS.  

I do support Ben's DISCUSS, and, even more, Eric's

Thank you for addressing my DISCUSS point in the security section in the latest revision. I am now balloting a NO OBJECTION so clearing my previous blocking DISCUSS.

Regards

-éric

PS: I still have doubts about the implementation status wiki page though ;-)

-------- IGNORE TEXT BELOW ----------
The rest below is for archival purpose as all the points have been addressed.

Thank you for the work put into this document. It is indeed due time to filter also those IPv6 packets ;-)

Please find below one blocking DISCUSS point, some non-blocking COMMENT points, and some nits.

I hope that this helps to improve the document,

Regards,

-éric

== DISCUSS ==

I am puzzled by the absence of a flow spec for the first Next-Header being a specific value and by the absence of a flowspec for the occurence of any extension header in the extension header chain. Extension headers are an important difference compared to IPv4 and could be 'nasty' as well (e.g., hop-by-hop header). Why was this not considered by the authors ? Or is there another document in the WG to address this issue ?

== COMMENTS ==

-- Section 3.3 --
How is the upper-layer defined here? Basically, how a node can determine whether it is an extension header or an upper-layer header? While I agree that there are not too many new upper-layer protocols being specified, I would prefer to have a definition of "upper-layer" here either by listing (or referring to a IANA registry) all existing extension headers (then all 'next header' not being 'extension header' are by default 'upper layer') or vice-versa.

-- Section 3.6 --
Just curious ;-) Why is bit 7 not used in this encoding ?

-- Section 3.7 --
I share Ben Kaduk's concern about the encoding of the flow label in less than 20 bits.

== NITS ==

-- Section 3.1 (and possibly others) --
Sometimes the field 'length' is all lower case and sometimes it is capitalized.

-- Section 3.8.2 (and possibly others) --
Please use RFC 5952 to write IPv6 addresses.

Thanks for clearing up my confusion on the DISCUSS point that I had, and for posting -18 with resolutions to that and my comments.

I’ll leave the remaining comment here for the record, leaving it for the editors to do what they think best.

   In the case Length minus Offset is 0 every address matches.  Length
   MUST always be in the range 0-128 and Length minus Offset MUST always
   be 0 or more, otherwise this component is malformed.

Is there actually value in allowing 129 ways to match every address (length=offset=0, length=offset=1, length=offset=87, and so on)?  If not, it seems less prone to error to say that length=offset=0 matches every address, and otherwise length MUST be greater than offset or the component is malformed.  No?

Thank you for addressing my discuss (and comment!) points!

IIUC the truth table for Type 12 (Fragment) is as follows:

          Offset = 0             Offset > 0

M = 0    NOT (FF | IsF)   (LF)         

M = 1      (FF)                    ?

IsF is the two right cells in the table, but there is no way to express the lower right cell using these semantics. If IsF meant "neither first nor last fragment" it would be possible to express all possibilities using the bitmask and bitmask_op. Maybe that's a use case we don't envision right now, but I don't see any downside in being able to express it.

Otherwise, this is a great document. Thanks!

Hi,

should Type 13 for IPv4 be Unassigned or Reserved ?


Nit:
s/and propose/and proposes/

Hi,

Thank you for this document.  Even without the specific domain knowledge I found this document easy to read and understand.

A couple of minor comments that you may wish to consider:

    3.  IPv6 Flow Specification components

       Types 4, 5, 6, 9, 10 and 11, as defined in [I-D.ietf-idr-rfc5575bis],
       also apply to IPv6.

Also giving the descriptive names for these types might aid the reader here.

    3.3.  Type 3 - Upper-Layer Protocol

       This component uses the Numeric Operator (numeric_op) described in
       [I-D.ietf-idr-rfc5575bis] Section 4.2.1.1.  Type 3 component values
       SHOULD be encoded as single octet (numeric_op len=00).

The "(numeric_op len=00)" threw me off at first, until I referenced back to section 4.2.1.1.  Possibly, this might be more clear as "(i.e., numeric_op len field=00)".  Obviously, if you decide to change this, then there are other places that need to be updated as well.
   

    3.4.  Type 7 - ICMPv6 Type

The text wasn't super clear to me whether the a Type 3 componet could/should be specified to match protocol-value 58 as well as a Type 7 field. I would presume that either is allowed, but I was unsure whether it might be helpful to clarify this further.

Regards,
Rob