Hi Pete Appreciate the review, I posted revision 06 now. I made the two editorial changes you suggested. Your suggestion is an improvement, and I agree about the capitalization as well.
When it comes to padding. I want to keep it as is, to minimize packet size. We are not using padding in for instance the pim protocol either. I'm aware that some architectures will not allow accessing an unaligned 16/32/64 bit value, I think I've seen segmentation faults on SPARC, or will take a performance hit. To be safe one may need to for instance use memcpy to copy the values into separate aligned variables. Thanks, Stig On Thu, Jan 13, 2022 at 3:45 PM Pete Resnick via Datatracker <nore...@ietf.org> wrote: > > Reviewer: Pete Resnick > Review result: Ready with Issues > > I am the assigned Gen-ART reviewer for this draft. The General Area > Review Team (Gen-ART) reviews all IETF documents being processed > by the IESG for the IETF Chair. Please treat these comments just > like any other last call comments. > > For more information, please see the FAQ at > > <https://trac.ietf.org/trac/gen/wiki/GenArtfaq>. > > Document: draft-ietf-pim-igmp-mld-extension-05 > Reviewer: Pete Resnick > Review Date: 2022-01-13 > IETF LC End Date: 2022-01-19 > IESG Telechat date: Not scheduled for a telechat > > Summary: One possible minor issue and a couple of nits, but otherwise ready. > > Major issues: > > None. > > Minor issues: > > In section 3 it says, > > There is no alignment or padding. > > Are you sure that implementations are going to work with this? In my old > brain, > there are still memories of trying to read 16-bit values out of an odd-aligned > location caused all sorts of problems. Are you sure you don't want to at least > pad this to even lengths? > > Nits/editorial comments: > > In section 3, this sentence confused me for a moment: > > A previously reserved bit in the IGMPv3 and MLDv2 headers is used to > indicate whether this extension is used. > > I suggest: > > For each of the IGMPv3 and MLDv2 headers, a previously reserved bit > is used to indicate the presence of this extension. > > In section 3: > > When this extension > mechanism is used, the number of Group Records in each Report message > should be kept small enough that the entire message, including any > extension TLVs can fit within the network MTU. > > That "should" looks pretty interoperability-related to me. Perhaps "SHOULD"? > > > > -- > last-call mailing list > last-c...@ietf.org > https://www.ietf.org/mailman/listinfo/last-call _______________________________________________ Gen-art mailing list Gen-art@ietf.org https://www.ietf.org/mailman/listinfo/gen-art
