Address Opsdir review comments

[mjethanandani]

Reviewer: Gyan Mishra Review result: Not Ready

The TCP FSM is of course the most critical component of the transport.

It would be good to list all that is not included in the Yang model that exists in the TCP MIB. Also would be helpful as to reasons why.

I see mentioned that RTO is not part of the Yang model, however to be complete I think it should be included.

The yang model seems to not have all the TCP FSM states listed below: Closed Listen SYN RCVD SYN Sent Established FIN wait 1 FIN wait 2 Closing Time wait Close wait Last Ack

Also the Yang model does not reference the TCP Flag bits set during state changes in the FSM below as well as flag combinations for example for establishment state you sent SYN, receive SYN/ACK, ACK

URG ACK PUSH RESET SYN FIN

I also don’t see anything in the Yang model on TCP window and window scaling and CWIN congestion control algorithm backoff.

Also I don’t see any mention in the Yang model about the well known port range 0-1023 and > 1023 anonymous port range for the TCP socket to be established.

Also mention about the TCP TCB control block.

Local IP Local Port Remote IP Remote Port Interface Process State Local/Send window Remote/Receive window Send SQ Ack Send SQ Un-ack Send SQ Next Not to be sent Receive Next RTT Buffer pointer

[michael-scharf]

This review deals with a number of quite different questions.

I suggest to distinguish them as follows:

1/ A more precise explanation of the diff to the TCP MIB. For some parameters, this is a low-hanging fruit and already partly written in the document. PR #95 improves this description further.

2/ The question whether the TCP connection table should include the TCP state. I don't think the process ID shoulds to be included as this is quite OS-specific, but the TCP state would be doable. The equivalent to the TCP MIB would be:

leaf tcpConnectionState { type enumeration { enum closed { value 1; } enum listen { value 2; } enum synSent { value 3; } enum synReceived { value 4; } enum established { value 5; } enum finWait1 { value 6; } enum finWait2 { value 7; } enum closeWait { value 8; } enum lastAck { value 9; } enum closing { value 10; } enum timeWait { value 11; } enum deleteTCB { value 12; } } config true; description "The state of this TCP connection.

        The value listen(2) is included only for parallelism to the
        old tcpConnTable and should not be used.  A connection in
        LISTEN state should be present in the tcpListenerTable.

        The only value that may be set by a management station is
        deleteTCB(12).  Accordingly, it is appropriate for an agent
        to return a `badValue' response if a management station

attempts to set this object to any other value.

        If a management station sets this object to the value
        deleteTCB(12), then the TCB (as defined in [[RFC793](https://datatracker.ietf.org/doc/html/rfc793)]) of
        the corresponding connection on the managed node is
        deleted, resulting in immediate termination of the
        connection.

        As an implementation-specific option, a RST segment may be
        sent from the managed node to the other TCP endpoint (note,
        however, that RST segments are not sent reliably).";
    }

3/ The TCP MIB includes a list of listeners, which could be included as well. The YANG equivalent to the MIB would be (w/o a process ID):

list tcpListenerEntry {

    key "tcpListenerLocalAddressType tcpListenerLocalAddress
         tcpListenerLocalPort";
    description
     "A conceptual row of the tcpListenerTable containing
      information about a particular TCP listener.";

    leaf tcpListenerLocalAddressType {
      type inet-address:InetAddressType;
      config false;
      description
       "The address type of tcpListenerLocalAddress.  The value
        should be unknown (0) if connection initiations to all
        local IP addresses are accepted.";
    }

    leaf tcpListenerLocalAddress {
      type inet-address:InetAddress;
      config false;
      description
       "The local IP address for this TCP connection.

        The value of this object can be represented in three
        possible ways, depending on the characteristics of the
        listening application:

        1. 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 tcpListenerLocalAddressType
           object being unknown (0).

        2. For an application willing to accept only IPv4 or
           IPv6 datagrams, the value of this object must be
           '0.0.0.0' or '::' respectively, with
           tcpListenerLocalAddressType representing the
           appropriate address type.

        3. For an application which is listening for data
           destined only to a specific IP address, the value
           of this object is the specific local address, with
           tcpListenerLocalAddressType representing the
           appropriate address type.

        As this object is used in the index for the
        tcpListenerTable, implementors should be
        careful not to create entries that would result in OIDs
        with more than 128 subidentifiers; otherwise the information
        cannot be accessed, using SNMPv1, SNMPv2c, or SNMPv3.";
    }

    leaf tcpListenerLocalPort {
      type inet-address:InetPortNumber;
      config false;
      description
       "The local port number for this TCP connection.";
    }

}

4/ A lot of other suggestions (e.g., congestion control) have been discussed in the WG and rejected for a base model. I don't believe that significantly changing the scope of the document makes sense.

In a nutshell, 2/ and 3/ need further analysis.

[mjethanandani]

@michael-scharf , I am going to post a PR for review. I would like you to analyze the model changes.

The tcp-listener-entry is particularly the one to pay attention to.

• The first is the type of addresses. Please check if they are correct.
• Next is the address itself. I have marked it as a union, but the string type has no regex pattern to them. I do not know how to represent an empty octet string.