Re: [Int-dir] INTDIR Review of draft-ietf-nvo3-bfd-geneve-12

[Donald Eastlake <[e-mail]>]

Hi Xiao Min,

OK, with the further tweaks below I considera all my comments to have
been resolved.

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

On Thu, Aug 17, 2023 at 2:54 AM <[e-mail]> wrote:
>
> Hi Donald,
>
>
> Thank you for the response.
> Please see inline my comments tagged with [XM-2].
>
> CC'd to [e-mail].
>
> Original
> From: DonaldEastlake <[e-mail]>
> To: 肖敏10093570;
> Cc: [e-mail] <[e-mail]>;[e-mail] <[e-mail]>;[e-mail] <[e-mail]>;[e-mail] <[e-mail]>;
> Date: 2023年08月17日 04:44
> Subject: Re: INTDIR Review of draft-ietf-nvo3-bfd-geneve-12
> Xiao Min,
>
> On Tue, Aug 15, 2023 at 4:30 AM <[e-mail]> wrote:
> >
> > Hi Donald,
> >
> > Thanks for your review and thoughtful comments.
> > Please see inline.
> >
> > Original
> > From: DonaldEastlake <[e-mail]>
> > To: [e-mail] <[e-mail]>;[e-mail] <[e-mail]>;
> > Cc: [e-mail] <[e-mail]>;[e-mail] <[e-mail]>;
> > Date: 2023年08月05日 19:23
> > Subject: INTDIR Review of draft-ietf-nvo3-bfd-geneve-12
> > I am an assigned INT directorate reviewer for
> > <draft-ietf-nvo3-geneve-12.txt>. These comments were written primarily
> > for the benefit of the Internet Area Directors. Document editors and
> > shepherd(s) should treat these comments just like they would treat
> > comments from any other IETF contributors and resolve them along with
> > any other Last Call comments that have been received. For more details
> > on the INT Directorate, see
> > https://datatracker.ietf.org/group/intdir/about/
> > <https://datatracker.ietf.org/group/intdir/about/>.
> >
> > Based on my review, if I was on the IESG I would ballot this document
> > as DISCUSS. I have the following DISCUSS/ABSTAIN level issues:
> >
> > - I do not understand the second half of the last paragraph of Section
> > 1. It says: "BFD for Geneve MUST be used within a TMCE unless BFD is
> > congestion controlled." But then seems to specify that it be
> > congestion controlled inside a TMCE. Would it be simpler to say that
> > BFD for Geneve must always be congestion controlled, if that is what
> > is intended?
> > [XM]>>> The last paragraph of Section 1 was introduced to address comments from Magnus Westerlund in his Tsvart last call review [1].
> >
> > In Magnus's comments, it says
> >
> > "So I think there are two paths forward. Either restrict the applicability of
> > this usage to paths where it is known to have provisioned capacity for the BFD,
> > as noted as required in RFC 5881 applicability statement. The alternative is to
> > extend BFD to actually have a real congestion control."
> >
> > I agree with Magnus that "have provisioned capacity for the BFD" (as required in RFC 5881) and "a real congestion control" are two different things.
> > To avoid confusion, I propose to change the text as below.
> >
> > OLD
> >
> > BFD for Geneve MUST
> > be used within a TMCE unless BFD is congestion controlled.  An
> > operator of a TMCE deploying BFD for Geneve is required to provision
> > the rates at which BFD is transmitted to avoid congestion and false
> > failure detection.
> >
> > NEW
> >
> > BFD for Geneve MUST
> > be used within a TMCE unless BFD is really congestion controlled.  As an alternative to a real congestion control, an
> > operator of a TMCE deploying BFD for Geneve is required to provision
> > the rates at which BFD is transmitted to avoid congestion and false
> > failure detection.
> >
> > [1] https://mailarchive.ietf.org/arch/msg/nvo3/Gps8423YoowFB0lN2UudpLeGxHk/
>
> OK.
>
> > - The wording in Section 4.1 first paragraph seems confusing and
> >
> > incomplete. (I believe this has been covered in other reviews.)
> > [XM]>>> Yes, this has been covered in John's review.
> >
> >
> > - In the first paragraph of Section 6: How can it be that both "Geneve
> > provides security" and "Geneve does not have any inherent security
> > mechanisms" ?
> > [XM]>>> Propose to change the text as below.
> >
> > OLD
> >
> > Geneve provides security between the peers and subject to the issue of overload described below.
> >
> > NEW
> >
> > Geneve (through IPsec, DTLS, or other means) provides security between the peers and subject to the issue of overload described below.
>
> I think you are talking about wrapping Geneve in IPSEC / DTLS / etc. I
> don't see how this is Geneve providing security. It's IPSEC / DTLS /
> etc. providing security for whatever their payload is including where
> that payload is something wrapped in Geneve.
>
> [XM-2]>>> You're right. Let me try again.
>
> OLD
> Geneve provides security between the peers and subject to the issue of overload described below.
> NEW
> The IP underlay network and/or the Geneve option can provide security between the peers, which are subject to the issue of overload described below.
>
>
> > The following are other issues I found with this document that SHOULD
>
> > be corrected before publication:
> >
> > - In section 4, the Inner Ethernet Header MAC addresses are in the
> > wrong order. The Destination MAC comes first, followed by the Source
> > MAC in an Ethernet header, the opposite of IP.
> > [XM]>>> OK. Propose to change the text as below.
> >
> > OLD
> >
> > Inner Ethernet Header:¶
> > Source MAC: MAC address of a VAP of the originating NVE.¶
> > Destination MAC: MAC address of a VAP of the terminating NVE.
> >
> > NEW
> >
> > Inner Ethernet Header:¶
> > Destination MAC: MAC address of a VAP of the terminating NVE.
> > Source MAC: MAC address of a VAP of the originating NVE.¶
>
> OK.
>
> > The following are minor issues (typos, misspelling, minor text
> > improvements) with the document:
> >
> > - Given the prominence of "tunnels" in the one sentence abstract, I
> > think it would be good to use that word in the first paragraph of the
> > Introduction. Possibly: "... an overlay network of tunnels by
> > decoupling ..."
> > [XM]>>> OK. Will make the change as you proposed.
> >
> >
> > - Section 1, last line of first paragraph on page 3: payload -> payloads
> > [XM]>>> I suspect you mean page 2, right?
>
> I'm pretty sure it's page 3:
> OLD
>    Geneve and VXLAN [RFC7348] is that Geneve supports multi-protocol
>    payload and variable length options.
> NEW
>    Geneve and VXLAN [RFC7348] is that Geneve supports multi-protocol
>    payloads and variable length options.
> [XM-2]>>> OK, I see. Will make this change.
>
>
> > - Section 4.1, first paragraph: "Protocol Type" -> "Ethertype"
>
> > [XM]>>> As defined in Geneve Header, this field is called "Protocol Type", do I miss something?
>
> Probably better to maintain consistency.
> [XM-2]>>> In both Section 4.1 and Section 5.1, "Protocol Type" is consistently used, so propose to remain it as is.
>
>
> Cheers,
>
> Xiao Min
>
>
> > - Section 5, last line: that -> when
>
> > [XM]>>> Considering  "Management VNI" is outside the scope of this document, I propose to remove the last sentence, and change SHOULD to MUST.
> >
> > OLD
> >       Virtual Network Identifier (VNI) field SHOULD be set to the VNI
> >       number that the originating VAP is mapped to.  One exception is
> >       that the Management VNI is used.
> >
> > NEW
> >       Virtual Network Identifier (VNI) field MUST be set to the VNI
> >       number that the originating VAP is mapped to.
> >
> > Note that the same change applies to the last paragraph of Section 4.
>
> OK.
>
> > - Section 6, "not low" -> "high"
> > [XM]>>> OK.
>
> Thanks,
> Donald
> ===============================
>  Donald E. Eastlake 3rd   [phone] (cell)
>  2386 Panoramic Circle, Apopka, FL 32703 USA
>  [e-mail]
>
> > Best Regards,
> > Xiao Min
>
>