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

Re: Design pattern



On Thu, 12 Jun 2003, David T. Perkins wrote:
> What do you yall think of the following design pattern found in the
> the MEGACO MIB module.
[ ... ]
> medGwyLinkIdTable OBJECT-TYPE
>        SYNTAX       SEQUENCE OF MedGwyLinkIdEntry
>        MAX-ACCESS   not-accessible
>        STATUS       current
>        DESCRIPTION
>         "This table contains a nextLinkId for each Gateway.
>          It provides the manager with the nextLinkId for use
>          in creating new Gateway Table Entries."
>    ::= { medGwyConfiguration 1}
> 
>    medGwyLinkIdEntry OBJECT-TYPE
>        SYNTAX       MedGwyLinkIdEntry
>        MAX-ACCESS   not-accessible
>        STATUS       current
>        DESCRIPTION
>        "This table contains the NextLinkId for this Gateway
>        and is indexed by mediaGatewayId."
>        INDEX        { medGwyGatewayId }
>    ::= { medGwyLinkIdTable 1 }
> 
>    MedGwyLinkIdEntry ::= SEQUENCE
>    {
>        medGwyNextLinkId                    TestAndIncr         -- rw
>    }
> 
>    medGwyNextLinkId OBJECT-TYPE
>        SYNTAX        TestAndIncr
>        MAX-ACCESS    read-write
>        STATUS        current
>        DESCRIPTION
>         "The Next Value for a MediaGateway LinkId.  Assists the
>          manager in selecting a value for medGwyGatewayLinkId.
>          Using the TestAndIncr syntax, A Manager will 'lock' this
>          variable, ensuring single access."
>    ::= { medGwyLinkIdEntry 1 }

I think this is completely bogus.  A simpler and better
approach would be to make this a r/o variable of type
MediaGatewayLinkId ::= Unsigned32 (1..2147483647), which
is the type of medGwyGatewayLinkId.

The problem is that medGwyNextLinkId is supposed to figure as a
currently unused value for the index object medGwyGatewayLinkId.
Presumably, a manager is supposed to read the current value,
create a row in a config table indexed by medGwyGatewayId and
medGwyGatewayLinkId using the value read from medGwyNextLinkId,
and at the same time increment medGwyNextLinkId by including it
and its current value as a varbind in the set request that does the
row creation. But this second step is not necessary to ensure that
the set only happens once ... the RowStatus variable already does
that.  And it's not sufficient to ensure uniqueness unless the agent
forces rows to be created in sequential order of medGwyGatewayLinkId
value.  Even that is not enough if there is a possibility that
index values may wrap around.  It is better and simpler to make
medGwyNextLinkId a r/o variable of type MediaGatewayLinkId and
have the agent responsible for keeping a currently unused value
in it.  That also solves another problem, namely that the range
of TestAndIncr is 0..2147483647, which does not match that of
medGwyGatewayLinkId.

If the rows of the config table need to have a lock to ensure
single access, then of course a TestAndIncr column would be
a reasonable thing to have.

Mike

P.S.

<rant>
This whole MIB module should be posted with the warning:
here there be dragons!

The problems range from inaccurate comments and DESCRIPTION
clauses, to syntax errors, to inaccessible index objects and
undefined status objects being referenced in notifications,
to use of IpAddress, to style issues like this:

     MediaGatewayId ::= TEXTUAL-CONVENTION
         STATUS      current
         DESCRIPTION
          "Possible Media Gateway Id that can be used to identify
           any media gateway uniquely"
         SYNTAX      INTEGER (1..2147483647)

     MediaGatewayLinkId ::= TEXTUAL-CONVENTION
         STATUS      current
         DESCRIPTION
          "Possible Media Gateway Link Id that can be used to identify
           any media gateway link uniquely"
         SYNTAX      Unsigned32 (1..2147483647)

     MediaGatewayTermId ::= TEXTUAL-CONVENTION
         STATUS      current
         DESCRIPTION
          "Possible Termination Id that can be used to identify
           any media gateway termination uniquely"
         SYNTAX      Unsigned32 (1..2147483647)

(i.e., two Unsigned32's and one INTEGER ... why?)

All of those things except the first (and maybe the last) could
be easily found with a MIB compiler.  I would not mind if someone
said to me:  I am having compile problems with this MIB module,
can you help me fix it?  And I do not mind reviewing the language
and helping people to re-write the DESCRIPTION clauses so that
someone not steeped in the technology can understand them.  But
I sure hate to waste my time on stuff that a simple run thru a
MIB compiler would catch.

The MIB module is now in WG last call.  If the WG last call ends
with no comments I will not be happy ...
</rant>

Sorry for the rant.  But this business of people sending out
half-baked stuff as if it were finished really gets my goat.
And it sure seems that stuff with MIB compilation errors is
very likely to have a number of much more serious problems
lurking ... I think I hear in this an echo of Patrik Fältström's
recent message to the problem-statement mailing list

http://www.alvestrand.no/pipermail/problem-statement/2003-June/002363.html

where he speaks of using ID-nits as a proxy for determining
if a document has had proper review.

//cmh