[netmod] LL review of draft-yeung-netmod-ospf-02

[Ladislav Lhotka <[e-mail]>]

Hi,

I reviewed the OSPF data model and I-D, concentrating mainly on YANG
issues and the integration with teh core routing data model.

Lada


*** General comments

    The "ospf" module is the result of an intensive cooperation of a
    multi-vendor expert group. On the one hand, it demonstrates that
    it is possible to design a YANG module for a relatively
    complicated routing protocol so as to be applicable to devices of
    different vendors. On the other hand, it also shows that the need
    to accommodate disparate configuration logics stemming from each
    vendor's CLI adds significant complexity to the data model.

    It is clear that the "ietf-routing" module is a nice fit for the
    "routing-instance-centric" design but the integration of the
    "protocol-centric" design is IMO quite awkward.

    While the (big) vendors participating in the module design may
    consider the result an acceptable compromise, I suspect it is not
    that much attractive for those who want to write a new
    implementation.

    This is not meant as a criticism of the "ospf" module but rather a
    reflection of reality that's by no means unique to OSPF. I think
    it is necessary to evaluate pros and cons of having one complex
    module that fits all versus multiple concurrent but cleaner
    modules.

*** Module design

**** Protocol-centric v. instance-centric model

     - The dichotomy of protocol-centric and instance-centric models
       should be better explained in the draft, maybe with examples.

     - In the protocol-centric case, do all routing protocols (not
       only OSPF) have to be defined in the default routing-instance?
       What are then the contents of non-default routing-instances?

     - References to interfaces use the "if:interface-ref" type, but I
       think they should always be confined to an appropriate
       routing-instance, i.e. refer to an entry of
       /rt:routing-instance/rt:interfaces/rt:interface and not to
       /if:interfaces/if:interface.

**** Inheritance

     In the "ietf-routing" module, the intent was to have different
     instances of the same routing protocol as entries of
     "rt:routing-protocol" list. The "ospf" module models OSPF
     instances as entries of
     "rt:routing-protocol/ospf:ospf/ospf:instance". I understand the
     main purpose of this organization is to allow instances to share
     data that are defined in "all-instances-inherit". Another way of
     achieving the same would be to define a special routing protocol
     type, e.g. "ospf:ospfv2-proto". An instance of this type would be
     a prototype and its data would be shared by all OSPFv2
     instances. The advantages of this are:
     - parameters that may be inherited can be defined just once,
       e.g., for "ospf:ospfv2" and "ospf:ospfv2-proto" at the same
       time.
     - leafrefs can refer to the same parameter no matter whether it
       is in the prototype or real instance;
     - in state data, the prototype instance needn't be present, the
       inherited parameters will appear as normal parameters of real
       instances.

**** Identities

     The "ospf:ospfv2" and "ospf:ospfv3" identities are defined with
     "rt:routing-protocol" as their base. I'd suggest to add a
     generic "ospf:ospf" identity. Taking into account the previous
     item, the identity hierarchy could be:
     
     rt:routing-protocol
       ospf:ospf
         ospf:ospf-proto
         ospf:ospfv2
           ospf:ospfv2-proto
         ospf:ospfv3
           ospf:ospfv3-proto

     Advantages:
     - Parameters from the "ospf:ospf" instance could be inherited by
       *both* OSPFv2 and OSPFv3 instances.
     - Using the new XPath function "derived-from()" (see sec. 10.4.1
       in draft-ietf-netmod-rfc6020bis-01), it will be possible to
       replace XPath expressions like

       "rt:type = 'ospf:ospfv2' or rt:type = 'ospf:ospfv3'"

       with

       "derived-from(rt:type, "ospf", "ospf")"

*** YANG issues

    - The module should be named "ietf-ospf".

    - In the definition

      list neighbor {
        description
          "List of neighbors.";
        leaf neighbor-id {
          type leafref {
            path "../../neighbor/neighbor-id";
          }
          description "Neighbor.";
        }
      }

      the "neighbor-id" leafref points to itself. This is is not
      allowed in YANG, the path should probably be something else.

    - Several augments have target nodes in the "ospf" module, for
      example

      augment "/rt:routing/rt:routing-instance/rt:routing-protocols/"
            + "rt:routing-protocol/ospf:ospf/ospf:instance" { ... }

      What is the purpose of doing so? The nodes defined inside such
      an augment can be put straight under "ospf:instance".

      Moreover, "when" expressions used for these augments are wrong,
      for example the above augment has

      when "../rt:type = 'ospf:ospfv2' or ../rt:type = 'ospf:ospfv3'"

      but it should be

      when "../../rt:type = 'ospf:ospfv2'" 
         + "or ../../rt:type = 'ospf:ospfv3'"

      - some descriptions are extremely terse and should be expanded.

*** I-D text

    - The standard text explaining symbols in tree diagrams should be
      included.

    - An example instance document would be helpful, e.g. an extension
      of the example in appendix D of draft-ietf-netmod-routing-cfg-16.

-- 
Ladislav Lhotka, CZ.NIC Labs
PGP Key ID: E74E8C0C