Thanks for the review. Some responses in line. > On Apr 20, 2018, at 10:29 PM, Qin Wu <bill.wu@xxxxxxxxxx> wrote: > > Reviewer: Qin Wu > Review result: Has Nits > > I have reviewed this document as part of the Operational directorate¡¯s ongoing > effort to review all IETF documents being processed by the IESG. These > comments were written with the intent of improving the operational aspects of > the IETF drafts. Comments that are not addressed in last call may be included > in AD reviews during the IESG review. Document editors and WG chairs should > treat these comments just like any other last call comments. Document reviewed: > draft-ietf-idr-bgp-gr-notification > > Summary: > This document updates RFC 4724 by defining an extension that permits the > Graceful Restart procedures to be performed when the BGP speaker receives a BGP > NOTIFICATION Message or the Hold Time expires. > > Major issue: None > Minor issue: Editorial > 1.Abstraction: > I am not familiar with history of RFC4724 and this document. I am wondering why > not relax BGP graceful restart and apply usage of BGP graceful restart message > to all BGP protocols messages rather than introduce a new flag within restart > flags? Why not make bis document to update RFC4724? For two reasons. First and most important, some network operators may not want to use GR semantics in the expanded manner this spec introduces. Second, because backwards-compatibility concerns might well arise if there were no on-the-wire signaling about what semantics your peer would support. (Note, I haven't given the second a lot of thought, because the first rules it out anyway.) > 2.Abstraction: > How session restart is different session reset or the session is fully > terminated? Do we use hard reset subcode to indicate those features separately? > If they are same things, please make sure to use consistent terminology in the > document. A fair criticism. I was tempted to say "there is no difference" but "restart" might have the shade of meaning that "the session has been terminated and a new one initiated" whereas "reset" might indicate something more like "the session has been terminated and we hope a new one will be initiated". Anyway, I'll give it some thought. > 3. Section 2, 3rd paragraph Why we use a different bit instead ¡°R¡± > bit to indicate graceful restart support for BGP notification message. The R bit already has defined semantics, entirely different from those of the N bit. Maybe I haven't understood your question properly. > 4. > Section 2, last paragraph Can you give an example what are these new parameters > and how these parameters apply? How these parameters are related to relevant > routes? Providing such an example is probably beyond the scope of this spec. Is the problem that you find the language "Graceful Restart parameters" to be not specific enough? It means to refer to the contents of the Graceful Restart capability, the point being that if your peer sends you a list of GR parameters you only start acting on them when (if!) the session transitions to ESTABLISHED. This language was added as the result of Bruno DeCraene's review, where he pointed out there might be some ambiguity related to a peer offering a new set of parameters that would (for example) cause GR-Notification support to be removed, but the session never establishing for some other reason (e.g. the wrong AS number in the OPEN). In such a case we are saying GR-Notification support would NOT be removed because the session related to that parameter never established. > 5.Section 3.1 said: ¡° In short, the Hard > Reset encapsulates another NOTIFICATION message in its data portion. > > ¡± > It looks one Notification message is embedded into another notification > message? Yes. > Is there any example for that. No, we didn't provide examples in the doc. My feeling was that the usage was clear. If there's consensus that it's not clear without examples we could add some of course, although I would like to see such a consensus before adding what would inevitably be a long examples section into what is now a pretty short doc. > Also it is not clear to me what do you > mean by as appropriate for in section 3.1 last paragraph? Each error code has a list of subcodes that can be used with it. These are documented in the IANA registry mentioned in the paragraph you're talking about. Basically, this just means that the payload of the Hard Reset is itself a properly-formatted NOTIFICATION message. > Would it be great to > add more text to explain the usage of subcode or add reference to point to > section 5 and point 5.2. OK. > 6.Section 4.1, 2nd paragraph I am wondering whether > the proposed procedures and rules are back compatible with the procedure and > rule defined in RFC4724? They're intended to be. In part this is provided by requiring that the N bit be exchanged before these procedures are used (which we discussed above). > 7.Section 5.1 Can you provide definition of Graceful > Cease in the terminology section? I'll do that, or back the term out and use something like "normal cease". Actually I thought I'd provided a definition, I guess I didn't, though. > How Graceful cease is different from Graceful > restart since I am confused. The intention was to draw a distinction between a Hard Reset and all other Cease messages. "Graceful Cease" in this context is supposed to mean "a Cease that isn't a Hard Reset". Thanks, --John
