[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

RE: printermib - problems other than compliances



On Thu, 13 Feb 2003, Wijnen, Bert (Bert) wrote:
> Some quick comments on your issues below:
> 
> 1. I don't think we have consensus between the MIB reviewers 
>    yet [regarding the handling of IANA-maintained MIB modules].
>    If we could drive to consensus on that quickly, then I am
>    willint to push for that consensus.
>
>    I agree that we need to change the copyright statement
>    somewhat [ ... ] But let me bring this to the IESG

I responded to this off-line, and I guess we will see what
results from your discussion with the other IESG members.

> 2. Is a [nit]. If they do a new rev, we will bring it up again.
>    If not, RFC-Editor will fix it.

Agreed.

> 3. I think this is cause by the fact that this was already in
>    RFC1759. Now... it is fair to ask to maybe change something 
>    here, since they make so many (sort of forbidden) changes
>    anyway. But I don't feel we need to push on this.

Here, I strongly disagree.  My issue here is not just that the
MIB module in the draft fails to compile;  in fact, it cannot
be implemented as written.  The object prtAlertIndex

prtAlertIndex OBJECT-TYPE
    -- NOTE: In RFC 1759, the range was not defined.
    SYNTAX     Integer32 (1..2147483647)
    MAX-ACCESS not-accessible
    STATUS     current
    DESCRIPTION
        [ ... ]
    ::= { prtAlertEntry 1 }

is not-accessible.  If it is implemented that way, it can't
be put into the OBJECTS list of the following notification:

printerV2Alert NOTIFICATION-TYPE
    OBJECTS { prtAlertIndex, prtAlertSeverityLevel, prtAlertGroup,
        prtAlertGroupIndex, prtAlertLocation, prtAlertCode }
    STATUS  current
    DESCRIPTION
        "This trap is sent whenever a critical event is added to the
        prtAlertTable.

        NOTE: The prtAlertIndex object was redundantly included in the
        bindings of the 'printerV2Alert' notification in RFC 1759, even
        though the value exists in the instance qualifier of all the
        other bindings.  This object has been retained to provide
        compatiblity with existing RFC 1759 implementaions."
    ::= { printerV2AlertPrefix 1 }

If, as the notification description says, the value of prtAlertIndex
is transmitted in this notification by existing implementations,
then those implementations are treating this object as
accessible-for-notify or read-only.  If, on the other hand, the
description is wrong and existing implementations _don't_ include the
value of prtAlertIndex, then it should not be in the OBJECTS list.
If some implementations do one thing, and others do something else,
then one behaviour needs to be picked as the "right" behaviour and
documented in the specification.

In any case, something needs to be changed to bring the MIB into
alignment with existing practice (or the "right" version of existing
practice, if there is more than one) and (IMnsHO) to make it compile.
Here are the possibilities:

(a) Change the MAX-ACCESS of prtAlertIndex to read-only.  This
is not allowed under the SMIv2 revision rules, and would not be
allowed in a new MIB module, but index objects are allowed by
RFC 2578 to be read-only in modules translated from SMIv1.  A
MIB compiler and other programs in SNMP toolkits therefore have
to accept it.  This is the proper solution (IMnsHO) if the
prevailing practice is to include prtAlertIndex in the notification.

(b) Remove prtAlertIndex from the OBJECTS list.  This also is
not allowed under SMIv2 revision rules, but it's how it would
be done in a new MIB module (and how RFC 1759 should have done
it).  This is the proper solution (IMnsHO) if the prevailing
practice is to exclude prtAlertIndex from the notification.

I'm not recommending to change MAX-ACCESS of prtAlertIndex to
accessible-for-notify because that's not permitted under SMIv2
rules and MIB compilers will complain.  I would consider that
"fix" to be unsatisfactory for a MIB module that is targeted
at the standards track.

It is true that neither of the proposed solutions is strictly
legal under SMIv2 revision rules, but a waiver is in order here
since the situation is a result in a bug in RFC 1759 that was not
caught before publication.  And as you point out, there are many
other changes in the draft that ignore SMIv2 revision rules,
with (IMnsHO) a lot less justification.  Finally, either of the
proposed solutions is a rather easy edit, so the amount of work
required should not be an issue.

> 4. Mmmm.. my latest email understanding with them was that they
>    were (for the most part) going to do away with the Type1/2/3
>    differentiation of TCs. I also had hoped they would have
>    taken your comment into consideration. Their text about the
>    violation of the rule was applicable in an earlier version
>    where they had just renamed and not deprected the old one.
>    Once we told them, they re-introduced the old one as
>    deprecated, and so they could have removed the "violates"
>    text.

That's exactly what I figured, and it's why I made that
comment last time.

> 5. Some 3 or 4 months ago I did go through all those changes
>    in very much detail (took me 2 days). And if we were to
>    take our rules to the "letter of the law" you are correct
>    and we should not accept them. We have made exceptions in
>    the past. We have exceptions aka:
>    - this is a clarification
>    - this is a bug fix
>    - this is what everyone implemented 
>    And so what I did when I reported this all to them is to
>    point out what I thought about the "violation", if I thought
>    it would be trouble, or if it would be kind of acceptable 
>    and so on. They also guaranteed me that all the participants
>    (so basically all the major implementers of this MIB) agree
>    on the changes. So what can we do. Let them do a complete
>    new MIB which nobody implements, or let them get away with
>    the things they feel are OK and are accepted by everyone.
>    Note that they DO RECYCLE at PS.
>    Juergen had suggested back then that they should add comment
>    lines as to where they sort of violated the rules and to
>    describe how and why they did so. SO that reviewers could
>    easily see it and so the during a Last Call it would be clear
>    that we're (the're) not sneaking somthing in under the covers.

