[tcpm] Robert Wilton's No Objection on draft-ietf-tcpm-yang-tcp-07: (with COMMENT)

Robert Wilton via Datatracker <noreply@ietf.org> Thu, 30 June 2022 13:29 UTC

Return-Path: <noreply@ietf.org>
X-Original-To: tcpm@ietf.org
Delivered-To: tcpm@ietfa.amsl.com
Received: from ietfa.amsl.com (localhost [IPv6:::1]) by ietfa.amsl.com (Postfix) with ESMTP id BB4FEC157B5E; Thu, 30 Jun 2022 06:29:14 -0700 (PDT)
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
From: Robert Wilton via Datatracker <noreply@ietf.org>
To: The IESG <iesg@ietf.org>
Cc: draft-ietf-tcpm-yang-tcp@ietf.org, tcpm-chairs@ietf.org, tcpm@ietf.org, nsd.ietf@gmail.com, nsd.ietf@gmail.com
X-Test-IDTracker: no
X-IETF-IDTracker: 8.5.1
Auto-Submitted: auto-generated
Precedence: bulk
Reply-To: Robert Wilton <rwilton@cisco.com>
Message-ID: <165659575476.26185.3417581885981216323@ietfa.amsl.com>
Date: Thu, 30 Jun 2022 06:29:14 -0700
Archived-At: <https://mailarchive.ietf.org/arch/msg/tcpm/2ZpVtYtUcWHP7ZJFJ9viCDWQiYE>
Subject: [tcpm] Robert Wilton's No Objection on draft-ietf-tcpm-yang-tcp-07: (with COMMENT)
X-BeenThere: tcpm@ietf.org
X-Mailman-Version: 2.1.39
List-Id: TCP Maintenance and Minor Extensions Working Group <tcpm.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/tcpm>, <mailto:tcpm-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/tcpm/>
List-Post: <mailto:tcpm@ietf.org>
List-Help: <mailto:tcpm-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/tcpm>, <mailto:tcpm-request@ietf.org?subject=subscribe>
X-List-Received-Date: Thu, 30 Jun 2022 13:29:14 -0000

Robert Wilton has entered the following ballot position for
draft-ietf-tcpm-yang-tcp-07: No Objection

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)


Please refer to https://www.ietf.org/about/groups/iesg/statements/handling-ballot-positions/ 
for more information about how to handle DISCUSS and COMMENT positions.


The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-tcpm-yang-tcp/



----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

Hi,

Thanks for another YANG module!

I have some non-blocking comments, I suspect that many of these have already
been raised/discussed so really just want to check that they had been
proactively considered.

1)
      TCP connection list: Access to status information for all TCP
      connections.  Note, the connection table is modeled as a list that
      is read-writeable, even though a connection cannot be created by
      adding entries to the table.  Similarly, deletion of connections
      from this list is implementation-specific.

I think that it would be helpful to further describe and perhaps constrain the
behaviour here:
 - I would suggest clarifying that the list is read/writable to allow
 configuration to be associated with existing connections, or connections that
 may be subsequently made. - It would be helpful to understand what happens if
 the configuration for a connection changes for an existing connection.  Does
 it force the connection to be closed and reopened?  Or does it depend on the
 configuration change? - I don't think that deleting the configuration
 properties should cause a connection to be closed (although it might cause the
 connection to flap, as per my comment above.)  E.g., for BGP, isn't it the BGP
 neighbour configuration that should control whether a connection logically
 exists?

2) With the ambiguity regarding YANG IP address and zones, can I clarify that
in all cases, the intention is that zoned ip-addresses are allowed where the
inet:ip-address type is used?

3) leaf enable-ao {
I presume that there was a conscious decision not to use a presence container
here (and hence avoid the need for the must statements)?

4) leaf enable-md5 {
I presume that implementations won't need to add extra MD5 specific parameters?
 I.e., it doesn't make sense for this to be a presence container?

5) Connection is closed. Connections in this state
                    may not appear in this list.";

In the operational state tree then what does the tcp/connections list
represent?  Does it contain all connections that either exist or have some
configuration associated with them?

6) I think that leaf "state" (describing the connection state) should be marked
as "config false".  A client should not be able to configure the current
protocol "state".

7)      leaf address {
           type union {
             type string;
             type inet:ip-address;
           }

Given the described behaviour, then I think that you should probably:
 (i) List the inet:ip-address type first.
 (ii) Add a length "0" restriction to the string type.

8)              For an application willing to accept both IPv4 and
                 IPv6 datagrams, the value of this object must be
                 ''h (a zero-length octet-string), with the value
                 of the corresponding 'type' object being
                 unknown (0).

What does the h mean?  I've got a nagging feeling that this is a known
convention ...  I presumed that the WG considered and dismissed the option of
having two separate listeners listed, one for v4 and one for v6 rather than the
union with the string type?

9)
list tcp-listeners {
         key "type address port";
This allows multiple listeners for the same port.  I just wanted to check that
is allowed/expected?

10) container statistics

These counters are global level, so there are not many of them.  Some of these
counters are 32 bits, I presume that there is no reasonable risk that they
would overflow over a reasonable time frame?

Nits:

included in TCP MIB => included in the TCP MIB
TCP-AO TCP-AO [RFC5925] => Duplicate TCP-AO.

Thanks,
Rob