Subject: Re: [MIB-DOCTORS] MIB Doctor review of draft-ietf-sidr-rpki-rtr-protocol-mib-04

From: Michael Baer <[email-masked]>
Date: Thu, 7 Feb 2013 13:25:34 -0800

Hi David,

After discussion with the other editors, I just submitted an updated
rfc.  I have listed changes from your comments inline below.

>>>>> On Tue, 15 Jan 2013 10:39:39 -0500, David Harrington <[email-masked]> said:

    DH> This review is officially done for the OPS AD, and the AD needs
    DH> to decide which points must be addressed.

    DH> Most of the comments should be non-controversial and easy to
    DH> address.

    DH> There are a couple syntax questions.

    DH> Most technical questions are about ensuring clear and
    DH> unambiguous descriptions to ensure interoperability.

    DH> Is LAST-UPDATED "201110140000Z" still correct?

updated 

    DH> We've crossed a year boundary, so the 2012 copyright might need
    DH> updating.

updated 

    DH> RpkiRtrConnectionType/other: s/non/none

fixed 

    DH> rpkiRtrCacheServerConnectionType: to prevent any ambiguity, I
    DH> think it might be better to describe this only as the connection
    DH> type, and remove the "or transport security suite" from the
    DH> object description. The fact that the connection type might be a
    DH> transport security suite can be spelled out in the connection
    DH> type T-C. Even there, I would be careful of ambiguity, to
    DH> provide clear guidance/constraints to future developers of added
    DH> enumerations.

removed

    DH> rpkiRtrCacheServerPreference: would a DEFVAL be a good idea to
    DH> specify the 255 default?

Object was changed to having no limits on size (other than inherit max
for Unsigned32) and DEFVAL added as max of Unsigned32.

    DH> rpkiRtrCacheServerTimeToRefresh could benefit from a
    DH> reference. Is the time to refresh always decremented by 1, or
    DH> can an implementation choose to only reflect time to refresh in,
    DH> say, 5 or 10 second increments? Is there a maximum negative
    DH> value that makes sense here? Is it possible to require a wrap if
    DH> no refresh is completed? Or should the value latch at the
    DH> maximum negative value?

reference added, left at 1 second units (for simplicity), description
has it latch at the max negative value.
 
    DH> rpkiRtrCacheServerId would benefit from a reference.
 
meant to, but not added

    DH> rpkiRtrCacheServerId value is used as a relation between
    DH> tables. What are the fate-sharing requirements? Must a
    DH> corresponding rpkiRtrPrefixOriginCacheServerId exist? Or if
    DH> rpkiRtrPrefixOriginCacheServerId exists, must a corresponding
    DH> rpkiRtrCacheServerId exist? If one entry is deleted, must the
    DH> corresponding entry be deleted to prevent one table from reusing
    DH> an identifier that is still in use in the other table?

added some description of fate in to the rpkiRtrPrefixOriginTableEntry 
    DESCRIPTION "An entry in the rpkiRtrPrefixOriginTable.
                 This represents one announced prefix. If a Cache
                 Server is removed from the local configuration, any
                 table rows associated with that server (indicated by
                 rpkiRtrPrefixOriginCacheServerId) are also removed
                 from this table."

    DH> s/debuging/debugging/

fixed 

    DH> The Errors table contains counters that represent "the number of
    DH> XXX errors received." This description seems to indicate that
    DH> counters start at zero (and implicitly reset to zero on a
    DH> discontinuity). An NMS developer certainly might interpret this
    DH> text as indicating zero-based counters. I think if there is an
    DH> assumption these are zero-based, there are TC-s that could be
    DH> used to show that assumption. OTOH, counters typically are not
    DH> zero-based because if errors occur during system startup, the
    DH> counter might be initialized partway through the startup and not
    DH> reflect all the errors that occurred during the startup phase. I
    DH> think this table would benefit from a discussion of the expected
    DH> initial values (zero or unpredictable/implementation-dependent)
    DH> and the nature of expected discontinuities, so NMS developers
    DH> can be sure to understand the relevant assumptions.  (is the
    DH> only discontinuity a restart of the management system?  What
    DH> about rollovers? Are all these error counters not preserved
    DH> across system restarts? Or can an implementation choose to
    DH> preserve them across resets to enhance debugging history?) Some
    DH> of this discussion would seem to apply to all the error
    DH> counters; if that is true of all current counters and should be
    DH> true for an potentially new counters added to this table, this
    DH> type of discussion could be captured in the Table or TableEntry
    DH> description.  (including the discussion of
    DH> rpkiRtrDiscontinuityTimer, which seems to apply to the whole
    DH> table).

