Re: [Idr] RtgDir review: draft-ietf-idr-bgp-gr-notification-07

["John G. Scudder" <[e-mail]>]

Belatedly closing the loop on this. Comments in line. 

I note we did a WGLC on this about a year ago :-o . I think this revision captures all the change requests that came out of that and from the review. I will follow up to the old WGLC thread, too.

Thanks,

--John

> On Sep 15, 2016, at 4:44 AM, Emmanuel Baccelli <[e-mail]> wrote:
> 
> Hi John,
> 
> see comments inline.
> 
> On Tue, Sep 13, 2016 at 8:43 PM, John G. Scudder <[e-mail] <mailto:[e-mail]>> wrote:
> Hi Emmanuel,
> 
> Thanks for your review. My questions and comments are in line below.
> 
> --John
> 
> 
> 
> > Abstract:
> > - for clarity, append at the end of last sentence "and for force a full
> > reset"?
> 
> Changed (in the source, not yet published as 08 pending this discussion) to "This document also defines a new BGP NOTIFICATION Cease Error subcode whose effect is to request a full session restart instead of a Graceful Restart. "
> 
> 
> Fine with me.
>  
> 
> > - recall that reserved/unspecified fields must be zeroed (and ignored)?
> 
> While this is a good suggestion as general practice, it seems beyond the scope of the present draft. If others disagree and think it's OK for the present draft to introduce clarifications and modifications to RFC 4724 beyond the definition of Notification support, I'd be happy to draft some text. But for now, I'd say it's better to hold this for a 4724bis.
> 
> 
> OK. This was just a suggestion. However, see next comment below.
>  
> 
> > - in Address family flags: remove "deprecated" specification text
> 
> To be clear, you are suggesting this text should be removed?
> 
> "The usage of second most significant bit "N" (which was defined in a
> previous draft version of this specification) is deprecated. This bit MUST
> be advertised as 0 and MUST be ignored upon receipt."
> 
> I am inclined to keep it since as it says, a previous version of the draft did spec such a flag and we have past experience indicating that this can cause problems when early implementations operate with more recent ones.  What's your reason for wanting to remove the text?
> 
> 
> I see. But on the other hand, in the long term, you are writing an RFC, and in my opinion, it is awkward and confusing to have this kind of text and reference to an early draft version in an RFC.
> One solution could be to remove the N bit in the figure and recall that as per RFC4724 all reserved bits must be set to zero?
> The good side is it could fix your problem without referring to "prior versions of the draft".
> The bad side would be that it is not so great to have exactly the same piece of spec duplicated from RFC4724.
> I'm not sure there is a perfect solution here.

On consideration, this doesn't seem as though it's one of the cases where we need to leave a warning for implementors for all time. Although this is sometimes called for when there has been a rush to implement an earlier draft, this doesn't seem to be the case for this one. So, in -09 I've taken a more radical approach to addressing your concern, namely to remove all text related to the AF flags (implicitly: leaving them alone, as per RFC 4724). This has the pleasant effect of further simplifying the document. Thanks!

If anyone objects to this change, please speak up.
 
> > Section 4:
> > - "When a BGP speaker resets its session due to a HOLDTIME expiry, it
> > should generate..."
> >  => s/should/SHOULD
> 
> I am not inclined to use of the all-caps RFC 2119 language since this is not a new requirement imposed by the current specification, it's merely restating an existing requirement. That is, the word "should" is being used in its normal English sense. To elaborate, it's my practice whenever using SHOULD in the RFC 2119 sense, to spend a little effort considering under what circumstances an implementor might ignore the "SHOULD"  and then discuss those in a "MAY"  clause. I think doing that would be outside the scope of this document. If the word "should" causes readers pain, I'm happy to revise the sentence in a way that uses a different word.
> 
> 
> No strong opinion on that. I let specialists talk it over, if needed. Else, let's do what you propose.
>  
> > Section 4.1:
> > - the last paragraph of section 4.2 of RFC4724 states that support for the
> > stale route retain timer is a MAY.
> > It seems appropriate to specify upfront that this timer is now a MUST?
> 
> It wasn't our intention to make the timer mandatory. Is there a reason you think it has to be? (As a practical matter, as far as I know all implementations do support the timer, but I think that's beside the point.)
> 
> 
> When one reads the current spec:
> on one hand, I understand that you MUST do something when this timer fires,
> on the other hand, in RFC4724, this timer MAY be used.
> So one naturally wonders what happens if this timer is not implemented: can it cause problems?

I don't think so, or rather I don't think the present spec changes whether or not problems could occur. To try to clarify the situation, I added the word "optional" in -09: "the optional timer mentioned in the final paragraph of [RFC4724] S. 4.2". Hopefully this helps the reader understand this was a deliberate choice without requiring a long explanation.

> I was thus suggesting that making this timer mandatory would be clearer.
> If all the implementations you know about have this timer, it seems like it's a really good idea to have it anyway.
> But there again, I let specialists talk it over.
> 
> Best,
> 
> Emmanuel
>