[pim] Comments on draft-ietf-pim-msdp-yang-03

[Stig Venaas <[e-mail]>]

Hi

We need to see how to make progress with this draft. We need to have
another WGLC. I'll
discuss with Mike. The draft looks great, but after careful reading I
have some comments I
hope can be addressed first. Please see below.

In the abstract and maybe the title, you should spell out MSDP.

OLD:
MSDP YANG Model
NEW:
A YANG Data Model for Multicast Source Discovery Protocol (MSDP)

OLD:
MSDP
NEW:
Multicast Source Discovery Protocol (MSDP)

Author data:
The address and affiliation needs to be updated for Mashesh.

I don't understand exactly how the crypto stuff would be used.
+--rw authentication
       |     |  +--rw (authentication-type)?
       |     |     +--:(key-chain) {peer-key-chain}?
       |     |     |  +--rw key-chain?          key-chain:key-chain-ref
       |     |     +--:(password)
       |     |        +--rw key?                string
       |     |        +--rw crypto-algorithm?   identityref

If password is used, what is the key?
I see it says "Support configuration of peer password.", but that
doesn't help me. Is it the actual password string? I'm not sure about
key chain either. Might that need to be defined in more detail? How
is this done in say the BGP YANG model? I guess that is similar.

rw peer-as?                 string {peer-as}?
Isn't there a defined way to handle ASNs other than strings in
IETF models? I see "type inet:as-number;" used for peer-as
another place in the model.

In section 4 there are 2 places where I think "Sa-cache" should be
"SA cache".

Some comments on the model.
"Support configuration of global ttl-threshold.";
Should it be TTL threshold?

"Support the rpc to clear SA cache.";
Should rpc be RPC?

Should the description really be the same for both of these?
    grouping authentication-container {
       description
         "A container defining authentication attributes.";
       container authentication {
         description
           "A container defining authentication attributes.";

"Attribute to configure peer tcp connection source.";
Should it say TCP?

"The container of originating-rp.";
Originating RP?

         description
           "true to enable peer;
            false to disable peer.";
Is it common to use lower case? Or should it be TRUE and FALSE?

For holdtime-interval it says
must ". > 3";

I'm not sure what "." means. Is this a common way to write this
in YANG models? Similarly for keepalive-interval.

For error and queue containers the description could be on one line.
E.g.
container error {
           description
             "A grouping defining error statistics
             attributes.";

container sa-cache {
           config false;
           description
             "The sa cache information.";
           list entry {
             key "group source-addr";
             description "A list of sa cache entries.";
             leaf group {
               type inet:ipv4-address;
               description "The group address of this sa cache.";
             }
I think SA should be capitalized in all the descriptions.

"Periodic SA advertisement timer exipiring time on
Typo.

description "Holddown timer value for SA forwarding.";
Hold-down?

presence "Present if a particular is cleared.";
Something missing here. "if a particular entry is ..."?

That's all I have.

Stig