RE: Melinda Shore: [midcom] WG LAST CALL

Hi Juergen, 

I am really sorry for the delay in response to your mail. My comments are inline. Please look for [ROHIT].

> > - On page 8, I suggest that extensions for protocol FOO go into a 
> > module named NAT-FOO-MIB to ensure that those things stay together 
> > if you sort the module names. 
> > 

[ROHIT] We have decided to remove the protocol extensibility from the nat-mib. This will cause all the sections related to the protocol extensibility to be removed from the MIB. 

> > - The whole document needs an editorial revision to comply with the 
> > MIB guidelines, see <draft-ietf-ops-mib-review-guidelines-01.txt>. I 
> > will not repeat those things here that apply in this particular 
> > case. 
> > 

[ROHIT] We will make changes according to the guidelines.. 

> > - It is unclear to me why there is a NAT-TC module which d!
 efines the 
> > NATProtocolType. If the idea is that this module goes to IANA 
> > control, then this needs to spelled out in an IANA considerations 
> > section and the module name should be something like 
> > IANA-NAT-MIB. As usual, the difference between none(1) and other(2) 
> > remains unclear since the DESCRIPTION clause of the NATProtocolType 
> > TC is silent what someone using this TC has to specify and the 
> > comments are not totally clear to me. 
> > 

[ROHIT] This section will also go away in the next revision as we will get rid of the protocol extensibility. 

> > - It appears to me that the *Port* object types should be using the 
> > InetPortNumber textual convention rather than Integer32. Note that 
> > this changes the tag and is therefore not just an editorial change. 
> > 

[ROHIT] will do this. 

> > - I find t!
 he naming scheme inconsistent (or I do not understand 
> > it
). If you really want to seperate configuration and statistic 
> > objects in diffferent tables, would it not make more sense to call 
> > the tables like this: 
> > 
> > o natAddrMapConfTable and natAddrMapStatsTable 
> > 
> > - Why is natConfEntry indexed by natConfInterfaceIndex and not just by 
> > ifIndex? And should this table not be called natInterfaceConfTable 
> > of shorter natIfConfTable? The name of the *Stats* augmentation 
> > should be aligned as well. 
> > 

[ROHIT] The idea was to start all the config tables with the NatConf and stats with the natStats but when natStats was added, the same convention was not followed. We will correct this inconsistency. Thinking again, i find your name convention better. We can us ifIndex as well; but by using natConfInterfaceIndex we wanted to indicate only the interfaces on which NAT is running. 

> > - How is na!
 tConfProtocol related to NATProtocolType? It appears to me 
> > that these two definitions are better kept in sync and dealt with in 
> > similar ways. 
> > 

[ROHIT] NATProtocolType will go away once we remove the protocol extensibility. 

> > - I suggest to move the scalars below a common root node just 
> > containing these scalars rather than having these timeouts in the 
> > middle of the table registrations. 
> > 
> > - The pointing from natConfProtSpecName to the real protocol specific 
> > tables is say interesting. If I understand correctly, then a 
> > natConfProtEntry basically maps a (natConfProtName, natConfProtType) 
> > tuple to a natConfProtSpecName in a table the management application 
> > has to know and it adds natConfProtIdleTimeout as the (only) 
> > attribute of this mapping. Perhaps this can be done in a simpler 
&g!
 t; > way? 
> > 

[ROHIT] We will rearrange protocol 
specific stuffs as the protocol extensibility is not one of the goals. 

> > - Since you already have scalars that control the generation of 
> > notifications, why is the throttling parameter not also accessible? 
> > 

[ROHIT] The design of notifications was mainly inspired by ENTITY MIB but yes we can add the throttling parameter as well. 

> > - You should not use the badValue error code in description which only 
> > exists for SNMPv1 compatibility (which is historic). My first guess 
> > is that you mean inconsistentValue... 
> > 

[ROHIT] Yes, it mean the inconsistentValue and we will change this. 

> > - Note that the natAddrBindTable has statistic counters which at first 
> > look seems inconsistent with the design to separate configuration 
> > from statistical objects. [Personally, I would just merge things and 
> > get rid of the!
  augmentations but that is a personal opinion not 
> > necessarily commonly agreed to.] The same comment also applies to 
> > the natAddrPortBindTable. 
> > 

[ROHIT] AddrBindTable and PortBindTables are not configuration tables; they are in fact status tables, so we decided to keep the stats in the same tables. IMHO, without augmentation tables will look like very huge and modularity will be lost. 

> > - The natAddrBindTable (and some other tables) lack a StorageType 
> > column - but you should check the MIB review guidelines anyway... 
> > 
> > - Is natAddrPortBindNumberOfEntries really needed? Such summary 
> > objects are often causing problems down the road. 
> > 

[ROHIT] This object was added on a request from the mailing grouop. 

> > - I found the name natProtocolStatsName confusing since it is actually 
> > a NATProtocolType and no!
 t what I call a name. 
> > 
> > - I find the descrip
tors for the various packet counters not very 
> > consistent (natProtocolStatsInTranslate 
> > vs. natProtocolStatsRejectCount vs. natAddrMapStatsNoResource 
> > vs. natInterfacePktsIn). I would have call them 
> > natProtocolStatsTranslateInPkts, natProtocolStatsRejectPkts, 
> > natAddrMapStatsNoResourcePkts, natInterfaceInPkts - at least the 
> > construction of the names would be consistent and the name says more 
> > precisely what is counted (namely packets). 
> > 

[ROHIT] We got this comment from others as well and we will change this. 

> > - Finally, are 32 bit counter enough for all these counters? 
> > 

[ROHIT] This is a debatable question and as we know that many vendos still support 32 bit counters. 

> > - You may want to check whether provide compliance statements for 
> > readonly implementations and whether you can go aw!
 ay with just 
> > requiring row one shot creation rather than dribble mode (see the 
> > diffserv MIB compliance statements for inspiration). 
> > 
> > - Some RowStatus columns are called *RowStatus while other are just 
> > called *Status. I suggest to call the *RowStatus consistently. 
> > 

[ROHIT] We will look in the DiffServ MIB. 

> > - Is the BIND id not in fact a session ID? 
> > 

[ROHIT] Noop; Every bind, address as well as address-port bind, is derived from an address map. The natAddrBindAddrMapName and natAddrPortBindAddrMapName objects provide the linkage between the bind and the address map it has been derived from. 

On the other hand, every NAT session is derived from a bind, address or address-port bind. 

> > - It is also surprising that the state tables which report the actual 
> > bindings are not related to the configuration tables. !
 However, the 
> > agent has to know this relationship I think
 to realize the 
> > notification. By modeling this relationship explicitely, I guess the 
> > unusual accessible-for-notify objects could go away. 
> > 

[ROHIT] Well. Binds are related to the addrMap and addrMaps are related to the config tables; so we do have the relationship but it is not implicit. 

Thanks 

Rohit 