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

From: Robert Sparks via Datatracker <[email-masked]>
Date: Tue, 09 Jun 2020 14:41:27 -0700

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://trac.ietf.org/trac/gen/wiki/GenArtfaq>.

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?

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?

Section 5.1 first paragraph: Why is TCP called out here?

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.

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.

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).

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.

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.

Nits:

Introduction, paragraph 3, second sentence: This is hard to read. I suggest
trying to break it into more than one sentence.

Introduction, paragraph 4, third sentence: This is hard to read. Please
simplify.

The document uses both "database" and "data base". Please pick one.

Top of page 7: "In case of former" does not parse. Please clarify.

Section 4.2.2, second paragraph, first sentence: Does not parse. I think it is
missing more than articles.

Section 4.3 at "In both modes": It's not clear what "modes" means here.

It would be worth calling out in section 5.1 that setting PD requires setting B
as specified in 5.2.2.

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.

Section 9.1 ignores leap-years and leap-seconds. It's worth explicitly noting
that.

---

From: Huaimo Chen <[email-masked]>
Date: Thu, 11 Jun 2020 00:35:24 +0000

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/

Best Regards,
Huaimo
________________________________
From: Robert Sparks via Datatracker <[email-masked]>
Sent: Tuesday, June 9, 2020 5:41 PM
To: [email-masked] <[email-masked]>
Cc: [email-masked] <[email-masked]>; [email-masked] <[email-masked]>; [email-masked] <[email-masked]>
Subject: Genart last call review of draft-ietf-pce-stateful-pce-lsp-scheduling-13

Reviewer: Robert Sparks

<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>.

Minor Issues:

[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.

[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.

[HC]: We have removed TCP.

[HC]: We have added some texts to say that only one of them is used.

[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.

[HC]: When R=1, Start-Time=0 means right now.
Because of transmission and processing delay, this cannot be achieved.
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.

[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.

[HC]: We have changed SHOULD NOT to MUST NOT.

Nits:

[HC]: We have split and rephrased it.

[HC]: We have rephrased it.

[HC]: We have changed "data base" to database.

[HC]: We have rephrased it.

[HC]: We have revised it.

[HC]: We have added some details.

[HC]: We have added some texts to state this.

[HC]: We have added some descriptions about these.

[HC]: We have added some texts about this.

---

From: Robert Sparks <[email-masked]>
Date: Thu, 11 Jun 2020 09:12:52 -0500

Thanks! Some initial replies inline (I haven't looked at the updated 
draft yet, but will do so soon):

On 6/10/20 7:35 PM, Huaimo Chen wrote:
> ------------------------------------------------------------------------
> *From:* Robert Sparks via Datatracker <[email-masked]>
> *Sent:* Tuesday, June 9, 2020 5:41 PM
> *To:* [email-masked] <[email-masked]>
> *Cc:* [email-masked] <[email-masked]>; [email-masked] 
> <[email-masked]>; 
> [email-masked] 
> <[email-masked]>
> *Subject:* Genart last call review of 
> draft-ietf-pce-stateful-pce-lsp-scheduling-13
> Reviewer: Robert Sparks

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.

> the start of the next repetition. We had added some texts to

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".

---

From: Huaimo Chen <[email-masked]>
Date: Thu, 11 Jun 2020 14:47:52 +0000

Hi Robert,

    Thank you very much for your quick reply.
    My answers/explanations are inline below.

Best Regards,
Huaimo
________________________________
From: Robert Sparks <[email-masked]>
Sent: Thursday, June 11, 2020 10:12 AM
To: Huaimo Chen <[email-masked]>; [email-masked] <[email-masked]>
Cc: [email-masked] <[email-masked]>; [email-masked] <[email-masked]>; [email-masked] <[email-masked]>
Subject: Re: Genart last call review of draft-ietf-pce-stateful-pce-lsp-scheduling-13

Thanks! Some initial replies inline (I haven't looked at the updated draft yet, but will do so soon):

    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%7Ce9d8ff99250149ef36fc08d80e118a6a%7C0fee8ff2a3b240189c753a1d5591fedc%7C1%7C0%7C637274815796667149&sdata=xqsVrqpFBP%2F7FZ1uXpePWT6m3X%2Fhy44K8pO8WcPO85M%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

<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%7Ce9d8ff99250149ef36fc08d80e118a6a%7C0fee8ff2a3b240189c753a1d5591fedc%7C1%7C0%7C637274815796667149&sdata=lFujiKxBpSoEGjvjxsBszLONJT2RajnEQLUwcQuqNYM%3D&reserved=0>>.

Minor Issues:

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

[HC]: We have added a note to consider transmission delay when R=1 and Start-Time is small.

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".

[HC]: It is the time from the start of one repetition to the start of the next repetition.

We have updated the document in version 15 (uploaded) accordingly.

Nits:

---

From: Robert Sparks <[email-masked]>
Date: Fri, 12 Jun 2020 10:32:52 -0500

> 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.

> _______________________________________________
> Gen-art mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/gen-art

---

From: Huaimo Chen <[email-masked]>
Date: Fri, 12 Jun 2020 15:49:05 +0000

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]>
Sent: Friday, June 12, 2020 11:32 AM
To: Huaimo Chen <[email-masked]>; [email-masked] <[email-masked]>
Cc: [email-masked] <[email-masked]>; [email-masked] <[email-masked]>; [email-masked] <[email-masked]>
Subject: Re: [Gen-art] Genart last call review of draft-ietf-pce-stateful-pce-lsp-scheduling-13

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.

I suggest

    A TLV can configure a non-zero grace period or elastic bound, but it MUST NOT provide both.

    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

Reviewer: Robert Sparks

<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>>.

Minor Issues:

86400.

Nits:

_______________________________________________
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>