Re: [netmod] Adam Roach's No Objection on draft-ietf-netmod-entity-07: (with COMMENT)

[Martin Bjorklund <[e-mail]>]

Hi,

Juergen Schoenwaelder <[e-mail]> wrote:
> On Wed, Jan 10, 2018 at 10:55:46PM -0800, Adam Roach wrote:
> > ----------------------------------------------------------------------
> > COMMENT:
> > ----------------------------------------------------------------------
> > 
> > I have one correction and one question about the ietf-hardware YANG module.
> > 
> >          enum exa {
> >            value 14;
> >            description
> >              "Data scaling factor of 10^15.";
> >          }
> >          enum peta {
> >            value 15;
> >            description
> >              "Data scaling factor of 10^18.";
> >          }
> > 
> > I believe this is backwards -- "peta" should be 10^15, while "exa" should be 10^18.
> 
> I agree this is wrong. This bug was most likely inherited from RFC
> 3433 but luckily there is a confirmed errata for RFC 3433. So Martin
> should fix this in the YANG module

Yes, it was inherited from the MIB.  I will fix this in the YANG module.

(and his copy of the MIB module).

Will do.  As it happens, I always just look into the MIBs distributed
by libsmi, and it seems the MIB is not updated there ;-)  Which leads
to an interesting issue - the errata for the MIB not only changes the
description in the comment, but it also changes the *value*.  I will
thus do the same in the YANG module:

      enum peta {
        value 14;
        description
          "Data scaling factor of 10^15.";
      }
      enum exa {
        value 15;
        description
          "Data scaling factor of 10^18.";
      }

This matches the verified MIB Errata, but since the original MIB is
probably present in most distributions, I wouldn't be surprised if
this object is not correctly implemented in real code...  When I
googled for the MIB I found several instances of NON-updated MIBs, and
zero instances of an updated MIB.

> >      typedef sensor-value-precision {
> >        type int32 {
> >          range "-8 .. 9";
> >        }
> > 
> > Why is this an int32 rather than an int8?
> 
> Likely because they way this was defined in the MIB module:
> 
>    SYNTAX Integer32 (-8..9)
> 
> I assume using int8 would be fine as well. (Note that YANG update
> rules allow to expand the range restriction but they do not allow to
> replace int8 with int32; so the range resulting from the type is a
> hard limit, the range restriction is an expandable limit. I guess in
> this case using int8 would be safe but then this is a slight (but
> likely not important) departure from the MIB module.)

I agree.  I will make the change to int8.


/martin