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* > > >
- [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