docs: added EP041 phase 1

[quadflow]

This supersedes #630, focusing on phase 1

You can see the blueprint rendered here

phase 2 will be sent in a new blueprint later on

[italovalcy]

great work @quadflow ! here are my comments:

On the Motivation, I would also include the benefits for capacity planning once the measurements of used resources for the static paths with pre-provisioning flows for backup will give exactly the estimative of used flows, no surprise when the backup needs to be provisioned (flows and s_vlan).

"Only the two UNI ingress FlowMods (...) during convergence" => "Only the two UNI ingress FlowMods (...) are installed/updated during convergence"

Not clear to me "O(path length) FlowMods per transit switch" maybe it should be just "O(path length) FlowMods"

Suggestion, when you mention "Egress/NNI" maybe should use EgressUNI / NNI

Disjoin for static paths should be defined again here

On the Static EVC configurations, the dual static mentions that backup should be disjoint, but on "single static + dynamic" this is not mentioned. Maybe we should explicitly state whether "single static + dynamic" will accept non-disjoin or only disjoin failover_path

For "dual static + dynamic" the disjoin will be calculated in regards to primary? Backup? Both?

Suggestion: remove the primary word here to avoid confusion with the semantic around primary through out the document, i.e "dynamic primary (full dynamic EVC)" => "full dynamic EVC"

Maybe it would be nice to have a section for definitions, for example to define "cold dynamic" or "cold-compute (path)" ?

On the second table, can you please clarify what happens to current_path on the rows 1-4,6,7 like you did for row 5?

drop ingress => remove ingress

ON the second table, row 6 when you say "none pre-installed, but a fresh disjoint path exists" do you mean the failover_path was also affected or empty (previously affected)? Row 7 goes in the same direction there (i.e., failover affected)?

One question please: single static + dynamic, whose failover_path is affected by a link failures will remain without the "fast failover protection" until the next consistency routine, right? Maybe this should be stated in the begin explicitly as a acceptable design decision

"configured paths still down but a dynamic escape exists" => "configured static paths still down but a dynamic path exists"

On the table right after Link up section, the 4th row applies to EVC state on recovery = running on primary path, right? In that case, the action is okay for single static or dual static (no op), but for single static + dynamic, for pure-dynamic I believe it is out of the scope of this blueprint, no? For single static + dynamic, it will be left for the consistency routine as well? Maybe the 4th row could be rewritten to explicitly describe the actions when "active on dynamic, backup recovers" (not sure if this is the case covered by that 4th row already)

When you say: "Reverting keeps the old dynamic path when it is still good" can you please provide example of a dynamic that eventually is no longer good?

Maybe you should standardize the term "dynamic escape" or "dynamic path"

On the "Upgrade considerations", would it be simple if the consistency routine checks and enforces the correct state?

In section "Static EVC configurations" you mentioned in the very end: "torn down again once a static recovers", but later on on the Link UP table it seems that the EVC will only switchover to the static path if the primary static path is the one which recovered. In other words, if the backup static recovered, the EVC will remain on the dynamic path, right?

"is only persisted" is not clear to me, can you please clarify what is persisted?

"really to work as a last resource path indeed" => last-resort path

For the phase 2 discussion, I think it is not necessary to be presented there. The phase 1 documentation is pretty intensive and requires full attention, then when we get into the phase 2 section the reader may be tired from all the previous discussions/concepts/etc. Maybe a shorter description or a reference to a second document would be better.

[Ktmi]

I'm liking this so far. It aligns pretty well with some of my own thoughts I had while working on changes to handle link down. Code wise, we should be able to get the static failovers working pretty easily. Install them like a regular dynamic failover, and just modify handle link down to have a separate path for static EVCs, so as to not take down the static paths.

