Re: [yang-doctors] Yangdoctors last call review of draft-ietf-i2nsf-nsf-monitoring-data-model-06

["Mr. Jaehoon Paul Jeong" <[e-mail]>]

Hi Andy,
Patrick and I have addressed the description on the dampening period in the
following revision.
https://datatracker.ietf.org/doc/html/draft-ietf-i2nsf-nsf-monitoring-data-model-08

I attach our revision letter.

Thanks.

Best Regards,
Paul


On Fri, Apr 9, 2021 at 2:08 AM Andy Bierman <[e-mail]> wrote:

> Hi,
>
> I reviewed your note, the diffs and the YANG module.
> I think draft-07 is ready (no issues or nits)
>
> There are probably some clarifications needed to adapt YANG Push dampening
> to I2NSF. YP refers to the data nodes changing within the dampening period.
> In this case, the notes to implementers should be clear about any events
> sent at the end
> of the dampening period because events were suppressed (if any). There
> might be a different
> procedure for each event or sub-event.
>
>
> Andy
>
>
> On Wed, Mar 31, 2021 at 7:09 PM Mr. Jaehoon Paul Jeong <
> [e-mail]> wrote:
>
>> Hi Andy, Linda, and Yoav,
>> Patrick and I have addressed all the comments from Andy.
>> Here is the revised draft -07:
>> https://tools.ietf.org/html/draft-ietf-i2nsf-nsf-monitoring-data-model-07
>>
>> I attach a revision letter to explain how we addressed the comments.
>>
>> There are the major updates in this revision as follows:
>> ---
>>    o  This version is revised according to the comments of Andy Bierman
>>       who is a YANG doctor.
>>
>>    o  This version updates its title as "I2NSF NSF Monitoring Interface
>>       YANG Data Model".  It clarifies the NSF Monitoring Interface to
>>       deliver NSF monitoring data to an NSF data collector (e.g.,
>>       Security Controller and NSF data analyzer).
>>
>>    o  This version adds an attack destination IP address for DDoS-attack
>>       event to provide an NSF data collector with more information about
>> the
>>       destination of DDoS-attack packets.
>>
>>    o  This version supports a notification for monitoring traffic flows
>>      to detect the source and destination (as a victim) of security
>> attacks
>>      such as DDoS attack.
>> ---
>> If you have questions and comments, please let me know.
>>
>> Thanks.
>>
>> Best Regards,
>> Paul
>>
>>
>> On Wed, Mar 24, 2021 at 3:34 AM Andy Bierman via Datatracker <
>> [e-mail]> wrote:
>>
>>> Reviewer: Andy Bierman
>>> Review result: Ready with Issues
>>>
>>>
>>> Status: Ready with Issues
>>>
>>> Most of the issues raised in the review of draft-04 have been
>>> addressed.
>>>
>>> Major Issues:
>>>
>>>  - None
>>>
>>> Moderate Issues:
>>>
>>> 1) too many YANG features
>>>
>>> There are 13 YANG features, one for each of the 13 notification-stmts
>>> defined.  There should be as few YANG features defined as possible.
>>> They should only be used if it is an unreasonable burden (compared
>>> to the feature value) for all servers to support the functionality.
>>>
>>> 2) list /i2nsf-monitoring-configuration/system-alarm
>>>
>>> This is yet another alarm management system created in the IETF.
>>> I guess the WG decided that RFC 8632 was not suitable.
>>>
>>> It is not clear how this system prevents excessive notifications
>>> sent to a client.
>>>
>>> What happens when the CPU, memory, or disk usage crosses back and
>>> forth over the threshold? Seems like an alarm is generated for each
>>> upward crossing of the threshold leaf.
>>>
>>> The precise behavior for triggering and then re-arming an alarm
>>> needs to be specified in the YANG module.
>>>
>>> RMON Alarms (RFC 2819) defines one way to prevent bursts of
>>> SNMP notifications, using an alarm reset threshold.
>>>
>>> YANG Push (RFC 8641) uses a dampening-period approach to prevent
>>> flooding the receiver with notifications.
>>>
>>> Also, it is not clear what use-case is served by "threshold = 0".
>>> The range is 0..100 instead of 1..100.
>>>
>>>
>>> Minor Issues:
>>>
>>> 3) too many notifications
>>>
>>> This module creates a lot of notifications to manage, and they are
>>> all optional to implement. This increases complexity in both
>>> the client implementation and operations.
>>>
>>> If you really need all 13 notifications then OK, but
>>> 13 notification events is a lot for one YANG module,
>>> especially if this set will get even larger over time.
>>>
>>> Here is one way to reduce the number of event definitions.
>>> The example below has 1 event and 13 sub-event types, but it could
>>> also apply to N event types each with some sub-event types.
>>>
>>> This design template adds one more layer in the notification message,
>>> but it is probably easier for the client and operator to manage.
>>> The deployment may require filters and access control rules that become
>>> more complex for a large number of notifications.
>>>
>>>
>>>     notification i2nsf-event {
>>>       description
>>>         "Wrapper for all I2NSF events";
>>>
>>>       choice sub-event-type {
>>>         description
>>>           "This choice must be augmented with cases for each allowed
>>>            sub-event.  Only 1 sub-event will be instantiated in each
>>>            i2nsf-event message. Each case is expected to define one
>>>            container with all the sub-event fields.";
>>>
>>>          // could put sub-events inline
>>>          case i2nsf-system-detection-alarm {
>>>            if-feature "i2nsf-system-detection-alarm";
>>>            container i2nsf-system-detection-alarm {
>>>              // contents of i2nsf-system-detection-alarm data
>>>            }
>>>          }
>>>
>>>        }
>>>      }
>>>
>>>
>>>      // could add sub-events via augments at any time
>>>      augment "/i2nsf-event/sub-event-type" {
>>>        case i2nsf-system-detection-event {
>>>          if-feature "i2nsf-system-detection-event";
>>>          container i2nsf-system-detection-event {
>>>            // contents of i2nsf-system-detection-event data
>>>          }
>>>        }
>>>      }
>>>
>>>
>>> Nits:
>>>
>>> 4) underscore vs. hyphen
>>>
>>> There are many field names in sec. 7 that are incorrect
>>> because they use an underscore instead of a hyphen char
>>> (e.g. req_cookies but leaf name is req-cookies)
>>>
>>> 5) verbose SNMP-style names
>>>
>>> The term -configuration in the object names is unusual.
>>> Repeating the parent name (like SMIv2) is not usually done in YANG.
>>> (e.g., i2nsf-system-detection-event-configuration)
>>>
>>> 6) identifiers should use well-known abbreviations or spell
>>> out the word if not too long.  E.g "ave" -> "average"
>>>
>>> 7) Is there a reason some identities are ALL-CAPS and others
>>> are all-lower-case? This should be explained in the YANG module.
>>>
>>>
>>>
>>>
>>>
>>>
>>>