[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[ipcdn] RE: MIB Doctor review: http://www.ietf.org/internet-drafts/draft-ietf-ipcdn-pktc-signaling-13.txt
Sorry for late reaction. I was on vacation.
So it seems we should expect a new revision then.
Which I think did appear on July 11?
I will take a look at it.
Bert Wijnen
> -----Original Message-----
> From: Sumanth Channabasappa [mailto:sumanth at cablelabs.com]
> Sent: Wednesday, June 20, 2007 4:11 PM
> To: Wijnen, Bert (Bert); gordon.beacham at motorola.com; Satish
> Kumar at Texas Instruments; ipcdn at ietf.org
> Cc: Jean-Francois Mule; Richard Woundy @ Comcast; Romascanu, Dan (Dan)
> Subject: RE: MIB Doctor review:
> http://www.ietf.org/internet-drafts/draft-ietf-ipcdn-pktc-sign
> aling-13.txt
>
> Bert,
>
> Comments inline.
>
> > Let me start to tell you that I am not a voice expert, so a
> lot of the
>
> > actual content of this MIB module is abacadabra for me. I
> am assuming
> > that other IPCDN WG members have evaluated (or will
> > evaluate) the actual content w.r.t. the technical details and
> > correctness.
> >
> > Let me also say that I find this MIB module pretty wieldy,
>
> Thanks for the review and this opening comment. It is nice to
> hear and the credit goes to all the ipcdn participants &
> implementers who contributed to, and revised, the mib module
> over the years.
>
>
> > More or less serious issues/concerns:
> >
> > - writable objects MUST specify (for example in their DESCRIPTION
> > clause) the expected persistence behavior.
>
> Good point. The following MIB Objects are writable:
>
> pktcSigDevCidSigProtocol, pktcSigDevR0Cadence,
> pktcSigDevR1Cadence, pktcSigDevR2Cadence,
> pktcSigDevR3Cadence, pktcSigDevR4Cadence,
> pktcSigDevR5Cadence, pktcSigDevR6Cadence,
> pktcSigDevR7Cadence, pktcSigDevRgCadence,
> pktcSigDefCallSigDscp, pktcSigDefMediaStreamDscp,
> pktcSigPulseSignalFrequency, pktcSigPulseSignalDbLevel,
> pktcSigPulseSignalDuration, pktcSigPulseSignalPulseInterval,
> pktcSigPulseSignalRepeatCount, pktcSigDevCidMode,
> pktcSigDevCidAfterRing, pktcSigDevCidAfterDTAS,
> pktcSigDevCidAfterRPAS, pktcSigDevRingAfterCID,
> pktcSigDevCidDTASAfterLR, pktcSigDevVmwiMode,
> pktcSigDevVmwiAfterDTAS, pktcSigDevVmwiAfterRPAS,
> pktcSigDevVmwiDTASAfterLR, pktcSigDevRingCadence,
> pktcSigDevCidDelayAfterLR, pktcSigDevCidDtmfStartCode,
> pktcSigDevCidDtmfEndCode, pktcSigDevVmwiSigProtocol,
> pktcSigDevVmwiDelayAfterLR, pktcSigDevVmwiDtmfStartCode,
> pktcSigDevVmwiDtmfEndCode, pktcSigDevrpAsDtsDuration.
>
> We did not intend to persist any of these MIB Objects. The
> DESCRIPTION clause will be updated to reflect this.
>
>
> > - I see various objects aka:
> > pktcSigDevR0Cadence OBJECT-TYPE
> > SYNTAX PktcRingCadence
> > MAX-ACCESS read-write
> > STATUS current
> > DESCRIPTION
> > " This object specifies ring cadence 0 (a user defined
> > field). This object is required for the L line
> package."
> > ::= { pktcSigDevObjects 5 }
> >
> > The fact that an object is required" is normally expressed in the
> > MODULE-COMPLIANCE. The above object (together with several others)
> > seems conditionaly optional, which means you would group all such
> > objects required for L line package in one OBJECT-GROUP and make
> > that OBJECT-GROUP conditionaly mandatory.
>
> Your proposal makes sense. I propose we add a new
> object-group and indicate it to be conditionally mandatory as
> you suggest.
>
> > - I wonder if the SYNTAX of SnmpAdminString makes sense for the
> > objects pktcSigCapabilityVersion and pktcSigCapabilityVendorExt.
> > It will work. But, SnmpAdminString is intended to contain human
> > readable (in any language/character set) strings. It
> seems that the
> > values that you allow are very restricted and certainly cannot
> > be in any other language/character-set.
> > I personally can live with it... but you might want to
> > think of just an OCTET-STRING that you define exactly what it can
> > contain.
>
> Can see your point; however, given the original intention to
> not be restrictive regarding the values and to restrict this
> to human readable strings only, we should probably let this be as-is.
>
>
> > - I find this strange:
> > pktcSigPowerRingFrequency OBJECT-TYPE
> > SYNTAX INTEGER {
> > f20Hz(1),
> > f25Hz(2),
> > f33Point33Hz(3),
> > f50Hz(4),
> > f15Hz(5),
> > f16Hz(6),
> > f22Hz(7),
> > f23Hz(8),
> > f45Hz(9)
> > }
> > UNITS "Hertz"
> >
> > Because tha value of the object is certainly not in "Hertz" units.
> > A value of 1 doe not represent 1 UNIT of "Hertz", but it
> represents
> > 20 Hertz. and so on... I think I would remove the UNITS clause.
>
> Yes, good catch, UNITS clause will be removed.
>
>
> > Same for pktcSigPulseSignalFrequency
>
> Yes, UNITS clause will be removed.
>
> > - pktcSigPulseSignalTable DESCRIPTION clause speaks about the
> > mandatory
> > nature of this table for E line package. This is MODULE-COMPLIANCE
> > stuff and should be expressed in the OBJECT-GROUP grouping and
> > MODULE-COMPLIANCE.
> >
> > similar comment for pktcSigDevRingCadenceTable
> I propose we add another new object-group and make it
> conditionally mandatory as you suggested earlier.
>
>
> >
> > - I see:
> > pktcSigPulseSignalDbLevel OBJECT-TYPE
> > SYNTAX TenthdBm (-350..0)
> > UNITS "dBm"
> >
> > I thought that the units were "1/10 of a dBm" ??
> >
> > same for pktcSigDevToneDbLevel
>
> Yes, will be updated.
>
>
> > - Given the DESCRIPTION clause for
> > pktcSigPulseSignalRepeatCount OBJECT-TYPE
> > SYNTAX Unsigned32
> > You might consider to make the syntax
> > SYNTAX Unsigned32 (1..50)
>
> Yes, will be updated.
>
>
> > - This DESCRIPTION clause has to be incorrect:
> >
> > pktcSigDevRingCadenceEntry OBJECT-TYPE
> > SYNTAX PktcSigDevRingCadenceEntry
> > MAX-ACCESS not-accessible
> > STATUS current
> > DESCRIPTION
> > " Unique value ranging from 0 to 127 that will
> correspond
> > to
> > the different ring cadences that are being supported by
> > the device."
> >
> > After all, it is an "entry" or "row" definition and so it
> does not
> > have a
> > value from 0-127. The INDEX object does.
> > The DESCRIPTION clause of the INDEX object seems to be describing
> > what an
> > Entry is all about.
>
> Good catch. Will be corrected.
>
>
> > - What sort of duration (UNITS?) must I assume for:
> > pktcSigDevToneFreqOnDuration OBJECT-TYPE
> > SYNTAX Unsigned32(0..5000)
> >
> > same question for pktcSigDevToneFreqOffDuration,
>
> According to the clarification provided by Satish Kumar and
> Phil Freyman, this is in milliseconds
>
>
> > - Have seen a SYNTAX of
> > SYNTAX INTEGER {
> > fsk(1),
> > dtmf(2)
> > }
> > for the signaling protocol multiple times.
> > Candidate for a TEXTUAL-CONVENTION?
>
> Yes, a TC will be created.
>
>
> > - For pktcNcsEndPntConfigPartialDialTO, what does a zero value mean?
> >
> > similar question for the other pktcNcs....TO objects
>
> I think the DESCRIPTION needs some additional explanation. To
> better respond to your question, here is the response from
> Phil Freyman who articulates the reasoning...
>
> "Logically the TO objects will be longer than any tone or
> frequency duration that it applies to since it is really a
> time out max for "bad"
> signaling situations. Per other text the TO object takes
> priority over any duration or cadence settings so if the TO
> object is set to "0"
> seconds, then the related tone would not be played out (timed out).
> Regardless of any duration or cadence setting. We cannot
> state that the TO must be longer than the associated tone
> duration since some durations are continuous and therefore
> the value of a time out would be lost...."
>
>
> > - I find it strange to see pktcNcsEndPntConfigStatus in the
> middle of
> > the
> > table. But... it is not an error.
> OK, I suggest we leave it as-is at this point.
>
>
> > - For the read-create table, I wonder where the read-only objects
> > pktcNcsEndPntStatusCallIpAddressType and
> > pktcNcsEndPntStatusCallIpAddress
> > come from? How does the agent determine those addresses.?
>
> The DESCRIPTION needs to clarify this. To explain further,
> the agent determines the CMS FQDN from the MIB Object
> 'pktcNcsEndPntConfigCallAgentId'. It then uses DNS to resolve
> the IP address. This resolution can lead to multiple IP
> addresses and it picks one. It then populates
> 'pktcNcsEndPntStatusCallIpAddress' with this IP address.
>
>
> > - For pktcNcsGroup the DESCRIPTION clause talks about this
> group being
> > mandatory for... That does NOT belong here. The fact if a group is
> > mandatory or not is specified in the MODULE-COMPLIANCE statement.
>
> Yes, this will be fixed.
>
> > Admin/Naming questions:
> >
> > - The title speaks about:
> >
> > Network-Based Call Signaling (NCS) MIB for PacketCable and
> >
> > while the MIB Module is named: PKTC-IETF-SIG-MIB and
> pktcIetfSigMib
> > Not that that is a bug... but it feels somewhat strange.
> >
> > Later in the document, at various places the "NCS MIB" term comes
> > back,
> > and so people might expect to see IETF-NCS-MIB or ietfNcsMib as
> > names?
>
> Let me check with the co-authors. I would leave it as-is, but
> I think we need to clean up the text accordingly.
>
>
> >
> > - Section 4 states:
> >
> > Terminal Adapter (MTA) devices. The IETF NCS MIB module
> (PKTC-IETF-
> > SIG-MIB) is intended to supersede various Signaling MIB modules
> > from
> > which it is partly derived:
> > - the PacketCable 1.0 Signaling MIB Specification
> > [PKT-SP-MIB-SIG-1.0],
> > - the PacketCable 1.5 Signaling MIB Specification
> > [PKT-SP-MIB-SIG-1.5],
> > - the ITU-T IPCablecom Signaling MIB requirements [ITU-T-J169],
> > - the ETSI Signaling MIB [ETSI-TS-101-909-9]. The ETSI
> Signaling
> > MIB requirements also refer to various signal characteristics
> > defined in [ETSI-TS-101-909-4], [ETSI-EN-300-001],
> > [ETSI-EN-300-659-1], [ETSI-EN-300-324-1] and
> [ETSI-TR-101-183].
> >
> > I know that many IPCDN WG members are all participating in
> > PackagetCable,
> > so I assume that superseding (is that same as obsoleting in IETF
> > terms?)
> > PacketCable documents is fine. But how about ITU-T and ETSI? Are
> > they
> > OK with the above statements?
>
> Good point, unless we formally receive a liaison statement
> about this, we should be more careful. Let's replace
> "intended to supersede" with "intended to update" which gives
> these 2 SDOs more room & control to do what they think is right.
>
> > - In section 4.1 I would use "MIB module" instead of "MIB"
>
> Ok, will be fixed.
>
> >
> > - The groups listed in section 4.1 are not the same as the groups
> > defined with the OBJECT-GROUP macros.
>
>
> Ok, will be fixed.
>
> >
> > - Sect 4.3 states:
> > pktcSigCompliances - this table has one object that has
> compliance
> > statements for devices that implement Signaling on the MTA.
> >
> > pktcSigGroups - this table contains group of objects for the
> > common
> > portion of the PacketCable NCS and Signaling MIB.
> >
> > pktcInternationalGroup - this table extends this MIB Module by
> > establishing a set of objects designed to support
> operations over
> > the widest possible range of markets.
> >
> > while none of these 3 are "tables" in MIB speak.
>
> Ok, s/table/object group - will be fixed.
>
>
> > - IN de MODULE-IDENTITY DESCRIPTION clause it states:
> >
> > Copyright (C) The Internet Society (2007). This
> version of
> > this MIB module is part of RFC yyyy; see the RFC
> itself for
> > full legal notices."
> >
> > That should be the new IETF Trust Copyright statement:
> >
> > Copyright (C) The IETF Trust (2007). This
> version of this
> > MIB module is part of RFC yyyy; see the RFC
> itself for full
> > legal notices."
>
> Ok, will be fixed.
>
>
> > - pktcNcsEndPntConfigTable and the objects in that table
> have a prefix
>
> > of
> > pktcNcs.... Why not pktcSigNcs.... ???
> > Just to better avoid any future name clashes in other MIB
> modules .
>
> I would be fine with this.
>
>
>
> >
> > - I see:
> > pktcSigDevEchoCancellation OBJECT-TYPE
> > SYNTAX TruthValue
> > MAX-ACCESS read-only
> > STATUS current
> > DESCRIPTION
> > " This object specifies if the device is capable of echo
> > cancellation."
> > ::= { pktcSigDevObjects 2 }
> >
> > And so I assume that the value true(1) means that the device is
> > capable of echo cancellation. I think it would be clearer if that
> > were explicitly stated.
> >
> > There are more of such objects of SYNTAX TruthValue that could
> > be clarified in a similar way.
> Will do.
>
>
> >
> > - there are some strange control characters in the document.
> > Specifically in the section titles, for example:
> >
> >
> > 9. ^M IANA Considerations
> >
> > - Security Considerations:
> >
> > Even if the network itself is secure (for example by
> using IPSec),
> >
> > pls change IPSec into IPsec which is the proper spelling
> and part of
>
> > the
> > latest security template.
>
> Will be fixed.
>
>
>
> >
> > ---------------------------
> >
> > references issues (found by my own tool):
> >
> > !! Missing citation for Informative reference:
> > P072 L048: [ITU-T-E.180] ITU-T E.180: "Various Tones Used in
> > National Networks,
> >
> > ---------------------------
> >
> > IDnits tells me:
> >
> > (See RFC 3967 for information about using normative
> references to
> > lower-maturity documents in RFCs)
> > -- Possible downref: Non-RFC (?) normative reference: ref.
> > 'ITU-T-J169'
> >
> > -- Possible downref: Non-RFC (?) normative reference: ref.
> > 'PKT-SP-CODEC'
> >
> > -- Possible downref: Non-RFC (?) normative reference: ref.
> > 'PKT-SP-MGCP'
> >
> > -- Possible downref: Non-RFC (?) normative reference: ref.
> > 'PKT-SP-PROV'
> >
> > probably OK, just listing it for completeness.
>
> Yes, this is ok.
>
> - S
>
_______________________________________________
IPCDN mailing list
IPCDN at ietf.org
https://www1.ietf.org/mailman/listinfo/ipcdn