[Gen-art] Genart last call review of draft-ietf-manet-dlep-pause-extension-05

[Dale Worley via Datatracker]

Reviewer: Dale Worley
Review result: Ready with Nits

I am the assigned Gen-ART reviewer for this draft. The General Area
Review Team (Gen-ART) reviews all IETF documents being processed
by the IESG for the IETF Chair.  Please treat these comments just
like any other last call comments.

For more information, please see the FAQ at

.

Document:  draft-ietf-manet-dlep-pause-extension-05
Reviewer:  Dale R. Worley
Review Date:  2019-03-20
IETF LC End Date:  2019-04-03
IESG Telechat date:  [not scheduled]

Summary:

   This draft is basically ready for publication, but has nits
   that should be fixed before publication.  There are a number of
   issues with technical content, but they appear to stem from
   editorial issues, rather than being unsettled technical
   decisions.

* Technical issues:

I notice that the Pause Extension cannot be used by a router to tell a
modem to not send.  I assume that the WG has considered this and has a
good reason for this asymmetry.

3.1.1.  Queue Parameter Sub Data Item

There need not be a Sub Data Item for a particular queue index.  Such
a queue has no declared size.  OTOH, it has no DSCPs, and so no
traffic can be sent for it, either.

   Queue Parameter Sub Data Items are an unordered list composed of sub
   data items with a common format.  The first sub data item is assigned
   a Queue Index value of 1, and subsequent data items are numbered
   incrementally.  The format of the Queue Parameter Sub Data Item is

   ...

   Queue Index:

  An 8-bit field indicating the queue index of the queue parameter
  represented in the sub data item.

These passages are contradictory.  Either the Sub Data Items are an
ordered list and indexes are assigned to them sequentially, they are
unordered and the indexes are given in the Queue Index subfield, or
the Sub Data Items are required to be in order by their Queue Index
fields.

   DS Field Qn:

  The data item contains a sequence of 8 bit DS Fields.  The
  position in the sequence identifies the associated queue index.
  The number of DS Fields present MUST equal the sum of all Num
  DSCPs field values.

This doesn't seem to match the defined format of the Sub Data Items.
The "DS Field Qn" fields contain exactly as many DS Fields as the
value of the Num DSCPs Qn field by definition.  And all of them are
associated with the one Queue Index in the Sub Data Item containing
the DS Field Qn.

I note that the Sub Data Items are not padded to a multiple of 4
octets.  I assume this is intended.

3.3.  Restart

   The sending of this data item parallels the Pause Data Item, see the
   previous section, and follows the same rules.  This includes that to
   indicate that transmission can resume to all destinations, a modem
   MUST send the Restart Data Item in a Session Update Message.  It also
   includes that to indicate that transmission can resume to a
   particular destination a modem MUST send the Pause Restart Item in a
   Destination Update Message.

Read literally, this means that there is a pause/transmit bit for each
destination/DSCP combination, and that the various messages (pause
vs. restart * Session Update vs. Destination Update * queue index
vs. 255) set some subset of the bits to "pause" or "transmit".  This
is opposed to the model where (to simplify) there is an overall
pause/transmit bit for all traffic and an independent pause/transmit
bit for each destination, and traffic may be sent for a destination
only if both the overall bit and the destination bit are "transmit".

* Editorial issues:

1.  Introduction

   The
   extension also optionally supports DSCP (differentiated services
   codepoint) aware, see [RFC2475], flow control.

The phrasing of this sentence is awkward because of the number of
interpolated phrases.  I suggest something like:

   The extension also optionally supports flow control that is DSCP
   (differentiated services codepoint) aware (see [RFC2475]).

Also, it seems that recent RFCs have tended to capitalize the phrase
"Differentiated Services Code Point".  (Probably check this point with
the Editor.)

   Note
   that this mechanism only controls traffic that is to be transmitted
   on the modem's attached data channel and not to DLEP control messages
   themselves.

The parallelism is not correct here, as it would read "this mechanism
 controls ... to DLEP".  Perhaps change to:  "this mechanism only
applies to traffic ...".

2.  Extension Usage and Identification

   To indicate that the Control Plane Based Pause
   Extension is to be used, an implementation MUST include the Control
   Plane Based Pause Extension Type Value in the Extensions Supported
   Data Item.

If I am reading RFC 8175 correctly, this is not exactly true.  Sending
the Value does not compel the peer device to use the Extension.
Instead, "To indicate that the implementation accepts use of the
C.P.B.P.E., an implementation includes 

Re: [Gen-art] Genart last call review of draft-ietf-ccamp-alarm-module-07

[stefan vallin]

Thanks Dan!

