Hi Xiao Min, Thanks for your reply. Some further comments in line below.
> On Aug 4, 2023, at 4:14 AM, xiao.m...@zte.com.cn wrote: > > > Hi John, > > > > Thank you for the review and thoughtful comments. > > Please see inline. > > Original > From: JohnScudderviaDatatracker <nore...@ietf.org> > To: The IESG <i...@ietf.org>; > Cc: draft-ietf-nvo3-bfd-gen...@ietf.org > <draft-ietf-nvo3-bfd-gen...@ietf.org>;nvo3-cha...@ietf.org > <nvo3-cha...@ietf.org>;nvo3@ietf.org <nvo3@ietf.org>;matthew.bo...@nokia.com > <matthew.bo...@nokia.com>;matthew.bo...@nokia.com <matthew.bo...@nokia.com>; > Date: 2023年08月04日 03:40 > Subject: John Scudder's Discuss on draft-ietf-nvo3-bfd-geneve-12: (with > DISCUSS and COMMENT) > John Scudder has entered the following ballot position for > draft-ietf-nvo3-bfd-geneve-12: Discuss > > When responding, please keep the subject line intact and reply to all > email addresses included in the To and CC lines. (Feel free to cut this > introductory paragraph, however.) > > > Please refer to > https://www.ietf.org/about/groups/iesg/statements/handling-ballot-positions/ > for more information about how to handle DISCUSS and COMMENT positions. > > > The document, along with other ballot positions, can be found here: > https://datatracker.ietf.org/doc/draft-ietf-nvo3-bfd-geneve/ > > > > ---------------------------------------------------------------------- > DISCUSS: > ---------------------------------------------------------------------- > > Thanks for this valuable and easy-to-read spec. I have one concern that I'd > like to have a discussion about; I hope this will be easy to resolve. > [XM]>>> I believe so. :-) > > > > There are several places where you use MUST in a way I think is unnecessary, > you seem to be saying, in effect, "to do Geneve you MUST do Geneve". Most of > these are harmless IMO (I put some examples in the COMMENT section just in > case > you're unclear what I'm talking about) but there are two that seem problematic > to me, nearly-identical sentences from Sections 4.1 and 5.1: > > Then the Destination IP, the UDP destination port and > the TTL or Hop Limit of the inner IP packet MUST be validated to > determine whether the received packet can be processed by BFD. > > and > > Then the UDP destination port and the TTL or Hop Limit > of the inner IP packet MUST be validated to determine whether the > received packet can be processed by BFD. > > In both cases, it's unclear to me if you're just saying "Geneve has certain > validation rules that have to be met before the packet can be passed to the > upper layer", or if you're introducing a new requirement. In the former case, > please be more transparent about that, possibly with a citation to the > validation rules in the underlying spec. You could also consider dropping the > RFC 2119 MUST. In the latter case, if you're truly introducing a new > requirement, I think the validation rules need to be spelled out much more > clearly. (I think it's probably the former case.) > [XM]>>> It seems a mix of the two cases, and for the former case the > underlying spec is RFC 5881. > > Propose to change the text as below. > > OLD > > Then the Destination IP, the UDP destination port and > the TTL or Hop Limit of the inner IP packet MUST be validated to > determine whether the received packet can be processed by BFD. > > and > > Then the UDP destination port and the TTL or Hop Limit > of the inner IP packet MUST be validated to determine whether the > received packet can be processed by BFD. > > > > NEW > > Then the Destination IP, the UDP destination port and > the TTL or Hop Limit of the inner IP packet MUST be validated to > determine whether the received packet can be processed by BFD, > i.e., the three field values of the inner IP packet MUST be in compliance > with what's defined in Section 4. Destination address and TTL are easy for me to work out from Section 4: - Destination IP: IP address of a VAP of the terminating NVE. If the VAP of the terminating NVE has no IP address, then the IP address 127.0.0.1 for IPv4 or ::1/128 for IPv6 MUST be used. - TTL or Hop Limit: MUST be set to 255 Port isn’t explicitly mentioned in Section 4, but I guess this is the relevant paragraph: The fields of the UDP header and the BFD Control packet are encoded as specified in [RFC5881]. When I look in RFC 5881, I think the relevant requirement is in Section 4: BFD Control packets MUST be transmitted in UDP packets with destination port 3784, within an IPv4 or IPv6 packet. Have I correctly identified all the requirements you’re looking to codify? If so, I think this text is good enough to clear the Discuss, although I would appreciate if the RFC 5881 reference included the section number to minimize inconvenience and ambiguity for the reader. > and > > Then the UDP destination port and the TTL or Hop Limit > of the inner IP packet MUST be validated to determine whether the > received packet can be processed by BFD, i.e., the two field values of the > inner IP packet MUST be in compliance with what's defined in Section 5. So: - TTL or Hop Limit: MUST be set to 255 And The fields of the UDP header and the BFD Control packet are encoded as specified in [RFC5881]. With the same question if I’ve understood correctly, the same redirect into RFC 5881 Section 4, and the same suggestion to cite the section specifically. > ---------------------------------------------------------------------- > COMMENT: > ---------------------------------------------------------------------- > > - Please consider expanding "FCS" where used, or glossing it elsewhere. > [XM]>>> OK. Will add "FCS" to the Abbreviations section. > > > - This sentence in Sections 4.1 and 5.1, > > > The Destination MAC of the inner Ethernet > frame matches the MAC address of a VAP which is mapped to the same as > received VNI. > > has a grammatical problem that prevents me from making sense of it. I *think* > you are missing a noun after "the same", so it should be something like "The > Destination MAC _address_ of the inner Ethernet frame matches the MAC address > of a VAP which is mapped to the same _???_ as _the_ received VNI." > > Or maybe some other rewrite is needed, but anyway, it's not clear as it > stands. > [XM]>>> Propose to change this sentence as below. > > OLD > > The Destination MAC of the inner Ethernet > frame matches the MAC address of a VAP which is mapped to the same as > received VNI. > > NEW > > The Destination MAC address of the inner > Ethernet > frame matches the MAC address of a VAP which is mapped to the same VNI as > the > received VNI. Looks good. > - Here are a few examples where I think you have MUSTs that may be > unnecessary, > > as referenced in my DISCUSS. I don't insist on any changes related to these, > I'm just providing them for your information. > > The Outer > IP/UDP and Geneve headers MUST be encoded by the sender as defined in > [RFC8926]. > > ("MUST be" could be "are"; occurs 2x in the doc) > [XM]>>> Will do s/MUST be/are. Cool. > Opt Len field MUST be set consistent with the Geneve specification > > > (This could be "The usage of the Opt Len field is specified in [RFC8926], and > depends on whether or not", etc; occurs 2x in the doc) > [XM]>>> In this case I prefer to remain it as is, because MUST/SHOULD is also > used for other fields of the Geneve header. OK. > Once a packet is received, the NVE MUST validate the packet as > > described in [RFC8926]. > > (Could be "... the NVE validates..."; occurs 2x in the doc) > [XM]>>> Will do s/MUST validate/validates. > > Best Regards, > > Xiao Min Regards, —John _______________________________________________ nvo3 mailing list nvo3@ietf.org https://www.ietf.org/mailman/listinfo/nvo3
