Subject: Re: [Pce] [Gen-art] Genart last call review of draft-ietf-pce-stateful-pce-lsp-scheduling-13

From: Alissa Cooper <[email-masked]>
Date: Wed, 8 Jul 2020 10:25:55 -0400

Robert, thanks for your review. Huaimo, thanks for addressing Robert’s comments. I entered a No Objection ballot.

Alissa

> On Jun 12, 2020, at 11:49 AM, Huaimo Chen <[email-masked]> wrote:
> 
> Hi Robert,
> 
>     Thank you very much for your suggestion.
>     We have updated the document (version 16 uploaded) according to your suggestion.
> 
> Best Regards,
> Huaimo
> From: Robert Sparks <[email-masked] <mailto:[email-masked]>>
> Sent: Friday, June 12, 2020 11:32 AM
> To: Huaimo Chen <[email-masked] <mailto:[email-masked]>>; [email-masked] <mailto:[email-masked]> <[email-masked] <mailto:[email-masked]>>
> Cc: [email-masked] <mailto:[email-masked]> <[email-masked] <mailto:[email-masked]>>; [email-masked] <mailto:[email-masked]> <[email-masked] <mailto:[email-masked]>>; [email-masked] <mailto:[email-masked]> <[email-masked] <mailto:[email-masked]>>
> Subject: Re: [Gen-art] Genart last call review of draft-ietf-pce-stateful-pce-lsp-scheduling-13
>  
> 
> On 6/11/20 9:12 AM, Robert Sparks wrote:
>> Thanks! Some initial replies inline (I haven't looked at the updated draft yet, but will do so soon):
> I've looked at the diffs between -13 and -15 and my comments have been addressed except for one point.
> 
>         Only one of them (i.e., elastic range and grace periods) is used for an LSP.
> 
> Isn't strong enough. I think you need normative language here.
> 
> I suggest
>     A TLV can configure a non-zero grace period or elastic bound, but it MUST NOT provide both.
>> On 6/10/20 7:35 PM, Huaimo Chen wrote:
>>> Hi Robert,
>>> 
>>>     Thank you very much for your time and valuable comments.
>>> 
>>>     My answers/explanations are inline below with prefix [HC].
>>> 
>>>     Your comments have been addressed in the updated document 
>>>  https://datatracker.ietf.org/doc/draft-ietf-pce-stateful-pce-lsp-scheduling/ <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdatatracker.ietf.org%2Fdoc%2Fdraft-ietf-pce-stateful-pce-lsp-scheduling%2F&data=02%7C01%7Chuaimo.chen%40futurewei.com%7Ca22642eaef594f08eb0308d80ee5e221%7C0fee8ff2a3b240189c753a1d5591fedc%7C1%7C0%7C637275727802251333&sdata=O2EvYJTW9ns6OFBSqAHwZSAp5qPgGGWUC3oQCFzLBw8%3D&reserved=0>
>>> 
>>> Best Regards,
>>> Huaimo
>>> From: Robert Sparks via Datatracker <[email-masked]> <mailto:[email-masked]>
>>> Sent: Tuesday, June 9, 2020 5:41 PM
>>> To: [email-masked] <mailto:[email-masked]> <[email-masked]> <mailto:[email-masked]>
>>> Cc: [email-masked] <mailto:[email-masked]> <[email-masked]> <mailto:[email-masked]>; [email-masked] <mailto:[email-masked]> <[email-masked]> <mailto:[email-masked]>; [email-masked] <mailto:[email-masked]> <[email-masked]> <mailto:[email-masked]>
>>> Subject: Genart last call review of draft-ietf-pce-stateful-pce-lsp-scheduling-13
>>>  
>>> Reviewer: Robert Sparks
>>> Review result: Ready with Issues
>>> 
>>> I am the assigned Gen-ART reviewer for this draft. The General Area
>>> Review Team (Gen-ART) reviews all IETF documents being processed
>>> by the IESG for the IETF Chair.  Please treat these comments just
>>> like any other last call comments.
>>> 
>>> For more information, please see the FAQ at
>>> 
>>> <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftrac.ietf.org%2Ftrac%2Fgen%2Fwiki%2FGenArtfaq&amp;data=02%7C01%7Chuaimo.chen%40futurewei.com%7C009e6885956a4c1957c908d80cbde38d%7C0fee8ff2a3b240189c753a1d5591fedc%7C1%7C0%7C637273356999321440&amp;sdata=x1%2BeFVCwSKtx16TqY5ycJgjrXK%2Bb%2Be4ttO0SuBrPStY%3D&amp;reserved=0 <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftrac.ietf.org%2Ftrac%2Fgen%2Fwiki%2FGenArtfaq&data=02%7C01%7Chuaimo.chen%40futurewei.com%7Ca22642eaef594f08eb0308d80ee5e221%7C0fee8ff2a3b240189c753a1d5591fedc%7C1%7C0%7C637275727802261328&sdata=891whMYACGrZ%2BzKXEmhvwpimP6wkNjd%2BL70QOehV9WQ%3D&reserved=0>>.
>>> 
>>> Document: draft-ietf-pce-stateful-pce-lsp-scheduling-13
>>> Reviewer: Robert Sparks
>>> Review Date: 2020-06-09
>>> IETF LC End Date: 2020-06-12
>>> IESG Telechat date: Not scheduled for a telechat
>>> 
>>> Summary: Essentially ready for publication as a Proposed Standard RFC, but with
>>> issues to consider before progressing
>>> 
>>> Minor Issues:
>>> 
>>> Section 4.2.2: It's not clear when the computation for a path satisfying the
>>> constraint happens. Is this done once when the periodical LSP arrives, or at
>>> each scheduled interval? If the former, what happens if there is no path
>>> solution for only one of the multiple intervals?
>>> 
>>> [HC]: We have made it clear in the document through revising the texts. 
>>> When a periodical LSP is configured, the paths for all the scheduled 
>>> intervals of the LSP is computed once. 
>>> If there is no path for some (e.g., one) of the intervals, 
>>> the constraints for the periodical LSP is not satisfied and
>>> the LSP will not be set up at all. 
>>> 
>>> Section 4.4, second paragraph, last sentence: If the path cannot be set, is the
>>> previous LSP left in effect? Or does the failure result in no there being no
>>> scheduled LSPs in effect?
>>> 
>>> [HC]: We have added the texts explicitly stating that 
>>> the previous LSP will not be impacted if the path for new 
>>> requirements cannot be set.
>>> 
>>> Section 5.1 first paragraph: Why is TCP called out here?
>>> 
>>> [HC]: We have removed TCP. 
>>> 
>>> You should be explicit about whether it's ok to have both grace periods and
>>> elastic bounds at the same time. The TLV allows that to happen. I'm not sure
>>> what it would mean, and I suspect it's unlikely that you would have two
>>> implementers compute the consequences the same way.
>>> 
>>> [HC]: We have added some texts to say that only one of them is used.
>>> 
>>> Section 5.2.1, definition of the R bit: You should call out that relative time
>>> is in seconds (I think?) when the R bit is 1, and you need a discussion
>>> somewhere about the necessity of synchronized clocks and potential problems
>>> with transmission delay when the R bit is 1.
>>> 
>>> [HC]: You are right. The relative time is in seconds. 
>>> We have explicitly stated this and had some discussions about 
>>> synchronizing clocks and potential problems with transmission delay.
>>> 
>>> Section 5.2.1, definition of Start-Time: Why must a value of 0 not be used? Is
>>> this true for both R=1 and R=0? For R=1, a start time value of 1 vs a start
>>> time value of 0 may, in practice, be indistinguishable (because of transmission
>>> or processing delay).
>>> 
>>> [HC]: When R=1, Start-Time=0 means right now. 
>>> Because of transmission and processing delay, this cannot be achieved.
>> And you don't have the same problem (or at least a race condition) for R=1, Start-Time=1?
>> (And perhaps other small values of Start-Time?). This isn't a big deal, but it looks like a rough
>> edge, and I want to make sure it's been thought through.
>>> For R=0, Start-Time=0 means the epoch (i.e., 1 January 1970 at 00:00 UTC).
>>> So for both R=1 and R=0, a value of 0 must not be used in Start-Time.
>>> 
>>> In section 5.2.2 at the definition of Repeat-time-length: Please be explicit
>>> about whether this is the interval between the start time of two repetitions or
>>> the interval between the end-time of one repetition and the start of the next
>>> repetition. I think you mean the former.
>>> 
>>> [HC]: It is the interval between the end-time of one repetition and
>>> the start of the next repetition. We had added some texts to 
>>> state this explicitly.
>> That surprises me. All the other values are start-start and not end-start.
>> With REPEAT-EVERY-DAY, you're aiming for the same time every day.
>> Suppose you didn't have REPEAT-EVERY-DAY and had to express that with REPEAT-EVERY-TIME-LENGTH.
>> If it were start-start, you could set the interval to 86400 (as you compute in 9.1) and be done.
>> With it being end-start you have to calculate the value by subtracting the Duration field value from
>> 86400. 
>> I think that's likely to surprise implementors trying to achieve something like "every other day".
>> 
>>> 
>>> At section 5.2.1 you say this TLV SHOULD NOT be included unless both PCEP peers
>>> have set the B bit. But in section 6.6, you say MUST NOT. Please choose one.. I
>>> think you want both places to say MUST NOT.
>>> 
>>> [HC]: We have changed SHOULD NOT to MUST NOT.
>>> 
>>> Nits:
>>> 
>>> Introduction, paragraph 3, second sentence: This is hard to read. I suggest
>>> trying to break it into more than one sentence.
>>> 
>>> [HC]: We have split and rephrased it.
>>> 
>>> Introduction, paragraph 4, third sentence: This is hard to read. Please
>>> simplify.
>>> 
>>> [HC]: We have rephrased it.
>>> 
>>> The document uses both "database" and "data base". Please pick one.
>>> 
>>> [HC]: We have changed "data base" to database.
>>> 
>>> Top of page 7: "In case of former" does not parse. Please clarify..
>>> 
>>> [HC]: We have rephrased it.
>>> 
>>> Section 4.2.2, second paragraph, first sentence: Does not parse. I think it is
>>> missing more than articles.
>>> 
>>> [HC]: We have revised it.
>>> 
>>> Section 4.3 at "In both modes": It's not clear what "modes" means here.
>>> 
>>> [HC]: We have added some details.
>>> 
>>> It would be worth calling out in section 5.1 that setting PD requires setting B
>>> as specified in 5.2.2.
>>> 
>>> [HC]: We have added some texts to state this.
>>> 
>>> It would be helpful in 5.2.2 at the definition of Opt: to point forward to the
>>> registry you are creating for its values. It would also be good to be explicit
>>> about what to do if an element receives a TLV with a value here it does not
>>> understand yet.
>>> 
>>> [HC]: We have added some descriptions about these.
>>> 
>>> Section 9.1 ignores leap-years and leap-seconds. It's worth explicitly noting
>>> that.
>>> 
>>> [HC]: We have added some texts about this.
>>> 
>> 
>> 
>> _______________________________________________
>> Gen-art mailing list
>> [email-masked] <mailto:[email-masked]>
>> https://www.ietf.org/mailman/listinfo/gen-art <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.ietf.org%2Fmailman%2Flistinfo%2Fgen-art&data=02%7C01%7Chuaimo.chen%40futurewei.com%7Ca22642eaef594f08eb0308d80ee5e221%7C0fee8ff2a3b240189c753a1d5591fedc%7C1%7C0%7C637275727802261328&sdata=ldR8LmGjXkxY%2Fw5J%2BrAIh%2FtUjRHgf5TsS7G8COZURSc%3D&reserved=0>
> _______________________________________________
> Gen-art mailing list
> [email-masked] <mailto:[email-masked]>
> https://www.ietf.org/mailman/listinfo/gen-art <https://www.ietf.org/mailman/listinfo/gen-art>