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

[[e-mail]]

Hi Tom,

Good catch on the address count. I’ll change it in the next update.

Thanks,
Ian

> On 23. Mar 2021, at 12:38, tom petch <[e-mail]> wrote:
> 
> From: [e-mail] <[e-mail]>
> Sent: 17 March 2021 12:57
> 
> Tom,
> 
> Thanks for your review. I’ve just posted -version -19, please see inline below for details on the changes in this version.
> 
> <tp>
> 
> Looks good.
> 
> On the question of auth information, some use string, some use binary, some use hex,  It is a question of what users are used to and how much entropy you can pack into a given space.  So, leave it as it is unless someone demands a change.
> 
> One small point.  In two places you have 
> end-address minus start-address
> for the total number.  Should that be 'plus one'?
> ie is 10.1.0.0 to 10.1.0.255 255 or 256?
> 
> Tom Petch
> 
> 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
>>> 
>