Hello authors,
This is very substantial and detailed piece of work. Many thanks for
the considerable effort it represents.
I have done my usual AD review of this document in order to process the
publication request. The purpose of the review is to catch any issues
that might otherwise show up in IETF last call or IESG review, and to
ensure that I can fully support the document.
I'm glad to hear that there are implementations underlying this work and
that should be used as a moderating influence on my comments and
questions. For many of the points I raise it may be OK to answer "We
have thought about it, but this is how we chose to implement it."
Please look through the comments and let me know your thoughts either by
providing a new revision, or by answering via email.
Thanks for the work,
Adrian
=====
Section 1 para 2
The PCE communication protocol (PCEP)
The convention these days seems to be "Path Computation Element
Communications Protocol" or "Path Computation Element communications
Protocol".
You might apply that to the document title as well.
---
Section 1
The PCE communication protocol (PCEP) is the communication protocol
between a PCC and PCE for point-to-point (P2P) path computations and
is defined in [RFC5440].
That reference to P2P seemed odd to me. Any reason to include it?
In particular, nothing in this module appears to be specific to whether
P2P or P2MP request are being supported.
---
That previous point does cause me to wonder about "PCE capabilities".
I can see that you have tried to limit this module to describing the
features that are core to 5440, but I wonder whether that is wise.
For example, RFCs 5088 and 5089 define a set of PCE capabilities that
may be advertised in the IGPs and are surely relevant to the model of
a PCE that speaks PCEP. That information might usefully be seen in the
module at the PCE, and also at the PCC so that an operator can check
"why does that PCC keep sending these requests to the wrong PCE?"
Similarly, the Open Object can carry TLVs that indicate further
capabilities. Obviously you can't just include all future capabilities
TLVs since you don't know what they are (well, you know some, but that
have already been defined, but you can't know the undefined ones). But
you might want to to look at how those capabilities will be hooked in
for future accessibility.
Of particular interest will be the OF-list TLV of RFC 5541.
---
Convention has it that you have written a "MIB module" which is part of
"the MIB", so can you look through the document and mainly change "MIB"
to "MIB module".
---
I'm wondering under what circumstances the pcepEntityTable has more than
one entry. You might describe that in 4.1 and also in the Description
clause for the table.
That would tie in with explaining why you have an index of
Unsigned32 (1..2147483647) which allows for quite a lot of entities!
For a moment I thought that maybe there would be one entry when acting
as PCE and one when acting as PCC (i.e., a max of 2 < 2147483647) but
there is no mode field in the entityTable, only the peerTable.
Actually, I'm a bit confused about the indexing of the three tables.
I think you think that every PCE and PCC in the network shows in the
entityTable. But that isn't how you have set up the entityTable to
contain information that is about the local PCEP entity/entities. So
my confusion...
Now, looking at the indexes to pcePcepPeerTable. There are two
questions.
1. Why do you use pcePcepEntityIndex as an index? It points back into
entityTable which we have established is basically the local PCEP
speaker or which there is probably just one.
The text in 4.2 says...
The pcePcepPeerTable contains one row for each PCEP peer that the
PCEP entity (PCE or PCC) knows about.
I think your intention is that pcePcepEntityIndex is different for
each peer. But pcePcepEntityIndex is the index to PcePcepEntityTable
which is full of local PCEP entity information and (I suspect) will
only ever have one entry.
Shouldn't the peerTable have its own unique index for each peer?
2. Making pcePcepPeerAddrType and pcePcepPeerAddr indexes as well would
appear to allow two or more entries for the same peer with different
addresses.
Is that the intention? If so, you might make it clear in section 4.2.
And you would also need to make clear that there is an expectation
that an implementation will assign the same value of
pcePcepEntityIndex (or the new per-peer index) to each table entry
that represents the same peer.
OTOH, if a peer has multiple addresses, will this allow you to have
multiple sessions to the same peer. Is that what you intended?
Alternatively, it seems unlikely that two different peers will have
the same address. So why is any additional index (i.e.,
pcePcepEntityIndex) needed?
The same questions apply to the indexing of pcePcepSessTable although
pcePcepSessInitiator is understandable.
The solution to all this will be:
- Decide how many entries you really expect in entityTable.
- Decide whether entries in peerTable and sessTable should be indexed
from entityTable, or with an index from peerTable.
---
A small worked example would be cool (either between Sections 4 and 5,
or in an Appendix). I would draw a figure such as:
PCE1---PCE2 PCE3
| / | / |
| / | / |
PCCa/ PCCb PCCc
...and give an example of the module as read at PCE2 and PCCb.
---
The OID structure is unusual. I'm not saying it is wrong, but it is
different around pcePcepObjects. It would probably help to include a
diagrammatic representation. I think you have something like the
following.
(BTW, the "unusual" is the additional indirection between pcePcepObjects
and the various tables.)
mib-2
|
|--- XXX pcePcepMIB
|
|--- 0 pcePcepNotifications
| |
| |--- 1 pcePcepSessUp
| : :
|
|--- 1 pcePcepObjects
| |
| |--- 1 pcePcepEntityObjects
| | |
| | |--- 1 pcePcepEntityTable
| |
| |--- 2 pcePcepPeerObjects
| | |
| | |--- 1 pcePcepPeerTable
| |
| |--- 3 pcePcepSessObjects
| |
| |--- 1 pcePcepSessTable
|
|--- 2 pcePcepConformance
|
|--- 1 pcePcepCompliances
|--- 2 pcePcepGroups
---
I'd also like to see a short section (probably just containing a figure)
that shows the relationship between the three tables in this module.
It can't be complex (there are only three tables!), but a figure that
shows which objects in which tables lead you to find rows in other
tables would be a help. And where the relationship is shared indexes or
augmentation that can be called out.
---
I am quite happy that this module is entirely read-only. But it makes
some of the "usual" objects a little odd without additional information
in their Description clauses. For example, pcePcepEntityAdminStatus says
"The administrative status of this PCEP Entity."
That is fine, but what does it mean?
You could probably add:
"... This is the desired operational status as currently set by an
operator or by default in the implementation. The value of
pcePcepEntityOperStatus represents the current status of an attempt
to reach this desired status."
---
I like the values you have offered in pcePcepEntityOperStatus
A common accompaniment to the failure cases (and maybe the inactive /
deactivating cases) is an unformatted reason string. Did you consider
adding one of these?
---
Do you support values of pcePcepEntityAddrType other than ipv4(1) and
ipv6(2)?
Maybe unknown(0) is used when pcePcepEntityAddr has not been set up?
If there is some limit on the full range of values from the Syntax
InetAddressType you should:
- say so in the Description clause
- say so in the Conformance statement.
That will make the text in pcePcepEntityAddr easier to cope with
unchanged.
---
Just asking...
Is there a value of pcePcepEntityConnectTimer that means "never give
up"? You could use zero if you wanted to, but you don't have to.
(18 hours is probably long enough.)
---
Similar for pcePcepEntityConnectMaxRetry
Is there a value that means continue indefinitely?
Although 2^32 attempts sounds like quite a few.
The Description for this object says "...before going back to the Idle
state." Could you reference that back to an object (presumably in the
peerTable since the entry in the sessTable does not exist in idle state)
and the associated value.
---
pcePcepEntityOpenWaitTimer same question about "wait forever".
Also, perhaps clarify that this is the time after the TCP connection has
come up.
I know the protocol spec makes this clear, but "...aborts the session
setup attempt" could be enhanced with "...terminates the TCP connection
and removes the associate entry from the sessTable"
---
pcePcepEntityDeadTimer is recommended to be 4 times the
pcePcepEntityKeepAliveTimer value.
I don't think that giving this advice is helpful since this object is
read-only and can only report the configured (through other means)
value.
---
pcePcepEntityMaxDeadTimer
DESCRIPTION
"The maximum value that this PCEP entity will accept from a
peer for the Dead timer. Zero means that the PCEP entity
will allow not running a Dead timer.
A Dead timer will not be accepted unless it is both greater
than the session Keepalive timer and less than this field."
Are you sure? Where does a zero Dead timer fit with that statement?
---
pcePcepEntityAllowNegotiation seems out of order in amidst the various
timer objects. Not important, but odd.
---
pcePcepEntityMinDeadTimer OBJECT-TYPE
DESCRIPTION
"In PCEP session parameter negotiation, the minimum value
that this PCEP entity will accept for the Dead timer. Zero
means that the PCEP entity insists on not running a Dead
timer.
A Dead timer will not be accepted unless it is both greater
than the session Keepalive timer and greater than this
field."
Again, the second paragraph seems to conflict with the use of zero.
---
pcePcepEntitySyncTimer needs to clarify that this object only has
meaning if the entity is a PCE.
So you should give a value that a PCC can return in this object, or
say that a PCC should not return this object.
Furthermore, the syncTimer is only recommended in RFC 5440. How should
an implementation that does not implement a syncTimer return this
object?
---
The backoff timer objects (pcePcepEntityInitBackoffTimer and
pcePcepEntityMaxBackoffTimer) might be better grouped next to the
other session-related timers (especially the OpenTimer).
---
pcePcepEntityMaxUnknownReqs OBJECT-TYPE
DESCRIPTION
"The maximum number of unrecognized requests and replies that
any session on this PCEP entity is willing to accept per
minute.
pcePcepEntityMaxUnknownMsgs OBJECT-TYPE
DESCRIPTION
"The maximum number of unknown messages that any session
on this PCEP entity is willing to accept per minute."
...before doing what?
---
Same issue wrt pcePcepPeerAddrType as for pcePcepEntityAddrType
---
In pcePcepPeerRole it might be more usual to have unknown(0).
---
It is not clear (to me) whether pcePcepPeerNumSessSetupFail is
incremented each time a retry fails or only each time a session
set-up attempt is abandoned.
---
pcePcepPeerSessionFailTime
DESCRIPTION
"The value of sysUpTime the last time a session with this
peer failed to be established.
This is consistent with other fields, but does not record a session
that was up, but then failed.
---
Now, I'll grant that pcePcepEntityMaxUnknownMsgs allows for a remarkably
high rate of receipt of unknown messages with...
SYNTAX Unsigned32
DESCRIPTION
"The maximum number of unknown messages that any session
on this PCEP entity is willing to accept per minute."
But, given that rate, it seems that pcePcepPeerNumUnknownRcvd could
overflow within just one minute.
SYNTAX Counter32
DESCRIPTION
"The number of unknown messages received from this peer."
The same is going to apply to pcePcepEntityMaxUnknownReqs and
pcePcepPeerNumReqRcvdUnknown.
Also, of course, the same applies to the counters in the sessTable.
---
Many of the same comments apply to the objects in the sessEntry as
applied to the peerEntry and entityEntry, and I won't repeat them here.
---
I don't think pcePcepSessOverloadTime and pcePcepSessPeerOverloadTime
are very useful. The values are presumably stored from sent or received
OVERLOADED-DURATION TLVs in PCNtf messages that indicate 'Overloaded'.
However, if I come and look at the object some time later I have no idea
how much longer the overloaded state may last.
I think you need (I use the peer as an example)
pcePcepSessPeerOverload TruthValue
Whether or not this peer is overloaded.
pcePcepSessPeerLastOverloaded Timestamp
Time at which last overload event occurred for this peer.
Not cleared when pcePcepSessPeerOverload becomes false.
pcePcepSessPeerLastOverloadTime Unsigned32
Duration of last overload event for this peer.
Not cleared when pcePcepSessPeerOverload becomes false.
---
Although this is carefully a read-only MIB module, I wonder whether you
need a way to quiesce the Notifications issued by an implementation.
Something like...
pcePcepNotificationsMaxRate OBJECT-TYPE
SYNTAX Unsigned32
MAX-ACCESS read-write
STATUS current
DESCRIPTION
"This variable indicates the maximum number of
notifications issued per second. If events occur
more rapidly, the implementation may simply fail to
emit these notifications during that period, or may
queue them until an appropriate time. A value of 0
means no notifications are emitted and all should be
discarded (i.e., not queued)."
...or...
pcePcepNotificationsEnable OBJECT-TYPE
SYNTAX TruthValue
MAX-ACCESS read-write
STATUS current
DESCRIPTION
"If this object is true, then it enables the
generation of notifications."
Without one of these, your management station will be bombed with
Notifications from the PCCs in the network.
_______________________________________________
Pce mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/pce
