> On Dec 12, 2023, at 10:23 AM, Greg Mirsky <[email protected]> wrote: > > Hi Russ, > thank you for your comments and suggestions; most helpful. Please find my > notes below tagged GIM>>. I've attached the new working version of the draft > that includes all the updates addressing your concerns. > > Regards, > Greg > > On Mon, Dec 11, 2023 at 1:17 PM Russ Housley via Datatracker > <[email protected] <mailto:[email protected]>> wrote: >> Reviewer: Russ Housley >> Review result: Almost Ready >> >> 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 wait for direction from your >> document shepherd or AD before posting a new version of the draft. >> >> For more information, please see the FAQ at >> <http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>. >> >> Document: draft-ietf-detnet-mpls-oam-13 >> Reviewer: Russ Housley >> Review Date: 2023-12-11 >> IETF LC End Date: 2023-12-19 >> IESG Telechat date: unknown >> >> Summary: Almost Ready >> >> >> Major Concerns: >> >> Section 3.1: In the description of the d-ACH Sequence Number, it says: >> >> ... The originator node >> MUST increase the value of the Sequence Number field by 1 for each >> active OAM packet. >> >> Since the field is 8 bits, the description should also talk about what >> happens when 1 is added to 255. (I assume it wraps.) > GIM>> You're correct, it is circular. Would the following update address your > concern: > OLD TEXT: > Sequence Number - is an unsigned 8-bit field. The sequence number > space is circular with no restriction on the initial value. The > originator DetNet node MUST set the value of the Sequence Number > field before the transmission of a packet. The originator node > MUST increase the value of the Sequence Number field by 1 for each > active OAM packet. > NEW TEXT: > Sequence Number - is an unsigned circular 8-bit field. The > sequence number space is circular with no restriction on the > initial value. The originator DetNet node MUST set the value of > the Sequence Number field before the transmission of a packet. > the initial value SHOULD be random (unpredictable). The > originator node MUST increase the value of the Sequence Number > field by 1 for each active OAM packet.
Yes, that resolves my concern. >> >> Minor Concerns: >> >> General: Based on the Abstract, ACH seems to mean Associated Channel, but >> other places it seems to mean Associated Channel Header. Please be >> consistent. > GIM>> Thank you for pointing this out. I've checked how ACH is extended in > RFC 5085 <https://www.rfc-editor.org/rfc/rfc5085.html>. It is extended as > Associated Channel Header, while Associated Channel is always used without > abbreviation. I've updated the document accordingly. Thanks. >> >> Nits: >> >> Section 1: s/of active and hybrid, as defined in [RFC7799], OAM methods./ >> /of active and hybrid OAM methods, as defined in [RFC7799]./ > GIM>> Thank you, done. Thanks. >> >> Section 2.1: Some terms have a hyphen between the term and the definition. >> Others do not. Please use some separator in all cases. >> >> Figure 3: s/DetNet Associated Channel Header/d-ACH/ > GIM>> Done Thanks. >> >> Section 4.1: I cannot parse this sentence: >> >> The manipulation makes the >> identification of the TSN Stream in the intermittent TSN nodes avoids >> the need to look for the S-Label afterward. >> >> Please reword. > GIMM>> Thank you for raising up this. Please consider the following update: > OLD TEXT: > The first component identifies the DetNet flow (using Clause 6.8 of > [IEEE.802.1CBdb]) and the second component creates” the TSN Stream > via manipulation of the Ethernet header. The manipulation makes the > identification of the TSN Stream in the intermittent TSN nodes avoids > the need to look for the S-Label afterward. > NEW TEXT: > The first component identifies the DetNet flow (using Clause 6.8 of > [IEEE.802.1CBdb]), and the second component creates the TSN Stream by > manipulating the Ethernet header. That manipulation simplifies the > identification of the TSN Stream in the intermediate TSN nodes by > avoiding the need for them to look outside of the Ethernet header. That works for me. Russ
_______________________________________________ Gen-art mailing list [email protected] https://www.ietf.org/mailman/listinfo/gen-art