> On 19 Mar 2019, at 19:54, Dan Romascanu  wrote:
> 
> Hi Stefan, 
> 
> Thank you for your answer and for addressing my concerns. I am comfortable 
> with your proposals. If your AD agrees, I would include these in a revised 
> version before submission to the approval of the IESG. 
> 
> Regards,
> 
> Dan
> 
> 
> On Tue, Mar 19, 2019 at 5:11 PM stefan vallin  > wrote:
> Hi Dan!
> Thanks for your review, an honour to have RFC 3877 in the loop :)
> See inline
> br Stefan
> 
> 
> > 
> > 
> > Major issues:
> > 
> > 1. The definition of Alarm is key for the whole model. It reads like this:
> > Alarm (the general concept): An alarm signifies an undesirable state in a
> > resource that requires corrective action.
> > 
> > However, RFC 3877 already defined a number of concepts including:
> >  Error
> >  A deviation of a system from normal operation.
> > 
> >   Fault
> >  Lasting error or warning condition.
> > 
> >   
> > 
> >   Alarm
> >  Persistent indication of a fault.
> > 
> > I believe that there is a need to show why the model defined by RFC 3877 
> > needs
> > to be changed, and why the difference that RFC 3877 was making between a 
> > Fault
> > and an Alarm is no longer needed.
> 
> Good comment, you are right, and we need to keep the distinction between 
> fault and alarm.
> That distinction is used in X.733, 3GPP IRP and others. The general pattern 
> is that “fault”
> refers to what is really broken, and the alarm the manifestation of that 
> underlying cause. 
> There is not a simple 1-1 relationship between a fault and an alarm
> * 1 fault may have many alarms due to limited root cause capabilities of the 
> system
> * There might be no underlying fault to an alarm, consider a non-optimal QoS 
> configuration 
>   which gives bad quality in VOIP calls. Certainly a MOS alarm from the VOIP 
> probe, but there
>   is no “fault” as such (if you do not consider a non-optimal config as a 
> fault)
> 
> So X.733
> X.733 fault: The physical or algorithmic cause of a malfunction
> 3GPP fault: a deviation of a system from normal operation, which may result 
> in the loss of operational capabilities of the element or the loss of 
> redundancy in case of a redundant configuration
> 
> I suggest we add the following to terminology:
> Fault: the underlying cause of an undesired behaviour
> 
> If we then turn to the term “alarm". I have added two aspects to the 
> definition of an alarm:
> 
> An alarm signifies an *undesirable state* in a resource that *requires 
> corrective action*.
> 
> Mostly based on the alarm standardization work in the process industry (see 
> draft references).
> 
> 1) Rather than “deviation from normal”, we say “undesirable”, subtle 
> difference.
>   In IT environments it is easier to define what is normal, a normal load to 
> a web server.
>   And anything deviation from that normal load could be an alarm.
>   In networking, things are more dynamic, and deviation from normal might be 
> the desired state.
>   So the definition stresses the fact that it is an undesired state, not just 
> deviation from normal.
> 
> 2) Adding the requirement that an alarm per definition should require an 
> action. This is a sound
>   requirement that puts requirements on what qualifies as an alarm and limits 
> the amounts of alarms.
>   (See for example the EEMUA, and ISA182 references in the draft). The 3GPP 
> Alarm standard
>   also added this to their definition at the later revisions to address the 
> alarm overload problem.
> 
> 
> 
> 
> 
> > Also, RFC 3877 defined in Section 3 a
> > Framework and an Architecture that was consistent with X.733. This document 
> > has
> > no such section, and while acknowledging the need for a mapping to X.733 it
> > states as a goal:
> > Mapping to X.733, which is a requirement for some alarm systems. Still, keep
> > some of the X.733 concepts out of the core model in order to make the model
> > small and easy to understand
> > 
> > More details about what is left out and why these are not needed would help.
> The alarm YANG model  does not *require* the X.733 parameter
> definitions of for example probable-cause enum values. Today, most networking 
> devices 
> and management systems do not rely on those enumerations.
> 
> Those are defined in the X733 augmentation module in order to keep the core 
> model as
> small and useful as possible. X733 requirements come more often from telecom 
> environments.
> 
> 
> > 
> > Minor issues:
> > 
> > 1. Section 2 makes a statement that includes
> > ... While IETF has not really addressed alarm management
> > 
> > This is is actually not accurate. RFC 3877 addressed Alarm Management. Maybe
> > there is a need to revise that approach, but this should be done explicitly,
> > not by stating that it did not exist.
> Correct, bad wording.
> OLD TEXT:
> Address alarm usability requirements, see Appendix G.  While IETF
>   has not really addressed alarm management, telecom