Re: [RTG-DIR] Rtgdir early review of draft-ietf-bier-ping-08

Tony Przygienda <tonysietf@gmail.com> Sun, 30 April 2023 17:18 UTC

Return-Path: <tonysietf@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 49177C1519AC; Sun, 30 Apr 2023 10:18:04 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -7.094
X-Spam-Level:
X-Spam-Status: No, score=-7.094 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_HI=-5, RCVD_IN_ZEN_BLOCKED_OPENDNS=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, URIBL_BLOCKED=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 3FJxtQBeuLfD; Sun, 30 Apr 2023 10:18:00 -0700 (PDT)
Received: from mail-ua1-x92b.google.com (mail-ua1-x92b.google.com [IPv6:2607:f8b0:4864:20::92b]) (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 30525C14CF1F; Sun, 30 Apr 2023 10:18:00 -0700 (PDT)
Received: by mail-ua1-x92b.google.com with SMTP id a1e0cc1a2514c-77d03730cdeso283895241.0; Sun, 30 Apr 2023 10:18:00 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1682875079; x=1685467079; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=Wt+AJCX9Og9fm88yOmrm1CrbxsL0DtsuAlFSHJp5dEQ=; b=CgJ29769AUwMWzlc+YQFiiq6x6xQKsi5LiPP7dNI6duJ4CjclbERJaP+pY8vcyY0q+ epIyJQTBUMcsqx6RICL6n64Xgx6d6nL9QVQPtJtU0fTq8u3gMAg+NxVjZ26hLDwXiHiQ ZSZGke6AgYCgukDjw8Jh+x3PTr/ymo7tdyJKSWTevmxc4G/GEVnByJIqXWpCulD9mUam XGERrnMptao6O+8Xte9fWsFOaLSElMlllbxWFP+ETD2UTQBELKI1QWMkUkQV1Tel/Rx7 bcTMYQhFHx9/gi/3JqBtFYEpZaT9j+VmPnyB4u3/ITq1ur8ypgMxC+EZe3fZAjUL60kZ 2BPQ==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1682875079; x=1685467079; 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=Wt+AJCX9Og9fm88yOmrm1CrbxsL0DtsuAlFSHJp5dEQ=; b=a4ibR/lm21OTSMjHqggC6p+o/NpA5LAupXvyCPAsF5BkCHAFYUK0Hz3HQhvs9u8qRT 1ZzjxKMEcEWRtS7wrLnNdBKMtvfpspGgTGy2N1Q6oTgjFjzduHvGHVq3bXtEX5TGMUeo KLsRkRjVXTMabXGl0B0kER7Gn9BLZVgiAeFp51BiU+3AaXgAEfb2Cd6EsvFC6z+U8Iro oAIYcZQW5LREEcGY1ktuBpoO1V+WS1HMxjNk7rZfnptbREysi4NK0yNrznyYOJ/KFAUU OWGDxV1n/GK0jU8kQvYSh992fy0vUv0FOA20gjj2OaIjXRESQUt98yeJAgpR6J1Y/tqb c4cA==
X-Gm-Message-State: AC+VfDytWF+9P1XtiPBSDnLGXRPOqJm9FptOXC0AKCm3K5JpZiGMlFJ+ lF6Iei0ZjZm/WjumxdlS92muohEpD19h4nI6dIKc3nXgQPYx1Q==
X-Google-Smtp-Source: ACHHUZ6t5lVapSh8t0pfai8lG7qzzYSC3Un8Gfkp/cWWBUikpUZg0mtYlgXSR73a72DbdZMw3t0PWIwmtjkXik4Er68=
X-Received: by 2002:a67:f701:0:b0:42e:4cc3:5b4 with SMTP id m1-20020a67f701000000b0042e4cc305b4mr5184211vso.12.1682875078897; Sun, 30 Apr 2023 10:17:58 -0700 (PDT)
MIME-Version: 1.0
References: <168267823708.49129.2910500144998874882@ietfa.amsl.com>
In-Reply-To: <168267823708.49129.2910500144998874882@ietfa.amsl.com>
From: Tony Przygienda <tonysietf@gmail.com>
Date: Sun, 30 Apr 2023 19:17:22 +0200
Message-ID: <CA+wi2hO0P3hRb4o_FGuD2WGX=E5UL2Gbbrg64pVWDZbTfE1XFA@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="00000000000047755f05fa90e393"
Archived-At: <https://mailarchive.ietf.org/arch/msg/rtg-dir/EcAXD4KxGaPBqxPiEOBR5Dh_XWw>
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 17:18:04 -0000

Dhruv, thanks, excellent review and good catches. appreciated. up to
authors/shepherd now ;-)

-- tony

On Fri, Apr 28, 2023 at 12:37 PM 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*
>
>
>