Hi Rob, Thanks a lot for your detail review and comments. I will work on them and get back.
Thanks, Ketan From: spring <[email protected]> On Behalf Of Rob Shakir Sent: 24 July 2018 17:04 To: [email protected]; SPRING WG List <[email protected]> Subject: [spring] Comments on draft-spring-segment-routing-policy (As an individual contributor) Hi Authors, I have a number of comments on draft-spring-segment-routing-policy, some are technical, some are editorial. Frankly, I find this document quite difficult to read since it doesn't coherently make up a specification - and rather jumps about quite a lot, with a number of things being re-stated. I understand that this happens during development of a specification, but now this is a WG doc, we should focus on making this document as clear as it can be. Please find my specific comments below: * (1. Introduction) In this section you use the wording "augmented with an ordered list of segments" - I would suggest rewording this. Whilst it might be the case for SRv6, for SR-MPLS, either we're doing pop+push, or simply pushing the new segment list. In this case, it's not really "augmenting", but adding a new stack of headers. * (2. SR Policy) The introduction to this section isn't really clear to me. * What is meant by a "specific intent"? I think that multiple "intents" might have the same SR policy - e.g., some latency optimisation and IGP shortest path are different "intents", but are likely to mean the same policy. Consider rewording this to a "common pathing behaviour" or similar. * Why do we need to refer to what the SR architecture says about instructions? It seems to me here the core point is "An SR Policy may consist of any type of segment". Is there a technical reason to not just word this more simply? * (2.1 Identification of an SR policy) Where there is a <headend, endpoint, colour> tuple, I believe that the intention here is to say that the headend IPv[46] address is domain unique -- is this expected, if so, should it be stated? * (2.1) "An implementation MAY allow assignment of a symbolic name" - the MUST NOT that is specified here is interesting. I believe Harish asked a similar question in the working group. What is the intention with the MUST NOT? Operationally, should these names be unique per policy (otherwise, it seems they are probably not that user friendly for identification). * (2..2) There is some repetition in this section which makes it quite unclear -- I believe the key points are: * A policy has a set of candidate paths. * Those candidate paths can be dynamic or explicit. * Candidate paths comprise of a set of one or more SID lists. * Explicit candidate paths have externally (to the router) specified SID lists. Dynamic ones are computed on the SID lists. * Within each SID list, traffic is load-balanced across the SID-lists according to a weight. Is this section saying something else other than the above -- if not, I recommend rewording it to not to have as many decision criteria in it to parse. * (2.2) The whole sub-paragraph about replication is hugely underspecified. We say above that traffic is load-balanced across the SID lists - but then we say that traffic might be replicated. The IDR draft, and this draft have no further mention of "replication". I would highly recommend removing this section, since I think specifying this fully requires significantly more work, and is better done in another draft. * (2.3) I would recommend that the "Local" should simply say "via configuration". "Yang model through NETCONF" isn't accurate since YANG and NETCONF are not coupled, and gRPC here is ambiguous * (2.3 + 2.12) There are a lot of priorities and preferences going on in this document. * We have protocol origin, preference, and then priority. I would recommend restructuring to discuss clearly how these all relate to each other. * From my parsing, your intention is to say: * Within an individual SR policy the candidate path to select is specified using preference. If this is the case, then the load-sharing section in 2.2 should state that this is only when the candidate paths are of the same preference. * See further comments on candidate paths and backups below. * If multiple SR policies that have the same <colour, endpoint> are specified, then the protocol-origin is used to select between them. You should explicitly state whether protocol-origin is better as lower or higher. * There is some head-end specific recomputation priority that is used only for dynamic candidate paths.. If this is the case, then I'd recommend explicitly calling this "recomputation-priority". * (2.3+2.12+9.3) There are many spanners thrown into the works throughout this document as to how things actually operate with different preferences. For example: * If I have multiple candidate paths all with the same weight - I think you say we should load-share across the weights. It doesn't seem to be stated when I should failover to the lower preference paths. * My assumption is that I do not compare preference across different protocol origins -- i.e., I pick the protocol-origin that is best, and then select within its candidate paths - such that it is only when ALL BGP (for example) paths have failed, that I go and look at ones configured statically. This would be my preference - but it should be clarified in the doc. * The backup semantics that are in this doc are really preference failover AFAICS? There is mention that "another ... candidate path MAY be designated as a backup for a specific or all ... candidate paths". How does this work? Where is this signalled that a particular policy is a backup for another? How does this then interact with the preferences and protocol origin values? * (2.8) "A candidate path is valid if it is usable" -- you should define usable somewhere. This is the only use of the word "usable" in the document. If it just means that it has met its validity criterion as specified in Section 5, then this should just be stated. * (2.7+2.8+2.9) For clarity, I would collapse these sections into something coherent - breaking them up makes the document less readable. * (2.8) It is only RECOMMENDED that a candidate path for an SR policy is given a different preference. There are then tiebreaker rules that are only MAY.. Please explicitly specify this -- it's going to be a nightmare to manage a multi-vendor network and test this if there are optional ways to tie-break, especially as the number of preferences/priorities et al. means the number combinations is large. * (2.11) This specification of WECMP seems like it says the same thing that is in 2.2 -- can you consider making these coherently stated in the document? * (3) What is the reason to define an SR-DB here? It seems like there is little reference to this elsewhere, and it replicates information from other places - e.g., BGP RIB, MPLS label tables, and IGP-TE database. Do you intend that implementors create some new lookup of this information, or is this conceptual? * This concept is only defined here in the document -- isn't it just an implementation detail that does not need to be standardised? * Why should we replicate the IGP contents in this SRDB? * You are recommending here that information such as topology is learnt from other domains - why is this a requirement for SR-TE policy? It certainly doesn't seem like a base requirement. To be clear, I would recommend removing this entire section from the document. * (5.2) I find this whole section under-specified again. The PCE draft referenced does not seem to tell me how to specify an optimisation objective, and there doesn't seem to be any explanation of where or how this would be specified at a head-end in configuration. I would recommend removing this from this document as it seems to be either: * Implementation specific -- if it's in configuration. The only difference might be if one were to want to specify a YANG model that specifically discusses how to specify such objectives. * A few years ago, we discussed this whole problem space, and the conclusion seemed to be that having such objectives specified on the head-end would end up with significant code churn - having an opaque identifier that could be handed to the PCE seemed more logical. * (6) Please can we stop writing IETF documents like they are marketing a technology - "X is fundamental to Segment Routing" is not a useful statement. Please objectively state the technical benefit. Either way, this seems like this section can be removed. * (6.2) Please make it a MUST that an alert of some form is generated when the BSID is not available. Otherwise it is entirely opaque to an operator that this wasn't accepted and that their policy is not in use. * (6.2) In the case of BSID stickiness, if CandidatePath1 specifies label 42 (assumed to be in SRLB), and is more preferable than CandidatePath2 then AIUI, we'll install this policy and use label 42. If we assume that there's some second CandidatePath2, which does not specify a BSID. If CandidatePath1 is subsequently withdrawn, even though CandidatePath2 doesn't specify a BSID, label 42 will be retained. If CP1 is now re-advertised, what happens - is this BSID considered "used" even though it's essentially now a dynamic allocation within the SRLB? * (6.4) Specifying binding SIDs can be assigned to any element doesn't seem to have any real benefit of being in this draft. If there are new BSID bindings that actually require standardisation - then I would highly recommend putting in their own draft. there are many questions that come from this short paragraph. For example, how are those binding SIDs considered valid? Is there OAM interworking that is needed with other networks for liveliness detection etc. We explicitly removed a lot of the BSID->RSVP including ERO advertisement in earlier revisions of SR related IGP drafts. * (7) The SR-TE Policy State is defined here to contain the reason that a policy or candidate path is not valid, however, the encodings in ietf-idr-te-lsp-distribution don't seem to say how this should be encoded. Is this the right reference? Is there extension to that work which is required? * (8.1 + 8.2) The document appears to have multiple different places where it discusses validity. Is there a reason that this can't be covered in 2.10 or 2.11? This would be a more natural place for the definition of "drop on invalid" too. * (8.2) Drop on invalid appears underspecified to me here -- should it be maintained when there was an active BSID but all remote ways of advertising it went away, or only when all paths in their entirety are invalid? If the policy did not specify a BSID - i.e., it was dynamically allocated, should this be freed? What is the behaviour when we have IP2MPLS entries that rely on that policy -- they should never fall back to other mechanisms of routing the traffic via an alternate MPLS tunnel, or alternate IP forwarding entry? * (9.2) It's very unclear to me how one actually provisions this link protecting policy. You have <colour, endpoint> as the unique tuple -- what is the policy that one installs using the framework that is defined in this document to define an SR-TE policy for link protection? What's the match criteria for it? * (9.3) This has a similar problem as the point raised above about how one specifies that a policy is a backup for another policy. Thanks for your review of these issues. Happy to discuss them on the list further. Kind regards, r.
_______________________________________________ spring mailing list [email protected] https://www.ietf.org/mailman/listinfo/spring
