[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: diverging views on adding range constraints in MIB modulerevisions
On Thu, 27 Feb 2003, Juergen Schoenwaelder wrote:
> >>>>> C M Heard writes:
> I strongly believe that the WGs have to make the final decision in
> such cases and our role is to explain to them the trade-offs and the
> impacts of their decisions.
As a general rule, I agree with that. If I see something that is
not done in quite the way I think is best, then I will tell the
authors so, but if I think that what they have done will work, then
I won't second guess their design decisions if they choose to stick
to them. And when I think that WG/author decisions are right, I
will defend those decisions to the AD (as Bert well knows).
On the other hand, if I am confronted with something that is clearly
broken, I'll stick to my guns and recommend to the AD that the
module not be published until it is corrected.
> I myself had to change a range of a published MIB object do fix
> a bug that slipped through and I personally believe that such
> fixes when a document recycles at proposed are sometimes much
> more valuable than blindly insisting on SMI rules. (In this
> particular case I had, the SMI correct way would have been to
> deprecate the object and to create a new one. If such on object
> is used as part of an index or being pointed to, even whole
> tables have to be redone. This is hardly the right cost/benefit
> tradeoff.)
Such a change was made at my recommendation in a MIB I reviewed when
it was undergoing revision (SONET-MIB, 1998: in going from RFC 1595
to RFC 2558 the SYNTAX value of sonetPathCurrentStatus was changed
from (1..14), which was inconsistent with its DESCRIPTION, to
(1..62), which agrees with the DESCRIPTION clause). So I think we
agree on this point, too -- at least in principle.
> In the particular case that you have in mind, I think we clearly made
> the MIB authors aware of the impact.
Indeed, that's what I tried to do in my comments on the MIB in
question (it was the printer MIB). And after making my comments I
shut up and have promised not to raise the issues again in last
call.
> I think the whole issue came up because some "pointer" objects
> had a range which was not consistent with the range of things
> the pointers are supposed to point to. The group responsible for
> the MIB module decided that they want to solve this by declaring
> that it is indeed Integer32 (1..65535) what everybody has
> implemented and I think MIB reviewers (and the IESG) should
> accept such decisions.
That's not an accurate description of what I saw and objected to.
If it had been as you describe I would have dropped my objection
once the situation was explained, or if it was obvious to begin with
I would not even have bothered to comment.
The suspicious changes that initially caught my eye were these:
prtOutputIndex, which was an INDEX object, and
prtOutputDefaultIndex, which was a pointer object taking on values
of prtOutputIndex, both had SYNTAX values of Integer32 with no
constraints in RFC 1759. The PWG decided to add range constraints to
both. Obviously, there could be no question if they had chosen
(0..2147483647), as that would just make explicit something that was
already implicit in the definition. But they did not do that. They
chose (1..65535). The reasons they gave were that (a) zero is an
invalid index value, which is simply untrue, and (b) nobody needs a
big index value, which may indeed be true but irrelevant as far as
backward compatibility is concerned. Other indices where they made
a similar range change were prtStorageRefSeqNumber,
prtDeviceRefSeqNumber, and prtConsoleLightIndex (in these cases from
(0..65535) to (1..65535)). The authors did not say so, but this all
appears to me to be "tidying up" to make everything match
prtInputIndex, which did have a range of (1..65535) in RFC 1759. In
my judgement -- with which you may not agree -- "tidying up" is not
a good enough reason to make those changes.
I think that the manufactured example I gave in the e-mail that
started this thread accurately characterizes the nature of the
changes that I found questionable in this case.
> My understanding of the job of a MIB doctor is to _help_ authors to do
> a good job. We should not enforce rules blindly.
I hope it's obvious that I agree with that.
> And we should also try hard to make the review itself a friendly
> act, making sure we try hard to explain things well, in an
> atmosphere that is friendly and such.
And indeed, I certainly agree with that.
> Another issue are turn around times - which are sometimes not as
> good as they should be.
That is also true. The fault is not always that of the MIB reviewer,
though. I've turned around reviews in as little as 48 hours when the
MIB module was exceedingly well-written. A more recent case took me
about two months to turn around because the authors were inexperienced
and made many design mistakes that had to be corrected in order for
the MIB module to be implementable. Reviewing that module and writing
up the comments -- which at the authors' request included specific
recommendations for changes -- took me quite a long time.
> Perhaps we actually need to draft some text
> about MIB reviewer ethics. I am serious.
I'm aware that there are some authors that have not been happy with
the process (cf. Andrew Smith's crack about "MIB quacks"), and
maybe what you are suggesting could help. Although I won't volunteer
to actually draft the text, I'll be happy to provide feedback if you
wish to do so.
One point that is worthwhile to note is that the reviewer's role is
that of advisor to the responsible AD. The reviewer does not have
final say; that's the AD's (and IESG's) job. In my view, my job as
reviewer is to give the responsible AD the best advice that I can,
and if I were to give the OK to broken stuff then I would be
derelict in that duty.
//cmh