Re: [I2nsf] [Last-Call] Yangdoctors last call review of draft-ietf-i2nsf-nsf-monitoring-data-model-06

Top-posting one more comment about the different I-Ds being different:-)

This I-D uses the leaf start-time where other I-D s use leaf start-date-time.  This inconsistency was apparent when I looked at
draft-lingga-i2nsf-application-interface-dm-00
where there are two examples one after the other with the different formats.  I think that start-date-time  is better but, as ever, I do like consistency!

Tom Petch

From: I2nsf <i2nsf-bounces@ietf.org> on behalf of t petch <ietfa@btconnect.com>
Sent: 02 September 2021 12:41

----- Original Message -----
From: "Mr. Jaehoon Paul Jeong" <jaehoon.paul@gmail.com>
Sent: Tuesday, August 24, 2021 3:27 PM

> Hi Tom,
> Patrick and I have addressed your comments below with -09 version:
>
> I attach the revision letter to explain how to address them.
>
> Please let us know where this revision satisfies you or not.

Paul

Getting there.

You have added some references to the YANG module - good - but you must
also add them to the I-D References

I see
RFC854, RFC913, RFC1081, RFC4340, RFC4960, RFC5321, RFC7230, RFC7231.
RFC1081 is obsoleted by RFC1225 so that would likely be a better
reference.  In other I-D you have cited RFC793bis - I do not know if
that is appropriate here.

