Re: [RTG-DIR] Rtgdir early review of draft-ietf-bier-ping-08
Greg Mirsky <gregimirsky@gmail.com> Sun, 30 April 2023 22:02 UTC
Return-Path: <gregimirsky@gmail.com>
X-Original-To: rtg-dir@ietfa.amsl.com
Delivered-To: rtg-dir@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 8AB64C14CEE3; Sun, 30 Apr 2023 15:02:18 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.095
X-Spam-Level:
X-Spam-Status: No, score=-2.095 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_ZEN_BLOCKED_OPENDNS=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, URIBL_DBL_BLOCKED_OPENDNS=0.001, URIBL_ZEN_BLOCKED_OPENDNS=0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.com
Received: from mail.ietf.org ([50.223.129.194]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 3-kvlH-0lBrK; Sun, 30 Apr 2023 15:02:14 -0700 (PDT)
Received: from mail-yw1-x112f.google.com (mail-yw1-x112f.google.com [IPv6:2607:f8b0:4864:20::112f]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id DC767C14CF09; Sun, 30 Apr 2023 15:02:14 -0700 (PDT)
Received: by mail-yw1-x112f.google.com with SMTP id 00721157ae682-559e53d1195so20101837b3.2; Sun, 30 Apr 2023 15:02:14 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1682892134; x=1685484134; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=V4jVnFUDDyzxYENk7lQ/HruZF0fUhQ2y9PtVdyebdjs=; b=qG0HMLzY+uPlW4ORFYHFNn6I9+fUkVWv1Z5BGh5Hm33+pXmVZJaKMFV/hSg1axWOQT YZBZYisKpn4YxZm6tX1e+ANM231V5A+t2AW7fJaThuQ+ot+1DlHtWHr6PLy1F0z4dwGN L7/Vs8I4dvutZex6x1ab1gJgrPLBi2lA/HL3fVDFc32wsEpV+Tq3LcIYRIP6uCdePS7i 8oB5zeyLgewJXIfHf5T0lax4KFluAJzDauxpdAzgeNLA0vG6M9h0TqkY5tTlOHiW84da yyudplH366rlAdflGPkI9q3qPXIrT0fR3ZaAtHXZP54vHulLxv4YftD0Iimk0b/RS5yE B7Rw==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1682892134; x=1685484134; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=V4jVnFUDDyzxYENk7lQ/HruZF0fUhQ2y9PtVdyebdjs=; b=Zvn6idNuWDqyt74URlgULKwk31d1RIS+qFjVjaWu11tATr0WO7S+TRgNlV4DxZDupp FJDM7KCU1ieguD8LLdQu+LjyJ/zXbn2DYNiO6zyhPSt0RmeU9hr/0W2qPjzTuWAIIcOY LuecrsfvG6EOohIyRgqvZeo7kaT33QLQSPqYR6bFrS6dE/iZRpTPDTl9++6GSV/ffa8D KwQyam1EmV3sdsa6ATNMduGx316NlmRTWOPBYZadC7W39d0ugpY3SfhT1dbVHcCnv6oS tIG23+xfRbn0kzmZk6Hqys2ZqHBQZo3Nbf7YQ41hTf7EA/W0S08AGpo6VRgDB7cyspbJ JBUQ==
X-Gm-Message-State: AC+VfDzYv+LEdAYIIZrenLriRo+YKeTbdw7I8AblDxhVgUjzspyieMHt Y37ZjtCxrWfb0K9/+uvUzLo+WYV9NSja070CrZM6+D7F
X-Google-Smtp-Source: ACHHUZ6qdBqwxy/RH6+xDttMEDrlZ6oEnywkJDhgo9rhRgF/UwBdQ34bdaEaHsfG0dH+g4BuS8lSK3X0FqNM33s7Kos=
X-Received: by 2002:a81:5403:0:b0:559:d1ea:8c7a with SMTP id i3-20020a815403000000b00559d1ea8c7amr6645351ywb.1.1682892133818; Sun, 30 Apr 2023 15:02:13 -0700 (PDT)
MIME-Version: 1.0
References: <168267823708.49129.2910500144998874882@ietfa.amsl.com>
In-Reply-To: <168267823708.49129.2910500144998874882@ietfa.amsl.com>
From: Greg Mirsky <gregimirsky@gmail.com>
Date: Sun, 30 Apr 2023 15:02:02 -0700
Message-ID: <CA+RyBmV9pkB82prURKLoNZ4RduWeaXyKFPgySMD9tmpzODHOGg@mail.gmail.com>
To: Dhruv Dhody <dd@dhruvdhody.com>
Cc: rtg-dir@ietf.org, bier@ietf.org, draft-ietf-bier-ping.all@ietf.org
Content-Type: multipart/alternative; boundary="000000000000d4e51405fa94db2b"
Archived-At: <https://mailarchive.ietf.org/arch/msg/rtg-dir/3yXrbLG2eR8Ywn8RCg03dUqBwN4>
Subject: Re: [RTG-DIR] Rtgdir early review of draft-ietf-bier-ping-08
X-BeenThere: rtg-dir@ietf.org
X-Mailman-Version: 2.1.39
Precedence: list
List-Id: Routing Area Directorate <rtg-dir.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/rtg-dir>, <mailto:rtg-dir-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/rtg-dir/>
List-Post: <mailto:rtg-dir@ietf.org>
List-Help: <mailto:rtg-dir-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/rtg-dir>, <mailto:rtg-dir-request@ietf.org?subject=subscribe>
X-List-Received-Date: Sun, 30 Apr 2023 22:02:18 -0000
Hi Dhruv, thank you for your detailed review and thoughtful comments; greatly appreciated. I'm working on updating the document to address Intdir early review <https://mailarchive.ietf.org/arch/msg/bier/TQ_j4T60ZyORX22HcOVqhhwhC7Q/>. I'll share updates with you as you and Brian pointed out similar issues in the draft. And then will continue working to improve the quality of the specification. Best regards, Greg On Fri, Apr 28, 2023 at 3:37 AM Dhruv Dhody via Datatracker < noreply@ietf.org> wrote: > Reviewer: Dhruv Dhody > Review result: Has Issues > > # RTGDIR early review of draft-ietf-bier-ping-08 > > Hello, > > I have been selected to do a routing directorate “early” review of this > draft. > https://datatracker.ietf.org/doc/draft-ietf-bier-ping/ > > The routing directorate will, on request from the working group chair, > perform > an “early” review of a draft before it is submitted for publication to the > IESG. The early review can be performed at any time during the draft’s > lifetime > as a working group document. The purpose of the early review depends on the > stage that the document has reached. As this document is > post-working-group-last-call, my focus for the review was to determine > whether > the document is ready to be published. > > For more information about the Routing Directorate, please see > https://wiki.ietf.org/en/group/rtg/RtgDir > > Document: draft-ietf-bier-ping-08 > Reviewer: Dhruv Dhody > Review Date: 28 April 2023 > Intended Status: Standards Track > > ## Summary: > > I have significant concerns about this document. It needs more work before > being submitted to the IESG. > > ## Comments: > > The need for OAM for BIER is well established, the mechanism is sound but > there > is a need to tighten up the text (as needed for a standards track > document) and > increase the quality of the WG output that is to be considered ready to be > sent > to the IESG for publication. > > The good news is that most of my comments are easy to handle. I wonder why > these were not caught during the normal WG review itself. Interesting to > see > that the WGLC was initiated way back in 2018. > > ### General > > * Request the shepherd to make sure that there is a valid justification > for 6 > authors. > > * The document could benefit from an overview of the OAM operation that > this > document defines before taking a direct jump into the packet formats and > TLVs. > > ### Section 2.1 > > * Consider adding QTF, RTF, MTU, DA, DIA > > ### Section 3 & 3.1 > > * The text in Section 3 states - > > ```` > [RFC8296] defines a 4-bit field as "Proto" to identify the payload > following the BIER header. > ```` > > whereas RFC 8296 says - > > ```` > Proto: > > This 6-bit "Next Protocol" field identifies the type of the > payload. > ```` > > I also see that the BIER OAM message format defined in this I-D is using > 4-bits. Should this not be consistent across BIER documents? - Both number > of > bits and the name! > > * The relationship between the first figure and the second needs to be more > explicit. Moreover, there are the following inconsistencies: > > * Missing OAM Message Length in the 2nd figure > * Initially non-zero 'Proto' values are allowed and then it says it > needs > to be zero. > > * You need to describe the handling of the first Reserved field i.e. it > MUST be > set to zero and ignored on receipt. > > * I also suggest stating clearly the size of each of the fields in the > text. > > * Think about adding some text about version number change as future > guidance. > (refer to RFC 4379 for inspiration) > > * Please add a normative reference for IEEE 1588-2008 (1588v2). > > * I am unsure about - "Any other value MAY be considered as sanity check > failure"; IMHO document should clearly state which timestamp format it > supports, it should have guidance on what the prefered one; it should > allow for > another format to be added in future, thus differentiating between a junk > value > v/s unsupported. Should this not have an IANA registry? > > * The timestamp fields in the figure and text are unclear. Is it to be > considered as two 32 bits fields for seconds and microseconds or a single > 64-bit field? > > * Can QTF and RTF be of different formats? > > ### Section 3.2 > > * What is the reason for missing value 7? If it is intentional, you can > mark it > as unassigned if that is the case. > > * There is no description for DDMAP mismatch. > > ### Section 3.3 > > * Consider adding some text on the handling of padding (if it can exist) > and > its handling in the length field in the TLV. > > * Consider having a table for OAM TLV types defined in this document > upfront. > > * Are multiple instances of TLV allowed? I see text for some but not all. > > * How to handle unknown TLV? > > * Without context it is difficult to appreciate the need for various TLVs > (Original, Target, Incoming, downstream....); Consider adding some > background > text. > > ### Section 3.3.4 > > * Add description for MTU. Just MTU is not specific enough. What is MTU in > BIER > context? > > * Add text for Rsvd - MUST be set to zero when sending and ignored on > receipt. > (please check this at all places) > > * Why is the Sub-TLV type not being encoded? If we call it a TLV we should > encode the T part of the TLV! I suggest both TLV and sub-TLV follow the > same > format as defined in Section 3.3. Or am I not understanding the meaning of > Sub-tlv Length (which anyway is not described in the text)? > > * Can both sub-TLVs be carried at the same time? > > ### Section 3.3.5 > > * Please mention the length field value. > > ### Section 3.4 > > * The text "This encoding is similar..." is unclear, is it the same or is > there > any difference because of entropy? If there are differences that need to be > stated. > > ### Section 4.1 > > * Consider adding references here for Router Alert, and TTL for BIER-MPLS. > > ### Section 4.2 > > * In this text - > > ```` > BIER > OAM MUST support ECMP path discovery between a BFIR and a given BFER > and MUST support path validation and failure detection of any > particular ECMP path between BFIR and BFER. > ```` > > Is this stating a requirement for BIER OAM solution to fulfill or are you > stating that an implementation MUST support it? If it is a requirement it > does > not make much sense here! > > ### Section 4.3 > > * Can the Sequence Number wrap? Why SHOULD for increment? > > ### Section 4.5 > > * Note that the 'MUST not' is not accepted usage as per RFC 2119. Please > change > that to MUST NOT in this paragraph: > > ```` > If Reply-Flag=0, BFR MUST release the variables and MUST not send > any response to the Initiator. If Reply-Flag=1, proceed as below: > ```` > > ### Section 5 > > * My suggestion would be to identify the registry. For the UDP Port, it > might > be useful and explicit to provide the other fields like service-name and > description as per the registry - > > https://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.xhtml > > * The allocation policy for the registry is missing. It is left for IANA to > figure out what are the fields to be maintained in each of the registries. > > * For section 5.3, please state that this is a new sub-registry and clearly > state its name. You state that usage is not limited to OAM, then does it > belong > in "BIER OAM Parameters"? I am also wondering about this mixed registry for > type and sub-type with a common value field - not sure if this is better as > compared to separate sub-registries for TLV and sub-TLV space. > > ### Section 6 > > * I would guess that the surface area for attack is higher because of BIER > which should be highlighted. Is that not the case? > > ## Nits: > > * Expand BIER in the document title, that is the usual practice in > published > BIER RFCs > > * Expand OAM in the Abstract as well. You expand it in the Introduction. > > * Your Abstract and Introduction are almost the same. IMHO they serve a > different purpose and thus consider having another look. If the intention > is to > keep them the same then consider making them exactly the same (currently > you > have this extra text in the Introduction - "without any dependency on other > layers like the IP layer."). > > * Consider adding figure numbers and titles > > * Section 3.1; Better to be explicit here and say UDP header. > > ``` > When the > Reply mode is set to 2, the Responder BFR encapsulates the Echo reply > payload with the IP header. > ``` > > * Section 3.3.1; for BS Len, RFC 8296 uses the term BSL. Consider being > consistent. I also see BitStringLen. > > * Add a "~" at the start of the field "BitString (last 32 bits)" -- > this > issue exists at multiple places. > > * Multipath encoding is used only in 3.3.4.1.1, why not just move section > 3.4 > there? > > * Some of the TLV has "TLV Type=" in the figure, please just use Type= for > consistency. > > * Section 4.4; In "... and the BIER header to BIER-Header."; I think you > mean > Header-H to BIER Header? > > * I don't think you intend to thank "DIA Length - Downstream Interface > Address > field Length" in the Acknowledgement :) > > * Please check the HTML version and you will see various formatting issues > with > figures, consider fixing them now itself. > > Thanks! > Dhruv > > --- > > *In case of bad formatting, refer to this message at - > https://notes.ietf.org/draft-ietf-bier-ping?view* > > >
- [RTG-DIR] Rtgdir early review of draft-ietf-bier-… Dhruv Dhody via Datatracker
- Re: [RTG-DIR] Rtgdir early review of draft-ietf-b… Tony Przygienda
- Re: [RTG-DIR] Rtgdir early review of draft-ietf-b… Greg Mirsky
- Re: [RTG-DIR] Rtgdir early review of draft-ietf-b… Greg Mirsky
- Re: [RTG-DIR] Rtgdir early review of draft-ietf-b… Greg Mirsky
- Re: [RTG-DIR] [Bier] Rtgdir early review of draft… Dhruv Dhody
- [RTG-DIR]Re: [Bier] Rtgdir early review of draft-… Greg Mirsky