Re: [bess] RtgDir Early review: draft-ietf-bess-evpn-irb-extended-mobility-10.txt

[Donald Eastlake <[e-mail]>]

Hi Neeraj,

Yes, version -13 looks good.

Thanks,
Donald
===============================
 Donald E. Eastlake 3rd   [phone] (cell)
 2386 Panoramic Circle, Apopka, FL 32703 USA
 [e-mail]

On Mon, Aug 21, 2023 at 1:21 PM Neeraj Malhotra (nmalhotr)
<[e-mail]> wrote:
>
>
>
> Hi Donald,
>
>
>
> Thanks again for the additional points. I have addressed all of the additional comments below in the latest rev13. Please do let me know if there is any additional input before we can move further.
>
>
>
> Thanks,
>
> Neeraj
>
>
>
> From: Donald Eastlake <[e-mail]>
> Date: Wednesday, August 16, 2023 at 10:53 AM
> To: Neeraj Malhotra (nmalhotr) <[e-mail]>
> Cc: [e-mail] <[e-mail]>, [e-mail] <[e-mail]>, [e-mail] <[e-mail]>, BESS <[e-mail]>
> Subject: Re: RtgDir Early review: draft-ietf-bess-evpn-irb-extended-mobility-10.txt
>
> Hi Neeraj,
>
>
>
> Sorry for the delay in responding.
>
>
>
> Generally my comments have been incorporated and, I think, all my issues addressed. However, some problems were introduced by the changes and there are some nits:
>
> - The RFC 2119 & 8174 boilerplate language should presumably be in Section 2 but has disappeared from the draft.
>
> - Square bracketed references are not allowed in the Abstract. You need to change those in the Abstract back to just "RFC 7432" or perhaps "RFC 7432bis".
>
> - Although in my comments I said the draft should be shown as updating "RFC 7432", the Updates: line on the title page takes only numbers so it should just say "7432".
>
>
>
> Also, the references to RFC 826 and RFC 4861 need the space removed at the square bracketed reference in the text body and to be added to the References section
>
> and you should update references:
>
> draft-ietf-bess-evpn-inter-subnet-forwarding -> RFC 9135
>
> draft-ietf-bess-evpn-proxy-arp-nd -> 9161
>
>
>
> I have no idea if the reason you state will be good enough for the IESG to allow >5 authors.
>
>
>
> Thanks,
> Donald
> ===============================
>  Donald E. Eastlake 3rd   [phone] (cell)
>  2386 Panoramic Circle, Apopka, FL 32703 USA
>  [e-mail]
>
>
>
>
>
> On Tue, Aug 15, 2023 at 4:00 PM Neeraj Malhotra (nmalhotr) <[e-mail]> wrote:
>
>
>
> Hi Donald,
>
>
>
> Rev12 of the draft also takes care of the missing note with respect to six authors on the draft. Please do let me know if there is anything else beyond the earlier review comments that needs to be addressed.
>
>
>
> If not, would like to request on behalf of all authors to move this forward.
>
>
>
> Thanks,
>
> Neeraj
>
>
>
> From: Neeraj Malhotra (nmalhotr) <[e-mail]>
> Date: Tuesday, August 1, 2023 at 4:23 PM
> To: Donald Eastlake <[e-mail]>, [e-mail] <[e-mail]>, [e-mail] <[e-mail]>
> Cc: [e-mail] <[e-mail]>, BESS <[e-mail]>
> Subject: Re: RtgDir Early review: draft-ietf-bess-evpn-irb-extended-mobility-10.txt
>
>
>
> Hi Donald,
>
>
>
> Many thanks for the details review and comments. I have published version 11 of the document that incorporates all of your comments. Please also see inline below for some additional clarifications.
>
>
>
> This document repeatedly says that it may be considered a clarification of RFC 7432. I believe it is true that the behavior specified in this document is permitted by RFC 7432 but other behaviors are permitted and perhaps common. In order to handle the mobility cases covered in this document the behaviors in the document would have to be implemented or some other solution adopted. Thus I think the title page header should show this document as updating RFC 7432 and this should be mentioned in the Abstract and Introduction.
>
>
>
> [NM]: Ack. I have updated the text in the abstract and introduction sections to say that this document updates sequence number procedures defined in [RFC7432] in addition to the title page header.
>
>
>
> It seems to me that the last paragraph of Section 7.2 ignores the case where Mx-IPx with sequence number N movez to Mz-IPx where child IP-MACs under Mz were currently being advertised with sequence number M where M > N. The paragraph says the new Mz sequence number must be incremented to N+1 but if M>N I think it must be incremented to M+1. I have suggested changes to the last two paragraphs of Section 7.2 in the attached.
>
>
>
> [NM]: that’s a really good catch. A later section (8) does cover this but the example in section 7.2 was missing this condition. Updated.
>
>
>
> Drafts should generally be worded so the text will be correct in the final RFC. So both occurrences of "proposed" in this draft should be replaced by "specified" or "defined" and occurrences of "draft" in the body text should be replaced with "document".
>
>
>
> [NM] updated.
>
>
>
> Section 2.1 lists subsequent sections as Informative or Normative but omits Sections 3 and 7. I think Section 3 is Informative. The right category for Section 7 is a bit unclear but I'm inclined towards normative.
>
>
>
> [NM]: Updated as above – except that I am also a bit unclear if section 7 should be listed as normative as it is doing some ground work for the specifications in subsequent normative sections using some examples but is not meant to provide a complete specification. I have left it out for now, but happy to include it as normative if needed.
>
>
>
> Section 10.2 refers to section 6.1 but there isn't any section 6.1. The bullet point in Section 10.2 seems essentially incomplete: What "MUST be higher than the "Mz" sequence number"?
>
> In the last sentence of Section 4.3.1, it is not completely clear what "It" refers to. Assuming it is the interpretation in the previous sentence, I suggest "It could be interpreted as" -> "This interpretation could be considered".
>
> "GW devices" occurs only once in this document in Section 2 and GW is never expanded. I suggest, assuming this is correct, that the phrase be replaced with "PE devices".
>
> In section 9, since it is not expanded and not listed in the glossary, I think "EXT-COMM" -> "Extended Community".
>
> Based on the usual order of RFC Sections and the RFC Editor's recommended table of contents, I think Sections 1 and 2 should be swapped.
>
> The requirements language boilerplate at the beginning of Section 1 needs to be updated to the latest version also normatively referencing RFC 8174.
>
>
>
> [NM]: incorporated all of the above comments and all inline changes in the marked-up document.
>
>
>
> The document references RFC 7432 but I think it should reference the rfc7432bis draft instead.
>
>
>
> [NM]: updated the reference link to point to RFC7432bis.
>
>
>
> I am doubtful that there are truly no new security considerations. At a minimum, I would think the Security Consideration section (section 11) should refer readers to the Security Considerations sections of [EVPN-IRB] and rfc7432bis and should state that the methods specified in this document will increase the consumption of sequence numbers.
>
>
>
> [NM]: added security section.
>
>
>
> RFCs are generally limited to a maximum of five authors. This document lists six but does say why it needs to list that many. This could be in a first page note to be deleted before publication.
>
>
>
> [NM]: missed adding this – let me wait a couple of days in case there are any additional comments and if not, update this by end of this week.
>
>
>
> Nits:
>
> Abstract: "Procedure to handle host mobility" -> "The procedure to handle host mobility"
>
> Section 2, first sentence: "EVPN-IRB enables capability ..." -> "EVPN-IRB enables the capability ...
>
> Section 2: "Purpose of this draft is to define additional ..." => "This document defines additional ... "
>
> Section 4.3.1: "Complication with this ..." -> "The complication with this ..."
>
> Section 8.8: "This sections is to be treated as optional ..." -> "This section is optional ..."
>
> Although the above stuck out a bit more to me, there are many other nits including some spelling typos and a duplicated word, so I went through marking what I consider to be fixes and these are shown in the attached.
>
>
>
> [NM]: incorporated all of the above comments and all inline changes in the marked-up document.
>
>
>
> I hope this review is helpful.
>
>
>
> [NM]: absolutely, a much cleaner read.
>
>
>
> Thanks,
>
> Neeraj