[ipcdn] Re: REVIEW:draft-ietf-ipcdn-qos-mib-11.txt

Murwin William-LWM008 <W.Murwin@motorola.com> Fri, 04 February 2005 16:25 UTC

Received: from ietf-mx.ietf.org (ietf-mx.ietf.org [132.151.6.1]) by ietf.org (8.9.1a/8.9.1a) with ESMTP id LAA11850 for <ipcdn-archive@ietf.org>; Fri, 4 Feb 2005 11:25:32 -0500 (EST)
Received: from megatron.ietf.org ([132.151.6.71]) by ietf-mx.ietf.org with esmtp (Exim 4.33) id 1Cx6Zo-0005gT-VV for ipcdn-archive@ietf.org; Fri, 04 Feb 2005 11:45:01 -0500
Received: from localhost.localdomain ([127.0.0.1] helo=megatron.ietf.org) by megatron.ietf.org with esmtp (Exim 4.32) id 1Cx6DD-0007gz-3b; Fri, 04 Feb 2005 11:21:39 -0500
Received: from odin.ietf.org ([132.151.1.176] helo=ietf.org) by megatron.ietf.org with esmtp (Exim 4.32) id 1Cx66Z-0006lN-0w for ipcdn@megatron.ietf.org; Fri, 04 Feb 2005 11:14:47 -0500
Received: from ietf-mx.ietf.org (ietf-mx.ietf.org [132.151.6.1]) by ietf.org (8.9.1a/8.9.1a) with ESMTP id LAA11024 for <ipcdn@ietf.org>; Fri, 4 Feb 2005 11:14:44 -0500 (EST)
Received: from motgate8.mot.com ([129.188.136.8]) by ietf-mx.ietf.org with esmtp (Exim 4.33) id 1Cx6PM-0005R4-Et for ipcdn@ietf.org; Fri, 04 Feb 2005 11:34:13 -0500
Received: from il06exr06.mot.com (il06exr06.mot.com [129.188.137.136]) by motgate8.mot.com (Motorola/Motgate8) with ESMTP id j14GGeJD019854 for <ipcdn@ietf.org>; Fri, 4 Feb 2005 09:16:40 -0700 (MST)
Received: from ma19exm01.e6.bcs.mot.com (ma19exm01.e6.bcs.mot.com [10.14.33.5]) by il06exr06.mot.com (Motorola/il06exr06) with ESMTP id j14GEc9b015646 for <ipcdn@ietf.org>; Fri, 4 Feb 2005 10:14:38 -0600
Received: by ma19exm01.e6.bcs.mot.com with Internet Mail Service (5.5.2657.72) id <ZH8S40KF>; Fri, 4 Feb 2005 11:14:37 -0500
Message-ID: <62173B970AE0A044AED8723C3BCF23810636B291@ma19exm01.e6.bcs.mot.com>
From: Murwin William-LWM008 <W.Murwin@motorola.com>
To: bwijnen@lucent.com, gen-art@alvestrand.no, gen-art@alvestrand.no, ipcdn@ietf.org, Patrick Michael-LZZ007 <Michael.Patrick@motorola.com>
Date: Fri, 04 Feb 2005 11:14:28 -0500
MIME-Version: 1.0
X-Mailer: Internet Mail Service (5.5.2657.72)
Content-Type: text/plain; charset="iso-8859-1"
X-Spam-Score: 0.0 (/)
X-Scan-Signature: ded6070f7eed56e10c4f4d0d5043d9c7
Subject: [ipcdn] Re: REVIEW:draft-ietf-ipcdn-qos-mib-11.txt
X-BeenThere: ipcdn@ietf.org
X-Mailman-Version: 2.1.5
Precedence: list
List-Id: IP over Cable Data Network <ipcdn.ietf.org>
List-Unsubscribe: <https://www1.ietf.org/mailman/listinfo/ipcdn>, <mailto:ipcdn-request@ietf.org?subject=unsubscribe>
List-Post: <mailto:ipcdn@ietf.org>
List-Help: <mailto:ipcdn-request@ietf.org?subject=help>
List-Subscribe: <https://www1.ietf.org/mailman/listinfo/ipcdn>, <mailto:ipcdn-request@ietf.org?subject=subscribe>
Sender: ipcdn-bounces@ietf.org
Errors-To: ipcdn-bounces@ietf.org
X-Spam-Score: 0.0 (/)
X-Scan-Signature: 963faf56c3a5b6715f0b71b66181e01a