I was not objecting to things that are obviously bug fixes.  The
things I'm objecting to are the changes that are not needed to
fix an actual bug (though in some cases the original definition
was not as good as it could have been) and that are not backward
compatible with the original definitions.  Here is one example.
RFC 1759 defined the following index object:

prtOutputIndex OBJECT-TYPE
    SYNTAX     Integer32
    MAX-ACCESS not-accessible
    STATUS     current
    DESCRIPTION
        "A unique value used by this printer to identify this
        output sub-unit. Although these values may change due
        to a major reconfiguration of the sub-unit (e.g.  the
        addition of new output devices to the printer), values
        are expected to remain stable across successive printer
        power cycles."
    ::= { prtOutputEntry 1 }

Although this is not technically illegal, it does merit a fix
because the SYNTAX clause really should exclude negative values.
Now, if all that was done was to add a range of (0..2147483647)
then it would be OK because that could not possibly change any
behaviour over the wire and so would not be a semantic change.
Indeed, our draft MIB review guidelines explicitly say that this
particular change should be allowed.  The draft, however, adds a
much more restrictive range constraint:

prtOutputIndex OBJECT-TYPE
    -- NOTE: In RFC 1759, the range was not defined.
    SYNTAX     Integer32 (1..65535)
    MAX-ACCESS not-accessible
    STATUS     current
    DESCRIPTION
        "A unique value used by this printer to identify this output
        sub-unit. Although these values may change due to a major
        reconfiguration of the sub-unit (e.g.  the addition of new
        output devices to the printer), values SHOULD remain stable
        across successive printer power cycles.

        NOTE: The above description has been modified from RFC 1759
        for clarification."
    ::= { prtOutputEntry 1 }

The problem with this is that an implementation that complied with the
original definition in RFC 1759 will not necessarily comply with this
definition.  In other words, this change is not backward compatible
and therefore constitutes an illegal semantic change.

It is true that RFC 2578 recommends that index values of zero be
reserved for special cases, and that we enforce that in new MIB
modules.  But we don't (or at least I hope we don't) generally
allow IETF WGs to make non-backward-compatible changes just to
comply with the recommendation than zero not be used.

Anyway, as I said, if the other things are fixed I will turn a blind
eye to this stuff.  But that does not mean that I think it is OK to
make chages of this nature when revising standards-track MIB modules,
and I hope we don't make a habit of allowing it.

> So with that, I know that Juergen and Dave Harrington have looked
> at this MIB several times... Can people react on the above.

Yes, please defend this if you think I am wrong.

