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

From: t petch <[email-masked]>
Date: Tue, 2 Mar 2021 12:40:21 +0000

Ian

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

server-module (part-reviewed)
prefix-delegation
6.3 would seem a better reference than 6.2

rapid commit
5.1 might be better than 5.2

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

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

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

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

duid-unstructured
I do not understand what the pattern is doing here

OPTION_AUTH
the RFC references auth-namespaces so I think that this should too here 
and in I-D 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

auth-information
string or binary?

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

More to come.

Tom Petch

On 02/02/2021 11:36, t petch wrote:
> Ian
>
> Looks good, although it will be some time before I digest all the
> detail.
>
> Some admin type thoughts to be going on with.
>
> Authors: the I-D has six, which may or may not be ok with the AD, but
> the YANG modules have five or six or seven which is not ok! Consistency
> please.
>
> Contact in YANG modules must include WG website and e-mail, in the YANG
> modules in Appendices as well.
>
> NMDA or lack thereof needs a mention in Introduction or Abstract
>
> References: the YANG modules  reference
> RFC826
> RFC2464
> RFC4122
> <https://www.iana.org/assignments/dhcpv6-parameters>
> which need to be in I-D Normative References
>
> References in the YANG modules are patchy. You need them, I think, for
> many more leaves, all the timers, all the counts, as RFC and section
> therein, and for IANA Enterprise numbers (the URI)
> Without them I do not know where to look to see if the YANG matches the
> underlying definitions.
>
> IANA Considerations must register the four namespaces
>
> RFC8513 appears in several places, which I find rather telling
>
> s.1 lacks reference for YANG and Netconf and lacks RESTCONF
>
> Options: I would like a list of all the options supported so I do not
> have to reverse engineer the YANG to find them
>
> Abbreviations need expanding on first use
>
> s.2.1
>    *  enabled: Enables/disables the function of the //DHCPv6 /server.
>
> s.3.1
>         leaf rapid-commit {
>           type boolean;
>           description "A value of 1 specifies that the pool supports
> boolean are true or false
>
>         leaf client-duid {
>           type binary;
> for me 'duid' cries out for a YANG type definition
>
> s.3.3
>           defined in [RFC8415] is unsuccessful.";
> looks like markup language which is not allowed in YANG modules
>
> s.3.4
>       typedef timer-seconds32 {
>         type uint32 {
>           range "1..4294967295";
> 4294967295 looks the maximum value which case you can say 'max'
>
> you exclude zero which used to be a valid value for such as T1 and T2
>
>         leaf type-code {
>           type uint16;
>            default 65535;
> why is the default 65535?
>
>           case duid-unstructured {
>           ...
>             leaf data {
>               type binary;
> as above, I think that this should be a type.  Were it binary, I think
> length should be restricted, such as min 3 octet max 128 octet
>
> /description "The replay detection method used/description "The Replay
> Detection Method used/
>
> Tom Petch
>
>
> ----- Original Message -----
> From: <[email-masked]>
> To: <[email-masked]>
> Sent: Friday, January 29, 2021 3:25 PM
> Subject: Re: [dhcwg] I-D Action: draft-ietf-dhc-dhcpv6-yang-17.txt
>
>
>> I've just posted -17 of the draft following discussion on the
> structure on the netmod mailing list. The discussion is here:
>>
>>
> https://mailarchive.ietf.org/arch/msg/netmod/uFNY9XfCcNANQNA34HpLVsF_lU8
> /
>>
>>
>> A number of changes were discussed and have been incorporated in this
> update. These are:
>>
>> * The element specific modules previously had a second module which
> described options relevant to the node. These options definitions have
> been incorporated into the relevant element modules. This means there
> are now total 4 modules in the draft  (instead of 7):
>>
>> Ietf-dhcvp6-common
>> Ietf-dhcvp6-client
>> Ietf-dhcvp6-relay
>> Ietf-dhcvp6-server
>>
>> * Options which are applicable to more than one node are now defined
> in the 'common' module to be imported and used by the relevant elements.
>>
>> * As a result, the identities for each node type is no longer needed,
> so these has been removed.
>>
>> * Additional option definition modules no long use 'RFCXXXX' in their
> naming. Short, descriptive names are used instead.
>>
>> * The appendix example for defining additional option definitions has
> been updated along with the accompanying text.
>>
>> * 'Enable' nodes have been added to the client, relay and server
> modules to enable disable overall function. Client and relay modules
> also have enable nodes for each DHCP interface included.
>>
>>
>> In addition, there are a number of small wording cleanups. Also,  in
> the security section, a bullet point about reconfiguring the
> relay-destination address has been removed. This was a duplicate bullet,
> copied in error under the read-only security descriptions.
>>
>> Thanks,
>> Ian
>>
>> On 29. Jan 2021, at 16:21,
> [email-masked]<mailto:[email-masked]> wrote:
>>
>>
>

---

From: t petch <[email-masked]>
Date: Thu, 4 Mar 2021 12:39:24 +0000

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)?

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?

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?
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

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

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

delete-prefix-lease
ditto

dhcpv6-relay

feature prefix-delegation
I think 6.3 a better reference

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

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

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

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

dhcpv6-client

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

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

container-ia-ta-options
ditto

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!

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

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

Tom Petch

---

From: [email-masked]
Date: Wed, 17 Mar 2021 13:57:39 +0100

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 <[email-masked]> wrote:
> 
> 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
> 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).";
            }

> 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.";
            }
]

> 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]

> 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";
        }
]

> 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";
        }

]

> I think 6.3 a better reference
[if - done]

> 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”;
]

> 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.]

> perhaps client may send, rather than will

[if -changed]

> 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?

[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.";
    }
  }

]

> 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”;
]

>> IANA status codes needs adding to the I-D references

[if - Done]

>> 6.3 would seem a better reference than 6.2
[if -Changed]

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

>> I do not understand this, nor do the RFC uses of subnet enlighten me

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

>> RFC8174 text in the module as well

[if - Included]

>> why a length of 260?  RFC allows 128 which sort of suggests two-byte

[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.
]

>> RFC has octets so I think that better

[if - changed]
>> 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]

>> the RFC references auth-namespaces so I think that this should too here

[if - Added references]

>> 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]

>> string or binary?

[if - The format of the contents depends on the authentication protocol that’s being used. Is binary a better choice 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}’;
]

---

From: tom petch <[email-masked]>
Date: Tue, 23 Mar 2021 11:38:23 +0000

From: [email-masked] <[email-masked]>
Sent: 17 March 2021 12:57

Tom,

<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

[if - Reworked:

]

[if - Reworked:

]

[if - Now modelled as:

[if - Now modelled as:

       leaf lease-prefix {
         type leafref {

]

[if - done]

[if - now modelled as:

      leaf link-address {
        type string {

[if - updated]

[if - changed to

[if -changed]

[if -changed]

]

[if - Done]

[if -Changed]

[if - Changed]

[if - Included]

[if - changed]

[if - Added references]

             type string {

---

From: [email-masked]
Date: Mon, 29 Mar 2021 10:23:24 +0200

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 <[email-masked]> wrote:
>