Comments on l2tpv3-yang-model: Covering keyed-v6-tunnel?

Dear authors, 

I have read draft-shen-l2tpext-l2tpv3-yang-model-00, which provides the configuration and management of l2tpv3 tunnels. A few comments below.

The most significant one is that, this YANG model is about managing RFC3931 in general, which does not cover the requirements from draft-ietf-l2tpext-keyed-ipv6-tunnel. Since the essence of draft-ietf-l2tpext-keyed-ipv6-tunnel (as I see) is to simplify L2TPv3 tunnel by taking some parts from RFC3931, the YANG model for L2TPv3 should also be able to cover the YANG model for keyed-v6-tunnel. 

I’ve worked out a YANG diagram tree for keyed-v6-tunnel (and a YANG model). The YANG model makes use of some parts from the l2tpv3-yang-model (especially the “static” case), and also adds some parameters for its own. Since the length of Cookie should always be 64 bits, the following YANG model doesn’t include it as a parameter to configure.

<YANG for keyed-v6-tunnel>
module: keyed-v6-l2tpv3
  +--rw keyed-v6-l2tpv3
     +--rw enabled?                          boolean
     +--rw l2tpv3TunnelInstances   
        +--rw l2tpv3TunnelInstance* [tunnelName]
           +--rw tunnelName                  string 
           +--rw srcIfName                     if:interface-ref // *unique*
           +--rw srcIPv6                         inet:ipv6-address  /* unique *
           +--rw dstIPv6                         inet:ipv6-address   *        */
           +--rw localSessionId?             uint32 
           +--rw remoteSessionId?         uint32 
           +--rw localCookies
           |  +--rw localCookie* [cookieName]
           |     +--rw cookieName            enumeration // "new"/"old"
           |     +--rw localHighCookie       hexBinary
           |     +--rw localLowCookie        hexBinary
           +--rw remoteCookie
           |  +--rw remoteHighCookie         hexBinary
           |  +--rw remoteLowCookie          hexBinary
           +--rw mtu                                 unit16
           +--rw (oam_method)
           |  +--:(ieee8021ag!)
           |  +--:(ituy1731!)
           +--rw vccv_enabled?               boolean
//There is a separate “operational state” subtree, which isn’t shown above. 
</YANG for keyed-v6-tunnel>

I’m thinking if it’s a good idea to merge the two models (with some explanation text or a separate section in the l2tpv3-yang-model draft), since there are some similarities. Otherwise, a separate I-D could be written to document the keyed-v6-tunnel YANG. 

Could you, authors and the WG, share your ideas on this? Thanks a lot!

Here are a few more comments on the content of the draft, detailed as follows.
1) According to RFC3931, the L2TPv3 tunnel should support the mode of “one tunnel, multiple sessions”. However, the current YANG model doesn’t support it. I would suggest the parameter of Sessionid be a list, with corresponding modifications. 
2) The expression of choice-case structure in the diagram tree is not the common style used by other YANG models. Please refer to RFC7277.
3) Typically, there is a separate operational state subtree for those “ro” parameters. Some restructures might be helpful to make the model clearer, if you like. Also refer to RFC 7277.

Hope that helps.

Cheers,
Qi