Hi Wes,
Thank you for your review.
> * Security considerations (since this is a secdir review, I'm listing
> it first!): Though you're right that this document includes all of
> the security considerations from the other document, I'm not sure
> that you can totally get away with saying there is nothing else
> new. Because this is an experimental draft, it may not be quite as
> important to be accurate about the security considerations, but you
> are introducing a brand new algorithm, which means that the
> ramifications associated with executing that algorithm may provide
> an additional security footprint.
I respectfuly disagree. This document changes nothing about the security
issues that were covered in RFC 9667. There is no new attack surface, no new
attack mechanisms. This is purely an algorithm for computation by the Area
Leader.
> Specifically, the network size can affect the order of the
> complexity, deliberately flapping states can cause a leader to
> possibly recalculate the topology on a frequent basis, some
> topologies may be more expensive to calculate than others, and all
> of the algorithm parameter choices affect the computation resources
> needed for the particular node. Maybe? (thinking out loud).
These concerns are not specific to this document. Yes, these are operational
considerations.
> Most centrally, for example, how frequently are these calculations
> re-performed and is this a possible denial of service factor by
> anyone that can alter the topology even by a flapping link state,
> eg.
If an attacker can flap the topology then there is a very serious security
issue but that is completely unrelated to Dynamic Flooding and may be a result
of deficient physical security, management plane security, or IGP security.
The impact on Dynamic Flooding is incidental.
> Do feel free to declare my thinking/wondering wrong, however.
>
> * Abstract: the wording "discloses the algorithm" makes me wonder if
> all the IPR was properly handled :-)
It was. Please note that RFC 9667 was published long before this document. It
took a while before Arista was willing to disclose the algorithm details.
> * In general because there are multiple uses of "topology" you should
> double check that all are properly identified with "sparse", "base",
> "dynamic", etc just to check clarity. I didn't check every usage.
Thank you, yes, this has been raised before resulting in several changes and I
believe that things are currently clear.
> * EG, in section 1 bullet "3." I believe the first sentence should end
> in "base topology" not just "topology"
Ok
> * 1.1.2: bullet 1: Since this is an experiment and these are supposed
> to be criteria, I wondered how "correctly interpreted" could be
> measured.
One would measure this by extracting the flooding topology from each
implementation and comparing it with the flooding topology as computed by the
Area Leader.
> * 1.1.2: bullet 7: Similar, how is robustness measured?
One reasonable measure would be consistently computing a new, valid flooding
topology in an acceptable time. What is ‘acceptable’ is somewhat subjective.
> * Section 3: It would be helpful if at the top of the section there
> was a note indicating that this is a high level summary and to see
> section 4 for more details. (I do agree the summary was very
> helpful though).
Ok
> * Section 3: I think the phrase "denoted by G'({}, {})" is misleading
> a bit. G' is really the name of a vertex/edge set where it starts
> as empty, and is thus G'({}, {}). Then bullet 1 reads a bit better,
> though "This cycle is added to the G' subgraph so it becomes
> G'(V(0), E(0))" may be even better.
Ok. I also changed the introductory sentence to: "The subgraph to be computed
is denoted by G', which
starts as an empty set of nodes and an empty set of edges,
G'({}, {})."
> * Section 3: Bullet 1 says to use a subset of edges, as does section
> 4, but given some advice somewhere on the right quantity to pick
> would be helpful.
This is very much implementation dependent and we do not have constructive
advice here. It would very much depend on the scale and diameter of the base
topology, but we don’t have anything like a quantitative of formulaic
recommendation. This is, after all, experimental.
> * Section 3, bullets 2 and 3 -- these say "not included in the
> subgraph" or "are included in the subgraph" in sort of a confusing
> way. Section 4.2 says that the start nodes of the subgraph *are*
> already included (in fact that's how you pick them), but that's not
> what I got out of this high level summary. My initial reading
> thought the new subset selection would contain zero nodes from the
> original set.
I’ve revised section 3, bullet 2 to read:
Select an arc path (V(i), E(i)) such that the first and last
node are in G', the remaining nodes are not in G', and all
edges are not in G'. That is, V(i) midpoints are selected
from V - V(0) - ... - V(i-1) and E(i) is selected from E -
E(0) - ... - E(i-1). This arc path is added to G', resulting
in G'( V(0) + V(1) + ... + V(i), E(0) + E(1) + ... + E(i) ).
> * Section 4: The final sentence in the first paragraph states "easily
> implemented" is immeditaely followed by the next paragraph with a
> list of implementation specific algorithms (DFS depth limit
> .. endpoint selection) made me laugh, as they may not be as easily
> implemented.
I am pleased that we have induced laughter. DFS and BFS are very well known
graph algorithms, found in almost every algorithm textbook. The modifications
that we propose are very minor. Did you have a specific suggestion for textual
improvement?
> * Section 4: DFS is mentioned here, but not expanded until 4.1.
Fixed
> * Section 4.1 bullet 2: find *the* starting node - I'd either state
> what node that should be (its hinted later it should be the one with
> the highest scale) or change "the" to "an" to be more generic.
Fixed.
> * Section 5: I was happy to see the example spelled out, as they
> greatly helped me understand the algorithm. You might include the
> resulting G() vectors for even better help to the reader.
Ok, added.
Tony
_______________________________________________
Lsr mailing list -- [email protected]
To unsubscribe send an email to [email protected]
