Re: [Last-Call] Genart last call review of draft-ietf-detnet-mpls-oam-13

[Russ Housley <housley@xxxxxxxxxxxx>]

On Dec 12, 2023, at 10:23 AM, Greg Mirsky <gregimirsky@xxxxxxxxx> 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 <noreply@xxxxxxxx> 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. 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

-- 
last-call mailing list
last-call@xxxxxxxx
https://www.ietf.org/mailman/listinfo/last-call