Re: [dhcwg] Benjamin Kaduk's Discuss on draft-ietf-dhc-dhcpv6-yang-24: (with DISCUSS and COMMENT)

[[e-mail]]

Hi Benjamin,

Please see inline below.

Thanks,
Ian

> On 15. Dec 2021, at 01:07, Benjamin Kaduk via Datatracker <[e-mail]> wrote:
> 
> Benjamin Kaduk has entered the following ballot position for
> draft-ietf-dhc-dhcpv6-yang-24: Discuss
> 
> 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/blog/handling-iesg-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-dhc-dhcpv6-yang/
> 
> 
> 
> ----------------------------------------------------------------------
> DISCUSS:
> ----------------------------------------------------------------------
> 
> Roman's comment touched on a related point, but I'd like to (hopefully
> briefly) discuss the way we encode certain opaque protocol fields.
> There are some places where we clearly intend a hex representation (a
> string with a pattern that's marked out as pairs of hex digits), but
> there are others where we just say "type string" with no indication of
> encoding, and even a "type binary".  If we want the specification to
> admit interoperable implementation we need to be more clear about what
> encoding we expect for all of these nodes.  I have noted most (possibly
> all, but please double check) in the COMMENT section, with a preference
> for "type binary" where we don't need to apply regex restrictions.  But
> that's just a personal preference, and choosing to use hex (or base64,
> or any other well-defined) encoding will suffice to resolve the discuss
> point.
> 

[if - In each of the cases that you identified (5 in total), the types have been changed to ‘binary’. I’ve checked through the models and you have identified all of the relevant instances in your review.

Here is a summary of the changes that have been made:


Ietf-dhcpv6-common.yang
----------------------------------
Auth-option-group
This is now modelled with a choice for the protocol type (conf-token or rkap). In both cases the relevant leaves (token-auth-information and auth-info-value respectively) are type binary.

Vendor-specific-information-option-group/
sub-option-data -> binary

Ietf-dhcpv6-relay.yang
———————————————
Interface-id-option-group
interface-id -> binary


ietf-dhcpv6-client.yang
——————————
User-class-option-group
-> user-class-data

Vendor-class-option-group
-> vendor-class-data


For reference, here are the associated review comments:

    grouping auth-option-group {
    [...]
      container auth-option {
      [...]
        leaf auth-information {
          type string;
          description
            "The authentication information, as specified by the
            protocol and algorithm used in this Authentication
            option.";

This should probably be binary, not a string.  Or, if it needs to be a
string, it should specify an encoding (e.g., hex).

------------------


            leaf sub-option-data {
              type string {
                pattern '([0-9a-fA-F]{2}){0,}';
              }
              description
                "The data area for the sub-option.";

Why does this need to be hex-encoded (vs. YANG binary)?
And, if it is hex-encoded, we should probably say so explicitly in the
description.

—————————




Section 4.3

    grouping interface-id-option-group {
      [...]
       leaf interface-id {
          type string;
          description
            "An opaque value of arbitrary length generated by the
            relay agent to identify one of the relay agent's
            interfaces.";

I think this is better as a YANG "binary" type than a string.  If not,
we should say something about the encoding.

--------------------


    grouping user-class-option-group {
      [...]
          leaf user-class-data {
            type string;
            description
              "Opaque field representing a User Class
              of which the client is a member.";

I think this would be better as YANG binary than string.  If we keep it
as string, we need to say what encoding is used.

-------------------

      container vendor-class-option {
        [...]
        list vendor-class-option-instances {
        [...]
          list vendor-class-data-element {
           [...]
            leaf vendor-class-data {
              type string;
              description
                "Opaque field representing a vendor class of which
                the client is a member.";

(likewise here)

-----------------