Response Inline:

>>-----Original Message-----
>>From: Michael A. Patton [mailto:MAP@MAP-NE.com]
>>Sent: Wednesday, February 02, 2005 12:30 PM
>>To: gen-art@alvestrand.no
>>Cc: ipcdn@ietf.org; Patrick Michael-LZZ007; Murwin William-LWM008;
>>bwijnen@lucent.com
>>Subject: REVIEW:draft-ietf-ipcdn-qos-mib-11.txt
>>
>>
>>To quote Bullwinkle T. Moose, "This time for sure..."
>>See previous messages for context, but use this for content.
>>
>>Summary: This draft is basically ready for publication as a Proposed
>>	Standard RFC, but has nits that should be fixed before
>>	publication.
>>
>>However, I'll note that I actually have a LOT of nits below, but
>>they're now mostly typos or things I can write off.  Only a few are
>>more than can be fixed in AUTH48.
>>
>>
>>Disclaimer:  I know enough about MIBs to be dangerous, but not enough
>>to be authoritative.  My review of the document (consistent with the
>>GenART charter) is primarily for readability and consistency.
>>
>>
>>I had only one substantive comment:
>>
>>I note that you have both "...TimeCreated" and "...TimeActive", but
>>isn't it the case that sysUpTime-"...TimeCreated" = "...TimeActive"?
>>That means that keeping both entries in every row is just extra work
>>and only one is needed.  Likewise in the Log table keeping all three
>>of "...TimeCreated", "...TimeActive", and "...TimeDeleted" is also
>>redundant duplication.  It's probably too late in the cycle to really
>>expect to fix this, but it seems like it would be an improvement to
>>drop all the "...TimeActive" and maybe include a comment on the simple
>>way to compute it.

No, sysUpTime - "...TimeCreated" does not equal "...TimeActive". Yes, time "...TimeCreated" is the time at a service flow has been created and/or an entry in this table was added. However, "...TimeActive" reflects the number of seconds the service flow was "active", not the number of seconds an entry has existed in the docsQosServiceFlowTable/docsQosServiceFlowStatsTable. This service flow could have been "provisioned", or even "admitted", but has never transitioned to "active".


>>
>>
>>There were a number of things that worried me:
>>
>>I'm not sure about the quality of the reference for [4].  (and what
>>happened to the other DOCSIS references?)  The title and code in
>>quotes might be enough for a knowledgeable person to find them, but
>>they don't mean anything to me.  The given URL doesn't refer to a
>>single document, and it's not clear which of tghe several available at
>>that URL is intended.  I suspect again that a knowledgeable person
>>could figure out the relationship.  So, since one of the GenART
>>charges is general understanding, I suggest the reference and/or web
>>page need some improvement in RFC prep.

(1) The other DOCSIS reference were removed because this draft made no direct mention or citation to any of those Specifications. I had added them for readers of the draft that were unfamiliar with DOCSIS, in case they were looking for all specification that made up DOCSIS. 

