Benjamin Kaduk's No Objection on draft-ietf-rtgwg-policy-model-30: (with COMMENT)

[Benjamin Kaduk via Datatracker <[e-mail]>]

Benjamin Kaduk has entered the following ballot position for
draft-ietf-rtgwg-policy-model-30: No Objection

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)


Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about DISCUSS and COMMENT positions.


The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-rtgwg-policy-model/



----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

Section 4.2

   Comparison conditions may similarly use options to change how route
   attributes should be tested, e.g., for equality or inequality,
   against a given value.

I'm not sure if this is stale text, or if still current, which
operations it refers to (e.g., where is an "inequality" option
offered?).  Perhaps it is referring to the potential behavior of future
augmentations?

Section 4.4

   Note that the called policy may itself call other policies (subject
   to implementation limitations).  The model does not prescribe a
   nesting depth because this varies among implementations.  For
   example, an implementation may only support a single level of
   subroutine recursion.  [...]

Is there a useful way to programmatically determine the nesting limit of
a given implementation, or is this more of just a "read the
documentation" thing?

Section 5

   Whether or not the route's pre-policy attributes are used for testing
   policy statement conditions is dependent on the implementation
   specific value of the match-modified-attributes leaf.  If match-
   modified-attributes is false and actions modify route attributes,
   these modifications are not used for policy statement conditions.
   Conversely, if match-modified-attributes is true and actions modify
   the policy application-specific attributes, the attributes as
   modified by the policy are used for policy condition statements.

Are these "actions" in question only for the current policy statement,
all policy statements in the current policy definition, or all policy
definitions?  (I see on second read that this is an "ro" node at
effectively global scope, and so likely to be at the broadest "all
policy statements" scope, but I would still welcome some clarification.

Section 7.2

      enum  add-metric {
        description
          "Add the specified value to the existing metric.
           If the result would overflow the maximum metric
           (0xffffffff), set the metric to the maximum.";
      }

Is the parenthetical always fully generic?  I seem to recall that, e.g.,
the Babel YANG model uses a 16-bit field for the metric.

    container apply-policy {

It slightly surprised me that the import-policy and export-policy lists
both required an instance, but I am rather distanced from what is
actually done in practice, and it doesn't seem too hard to put in a
"reject everything" policy if needed.  OTOH, there are *also* separate
default-{import,export} policy leaves, that do have a default reject, so
the need to explicitly set at least one policy in both direction may be
overkill.

          leaf-list tag-value {
            type tag-type;
            description
              "Value of the tag set member.";

The tag-type is a union of uint32 and hex-string; does the notion of
"equivalence" for matching tags do any type conversion between those?
If not, a warning that the value set here must be of the correct type to
match the received (or generated?) route might be in order.

              container match-interface {
                leaf interface {
                  type leafref {
                    path "/if:interfaces/if:interface/if:name";

Is it always the case that the interface over which we receive a route
and the interface that traffic on that route is sent will be the same?
Or do we have to worry about potential skew between the two ways that
the interface comes into play?

              container match-neighbor-set {

I don't understand why there's no "match-set-options" analogue for
match-neighbor set.  Does the neighbor set use "ANY" matching semantics,
then?

              leaf set-route-preference {
                type uint16;
                description
                  "Set the preference for the route. It is also
                   known as 'administrative distance', allows for
                   selecting the preferred route among routes with
                   the same destination prefix. A smaller value is
                   more preferred.";

Thank you for including the last sentence!

Section 8

We should probably mention the apply-policy grouping as being
security-relevant (and how).

Section 11.1

Since RFCs 6241 and 8040 are only referenced from the security
considerations boilerplate (as examples of how YANG modules can be
used), they may not need to be classified as normative.

Appendix A,B

I didn't do much of a review here, since I'd need to understand
draft-ietf-idr-bgp-model in order to have very much useful to say, and
any such comments would accordingly be better directed at that draft.

NITS

Section 4.4

As an editorial matter, I'd consider adding a statement along the lines
of "when call-policy elements are present, a given policy statement
matches if all called policies return accept-route and all the other
conditions in the policy statement also match".  There don't seem to be
any other ways to read the current text that are reasonable, but it took
me a bit of thinking to work it out.

Section 7.2

  identity isis-level-1-2 {
    base route-level;
    description
      "Identity for IS-IS Level 1 and Level 2 area importation. It
       is only applicable to routes imported into the IS-IS
       protocol.";

The "importation into both" phrasing from ospf-normal-nssa reads much
more clearly to me than what's here.

          container prefixes {
            description
              "Container for the list of prefixes in a policy
               prefix list. Since individual prefixes do not have
               unique actions, the order in which the prefix in
               prefix-list are matched has no impact on the outcome
               and is left to the implementation. A given prefix-set
               condition is satisfied if the input prefix matches
               any of the prefixes in the prefix-set.";

The last sentence seems like it could be removed, since the
match-set-options leaf is what actually specifies the matching behavior
(and is not always "any").

              leaf set-application-tag {
                type tag-type;
                description
                  "Set the application tag for the route.
                   The application-specific tag is an additional tag
                   that can be used by applications that require
                   semantics and/or policy different from that of the
                   tag. For example, the tag is usually automatically
                   advertised in OSPF AS-External Link State
                   Advertisements (LSAs) while this application-specific
                   tag is not advertised implicitly.";

This seems to make more sense if I apply s/implicitly/explicitly/.

Section 8

      /routing-policy/defined-sets/prefix-sets -- Knowledge of these
      data nodes can be used to ascertain which local prefixes are
      susceptible to a Denial-of-Service (DoS) attack.

      /routing-policy/defined-sets/prefix-sets -- Knowledge of these
      data nodes can be used to ascertain local neighbors against whom
      to mount a Denial-of-Service (DoS) attack.

The second one reads like it should be "neighbor-set" rather than
"prefix-sets".

      /routing-policy/policy-definitions/policy-definition /statements/

Spurious space.