[Idr] Comments on draft-li-idr-flowspec-srv6-05

[Jeffrey Haas <[e-mail]>]

Authors,

I've been selected as the document shepherd from among the IDR chairs for
this draft.  Thank you for your patience.

A few questions and comments on your draft version -05:

- Please note per Working Group consensus, this feature MUST implemented in
  the Flowspec-v2 work and will not currently be permitted to advance as
  part of Flowspec-v1.
- The document is clearly for IPv6 functionality.  It would be good to call
  out in the document that you would expect to use a Flowspec version that
  is IPv6 capable.  I.e., AFI=2.  See the similar IPv6-only components in
  RFC 8956.
- The functionality in this document is intended to operate on the IPv6
  destination address in the IP Header.  The presumption is that the new
  Flowspec match fields are appropriate when the IPv6 destination address is
  an IPv6 SID.  Would it make sense for the match capabilities of the
  proposed features only be permitted to match when there is an accompanying
  SR Header?
- In section 3.1, Whole SID, there are three '0' bits:
  + The Operator field shares similar encoding to the Numeric Operator field
    from RFC 8955, section 4.2.1.
  + One of these bits overlaps the '0' in that Numeric Operator field.  Its
    text reads:
    "MUST be set to 0 on NLRI encoding and MUST be ignored during decoding"
  + The other two of the '0' bits overlaps the 'len' encoding of the Numeric
    Operator field.
  + The text in your draft for these bits reads: "0 - SHOULD be set to 0 on
    NLRI encoding, and MUST be ignored during decoding."  What is the
    motivation to have a SHOULD rather than a similar MUST?
- In section 3.2, Some bits of SID:
  + The value field includes a LOC Length, FUNCT Length, and ARGS Length
    field.  Since LFA <= 128, is there any desired match behavior on the
    bits > LFA when LFA < 128?
  + Related to the prior point, when LFA < 128, there's no "field type" that
    matches on LFA.  It's understood that when LFA = 128, the feature in
    section 3.1 is appropraite.
  + There is no LOC:ARGS field type.  Is that intentional?
  + Since the "field type" match has options that can specify sets of fields
    that are adjacent, error handling is interesting:
    * For LOC match, FUNC Length and ARGS Length are irrelevant.  However,
      since this is part of BGP NLRI, we have need of a canonical format
      when possible for such filters.
    * The same also applies for LOC:FUNCT matches, where ARGS Length is
      irrelevant.
  + This feature is somewhat complicated and would benefit from encoding
    and use examples.
- The behavior of the operator bits has clear symmetry with RFC 8955's
  Numeric Operator field.  I'd suggest pointing to that portion of the
  document to highlight how the combinations of bits are expected to be
  encoded to achieve the desired filtering logic.
- Error handling deserves some additional discussion.  The Chairs understand
  that some of the handling of such behaviors may be deferred to the
  Flowspec v2 document.  However, it's worth highlighting some known
  possible issues:
  + For Some bits of SID, the component length will have similar
    considerations.
  + The field type bits are not exhaustively specific in the document.  What
    is the error condition when receiving an unknown type?
  + The LOC Length, FUNCT Length, and ARGS Length fields are one octet in
    size.  What is the expected error handling to be when L+F+A > 128?
  + For Whole SID, the component length is expected to variable.  If the
    full 128 bits is accounted for as L+F+A, the component length would be 1
    octet type followed by multiples of 20 octets for the operator byte plus
    the 128-bit SID.  The minimum component length is 1 octet component
    type, one octet operator, and ... 3? octets for L + F + A length?
    The text surrounding treating SID as a "Prefix" could generally use
    attention as part of addressing the error handling.
- The IANA Considerations refer to RFC 7153.  Since this document defines no
  Extended Communities, it's not clear why this is present?

-- Jeff