Re: [manet] Benjamin Kaduk's Discuss on draft-ietf-manet-dlep-pause-extension-06: (with DISCUSS and COMMENT)

[Benjamin Kaduk <[e-mail]>]

On Sun, May 05, 2019 at 09:22:16PM -0400, Lou Berger wrote:
> Hi,
> 	My apologies, I thought I responded to this, but can't seem to find the
> response (hopefully my responses are consistent if I did!).
> 
> On 4/10/19 9:00 PM, Benjamin Kaduk via Datatracker wrote:
> > Benjamin Kaduk has entered the following ballot position for
> > draft-ietf-manet-dlep-pause-extension-06: Discuss
> > 
> > When responding, please keep the subject line intact and reply to all
> > email addresses included in the To and CC lines. (Feel free to cut this
> > introductory paragraph, however.)
> > 
> > 
> > Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
> > for more information about IESG DISCUSS and COMMENT positions.
> > 
> > 
> > The document, along with other ballot positions, can be found here:
> > https://datatracker.ietf.org/doc/draft-ietf-manet-dlep-pause-extension/
> > 
> > 
> > 
> > ----------------------------------------------------------------------
> > DISCUSS:
> > ----------------------------------------------------------------------
> > 
> > Section 3.1 defines 'Scale' as a four-bit unsigned integer field but only the
> > values 0-3 are assigned, leaving 4-16k usable.  How will future values be
> > assigned?
> > 
> 
> A future revision or update of this document.  We discussed a IANA
> registry, but thought it overkill as future additions seem quite unlikely.

Okay.  The Discuss was just to make sure it was considered in some fashion;
we do have a bunch of documents that explictly note "additional values are
to be allocated by updates to this specification" but I do not insist on
it.

> > In Section 3.1.1:
> > 
> >    DS Field Qn:
> > 
> >       The data item contains a sequence of 8 bit DS Fields.  The number
> >       of DS Fields present MUST equal the sum of all Num DSCPs field
> >       values.
> > 
> > Perhaps I'm misreading, but I thought the Num DSCPs Qn field was global
> > for this queue index, so there would just be one of them and no need to
> > take a sum.
> 
> It's just saying that more than one DSCP can match to a single queue
> index value.

Okay, if I'm looking at the figure properly then this is just an editorial
issue, then:

        0                   1                   2                   3
        0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
       |  Queue Index  |             Queue Size Qn                     |
       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
       | Num DSCPs Qn  |  DS Field Qn  |              ...              :
       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
       :                          ...                  |  DS Field Qn  |
       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

So we have the chunk of stuff for a given queue (index), and the size of
that queue, then a separate array/length for the DS values.  The various DS
values that can match this queue index are the contents of this array, but
this is the only array in the sub data item describing this queue index.
If that's correct, then isn't the description of "DS Field Qn" just "The
sub data item contains a sequence of 8 bit DS Fields for this queue index.
The number of DS Fields present MUST equal this value."?

> 
> > 
> > In Section 3.3:
> > 
> >    A router which receives the Restart Data Item SHOULD resume
> >    transmission of the identified traffic to the modem.
> > 
> > Why is this only a "SHOULD"?
> 
> to cover the case where the router is choosing not to send data, e.g.,
> due to lack of available data or other policy such as traffic rate shaping.

Okay.  Frequently documents will give the reader some hint as to what the
exceptional cases might be, but I will not insist on it here.


> > 
> > Section 4
> > 
> >                                  The extension does not inherently
> >    introduce any additional vulnerabilities above those documented in
> >    [RFC8175].  [...]
> > 
> > As noted by others, this sentence is just not true.
> > I will not duplicate the suggestions for additional considerations that
> > need to be documented.  
> 
> Please see comments / discussion elsewhere on this.

Sorry, most of these threads have aged out of my cache; could you give me a
hint about whose ballot thread to look in, or a link?

> > In particular, I agree with Roman that the use
> > of TLS needs to be mandatory, especially since this protocol is
> > nominally only defined for use with radio links.
> 
> 
> As covered elsewhere, in particular this is really a comment on base
> DLEP and I think this extension should be consistent with that document.
> (Consider an example from when 8175 was issued, consider the case where
> the modem and router use MACsec, 802.1X and 802.1 AE, why is TLS needed
> as well.)

Perhaps.  I think it's highly dependent on whether DLEP is only conveyed
over wired links and the physical scope of those links, though -- my
understanding is that most of the IESG was treating this protocol as
running over the radio link.

> > 
> > 
> > ----------------------------------------------------------------------
> > COMMENT:
> > ----------------------------------------------------------------------
> > 
> > Section 3.1
> > 
> >                                 Any errors or inconsistencies
> >    encountered in parsing Sub Data Items are handled in the same fashion
> >    as any other Data Item parsing error encountered in DLEP.
> > 
> > I had a hard time finding the indicated behavior in RFC 8175 -- "error"
> > does not appear at all (and "erroneous" only twice, for IP address
> > processing, v4 and v6); "parse" only appears once (in the Session
> > Initialization State); "inconsistent" apprears several times but I did
> > not see one that seemed to be a generic catch-all case.  It's probably
> > worth putting in a section reference or changing the text to use
> > keywords that are searchable in RFC 8175.
> > 
> I've added the typical text from 8175to remove any ambiguity:
> 
>       In particular, the receiving implementation MUST issue a
>       Session Termination Message containing a Status Data Item with
>       status code set to 130 'Invalid Data' and transition to the
>       Session Termination state.

Thanks.

> > Section 3.1.1
> > 
> >    Queue Size Qn:
> > 
> >       A 24-bit unsigned integer representing the size, in the octet
> >       scale indicated by the Scale field, of the queue supporting
> >       traffic with the DSCPs associated with the queue index.
> > 
> > Is there any value in including a specific sentinel value that indicates
> > that the modem cannot or does not wish to state the size of the
> > corresponding queue?
> 
> As here is no requirement for the modem to set this, I'm not sure if
> there would be any value in such.
> 
> > 
> > Section 3.2
> > 
> >    A router which receives the Pause Data Item MUST cease sending the
> >    identified traffic to the modem.  This may of course translate into
> >    the router's queues exceeding their own thresholds.  If a received
> >    Pause Data Item contains a Queue Index value other than 255, or a
> >    queue index established by a Session Initialization or Session Update
> >    Message, the router MUST terminate the session with a Status Data
> >    Item indicating Invalid Data.
> > 
> > This would be a nit, but actually has serious functional consequences:
> > remove the comma after "255".  The current text says that if the router
> > receives a queue index established by a Session Initialization message,
> > the router MUST terminate the session, which does not seem to be the
> > intent!
> 
> done.
> 
> > 
> > Section 3.3
> > 
> >    The Restart Data Item is used by a modem to indicate to its peer that
> >    transmission of previously suppressed traffic may be resumed.  An
> >    example of when a modem might send this data item is when an internal
> >    queue length drops below a particular threshold.
> > 
> > This seems like a fine place to suggest the application of hysteresis
> > (i.e., that the "particular threshold" here might be lower than the one
> > indicated in Section 3.2).
> > 
> > Section 5.x
> > 
> > It's not clear to me what value there is in repeating the registration
> > policy of the registries from which codepoints are being allocated.
> > IANA will apply the correct policy, and the reader of this document
> > doesn't need to care what policy was applied.
> 
> This section will get revised as part of final processing so defer to
> both IANA and the RFC editor on this.

Sounds good!

Thanks,

Ben