[lmap] AD evaluation: draft-ietf-lmap-yang-10

[Alissa Cooper <[e-mail]>]

I have reviewed this document in preparation for IETF last call. I have a few substantive comments and questions that I’d like to discuss before issuing the LC. I’ve also included some nits and minor comments that should be resolved together with any IETF LC comments.

Substantive comments and questions:

= Section 3 =

"Configuration Information: This is modeled in the /lmap/agent
      subtree, the /lmap/schedules subtree, and the /lmap/tasks subtree
      described below.  Some items have been left out because they are
      expected to be dealt with by the underlying protocol."

I think this needs some further elaboration about the items that have been left out. As far as I can tell the only items from the information model that this data model does not define are ma-config-credentials and ma-config-control-channels. There is already text further below to explain the expectation about how channels will be modeled, which should be referenced here. Is there not something more specific that can be said about how credentials will be handled/modeled, assuming the "underlying protocol" referenced for configuration is NETCONF?

= Section 4.2 =

(1) "It is generally a good idea to always configure
            an end time and to refresh the configuration
            of event object as needed to ensure that agents
            that loose connectivity to their controller
            do not continue their tasks forever."
            
This is more of a comment for the information model, but if this is worth saying here, shouldn't it (also) be mentioned in the corresponding information model definition in draft-ietf-lmap-information-model? This doesn't seem specific to this particular data model.

(2) For all the counters defined here, I think you need to specify the time period over which the counts are to be calculated.

(3) 'list action {
             key name;
             description
               "An action describes a task that is invoked by the
                schedule. Multiple actions are invoked sequentially.";'

I don't get this -- doesn't the execution mode define whether the actions are executed sequentially, in parallel, or pipelined?

(4) "A queue is internally used to pass
                  results to another schedule."

I thought it was up to the implementation to decide how to implement this?

(5) I don't understand why the 'program' elements are included in the task configurations and capabilities. It doesn't seem wise to allow the controller to tell the MA which program it needs to use to complete a task, and I don't understand why the MA would need to communicate that information to the controller either. I thought the recent list discussion indicated that if an MA was not capable of performing an action, that action would simply fail, which seems like all that is needed.


Nits and minor comments:

= Section 4.1 =

Please provide a reference for ISO 8601.

= Section 4.2 =

s/loose connectivity/lose connectivity/

s/schedule determins/schedule determines/

s/A set of month/A set of months/

s/A set of second/A set of seconds/

= Section 5 =

(1) Just a suggestion, but I think this is worth calling out specifically:

OLD
Unauthorized access may leak measurement results.

NEW
Unauthorized access may leak measurement results, including from passive measurements.

(2) "Implementers MUST taken care that
   option names and values are passed literally to programs.  In
   particular, it MUST be avoided that any shell expansions are
   performed that may alter the option names and values."

This text strikes me as a bit odd. Surely there are a whole selection of good programming practices that are necessary to ensure that things don't go haywire when implementing LMAP -- why call out these two with normative recommendations? Why does this guidance only apply to options? I would recommend having this text be non-normative, but if you do keep it I would suggest the following:

Implementers MUST take care that
   option names and values are passed literally to programs.  In
   particular, shell expansions that may alter the option names and values MUST NOT be performed.