Re: [Lime] WGLC: draft-ietf-lime-yang-connectionless-oam-03

["Adrian Farrel" <[e-mail]>]

Hi, 

I read this document as part of the WG last call. 

In my opinion the document is *almost* ready to move forward, but has
some small issues that need to be resolved first.

Thanks for the work,
Adrian

===

The use of English could use some work. The RFC Editor will catch this,
but there is a risk that they will break something, so if you have the
chance to get someone to edit the document first, that would help.

---

There are a number of nits reported at
https://www.ietf.org/tools/idnits?url=https://www.ietf.org/archive/id/draft-ietf
-lime-yang-connectionless-oam-03.txt

---

The datatracker is reporting "Submission Yang Validation returned 
warnings or errors." No idea about this, but sounds like something that
should be fixed.

---

Need to expand "OAM" in the Abstract.

---

Abstract s/Based model/Base model/

---

I think the Introduction would benefit from a short paragraph 
referencing draft-ietf-lime-yang-oam-model and explaining the 
difference.

---

Section 1 has tree citations of "[lime retrieval methods]". You need
to sort this out.

(I suspect idnits catches this)

---

Section 2.2. See my comments on the equivalent section in 
draft-ietf-lime-yang-oam-model

---

Section 3, etc.
Probably best to use single quotes (') around names of data nodes etc.
Thus, for example, 
   At the top of the Model, there is an 'oper' container for session

---

In 3.3 I can't make sense of
   Level is provided for scenarios where
   it might be possible to define layering relationship as it can be
   used to stitching fault at related OAM layers.

It seems to me (from the example that follows) that you are not talking
about layering of network technologies so much as concatenation of path
segments each of which is at a different technology. Maybe rewrite the 
text
as...
   'Level' defines the relative technology level in a sequence of 
   network portions, and is provided to allow correlation of faults in
   related OAM domains.
Or maybe I am only guessing.

---

3.3
s/thesecond/the second/

---

3.3

                  leaf level {
                      type int32 {
                           range "-1..1";
                      }
                      description
                        "Level";
                  }

It's not important, but I did wonder whether you would be better 
defining "up", "same", and "down" rather than using an int32.

---

A number of description clauses need to begin with capital letters.

---

I am trying to understand how a test point location can be identified 
by a multicast group address. I think you might want to target some of
the OAM tools at a multicast address (e.g., ping a mcast address) but
I don't see this as the same as a text point location. Am I missing
something?

---

You define oam-counter32 but then a number of leaf nodes (such as
session-count) are of type uint32. In fact, I don't see oam-counter32
used at all.

---

    grouping session-packet-statistics {
      description "Grouping for per session packet statistics";
      container session-packet-statistics {

        description "Per session packet statistics.";
        leaf rx-packet-count {
          type uint32;
          description "Total received packet count.";
        }
        leaf tx-packet-count {
          type uint32;
          description "Total transmitted packet count.";
        }
        leaf rx-bad-packet {

          type uint32;
          description "Total received bad packet.";
        }
        leaf tx-packet-failed {
          type uint32;
          description "Total send packet failed.";
        }
      }
    }

Please clarify whether this is "Total number of OAM packets..." If it is
counting normal data packets, uint32 is not large enough.

---

    grouping session-path-verification-statistics {
      description "Grouping for per session path verification statistics";
      container session-path-verification-statistics{
        description "OAM per session path verification statistics.";
        leaf verified-count {
          type uint32;
          description "Total number of packets that went through a path as
intended.";
        }
        leaf failed-count {
          type uint32;
          description "Total number of packets that went through an unintended
path.";
        }
      }
    }

Please clarify whether this is "Total number of OAM packets..." If it is
counting normal data packets, uint32 is not large enough.

---

Not sure why you need to define IP-Multicast-Group-Address since a mcast
IP address surely looks a lot like an IP address for which types already
exist.

---

Quite a few places "IP" is presented as "Ip"

---

      container path-verification {
        description "Optional path verification related information.";
        leaf flow-info {
          type string;
          description
            "ACL name that refers to the flow, if any.";
        }
        uses session-path-verification-statistics;
      }

Why "ACL name"? Why does this have anything to do with ACLs?

(But if it does, you need to expand ACL.)

---

Superfluous "YANG module of OAM" right at the end of the YANG module

---

5.  CL model applicability

Spell out "CL" or use actual model name.

---

6.  Security Considerations

   TBD.

I don't think that will be good enough :-)
You can model on the text in draft-ietf-lime-yang-oam-model

---

Section 8

   The authors of this document would like to thank Greg Mirskey and
   others

Greg spells it "Mirsky".
The others wished to remain anonymous?

---

It is possible that all your references are normative, but I doubt it.
You should go through them and work out which provide information and
which are required reading in order to process this document.

> -----Original Message-----
> From: Lime [mailto:[e-mail]] On Behalf Of Ron Bonica
> Sent: 19 January 2017 16:12
> To: [e-mail]; [e-mail]
> Subject: [Lime] WGLC: draft-ietf-lime-yang-connectionless-oam-03
> 
> Folks,
> 
> This message begins a Working Group Last Call on draft-ietf-lime-yang-
> connectionless-oam-03. Please submit comments by February 3, 2017.
> 
>
Ron
> 
> _______________________________________________
> Lime mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/lime