You import
  ietf-i2nsf-policy-rule-for-nsf {
with   prefix nsfi but in nsf-facing the prefix is nsfintf
Needs to be consistent

In identity, derived from application-protocol (a base which I like), I
note that 'imap' is present elsewhere but not here.  I do not know if
that is relevant to this module or not.

In the data module:

/http:/https:/

        leaf src-zone {
I was thrown by this thinking of IPv6 address zones but the description
makes in clear that this is nothing of the sort.  Probably not worth
changing but if you do I would suggest src-location as is used in the
description clause.

              leaf-list user-agent {
I think that the description violates (!) the limit on line length for
an RFC

      container i2nsf-counters {
       description
          "This is probably better covered by an import as this
This seems unfinished

            leaf alarm-type {
elsewhere you have switched to lower case (which I think right) but this
needs bringing in line (I do like consistency).

10.  I2NSF Event Stream
you are using the NETMOD convention for line breaks; would benefit from
a reference
"line breaks as per [RFC8792]

The I-D is big and I hope to find time this month to go through it again
in more detail.  Meanwhile, on to capability.

In passing, I get a bounce for skku-iotlab-members every time.

Tom Petch

>
> Thanks.
>
> Best Regards,
> Paul
>
> On Wed, May 5, 2021 at 7:44 PM t petch
<ietfa@btconnect.com<mailto:ietfa@btconnect.com>> wrote:
> Paul
>
> Top posting since this is a more general response (and leaving in YANG
> doctors since I note that five different YANG doctors reviewed the
five
> I-D and so might not see the issue that concerns me).
>
> As you have probably realised, I have now looked at the five YANG I-D
of
> I2NSF and am concerned at the disparate approaches to the same topics
> that I think will confuse a user and, likely, induce mistakes.  I
> provided some detailed comments  in response to WG LC on
> capability-data-model but really it cuts across all five.  It may be
> that the inconsistenicies that I see can be justified but if so, then
I
> think that the I-D may need some text to say so, to relate one I-D to
> another.
>
> The treatment of YANG identity for ICMP is to me a clear example.  I
> think that nsf-monitoring is good here, deriving icmpv4 and icmpv6
from
> icmp (and ipv4 and ipv6)
> while capability is not good having icmp (sic) and icmpv6 as two
> unrelated identity, no common base.
>
> But at a higher level it may be that capability has a better treatment
> where it has
>    base event; [from which is derived]
>      identity system-event-capability {
>      identity system-alarm-capability {
>
>    base system-event-capability;
>      identity access-violation {
>      identity configuration-change {
>
>    base system-alarm-capability;
>      identity memory-alarm {
>      identity cpu-alarm {
>      identity disk-alarm {
>      identity hardware-alarm {
>      identity interface-alarm {
>
> while nsf-monitoring has
>
>    base alarm-type;
>      identity mem-usage-alarm {
>      identity cpu-usage-alarm {
>      identity disk-usage-alarm {
>      identity hw-failure-alarm {
>      identity ifnet-state-alarm {
>
>    base event-type;
>      identity access-denied {
>      identity config-change {
>
> Different structure, different terminology, and these examples are
quite
> close compared to some others.  I would expect at least the root of
the
> identifier to be the same if not the whole identifier.
>
> What is missing, for me, is an underlying, high-level, information
model
> to provide a consistent structure and a consistent terminology for the
> I2NSF YANG I-D.
>
> Tom Petch
>
>
> ----- Original Message -----
> From: "Mr. Jaehoon Paul Jeong"
<jaehoon.paul@gmail.com<mailto:jaehoon.paul@gmail.com>>
> To: <tom petch>
> Cc: <Last Call>; <i2nsf@ietf.org<mailto:i2nsf@ietf.org>>; <Andy
Bierman>; <Yoav Nir>;
>
<draft-ietf-i2nsf-nsf-monitoring-data-model.all@ietf.org<mailto:draft-ie
tf-i2nsf-nsf-monitoring-data-model.all@ietf.org>>; <Linda
> Dunbar>; <Patrick Lingga>; <YANG Doctors>; <skku-iotlab-members>; <Mr.
> Jaehoon Paul Jeong>
> Sent: Thursday, April 29, 2021 3:49 PM
> Subject: Re: [I2nsf] [Last-Call] Yangdoctors last call review of
> draft-ietf-i2nsf-nsf-monitoring-data-model-06
>
>
> > > Hi Tom,
> > > Patrick and I have addressed all your comments below with the
> following revision.
> > >
>
https://datatracker.ietf.org/doc/html/draft-ietf-i2nsf-nsf-monitoring-da
>
ta-model-08<https://datatracker.ietf.org/doc/html/draft-ietf-i2nsf-nsf-m
onitoring-data-model-08>
> > >
> > > I attach our revision letter.
> > >
> > > Thanks.
> > >
> > > Best Regards,
> > > Paul
> > >
> > > On Mon, Apr 12, 2021 at 6:59 PM tom petch
>
<daedulus@btconnect.com<mailto:daedulus@btconnect.com><mailto:daedulus@b
tconnect.com<mailto:daedulus@btconnect.com>>> wrote:
> > > Paul
> > >
> > > Some admin comments on -07; I think that you need to:
> > >
> > > - change the title in YANG revision reference
> > >
> > > - add to the I-D references
> > > RFC959
> > > RFC8632
> > >
> > > - shorten lines. There is a limit to line length in RFC as per the
> Style
> > > Guide.  This is exceeded in the YANG where some of the path
statements
> > > take it over 80 while some of the examples are over 100.
> > >
> > > - add a reference for the import of
> > > ietf-i2nsf-policy-rule-for-nsf
> > >
> > > HTH
> > >
> > > Tom Petcb
> > >
> > > On 01/04/2021 03:09, Mr. Jaehoon Paul Jeong wrote:
> >>> > > > Hi Andy, Linda, and Yoav,
> >>> > > > Patrick and I have addressed all the comments from Andy.
> >>> > > > Here is the revised draft -07:
> > > ATT00001.txt 130 bytes
>
> Attachments:
>
Revision-Letter-for-NSF-Monitoring-YANG-Data-Model-version-09-20210824.d
ocx 103 kB
>
Revision-Letter-for-NSF-Monitoring-YANG-Data-Model-version-09-20210824.p
df 420 kB

_______________________________________________
I2nsf mailing list
I2nsf@ietf.org
https://www.ietf.org/mailman/listinfo/i2nsf