[secdir] Secdir last call review of draft-ietf-lsr-isis-fast-flooding-07

Barry Leiba via Datatracker <noreply@ietf.org> Thu, 14 March 2024 05:21 UTC

Return-Path: <noreply@ietf.org>
X-Original-To: secdir@ietf.org
Delivered-To: secdir@ietfa.amsl.com
Received: from ietfa.amsl.com (localhost [IPv6:::1]) by ietfa.amsl.com (Postfix) with ESMTP id 94DE9C18DB9B; Wed, 13 Mar 2024 22:21:06 -0700 (PDT)
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit
From: Barry Leiba via Datatracker <noreply@ietf.org>
To: secdir@ietf.org
Cc: draft-ietf-lsr-isis-fast-flooding.all@ietf.org, last-call@ietf.org, lsr@ietf.org
X-Test-IDTracker: no
X-IETF-IDTracker: 12.8.0
Auto-Submitted: auto-generated
Precedence: bulk
Message-ID: <171039366659.20498.10089613218127593389@ietfa.amsl.com>
Reply-To: Barry Leiba <barryleiba@computer.org>
Date: Wed, 13 Mar 2024 22:21:06 -0700
Archived-At: <https://mailarchive.ietf.org/arch/msg/secdir/db2YqUWtKn3660Y-JdwI5Ts3cuI>
Subject: [secdir] Secdir last call review of draft-ietf-lsr-isis-fast-flooding-07
X-BeenThere: secdir@ietf.org
X-Mailman-Version: 2.1.39
List-Id: Security Area Directorate <secdir.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/secdir>, <mailto:secdir-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/secdir/>
List-Post: <mailto:secdir@ietf.org>
List-Help: <mailto:secdir-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/secdir>, <mailto:secdir-request@ietf.org?subject=subscribe>
X-List-Received-Date: Thu, 14 Mar 2024 05:21:06 -0000

Reviewer: Barry Leiba
Review result: Has Issues

Only some minor things here:

— Section 3 —

   Although modern implementations have not strictly adhered to the 33
   millisecond interval, it is commonplace for implementations to limit
   the flooding rate to the same order of magnitude similar as the 33 ms
   value.

This sentence seems ungrammatical.  I think I know what you’re saying, so
perhaps this will work?:

NEW
   Although modern implementations have not strictly adhered to the 33
   millisecond interval, it is commonplace for implementations to limit
   the flooding rate to the same order of magnitude: tens of milliseconds,
   and not the single digits or fractions of milliseconds that are needed today.
END

If that’s not quite right, please riff on it as appropriate.

— Section 4 —

   For a parameter which
   has never been advertised, an IS SHOULD use its local default value.
   That value SHOULD be configurable on a per-node basis and MAY be
   configurable on a per-interface basis.

Nit: I think the first SHOULD here ought not to be a BCP 14 key word, and only
the second is.  I would write the first part of the sentence as a fact, and
only have the second be a directive:

NEW
   For a parameter that
   has never been advertised, an IS uses its local default value.
   That value SHOULD be configurable on a per-node basis and MAY be
   configurable on a per-interface basis.
END

— Section 4.4 —

   Length: Indicates the length in octets (1-8) of the Value field.  The
   length SHOULD be the minimum required to send all bits that are set.

The SHOULD seems very odd: what would be a good reason to make it longer than
necessary?  Is there a real reason not to straightforwardly say, “The length is
the minimum required…”?

— Section 6 —

Just a “thanks” comment here: I found Section 6 and its subsections to be clear
and informative.

— Section 8 —

I think the additional implications of having the new TLV have been well
thought out, and I don’t see anything missing.