Re: [dhcwg] I-D Action: draft-ietf-dhc-dhcpv6-yang-18.txt

[[e-mail]]

Tom,

Thanks for your review. I’ve just posted -version -19, please see inline below for details on the changes in this version.

Ian


> On 4. Mar 2021, at 13:39, t petch <[e-mail]> wrote:
> 
> On 02/03/2021 12:40, t petch wrote:
>> Ian
> 
> Ian
> 
> I have now been through all four modules and have the following comments in addition to my last message.  (A bad habit of mine that I seem unable to break is that I never see everything the first time, or the second, or.... but this is all I have so far:-)
> 
> Carrying on with dhcpv6-server, there are parts that I do not understand.  Several relate to threshold as defined in common as an integer percentage but I do not know how it is calculated.
> 
> leaf max-address-count
> maximum number of addresses
> but it is type threshold not a count, a number and how is it calculated? something over (end-address less start address plus one)?

[if - Renamed and description updated:

            leaf max-address-utilization {
              type dhcpv6-common:threshold;
              mandatory true;
              description "Maximum amount of the addresses in the
                pool which can be simultaneously allocated,
                calculated as a percentage of the available
                addresses (end-address minus start-address).";
            }

> 
> leaf-max-pd-space-utilization
> if the pool prefix is /48 and client-prefix-length /56, then how do you arrive at an integer percentage if e.g. 173 prefix out of 256 are in use? round up, round down?

[if - Reworded description as:

            leaf max-pd-space-utilization {
              type dhcpv6-common:threshold;
              mandatory true;
              description "Maximum amount of the prefixes in the
                pool which can be simultaneously allocated,
                calculated as a percentage of the available
                prefixes, rounded up.";
            }
]

> 
> two notification for 'threshold-exceeded' one for address, one for prefix.
> For address, you have three counts, total, max, allocated; how do these relate to the objects defined earlier under address-pools?

[if - Reworked:

  notification address-pool-utilization-threshold-exceeded {
    description "Notification sent when the address pool
      utilization exceeds the threshold configured in
      max-address-utilization.";
    leaf pool-id {
      type leafref {
        path "/dhcpv6-server/network-ranges/network-range/" +
          "address-pools/address-pool/pool-id";
      }
      mandatory true;
      description "Leafref to the address pool that the notification
        is being generated for.";
    }
    leaf total-pool-addresses {
      type uint64;
      mandatory true;
      description "Total number of addresses in the pool
        (end-address minus start-address).";
    }
    leaf max-allocated-addresses {
      type uint64;
      mandatory true;
      description "Maximum number of addresses that can be
        simultaneously allocated from the pool. This value may be
        less than count of total addresses. Calculated as the
        max-address-utilization (percentage) of the
        total-pool-addresses, rounded up.";
    }
    leaf allocated-address-count {
      type uint64;
      mandatory true;
      description "Number of addresses allocated from the pool.";
    }
  }

]

> For prefix, you have max-pd which does point to an earlier definition which is a threshold ie percentage but pd-space-utilization is uint64 and so clearly not a percentage
> I see inconsistency of names and of the objects used in the notifications which so I am confused
> 

[if - Reworked:

  notification prefix-pool-utilization-threshold-exceeded {
    if-feature prefix-delegation;
    description "Notification sent when the prefix pool
      utilization exceeds the threshold configured in
      max-pd-space-utilization.";
    leaf pool-id {
      type leafref {
        path "/dhcpv6-server/network-ranges/network-range/" +
          "prefix-pools/prefix-pool/pool-id";
      }
      mandatory true;
      description "Unique identifier for the pool.";
    }
    leaf total-pool-prefixes {
      type uint64;
      mandatory true;
      description "Total number of prefixes in the pool.";
    }
    leaf max-allocated-prefixes {
      type uint64;
      mandatory true;
      description "Maximum number of prefixes that can be
        simultaneously allocated from the pool. This value may be
        less than count of total prefixes. Calculated as the
        max-precfix-utilization (percentage) of the
        total-pool-prefixes, rounded up.";
    }
    leaf allocated-prefixes-count {
      type uint64;
      mandatory true;
      description "Number of prefixes allocated from the pool.";
    }
  }

]

> invalid-client-detected
> I assume this is the result of a message in which case message type would be useful.

[if - Added an enumeration for the message type]

> 
> delete-address-lease
> should this point to an entry in the database?
> I see that objects named -lease are state data not configuration so I am unclear what is being deleted
> 
[if - Now modelled as:

      leaf lease-address-to-delete {
        type leafref {
          path "../../dhcpv6-server/network-ranges/network-range" +
            "/address-pools/address-pool/active-leases/active-lease" +
            "/leased-address";
        }
]

> delete-prefix-lease
> ditto
[if - Now modelled as:

      leaf lease-prefix-to-delete {
        type leafref {
          path "../../dhcpv6-server/network-ranges/network-range" +
            "/prefix-pools/prefix-pool/active-leases/active-lease" +
            "/leased-prefix";
        }

I’ve also added a leafier in the relay module for the clear-prefix-entry roc:

       leaf lease-prefix {
         type leafref {
           path "/dhcpv6-relay/relay-if/prefix-delegation/pd-leases/" +
           "ia-pd-prefix";
        }

]

> 
> 
> dhcpv6-relay
> 
> feature prefix-delegation
> I think 6.3 a better reference
> 
[if - done]

> leaf link-address
> RFC s.9.1 appears to specify 16 octet, not binary 0..16

[if - now modelled as:

      leaf link-address {
        type string {
          pattern '[0-9a-fA-F]{32}';
        }
]

> 
> notification relay-event
> the description of this would seem to make it applicable to servers as well as relays

[if - The configuration of the implementation specific elements of server configuration (including listening interfaces) is only given as an example module in the appendix. Notifications related to server interface / topology changes would be part of this.]

> 
> clear-prefix-entry
> the reference is now RFC8987 which needs to be in the I-D references; this appears in other reference clauses

[if - updated]

> 
> clear-interface-prefixes
> should the input interface-ref be to part of the DHCP configuration rather than to any interface?

[if - changed to 
          path "../../dhcpv6-relay/relay-if/if-name”;
]

> 
> 
> dhcpv6-client
> 
> list user-class-data
> list plural, entry singular, but I wonder how many AD know their Latin?

[if - The list has been renamed to user-class-data-instances with user-class-data-id and user-class-data.]

> 
> container-ia-na-options
> perhaps client may send, rather than will

[if -changed]

> 
> container-ia-ta-options
> ditto
> 
[if -changed]

> invalid-ia-detected
> I cannot find this in s.18.2.10.1 nor does it seem right to me. This is client module so how does a client find itself to be invalid?
> The description does not help me either!

[if - It was meant to be for address conflicts. I’ve reworded the description as:

  notification invalid-ia-address-detected {
    description "Notification sent when an address received
      in an identity association option can be proved to be invalid.
      Possible conditions include a duplicate or otherwise illegal
      address.";
    reference "RFC 8415: Dynamic Host Configuration Protocol for
      IPv6 (DHCPv6), Section 18.2.10.1”;

The rest of the notification has also been re-worked to contain the rest of the info in the address allocation (timers etc.).]

> 
> MRC-exceeded
> MRD-exceeded
> s.7.6 offers a choice for each of these for different counts/timers so I think that the data notification should be more explicit and a message type would help as it would with many such notification

[if - I’ve reworked the notification as follows:

  notification transmission-failed {
    description "Notification sent when the transmission or
      retransmission of a message fails.";
    reference "RFC 8415: Dynamic Host Configuration Protocol for
      IPv6 (DHCPv6), Section 7.6";
    leaf failure-type {
      type enumeration {
        enum "solicit-timeout" {
          description "Max Solicit timeout value (SOL_MAX_RT)
            exceeded.";
        }
        enum "request-timeout" {
          description "Max Request timeout value (REQ_MAX_RT)
            exceeded.";
        }
        enum "request-retries-exceeded" {
          description "Max Request retry attempts (REC_MAX_RC)
            exceeded.";
        }
        enum "confirm-duration-exceeded" {
          description "Max Confirm duration (CNF_MAX_RD)
            exceeded.";
        }
        enum "renew-timeout" {
          description "Max Renew timeout value (REN_MAX_RT)
            exceeded.";
        }
        enum "rebind-timeout" {
          description "Max Rebind timeout value (REB_MAX_RT)
            exceeded.";
        }
        enum "info-request-timeout" {
          description "Max Information-request timeout value
            (INF_MAX_RT) exceeded.";
        }
        enum "release-retries-exceeded" {
          description "Max Release retry attempts (REL_MAX_RC)
            exceeded.";
        }
        enum "decline-retries-exceeded" {
          description "Max Decline retry attempts (DEC_MAX_RT)
            exceeded.";
        }
      }
      mandatory true;
      description "Description of the failure.";
    }
    leaf description {
      type string;
      description "Information related to the failure, such as
        number of retries and timer values.";
    }
  }

]

> 
> server-duid-changed
> a good case for a notification but I cannot see it described in the RFC

[if - I’ve updated the description and the reference:

  notification server-duid-changed {
    description "Notification sent when the client receives a lease
      from a server with different DUID to the one currently stored
      by the client, e.g. in response to a Rebind message.";
    reference "RFC 8415: Dynamic Host Configuration Protocol for
      IPv6 (DHCPv6), Section 18.2.5”;
]

> 
> Tom Petch
> 
> 
>> I put your last e-mail to me somewhere safe, too safe for me to find
>> just now.  So, I am going through -18 and thoughts so far
>> 
>> I like the treatment of DUID.
>> 
>> IANA status codes needs adding to the I-D references

[if - Done]

>> 
>> server-module (part-reviewed)
>> prefix-delegation
>> 6.3 would seem a better reference than 6.2
>> 
[if -Changed]

>> rapid commit
>> 5.1 might be better than 5.2
>> 
[if - Changed]

>> leaf id
>>  Equivalent to subnet ID
>> I do not understand this, nor do the RFC uses of subnet enlighten me

[If - Description now reads: "Unique identifier for the network range.”]

>> 
>> 
>> common module
>> You have a MUST in the common module which means you should have the
>> RFC8174 text in the module as well

[if - Included]

>> 
>> duid-base
>> why a length of 260?  RFC allows 128 which sort of suggests two-byte
>> character sets!

[If - Replaced the length statement with pattern '^([0-9a-fA-F]{2}){3,130}$’;

The length is 130 to include the 2 octet type as well.
]

>> 
>> between 1 and 128 bytes
>> RFC has octets so I think that better

[if - changed]
>> 
>> duid-unstructured
>> I do not understand what the pattern is doing here

[if - The pattern allows you to define any set of values except ones with 0001-0004 in the DUID-Type field which need to follow the formats for their corresponding defined type type]

>> 
>> OPTION_AUTH
>> the RFC references auth-namespaces so I think that this should too here
>> and in I-D references

[if - Added references]

>> 
>> I note that this uses HMAC-MD5; I do not know how Security ADs view
>> this; it may need a note in the Security Considerations
>> 
[If - I’ve checked RFC8415 and it isn’t mentioned in the Sec. Section there. I’ll add something if it’s flagged at the review stage]

>> auth-information
>> string or binary?

[if - The format of the contents depends on the authentication protocol that’s being used. Is binary a better choice here?]

>> 
>> sub-optiondata
>> again string or binary?  RFC does not help me here
>> I note that there is a two octet length field ie 65535 max

[If - I’ve left it as string and reworked as;

             type string {
            pattern '([0-9a-zA-F]{2}){,65535}’;
]

>> 
>> More to come.
>> 
>> Tom Petch
>>