Re: [RTG-DIR] Rtgdir early review of draft-ietf-idr-segment-routing-te-policy-18

Ketan Talaulikar <ketant.ietf@gmail.com> Fri, 08 July 2022 14:15 UTC

Return-Path: <ketant.ietf@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 4752EC159482; Fri, 8 Jul 2022 07:15:46 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.107
X-Spam-Level:
X-Spam-Status: No, score=-2.107 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_ZEN_BLOCKED_OPENDNS=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01] 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 3t1CCAN1z6K4; Fri, 8 Jul 2022 07:15:45 -0700 (PDT)
Received: from mail-vs1-xe2b.google.com (mail-vs1-xe2b.google.com [IPv6:2607:f8b0:4864:20::e2b]) (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 609F5C14792F; Fri, 8 Jul 2022 07:15:45 -0700 (PDT)
Received: by mail-vs1-xe2b.google.com with SMTP id a184so12310596vsa.1; Fri, 08 Jul 2022 07:15:45 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=JoobdfwmXaKrD9ip2OrsLHLRBlCNcxfgo2GEIBICStk=; b=Bj9A73oBlDCiwllr/THIocEuTL7nfa/T2GBoHRUkOyoJOJ3G7Hcf8d3d+icbnw/n8F +78iISAmAnLQFCXivHAdxWmniosoPGC3mm5sHJvL3UsfNVPphR8wqjzVuAyvoHy7cqyU JQZrfTSBn5T2CSqwsJmbxp3gHj0+sisT8/AD6JMRxgQ+XqJ8pK+G1K/sO5tlQETHYo0l +kAGLVLaflC1dnLzMmbjGKpRfswf4x45Sy8l/dW1c+3RFnRSCTORIKnsKL2Jli2kxnF7 E13c2Xr31GApbT1QARiuSkAu/7n75evFVDBSii/R6jXAdNGb9Ny7c62L5NVE2VnZoYKv 4ThA==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=JoobdfwmXaKrD9ip2OrsLHLRBlCNcxfgo2GEIBICStk=; b=ZOF+aaVD3abBEungWNRb6CBEu5/UmP7A85FD4oiYwk8RoLlCuQ9vueCqEkj/4gLCTM 65c+HUznc4iN7o2+ALPDe/ptTxfYGD8jxwlTPhAg838VOcVMXqg01M9EESrnYvSB1kXW RvNAd7bRuaPn7XJFhQXt+RwlaWAppoAE9qxNj8ymDnfGfQmaynT3wU26cWuvdHN3C4fI GqvBoHAUyIz2eNYMaRHJgltNPmfL8LcJvCHfsuz6AFRSQWPFeIRENdu/5+F+fd8QLNMe CZmajderdFoKtJZ3+SWh5bMsvuTiDEsQAANVLgEFLBIgVTIQ/AaiwUidS84TtZwU+/Cu tnOA==
X-Gm-Message-State: AJIora9wsyH85oSZnlhLJI7mGNLmNMZ6vlhQCihCM0fwIemRA+jvbNeE JlIfU71IKRv5GvF6zha1T/gfVRQAVwI8j2jky0w=
X-Google-Smtp-Source: AGRyM1tuLxmcM0LZMV7Rye2LEGjKtFgBckAGZ4NllPBCZDUm5KuydvKUbJ+B9+O5Xjo//PpnjjkFjSaVTLyRICOgXyM=
X-Received: by 2002:a05:6102:c07:b0:356:f9cc:b3b6 with SMTP id x7-20020a0561020c0700b00356f9ccb3b6mr1736743vss.34.1657289744265; Fri, 08 Jul 2022 07:15:44 -0700 (PDT)
MIME-Version: 1.0
References: <165728555482.56317.5289542263604707936@ietfa.amsl.com>
In-Reply-To: <165728555482.56317.5289542263604707936@ietfa.amsl.com>
From: Ketan Talaulikar <ketant.ietf@gmail.com>
Date: Fri, 08 Jul 2022 19:45:33 +0530
Message-ID: <CAH6gdPxeiHijpo3apNBXMnzvVtg2-9=mcc-T67-si1RfnggkLw@mail.gmail.com>
To: Mohamed Boucadair <mohamed.boucadair@orange.com>
Cc: rtg-dir@ietf.org, draft-ietf-idr-segment-routing-te-policy.all@ietf.org, "idr@ietf. org" <idr@ietf.org>
Content-Type: multipart/alternative; boundary="0000000000007f33ac05e34bd6bd"
Archived-At: <https://mailarchive.ietf.org/arch/msg/rtg-dir/BdHVLcUY1yF2dRHoh6v4VJ_GyuQ>
Subject: Re: [RTG-DIR] Rtgdir early review of draft-ietf-idr-segment-routing-te-policy-18
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: Fri, 08 Jul 2022 14:15:46 -0000

Hi Mohamed,

Thanks for your very detailed review and helpful comments. We'll work on
them and get back to you with responses and an updated version.

Thanks,
Ketan


On Fri, Jul 8, 2022 at 6:35 PM Mohamed Boucadair via Datatracker <
noreply@ietf.org> wrote:

> Reviewer: Mohamed Boucadair
> Review result: Has Issues
>
> Document: draft-ietf-idr-segment-routing-te-policy-18
> Reviewer: Mohamed Boucadair
> Review Date: 08/07/2022
> IETF LC End Date: N/A
> Intended Status: Standards Track
>
> I appreciate the effort that was spent to progress this draft since more
> than 6
> years!
>
> Before reviewing the document, I started first by re-reading
> RFC8024/RFC9012
> and reading draft-ietf-spring-segment-routing-policy for establishing the
> context. Overall, the approach documented in
> draft-ietf-idr-segment-routing-te-policy is sound and straightforward.
>
> I didn’t find major concerns from a routing standpoint other than the need
> to
> motivate some few claims (see the detailed review file about RRs, for
> example)
> and the lack of considerations related to the handling of the various
> sub-TLVs
> by intermediate routers (if any).
>
> However, there are a number of generic issues that I would recommend to
> consider (see the detailed review for the full list). All these are
> easy-to-fix
> issues.
>
> # General Comments (in no specific order)
>
> ## Consistency
>
> ### Single or multiple paths
>
> There is an apparent inconsistency in the document about the handling of
> multiple paths. For example, Section 1 says :"Selection of the best
> candidate
> path for an SR Policy" while the same section says also “this will result
> in
> one or more candidate paths being installed into ..”.
>
> If multipath is supported, then please add an explicit statement and make
> sure
> the overall text is consistent.
>
> ### Value 0 is marked as reserved for some registries, while that value is
> associated with a meaning for other registries.
>
> Is there any reason why a consistent approach isn’t followed here? what is
> the
> issues if value 0 is open for assignment?
>
> ## Modifications to the format of the Color Extended Community
>
> The text says that you are modifying the format the Color Extended
> Community,
> while this is not true. What this draft does is just associating a meaning
> with
> some bits. I would update the text accordingly.
>
> ## Normative language
>
> The use of the normative language should be double-checked. The most
> apparent
> concern is related the statement related to the handling of the reserved
> bits
> (SHOULD) while this RFC9012 uses MUST (which is correct, IMO).
>
> I tagged many others in the full review, fwiw.
>
> ## Lack of description
>
> Many fields are provided without acceptable description (e.g., “Local IPv4
> Address: a 4-octet IPv4 address.” or “Preference: a 4-octet value” !!).
>
> Also, some fields are provided with a structure but the text says also that
> these are reserved (e.g., 2.4.2 says “TC, S and TTL (Total of 12 bits) are
> RESERVED”).
>
> I wonder whether you can add a statement to say that multiple flags can be
> set
> simultaneously unless this is precluded by future flag assignments.
>
> Last, the document does not include the expected behavior of intermediate
> routers (e.g., whether it is allowed or not to alter some fields). I guess,
> they must not touch the content of the attributes but it is better if this
> is
> explicitly mentioned in the text.
>
> ## Reserved vs. Unassigned
>
> Almost all the “reserved” bits in the spec can be assigned in the future. I
> would use “Unassigned” as per RFC8126.
>
> FWIW, 8126 says the following:
>
>       Unassigned:  Not currently assigned, and available for assignment
>             via documented procedures.
>
>       Reserved:  Not assigned and not available for assignment.
>             Reserved values are held for special uses, such as to extend
>             the namespace when it becomes exhausted.
>
> ## Deprecated values
>
> The document includes notes about some “deprecated” codepoints. I’m not
> sure
> there is a value in having such notes in the final RFC.
>
> ## IANA considerations
>
> ### The document uses a mix of TBD statements (e.g., Section 2.4.3) and
> hard-coded values (early assignments). Not sure what’s was the rationale
> especially that code 20 was assigned but not listed as such.
>
> ### The IANA actions should be more explicit and ask IANA to update
> existing
> entries. For example, the current registry for code 73 points to
> [draft-previdi-idr-segment-routing-te-policy]. Need to update that entry
> and
> similar ones.
>
> ### The document lists (under IANA section) some values that are
> deprecated.
> The document should be clear whether these codes are available for future
> assignment or not.
>
> ### Many sub-TLVs have flag bits but not all of them have a registry to
> track
> future flag bit assignments.
>
> ## Manageability considerations
>
> No such considerations are included in the document.
>
> # Detailed review
>
> FWIW, you can find my full review at:
>
> * pdf:
>
> https://github.com/boucadair/IETF-Drafts-Reviews/raw/master/draft-ietf-idr-segment-routing-te-policy-18-rev%20Med.pdf
> * doc:
>
> https://github.com/boucadair/IETF-Drafts-Reviews/raw/master/draft-ietf-idr-segment-routing-te-policy-18-rev%20Med.doc
>
>
>
>