I'm interested to see what we do with phase 2. I've considered N-Path scenarios before, though my idea for that was to support both static and dynamic, by replacing the current model with a list of path_specs and deployed_paths. path_specscould support a variety ofpath_types`, such as dynamic and static, and allow for using different metrics for the dynamic path, and replacing the flexible metrics to an extent.

[quadflow]

@italovalcy thanks for your review, all of the questions were addressed, answers are also inlined to facilitate:

I would also include the benefits for capacity planning

Done.

"Only the two UNI ingress FlowMods (...) during convergence" => "Only the two UNI ingress FlowMods (...) are installed/updated during convergence"

Fixed the grammar issue.

Not clear to me "O(path length) FlowMods per transit switch" maybe it should be just "O(path length) FlowMods"

Correct. The transit stuff was left over from prior writings, removed it.

Suggestion, when you mention "Egress/NNI" maybe should use EgressUNI / NNI

Yes, that's more accurate. On second thought, let's also call "Egress UNI / NNI" as "path flows", makes easier to read too. I'll use "Egress UNI / NNI" only once, and alias them as path flows.

Disjoint for static paths should be defined again here

Done

"single static + dynamic" will accept non-disjoin or only disjoin failover_path

It'll find (partially) disjoint only. Augmented the text

For "dual static + dynamic" the disjoin will be calculated in regards to primary? Backup? Both?

Disjoint from the primary. But only computed when both as down, last resort escape. Also augmented in the text

Suggestion: remove the primary word here to avoid confusion with the semantic around primary through out the document, i.e "dynamic primary (full dynamic EVC)" => "full dynamic EVC"

Good catch. That was an awkward naming. Fixed.

Maybe it would be nice to have a section for definitions, for example to define "cold dynamic" or "cold-compute (path)" ?

Yes. Added a new section for terminology. Also unified "cold dynamic" as "escape"

drop ingress => remove ingress

Done

ON the second table, row 6 when you say "none pre-installed, but a fresh disjoint path exists" do you mean the failover_path was also affected or empty (previously affected)? Row 7 goes in the same direction there (i.e., failover affected)?

Yes, also affected. Updated the row to be more descriptive check it out.

One question please: single static + dynamic, whose failover_path is affected by a link failures will remain without the "fast failover protection" until the next consistency routine, right? Maybe this should be stated in the begin explicitly as a acceptable design decision

Single|Dual static + dynamic will also try to (cold) compute a dynamic escape after all the fast swaps, if it doesn't succeed, the consistency will try to recompute too. I also added a table for clarity. So, in summary in many cases it'll be able to compute it sooner without slowing down hot paths.

"configured paths still down but a dynamic escape exists" => "configured static paths still down but a dynamic path exists"

Reworded.

On the table right after Link up section, the 4th row applies to EVC state on recovery = running on primary path, right? In that case, the action is okay for single static or dual static (no op), but for single static + dynamic, for pure-dynamic I believe it is out of the scope of this blueprint, no? For single static + dynamic, it will be left for the consistency routine as well? Maybe the 4th row could be rewritten to explicitly describe the actions when "active on dynamic, backup recovers" (not sure if this is the case covered by that 4th row already)

Correct. Reworded with two explicit rows for clarity. Yes, in summary, consistency still behaves as it's always been to also recompute a failover_path for single + dynamic and pure dynamic when one doesn't exist. A link_up won't trigger an escape.

When you say: "Reverting keeps the old dynamic path when it is still good" can you please provide example of a dynamic that eventually is no longer good?

Good catch. Removed "it is still good" was redundant here. Reworded.

Maybe you should standardize the term "dynamic escape" or "dynamic path"

Improved the terminology. Standardized on "escape" for the cold-computed one, kept "dynamic path" only where it could be either that or a pre-installed failover_path. The differentiation exists to explain how the path setup will work during convergence, but at a API level nothing else will change.

On the "Upgrade considerations", would it be simple if the consistency routine checks and enforces the correct state?

Initially, I steered away from it, but we'll have the guards in place that won't swap to a non UP non deployed standby path. Great suggestion, I think this is an improvement, plus with the benefit of extra self healing capability (which is already aligned with what we want with consistency checks), and also not requiring to ship a console script. Let's go for it. Updated the blueprint.

In section "Static EVC configurations" you mentioned in the very end: "torn down again once a static recovers", but later on on the Link UP table it seems that the EVC will only switchover to the static path if the primary static path is the one which recovered. In other words, if the backup static recovered, the EVC will remain on the dynamic path, right?

Good catch, that was missing the qualifier for which static. Yes. I meant to write the escape is torn down when the EVC reverts to primary, not when any static recovers. Text updated.

"is only persisted" is not clear to me, can you please clarify what is persisted?

Leftover poor unnecessary wording. Persisted here meant evc.sync(). Cleaned up the text.

"really to work as a last resource path indeed" => last-resort path

Good catch, wrong word.

For the phase 2 discussion, I think it is not necessary to be presented there. The phase 1 documentation is pretty intensive and requires full attention, then when we get into the phase 2 section the reader may be tired from all the previous discussions/concepts/etc. Maybe a shorter description or a reference to a second document would be better.

Sounds good. Removed it, and saved it locally.

[quadflow]

I'm liking this so far. It aligns pretty well with some of my own thoughts I had while working on changes to handle link down. I'm interested to see what we do with phase 2. I've considered N-Path scenarios before, though my idea for that was to support both static and dynamic, by replacing the current model with a list of path_specs and deployed_paths.

Cool, thanks for your review @Ktmi. One thing that will be key is that static paths are essentially the materialized state while, and current_path acting like a pointer while only having one extra dynamic so that will make states (and clean up) more manageable, assuming the requirements and use case last spoke with @jab1982 still holds that N-paths will be for N statics + dynamic, we'll also confirm this with @italovalcy in phase 2 in a new blueprint. But indeed, if N paths were for any kind of paths, having a more decoupled data modeling has its merits, that'd be mef_eline v3 though.

It'll be cool to see N static paths in a non distant future in prod.

[italovalcy]

Nice work @quadflow ! thank you very much for reviewing my previous comments and commenting all of them! Appreciate you going over all the details! Congrats!