I meant to add clarifying text to the discontinuity time (e.g. a manager
can only compare values pulled since the discontinuity time...). But
double checking now, I didn't get this change into the current draft.

    DH> The comment discussing ROATable probably isn't needed in the
    DH> final document, since no official REVISION included ROATable,
    DH> right?

removed 

    DH> s/recieved/received/

fixed

    DH> rpkiRtrPrefixOriginAddressType: "Only IPv4 and IPv6 are
    DH> supported." - This is an enumeration that could be extended in
    DH> the future if RPKI ever supports additional address types (like
    DH> ipv7 or something); the limitation of ipv4/ipv6 is presumably a
    DH> limit set by RPKI; it should not be a limit set by the MIB
    DH> object, which could not be updated to match an updated
    DH> RPKI. Each of these objects should reference the current limit
    DH> in RPKI, and allow for future SMIv2 extensibility of the
    DH> enumeration so extending it would not involve obsoleting each
    DH> object and defining a new object without limits. By indicating
    DH> that the enumeration could legally be extended at some point in
    DH> the future, NMS developers can plan accordingly; the current
    DH> approach could inadvertently encourage hard-coding support for
    DH> these, and only these, two enumeration values.

    DH> Would putting the limit in a compliance clause rather than the
    DH> object description clause work?

I think so, we removed the limits and changed the compliance description
to only require support for ipv4, ipv6, and dns.

    DH> rpkiRtrPrefixOriginAddress says "The format of the address is
    DH> defined by the
    DH>                     value of the corresponding instance of
    DH>                     rpkiRtrCacheServerAddressType."
    DH> Shouldn't this be rpkiRtrPrefixOriginAddressType?

yes, fixed 

    DH> rpkiRtrCacheServerConnectionStateChange says:
    DH> " The SNMP agent MUST throttle the generation of
    DH>                     consecutive
    DH> rpkiRtrCacheServerConnectionStateChange
    DH>                     notifications such that there is at least a
    DH>                     5 second gap between them."

    DH> A MIB is theoretically separate from the protocol used to carry
    DH> the information. What happens if the notification is being
    DH> reported by something other than SNMP? Is throttling still
    DH> required?

changed to 'management agent'.

    DH> I am a bit concerned by the text that describes throttling
    DH> generation; should generation be throttled, or should sending be
    DH> throttled? The need to recognize a status change event and
    DH> request a notification be sent (i.e., generation) might be
    DH> handled by the underlying instrumentation, while sending the
    DH> notification would seem to be the purview of the protocol being
    DH> used (i.e., the SNMP engine). If I have a down event followed by
    DH> an up event 2 seconds later, should I not generate a
    DH> notification reporting the state change? What if I have no
    DH> subsequent up event to report? Should the NMS continue to
    DH> believe the state is down? Or should I just queue the up
    DH> notification for 3 seconds and then send it?

    DH> (and I realize queuing could just push the excessive generation
    DH> problem down the road, AND lead to untimely reports). The
    DH> current text is unclear about this and I am concerned whether
    DH> there might be interoperability problems caused by differing
    DH> interpretations of how to throttle these notifications.  I know
    DH> we require a discussion of throttling; is this the best way to
    DH> document the throttling for these notifications?

Added to the description clause:
       If more than one notification has occurred locally during that
       time, the most recent notification is sent at the end of the 5
       second gap and the others are discarded."

    DH> In rpkiRtrReadOnlyCompliance, s/Implemntation/Implementation/

fixed

-Mike

-- 
Michael Baer
Senior Research Scientist
SPARTA National Security Sector
[email-masked]
C:530.902.3131

---

From: Michael Baer <[email-masked]>
Date: Thu, 7 Feb 2013 13:34:59 -0800

Hi Benoit,

A update to the draft was sent in earlier, I have comments on the
changes made inline below.

>>>>> On Mon, 21 Jan 2013 23:42:22 +0100, Benoit Claise <[email-masked]> said:

    BC> Hi, On top of David's feedback, here is mine.

    BC> - http://tools.ietf.org/html/draft-ietf-sidr-rpki-rtr-26 is now
    BC> RFC 6810

changed

    BC>                      Copyright (c) 2011 IETF Trust and the
    BC> persons identified as authors of the code. All rights reserved.
    -> 2013

changed

    BC>    rpkiRtrCacheServerPreference OBJECT-TYPE SYNTAX Unsigned32
    BC> (0..255) MAX-ACCESS read-only STATUS current DESCRIPTION "The
    BC> routers' preference for this RPKI cache server.

    BC>                     A lower value means more preferred. If two
    BC> entries have the same preference, then the order is arbitrary.
    BC> If no order is specified in the configuration then this value is
    BC> set to 255."

    BC> I don't understand the sentence:

    BC> 		    If no order is specified in the
    BC> configuration then this value is set to 255."

    BC> Configuration of what? This is misleading as this is a non
    BC> read-write (monitoring) MIB module.  Do you mean a config knob
    BC> in RFC 6810? Please specify.

