A YANG Data Model for Routing Policy
RFC 9067
Yes
No Objection
Note: This ballot was opened for revision 30 and is now closed.
Alvaro Retana Yes
Erik Kline No Objection
John Scudder No Objection
In the acks you have a misspelling: “Chris Bower” should be “Chris Bowers”.
Lars Eggert No Objection
No reference entries found for: [draft-ietf-idr-bgp-model-09] ------------------------------------------------------------------------------- All comments below are about very minor potential issues that you may choose to address in some way - or ignore - as you see fit. Some were flagged by automated tools (via https://github.com/larseggert/ietf-reviewtool), so there will likely be some false positives. There is no need to let me know what you did with these suggestions. "Table of Contents", paragraph 2, nit: > manage policy configuration in a consistent way in environments with routers > ^^^^^^^^^^^^^^^^^^^ Consider replacing this phrase with the adverb "consistently" to avoid wordiness. Section 3. , paragraph 4, nit: > ations, and similarly, actions may effect multiple changes to route attribut > ^^^^^^ Did you mean "affect" (have an effect upon)? Section 4.2. , paragraph 10, nit: > a reject-route action returns false and the calling policy evaluation will > ^^^^ Use a comma before "and" if it connects two independent clauses (unless they are closely connected and short). Section 4.3. , paragraph 4, nit: > eating policies nested beyond a small number of levels (e.g., 2-3) is discou > ^^^^^^^^^^^^^^^^^ Specify a number, remove phrase, use "a few", or use "some". Section 4.3. , paragraph 4, nit: > o ensure that there is no recursion amongst nested routing policies. 5. Polic > ^^^^^^^ Do not mix variants of the same word ("amongst" and "among") within a single text. Section 4.4. , paragraph 2, nit: > n is specified for the chain). Whether or not the route's pre-policy attribut > ^^^^^^^^^^^^^^ Consider shortening this phrase to just "Whether". It is correct though if you mean "regardless of whether". Section 7.2. , paragraph 31, nit: > existing metric. If the result would overflow the maximum metric (0xffffffff > ^^^^^^^^^^^^^^ Consider removing "would". (Usually, "would" does not occur in a conditional clause, unless to make a request or give a polite order.). Section 7.2. , paragraph 33, nit: > he existing metric. If the result would be less than 0, set the metric to 0. > ^^^^^^^^ Consider removing "would". (Usually, "would" does not occur in a conditional clause, unless to make a request or give a polite order.).
Murray Kucherawy No Objection
Roman Danyliw No Objection
Thanks to Dan Harkins for the SECDIR review. ** Section 5. If none of the policy statement conditions are satisfied, then evaluation of the current policy definition stops, and the next policy definition in the chain is evaluated. Is it worth mentioning in this paragraph that various implementation specific optimizations may be possible. For example, Section 4.2 notes policy match conditions. If the match condition is ALL and the first condition is not satisfied, is it necessary to evaluate the next policy statement? ** Section 8. The text helpful notes the read sensitivity of “/routing-policy/policy-definitions/policy-definition” with “Additionally, policies and their attendant conditions and actions should be considered proprietary and disclosure could be used to ascertain partners, customers, and supplies.” It seems like “defined-sets/prefix-sets” could also reveal these relationships with partners, customers or suppliers.
Warren Kumari No Objection
Thank you for this document; it seems like it will be useful.
Éric Vyncke No Objection
Thank you for the work put into this document. I really admire the 4 authors managing to reach a consensus even while having different affiliations: IETF at its best! Please find below some non-blocking COMMENT points (but replies would be appreciated). I hope that this helps to improve the document, Regards, -éric == COMMENTS == -- Section 2 -- Having "Policy chain: A policy chain is a sequence of policy definitions (described in Section 4)." in the terminology section does not really help the reader... -- Section 4.1 -- While I am not a YANG expert, I wonder about the "*" (usually meaning 0 or more) for address in the neighbor-set container ? How can a neighbor exist w/o an address ? Why not using the "min-elements' YANG statement ?
(Benjamin Kaduk; former steering group member) No Objection
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.
(Martin Vigoureux; former steering group member) No Objection