(2) The title,date,and DOCSIS versions are minimum needed in order to find the specification, and are provided in the reference.  The code given is a notation that the DOCSIS community uses to quickly identify a specification. Here is the code: SP-{Radion Frquency Interface Specification(RFI), Operations Support System 
Interface Specification(OSSI), Baseline Privacy Plus Interface Specification(BPI+)}v{ version 1.0, version 1.1, version 2.0 }-I{version # of Specification}-Date.

(3) The URL is not maintained by the authors of the draft, and is the URL that Cablelabs/IPCDN perferred to be in the refernces. As author, I can ask for another URL from IPCDN Working Group, and see if they also perfer to have the extact location of the specification as well. 

(4) This reference is the same syntax as all other DOCSIS MIB references to DOCSIS Specifications, that have passed IESG:
	RFC2670, BPI+ MIB,  Subcriber Management

>>
>>I notice that the masks for IP addresses default to all ones, while
>>the mask for dest MAC address defaults to all zeroes.  This seems
>>strange to me, it seems that they should default the same.  If there's
>>some real reason for the distinction it should be explained somewhere.
>>If not it should be considered making them the same.  To my personal
>>taste all ones seems to make more sense.

The DOCSIS Specification does state that the default for the IP MASK should be 0xFFFFFFFF. The Specification does not specify the default for the Destination 
MAC Mask, if not present. 

The reason why the mask is "000000000000" from the authors point of view was to be more consitent with the Destination MAC Address, since the DEST MAC Address and DEST MAC Mask are single TLV together. Thus if the Destination MAC Address is present then neither is the Destination MAC Mask. If this TLV is not present than MASK is not ANDed with the  (pkts etherdst) = docsIetfQosPktClassDestMacAddr.

The IP Addresses are in their own TLV and each of the MASKs are in their own TLV. If a DEST IP MASK/SRC IP MASK is not present, then the MASK are defaulted to 0xFFFFFFFF and are still ANDed with the IP Address in the packets to see match the docsIetfQosPktClassInetXxxAddr.


>>
>>It also seems strange that only the dest MAC can be masked, and the
>>source MAC can't, but I assume that's inherited from the master spec I
>>can't find, and therefore not something that could be addressed (no
>>pun intended) in this document.
>>
This is from the specification:
http://www.cablelabs.com/specifications/archives/CM-SP-RFIv2.0-I06-040804-superseded.pdf



>>
>>Simple suggestions:
>>-------------------
>>
>>Section 1.2: the definition of SFID makes a point of saying that it's
>>	unsigned, but the definition of the SID doesn't say if it's
>>	signed or not.  Since these are just tokens for lookup, that
>>	probably doesn't matter, but it would be nice if thewse were
>>	more similar...

I will up the definition of a SID.

>>
>>In Section 2.2.3 the second paragraph essentially contains a
>>	definition for "primary SF", shouldn't that be defined in the
>>	glossary?

Will do.

>>
>>Typos
>>-----
>>
>>Section 1.2: "connecting a subscriber's LAN the CATV RF network."
>>	  => "connecting a subscriber's LAN to the CATV RF network."

Will do
>>
>>Section 2: "described in [5]" => "described in RFC 2119 [5]"
>>	(for consistency with other references)
>>
Will do, but in most case the RFC is not mention in the Citation, 

>>Section 2.2.1: "an CATV" => "a CATV"

Will do
>>
>>Section 2.2.2.1: "both DOCSIS 1.0, DOCSIS 1.1, and DOCSIS 2.0"
>>	      => "all of DOCSIS 1.0, DOCSIS 1.1, and DOCSIS 2.0"
>>	or maybe you meant
>>	      => "DOCSIS 1.0 as well as DOCSIS 1.1 and/or DOCSIS 2.0"
>>
Will do.

>>Section 2.2.4: "the number of packet delayed"
>>	=> "the number of packets delayed"
>>
Will do.

>>Section 2.2.11: "provides describes" => "describes"

Will do.
>>
>>Section 2.2.11: "mac addresses" => "MAC addresses"
Will do.

>>
>>Section 3: "with it Service Flow Classifier table"
>>	=> "with its Service Flow Classifier table"

Will do.
>>
>>Section 3: "co-ordination" => "coordination"

Will do.
>>
>>Section 3: "co-ordinate" => "coordinate"

Will do.
>>
>>In MIB definition of docsIetfQosPktClassBitMap: "A bit of of 
>>this object"
>>	=> "A bit of this object"

Will do.
>>
>>In MIB definition of docsIetfQosParamSetTosAndMask: "the this object"
>>	=> "this object"

Will do.
>>
>>In MIB definition of docsIetfQosParamSetType: "reserved by by the"
>>	=> "reserved by the"

Will do.
>>
>>In MIB definition of docsIetfQosServiceClassPolicyStatus:
>>	"it is reference by" => "it is referenced by"

_______________________________________________
IPCDN mailing list
IPCDN@ietf.org
https://www1.ietf.org/mailman/listinfo/ipcdn