Subject: [IPFIX] Review of [email-masked]

From: "Ersue, Mehmet (NSN - DE/Munich)" <[email-masked]>
Date: Tue, 14 Dec 2010 14:04:28 +0100

Hi All,

below are a few review comments on the IPFIX YANG module in
draft-ietf-ipfix-configuration-model-08 from Bernd and myself.

In general I agree with Lada that the model is too long and should be
probably devided into two parts covering the collector subsystem and the
rest functionality.

The grouping "selectorParameters" is very long (spans several pages) and
therefore is hard to read and understand.
Main reason is that it contains a huge choice statement ("Method") that
hosts a container for every type of selector, each containing several
leafs. 
Perhaps it would be better to move the contents of each of those
containers in an own grouping which are then used inside the choice
statement.

The applicability of certain cacheMode identities is feature depended.

For example:
    identity timeout {
      base "cacheMode";
      description "Flow expiration after active and inactive timeout;
        generation of Flow Records.
        This identity MUST NOT be referred to unless the feature
        cacheModeTimeout is supported by the Monitoring Device.";
    }

In effect, the constraint is described only in the description but not
enforced by the way the module is defined.
The current model saves some space though it would be better to model
Constraint-based.
I think it would be better to introduce a Container per cachMode and
protect it with if-feature (see model excerpt below with some skipped
parts).
The Container could be then put in a Choice statement and groupings
could be used to avoid repetitions (not in the example).

        container timeout {
           if-feature cacheModeTimeout ;
      leaf maxFlows {
        type uint32;
        units flows;
...
      }
      leaf activeTimeout {
        type uint32;
        ...
      }
     leaf inactiveTimeout {
        type uint32;
        ...
      }
      ...
       }
      conatiner natural {
           if-feature cacheModeNatural ;
      leaf maxFlows {
        type uint32;
        units flows;
...
      }
        leaf activeTimeout {
        type uint32;
        ...
      }
     leaf inactiveTimeout {
        type uint32;
        ...
      }
      ...
    }
   conatiner permanent {
          if-feature cacheModePermanent;
      leaf maxFlows {
        type uint32;
        units flows;
...
      }
        leaf exportInterval {
        when "../cacheMode = 'ipfix:permanent'" {
          description "This parameter is only available for Cache Mode
            'permanent'.";
        }
      ...  
 }

Hope this helps.

Cheers,
Mehmet

---

From: Gerhard Muenz <[email-masked]>
Date: Sun, 19 Dec 2010 12:21:14 +0100

Dear Mehmet, Bernd,

Thanks a lot for your valuable comments.

Please find my replies inline.

Gerhard

> containers in an own grouping which are then used inside the choice

As already stated in my reply to Lada, I would not separate CP from the 
other processes.

However, we could put the selector parameters in a separate module. As a 
result, we would have the same distinction between an ipfix-psamp 
module (which then could be renamed ipfix module) and a selector module 
as between IPFIX-MIB and SELECTOR-MIB.

The motivation for this separation is that adding a new selector only 
affects the selector module, but not the ipfix module.

Hence, we would put "choice Method" in a grouping within the selector 
module. The ipfix module would refer to this external grouping with a 
"uses" statement.

I hope that such external references are possible?

>            description "This parameter is only available for Cache Mode

That's a good idea.