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 =20

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