It's a 6810 knob, section 8.  This object was changed to not have the
255 limit, a reference and more descriptive text were added:

                 In two cases the maximum value for an Unsigned32
                 object should be returned for this object:
                 - If no order is specified in the RPKI Router
                   configuration.
                 - If a preference value is configured that is larger
                   than the max value for an Unsigned32 object."

    BC> - rpkiRtrCacheServerTimeToRefresh OBJECT-TYPE SYNTAX Integer32
    BC> UNITS "seconds" MAX-ACCESS read-only STATUS current DESCRIPTION
    BC> "The number of seconds remaining before a new refresh is
    BC> performed via a Serial Query to this cache server over this
    BC> connection.
    BC>                     A negative value means that the refresh time
    BC> has passed this many seconds and the refresh has not yet been
    BC> completed.
    BC>                     Upon a completed refresh (i.e. a successful
    BC> and complete response to a Serial Query) the value of this
    BC> attribute will be re-initialized with the value of the
    BC> corresponding rpkiRtrCacheServerRefreshTimer attribute."

    BC> A reference to RFC 6810 section 5.3 would be useful (as flagged
    BC> by David)

a reference was added

    BC> - Clarifying question: rpkiRtrPrefixOriginTableEntry OBJECT-TYPE
    BC> SYNTAX RpkiRtrPrefixOriginTableEntry MAX-ACCESS not-accessible
    BC> STATUS current DESCRIPTION "An entry in the
    BC> rpkiRtrPrefixOriginTable.  This represents one announced
    BC> prefix."  INDEX { rpkiRtrPrefixOriginAddressType,
    BC> rpkiRtrPrefixOriginAddress, rpkiRtrPrefixOriginMinLength } ::= {
    BC> rpkiRtrPrefixOriginTable 1 }

    BC> This table gives the link to the unique ID of the connection
    BC> with

    BC>    rpkiRtrPrefixOriginCacheServerId OBJECT-TYPE SYNTAX
    BC> Unsigned32 (1..4294967295) MAX-ACCESS read-only STATUS current
    BC> DESCRIPTION "The unique ID of the connection to the cache server
    BC> from which this announcement was received.  That connection is
    BC> identified/found by a matching value in attribute
    BC> rpkiRtrCacheServerId."  ::= { rpkiRtrPrefixOriginTableEntry 6 }

    BC> Am I assuming correctly that we can only receive a (addressType,
    BC> address, minLength) from one and only one connection?

No, so that table was changed to use all the columns as and INDEX.  The
set of values are not unique unless all the columns are used.

-Mike

---

From: Benoit Claise <[email-masked]>
Date: Mon, 11 Feb 2013 17:05:53 +0100

Hi Michael,

Thanks for addressing my points.
See my following email discussing David's concerns.

Regards, Benoit

---

From: Benoit Claise <[email-masked]>
Date: Mon, 11 Feb 2013 17:09:42 +0100

Hi Michael,

Thanks for addressing the MIB doctor points.

> double checking now, I didn't get this change into the current draft.
BC> The document would really benefit from this clarification.
BC> Please post a new version with this improvement, and I'll clear my 
DISCUSS.

>      DH> rpkiRtrCacheServerId would benefit from a reference.
BC> It would nice to add it, along with above improvement.

Regards, Benoit

---

From: Michael Baer <[email-masked]>
Date: Mon, 11 Feb 2013 09:40:19 -0800

Hi Benoit,

A updated 06 version of the draft was just submitted.  I have inline
comments below.

>>>>> On Mon, 11 Feb 2013 17:09:42 +0100, Benoit Claise <[email-masked]> said:

    BC> Hi Michael, Thanks for addressing the MIB doctor points.

    >> 
    >> I meant to add clarifying text to the discontinuity time (e.g. a
    >> manager can only compare values pulled since the discontinuity
    >> time...). But double checking now, I didn't get this change into
    >> the current draft.
    BC> The document would really benefit from this clarification.
    BC> Please post a new version with this improvement, and I'll clear
    BC> my DISCUSS.

I added text to the rpkiRtrDiscontinuityTimer object:

    For objects that use rpkiRtrDiscontinuityTimer to indicate
    discontinuity, only values received since the time indicated by
    rpkiRtrDiscontinuityTimer are comparable to each other. A manager
    should take the possibility of rollover into account when
    calculating difference values.

I did add a reference clause to this object (as well as fixing a
spelling nit mentioned by Brian Carpenter).

Thanks,
Mike

---

From: Benoit Claise <[email-masked]>
Date: Mon, 11 Feb 2013 19:31:58 +0100

Thanks.
I just cleared my DISCUSS.

Regards, Benoit