> Mike.... feel free to post your concerns to the authors of the
> MIB (it may become time-consuming, you are warned) and copy me pls.
> So they can answer your questions and try to address your concerns.
> Maybe better is to try and get some consensus here as to which
> topics really hurt and so that we focus on those and not on some
> minor details.

OK, I've cc'd the authors (and bcc'd the MIB doctors list in case
anyone else wants to join in).

> Again, I do appreciate your miticulous (spelling?) review and
> checking of things.

Bert, for my part, I thank you (and the other MIB doctors) for your
patience with my incessant nagging.

Mike Heard

> Thanks,
> Bert 
> 
> > -----Original Message-----
> > From: C. M. Heard [mailto:heard@pobox.com]
> > Sent: donderdag 13 februari 2003 8:23
> > To: Mreview (E-mail)
> > Subject: RE: printermib - problems other than compliances
> > 
> > 
> > On Tue, 11 Feb 2003, Wijnen, Bert (Bert) wrote:
> > > A new revission 14 has come out (or will today).
> > > 
> > >   draft-ietf-printmib-mib-info-14.txt (PS)
> > >   
> > > Also a new rev for finishing mib will come out tiday.
> > > 
> > >   draft-ietf-printmib-finishing-15.txt (Informational)
> > > 
> > > When they do, I think I am ready to approve them.
> > > If anyone wants to take a quick look and warn me if they 
> > > still see any serious concerns, then pls do so.
> > 
> > I just looked at <draft-ietf-printmib-mib-info-14.txt>.
> > The list of problems is smaller, but not small enough
> > for me to support having it on the standards-track.  If
> > it goes forward to IETF last call in its present form,
> > you may expect negative comments from me.  I've attached
> > a problem report below.  The items on that report I
> > consider it vital to fix are item 1 (IANA module issues)
> > and 3 (MIB compilation error).   Item 2 is a nit that
> > the RFC Editor can take care of (but the authors could
> > do so very easily).  Items 4 and 5 really should be
> > fixed (and I think leaving item 5 unfixed should disqualify
> > the module for standards-track status), but I will
> > pretend that I did not see them if items 1 and 2 are
> > fixed before IETF last call.
> > 
> > BTW, all of these items were on the first report I
> > made on the -13 version, except that some of the
> > specifics of my issues with the IANA-PRINTER-MIB
> > module are different now.
> > 
> > > I had originally intended to push back on the PS for the
> > > Printer-MIB (first doc) and rather see it as informational.
> > > But I did see more support for recycling at PS so probably
> > > will go that way.
> > 
> > Well, I still don't agree with letting all the illegal
> > revisions go by, but it appears that I am overruled.
> > 
[ ... ]
> > 
> > //cmh
> > 
> > Problem report on <draft-ietf-printmib-mib-info-14.txt>
> > 
> > 1.)  The initial version of the IANA-PRINTER-MIB module
> > is not prepared in the generally accepted manner.
> > Specifically, the IANA-PRINTER-MIB module now in
> > Section 5 either needs to go into an appendix with a
> > note to the RFC Editor that it is to be removed upon 
> > ublication when the IANA-maintained module goes on-line
> > (recommended), or else the section in which it resides
> > needs to be clearly marked as non-normative (and
> > preferably moved to an appendix).  In either case
> > there needs to be a normative reference to the URL
> > of the actual IANA-maintained IANA-PRINTER-MIB module
> > (similar to the one for [CHARMIB]) , since that one
> > (not the "base version" that is not in the draft) is
> > what is needed to make the scheme work.  Finally,
> > even if the initial version of IANA-PRINTER-MIB is
> > published in a non-normative section or appendix,
> > the following copyright notice
> > 
> >   Copyright (C) The Internet Society (year).  This
> >   version of this MIB module is part of RFC xxxx;
> >   see the RFC itself for full legal notices.
> > 
> > will be incorrect once IANA has made the first change
> > to the module (since the changed module will no longer
> > be the version published in RFC xxxx).  That needs to
> > be fixed.  I strongly recommend that the module have a
> > stand-alone copyright similar to the one in
> > ftp://ftp.rfc-editor.org/in-notes/mibs/current.mibs/rmon.mib
> > since that will work for all the IANA-modified versions
> > whether or not the initial version appears in an
> > RFC somewhere.
> > 
> > These are serious errors, and in my opinion MUST be fixed.
> > 
> > Note:  similar complaints apply to
> > draft-mcdonald-iana-charset-mib-02.txt
> > 
> > 2.)  There is a reference to [RFC2026] on
> > the front page.  That's a no-no, according
> > to http://www.ietf.org/ID-nits.html
> > 
> > 3.) smilint complains about an inaccessible object
> > used in a notification:
> > 
> > Printer-MIB:3703: [3] {notification-object-access} object 
> > `prtAlertIndex' of notification `printerV2Alert' must not be 
> > `not-accessible'
> > 
> > prtAlertIndex OBJECT-TYPE
> >     -- NOTE: In RFC 1759, the range was not defined.
> >     SYNTAX     Integer32 (1..2147483647)
> >     MAX-ACCESS not-accessible
> >     STATUS     current
> >     DESCRIPTION
> >         [ ... ]
> >     ::= { prtAlertEntry 1 }
> > 
> > [ ... ]
> > 
> > printerV2Alert NOTIFICATION-TYPE
> >     OBJECTS { prtAlertIndex, prtAlertSeverityLevel, prtAlertGroup,
> >         prtAlertGroupIndex, prtAlertLocation, prtAlertCode }
> >     STATUS  current
> >     DESCRIPTION
> >         "This trap is sent whenever a critical event is added to the
> >         prtAlertTable.
> > 
> >         NOTE: The prtAlertIndex object was redundantly included in the
> >         bindings of the 'printerV2Alert' notification in RFC 1759, even
> >         though the value exists in the instance qualifier of all the
> >         other bindings.  This object has been retained to provide
> >         compatiblity with existing RFC 1759 implementaions."
> >     ::= { printerV2AlertPrefix 1 }
> > 
> > In fairness, this was a problem inherited from RFC 1759, and
> > there is no strictly legal way to fix it.  Probably the least
> > evil thing would be to raise the MAX-ACCESS of prtAlertIndex
> > to read-only;  if its presence in the OBJECTS list is needed
> > for compatibility, then previous implementations must have
> > been doing something like that anyway.
> > 
> > In any case, this is a serious error, and in my opinion
> > it MUST be fixed.
> > 
> > 4.)  I see lots of ASN.1 comments that say things that do not
> > seem right.  For instance:
> > 
> > PrtMediaUnitTC ::= TEXTUAL-CONVENTION
> >     -- This is a type 1 enumeration.  Replaces MediaUnit in RFC 1759.
> >     -- Even though it is formally not allowed to change TC names, the WG
> >     -- decided this name change was necessary to clearly indicate this
> >     -- TC is from the Printer MIB.
> > 
> >     STATUS    current
> >     DESCRIPTION
> >         "Units of measure for media dimensions."
> >     SYNTAX    INTEGER {
> >                   tenThousandthsOfInches(3),  -- .0001
> >                   micrometers(4)
> >                   }
> > 
> > MediaUnit ::= TEXTUAL-CONVENTION
> >     -- Replaced by prtMediaUnitTC.
> >     STATUS    deprecated
> >     DESCRIPTION
> >         "Units of measure for media dimensions."
> >     SYNTAX    INTEGER {
> >                   tenThousandthsOfInches(3),  -- .0001
> >                   micrometers(4)
> >                   }
> > 
> > There is actually nothing illegal with what has been
> > done here, i.e., deprecating the old TC and defining
> > a new one.  As far as I can tell, since the new one
> > has the same syntax & semantics as the old one, it can be
> > legally substituted in a SYNTAX clause (if substituting
> > a TC for a base type is legal, then surely one TC for
> > another with same base type and semantics is OK too).
> > So the comments PrtMediaUnitTC are not really
> > correct and should be removed.
> > 
> > 5.)  On the other hand, many of the revisions that have
> > been made clearly are not legal revisions under SMIv2
> > rules.  smidiff lists 112 questionable changes.  Some of
> > them may be OK, but many of the ones that I have checked
> > are not;  I have not checked them all.  An smidiff
> > report will be made available upon request.