Re: [manet] Tsvart early review of draft-ietf-manet-dlep-credit-flow-control-09

["Velt, R. (Ronald) in 't" <[e-mail]>]

David, Lou,

Here is my take on some of the remaining issues from the TSV ART Early Review.
Disclaimer: I am not an author, merely a document shepherd (and a first-timer
at that). While my aim is to bring the resolution of these issues closer,
there is the risk that second-guessing the authors' intent has the opposite
effect...

Please see inline.

Thanks,
Ronald

> -----Original Message-----
> From: manet <[e-mail]> On Behalf Of Black, David
> Sent: dinsdag 2 augustus 2022 00:30
> To: Lou Berger <[e-mail]>; [e-mail]
> Cc: MANET IETF <[e-mail]>; Black, David <[e-mail]>
> Subject: Re: [manet] Tsvart early review of
> draft-ietf-manet-dlep-credit-flow-
> control-09
>
> Lou,
>
> Top posting to try to keep this short (anything not listed below is
> resolved):
>
> [1] -- Number of documents
> 	As we discussed, please record the WG's view of the appropriate
> 	number of documents in the draft shepherd writeup(s).

Done, I hope.

>
> [2] -- Flow Match Criteria
> 	The quoted existing text provides TC Sub-Data Item uniqueness (FID
> scope) within
> 	a TC Data Item (TID scope), but the second concern was across
> multiple TIDs
> 	(i.e., multiple TC Data items):
>
> 	 > I think there's an implicit assumption that TIDs do not
>  	 > overlap, perhaps because they are associated with non-overlapping
>  	 > destinations.  Under that assumption, the same packet could not
> match
>  	 > FIDs under more than one TID.
>
> 	Hence the quoted existing text does not resolve the second concern,
> 	so something needs to be done:
>
> 	 > Under that assumption, the same packet could not match
> 	 > FIDs under more than one TID.  If that's correct, that assumption
> needs
> 	 > to be explicitly stated (or referred to in the base DLEP spec?).  If
> that's
> 	 > not correct, then an additional precedence rule is needed, e.g.,
> lowest
> 	 > numbered TID takes precedence.

Section 2.3.2., Credit Window Association (Data Item), of
draft-ietf-manet-dlep-credit-flow-control-12 stipulates that  *single* Credit
Window Association Data Item be included in all Destination Up and Destination
Update Messages sent by a modem. The CWA Data Item contains the (one and only)
TID to be used for the Destination. The Destination Up and Destination Update
Messages will always also contain a MAC Address Data Item, per sections 12.11
and 12.17 of RFC 8175, respectively. This effectively binds a single TID to a
Destination. A "user" packet can only have a single Destination (even if that
Destination can be a multicast one). Thus, a packet can match (at most) one
FID associated with just that TID. Multiple Destinations can use the same TID,
of course. (How the binding works in case the Destination is Announced to the
modem by the router rather than signaled to the router by the modem by means
of a Destination Up Message is not clear to me. Should section 2.3.2 say that
the Credit Window Association DI MUST be included in all Destination Announce
Response Messages as well?)

>
> [3] -- Router MAY ignore flow control
> 	The crucial point in the response below can be restated as - nothing in
> the
> 	Management Considerations section of either draft overrides this
> sentence
> 	in Section 2 of the flow-control draft:
>
>    		When using credit
> 		windows, data traffic is only allowed to be sent by the router to
> the
> 		modem when there are credits available.
>
> 	The concern that remains is the distance between that sentence and
> the
> 	Management Considerations sections - they are 10 pages apart in the
> flow
> 	control draft, and that sentence does not appear in the DA credit
> extension draft.
>
> 	In addition, updates to the Management Considerations section of the
> flow
> 	control draft have not been applied to the Management
> Considerations section
> 	of the DA credit extension draft (which would not be a problem if the
> DA credit
> 	extension draft were not in a separate draft, but I digress ...). After
> aligning
> 	the text in the Management Considerations sections of both drafts, I
> suggest
> 	adding this sentence to the end of both Management Considerations
> sections:
>
> 		In all cases, if credit windows are in use, traffic for which
> credits are not
> 		available MUST NOT be sent to the modem by the router.

I tend to agree, but I need to think about this a bit more. I'll come back to
this.

>
> [5] -- Initialization vs. in-flight traffic
> 	I'm still looking for a response to:
>
> 	 > That does not solve the problem if the amount of traffic in flight
> between router
> 	 > and the modem is a significant fraction of the credit window size.
> E.g., suppose we
> 	 > have a very slow modem with a 1kbyte queue and there could be
> 512 bytes in flight
> 	 > between router and modem.  If the Credit Window Status arrives
> with 512 bytes in
> 	 > flight behind it, and the modem immediately does a Credit Window
> Initialization to 1k,
> 	 > the router can then send another 1k for a total of 1.5k which
> overruns the 1k modem
> 	 > queue by 50% (oops).
> 	 >
> 	 > I expect the reaction to that example to be along the lines of: That
> can't happen
> 	 > because of <A>, <B> and <C>.  If so, then <A>, <B> and <C> need to
> be clearly stated
> 	 > in the draft (e.g., <A> might be " the amount of traffic in flight is
> miniscule by
> 	 > comparison to both the Credit Window size and initial credit
> balance").

Section 2.3.1., Credit Window Initialization, of
draft-ietf-manet-dlep-credit-flow-control-12 states, that the CWI Data Item
can be carried in either a Session Initialization Response Message or a
Session Update Message. In the former case, there should not be any in-flight
data: a router-side DLEP implementation claiming to support credit-based flow
control should not be sending packets to the modem before the DLEP Session has
been established and credits have been granted by the modem. In the latter
case, the modem will presumably have a good reason to send a Session Update
Message containing a Credit Window Initialization Data Item. Most likely, it
will have learned from a Credit Window Status Data Item sent by the router,
that there is a discrepancy between the router's perceived number of available
credits for some flow and the modem's queue state for that flow. Since
something is wrong, it seems acceptable for the modem to discard further
(in-flight) packets matching that flow until it can be sure the router is
respecting the adjusted Credit Value from the CWI Data Item. Section 12.8 of
RFC 8175 states that a Session Update Response Message MUST be sent by a DLEP
participant when a Session Update Message is received. The reception of such a
Session Update Response Message by the modem provides a clue that the router
has adjusted its Credit Window for the flow (i.e., router and modem are now in
sync) and that packets belonging to the flow can be accepted again. I would
suggest adding a sentence to the last paragraph of section 2.3.4., Credit
Window Status, to clarify this re-synchronization process, e.g., "The modem
MAY, at its discretion, after sending the Session Update Message with one or
more Credit Window Initialization Data Items, discard any further packets
matching the indicated FIDs until it receives a Session Update Response
Message from the router." Thoughts?

>
> [C] Data item usage:
> 	 >> Unexpected DIs in messages is defined by the base spec -- and yes,
> it's
>  	 >> an error.
>
> 	Please add text to state the first part of that sentence to both the flow
> control
> 	and data classification drafts, perhaps at the end of the Compatibility
> section, e.g.:
>
> 		The DLEP specification [RFC8175] defines handling of
> unexpected
> 		appearances of 	any data items, including those defined in
> this
> 		document.

Seems like a good suggestion to me.

<end of shepherd's remarks>

>
> Thanks, --David
>
> -----Original Message-----
> From: Lou Berger <[e-mail]>
> Sent: Friday, July 29, 2022 8:00 AM
> To: Black, David; [e-mail]
> Cc: MANET IETF
> Subject: Re: Tsvart early review of
> draft-ietf-manet-dlep-credit-flow-control-
> 09
>
>
> [EXTERNAL EMAIL]
>
> David
>
> I apologize for missing this back in March. See below for responses..
>
> ----------
> On March 22, 2022 8:13:47 PM "Black, David" <[e-mail]>
> wrote:
>
>  > Lou,
>  >
>  > Thanks for the comprehensive response.  I'm copying and replying to
> only the items that merit further attention - any item not responded to
> is ok with me.
>  >
>  >>> -- [1] -- Number of Documents
>  > [...]
>  >>> I strongly suggest folding the DA credit extension draft into the
> traffic
>  >>> classification draft, as the resulting two-draft structure will be
> easier to
>  >>>  explain, and more importantly, should be easier for implementers to
> understand.
>  >>
>  >> This was the decision of the WG made a long time ago. I defer to the
>  >> chairs on this.
>  >
>  > I would ask the WG to revisit this decision based on the surprisingly
> short
> length
>  > of the DA credit extension draft (about a page of actual protocol
> specification
>  > plus one addition to an IANA registry).
>  >
>
> This has been rehashed quite a number of times in the working group. You
> made a comment to me that it could have been good to have put that in
> the shepherd right up - that's a valid ask in my opinion.
>
>
>  >>> -- [2] -- Flow Match Criteria
>  >>>
>  >>> The Traffic Classification draft does not explain how traffic
> classification
> is
>  >>> actually performed, i.e., how the applicable FID is determined for a
> packet,
>  >>> particularly when more than one FID is possible.
>  > [...]
>  >>
>  >> It took me a bit to decide how to address this (valid) comment. I
>  >> propose the following change to
>  >> draft-ietf-manet-dlep-traffic-classification-06:
>  >>
>  >> At the end of section 2.3.1, I propose adding the following text:
>  >>
>  >>          If a packet matches both a DSCP Field Value,see Section 2.2.
>  >>          and a Priority Field value, the DSCP
>  >>          associated TID MUST take precedence.
>  >
>  > That's headed in the right direction, but more work is needed on a
>  > couple of aspects.
>  >
>  > The above text does not cover situations where 0 DSCPs and/or 0 PCPs
>  > are used in a FID, and does not mention the VLAN identifier.
>
> I actually don't see why you say this.  (But I'm fine with your text, so
> no need to discuss this;-)
>
>  >  Suggestion:
>  >
>  > If a packet matches both a DiffServ Traffic Classification
>  > Sub-Data Item (see Section 2.2), e.g., DSCP match, and
>  > an Ethernet Traffic Classification Sub-Data Item (see Section
>  > 2.3), e.g., PCP match, then the TID with which the DiffServ
>  > Traffic Classification Sub-Data Item is associated MUST
>  > take precedence.
>
> No objections to this
>
>  >
>  > In addition, I think there's an implicit assumption that TIDs do not
>  > overlap, perhaps because they are associated with non-overlapping
>  > destinations.  Under that assumption, the same packet could not match
>  > FIDs under more than one TID.  If that's correct, that assumption needs
>  > to be explicitly stated (or referred to in the base DLEP spec?).  If
> that's
>  > not correct, then an additional precedence rule is needed, e.g., lowest
>  > numbered TID takes precedence.
>
> Is this already covered by the existing text:
>
>
>     Once validated, the receiver MUST ensure that each DS Field value is
>     listed only once across the whole Traffic Classification Data Item.
>     Note, this check is across the Data Item and not the individual Sub-
>     Data Item.  If the same DS Field value is listed more than once
>     within the same Traffic Classification Data Item, the Data Item MUST
>     be treated as an error as described above.
>
> and
>
>
>     Once validated, the receiver MUST ensure that each Priority Field
>     value is listed only once across the whole Traffic Classification
>     Data Item.  Note, this check is across the Data Item and not the
>     individual Sub-Data Item.  If the same Priority Field value is listed
>     more than once within the same Traffic Classification Data Item, the
>     Data Item MUST be treated as an error as described above.
>
>  >
>  >>> -- [3] -- Router MAY ignore flow control
>  >>>
>  >>> The Management Considerations section in both the traffic
> classification
> and DA
>  >>> credit extension drafts contains this text:
>  >>>
>  >>>     When modem-provided credit window information exceeds the
> capabilities of a
>  >>>     router, the router MAY use a subset of the provided credit windows.
>  >>>
>  >>> In other words, the router MAY ignore any credit windows that exceed
> the
>  >>> routers capabilities ... which in practice could amount to "MAY ignore
> any
>  >>> credit windows that the router implementer views as inconvenient" ...
> with the
>  >>> router proceeding to overrun the corresponding modem queues.  That
> doesn't seem
>  >> right, so if this is intended, some serious rationale/explanation ought
> to
> be
>  >>> provided.
>  >>
>  >> I personally have no objection to MUST, but perhaps a SHOULD is the
>  >> right answer given that there is an alternative defined.
>  >
>  > This isn't about MUST/SHOULD/MAY - it's about queue overrun which isn't
> supposed to
>  > happen and can't be addressed via just a keyword change. Suppose a
> router ignores the
>  > 17th modem credit window, *and* proceeds to overrun the corresponding
> modem
>  > queue.  That overrun is an undesirable failure mode.
>  >
>
> that's not how I read the current text.  I read it as if the modem
> describes more queues/windows than the router implements -- it may just
> send traffic against a subset.
>
>  > Would it be reasonable for a router that ignores at least one credit
> window
> to be
>  > prohibited from sending traffic that does not match any credit window (or
> that
>  > could match an ignored credit window)?  That would address the overrun
> problem.
>  >
>
> I'm not sure what the right behavior is here and don't believe it was
> discussed by the WG.  At this very late stage, how do you feel about
> leaving it unspecified behavior?
>
>  > Suggestion - at the end of Section 2.1 in the flow control draft, add a
> sentence that
>  > begins with:
>  >
>  > If a router is unable to associate a credit window with traffic to be
>  > sent a modem, then ...
>  >
>  >>> -- [4] -- Association Not Well-Specified
>  > [...]
>  >> Okay I'll try to improve the language in section 2 to cover (I) and (II)
>  >
>  > Better, but still hard to follow.    Suggestion:
>  >
>  > OLD
>  >   The TID provides the information to
>  >    support router traffic classification, based on the FIDs contained in
>  >    the TID, see [I-D.ietf-manet-dlep-traffic-classification].  As
>  >    defined, each credit window has a corresponding FID.
>  > NEW
>  >   The TID provides the information to
>  >    support router traffic classification, based on the FIDs contained in
>  >    the TID, see [I-D.ietf-manet-dlep-traffic-classification].  As
>  >    defined, each credit window has a corresponding FID, so traffic is
>  >    mapped to a credit window by locating a matching FID that
>  >    is contained in the TID that is associated with the traffic's
> destination.
>  >
>
> WFM
>
>  >> > -- [5] -- Initialization vs. in-flight traffic
>  >> >
>  >> > The Credit Window Initialization data item appears to be intended to
> establish
>  >> > common state for a Credit Window (e.g., size, number of credits)
> across
> the
>  >> > router and modem ... but ... that data item appears to be allowed to
> be
> sent
>  >> > while there's traffic in flight.  The result would be that the modem
> would
>  >> > count in-flight traffic against the initialized Credit Window, but the
> router
>  >> > would not.   The resulting inconsistency deserves discussion - it may
> be
>  >> > acceptable if the amount of traffic in flight is miniscule by
> comparison to
>  >> > both the Credit Window size and initial credit balance.
>  >>
>  >> This is the purpose of the credit window status DI.
>  >
>  > That does not solve the problem if the amount of traffic in flight
> between
> router
>  > and the modem is a significant fraction of the credit window size.  E.g.,
> suppose we
>  > have a very slow modem with a 1kbyte queue and there could be 512
> bytes in flight
>  > between router and modem.  If the Credit Window Status arrives with 512
> bytes in
>  > flight behind it, and the modem immediately does a Credit Window
> Initialization to 1k,
>  > the router can then send another 1k for a total of 1.5k which overruns
> the
> 1k modem
>  > queue by 50% (oops).
>  >
>  > I expect the reaction to that example to be along the lines of: That
> can't
> happen
>  > because of <A>, <B> and <C>.  If so, then <A>, <B> and <C> need to be
> clearly stated
>  > in the draft (e.g., <A> might be " the amount of traffic in flight is
> miniscule
> by
>  > comparison to both the Credit Window size and initial credit balance").
>  >
>  >> -- [6] -- Security Considerations
>  > [...]
>  >> How about:
>  >>
>  >>      This document introduces credit window control and flow mechanisms
>  >>      to DLEP.  These mechanisms expose vulnerabilities similar to
>  >>      existing DLEP messages, e.g., Destination UP or Down message
>  >>      injection attacks. The security mechanisms documented in <xref
>  >>      target="RFC8175"/> can be applied equally to the mechanism defined
>  >>      in this document.
>  >
>  > Second sentence would be better if the example were specific to this
> draft,
> perhaps:
>  >
>  >         These mechanisms expose vulnerabilities analogous to
> vulnerabilities
>  >         exposed by existing DLEP messages, e.g., an injected message
> resizes
>  >         a credit window to a tiny value causing denial of service.
>
> destination Up is used extensively by this document, but sure.an
>
>  >
>  >>> [C] Data item usage:  It's not clear which data items are required,
> allowed
> or
>  >>> prohibited in which DLEP messages.
>  > [...]
>  >> Unexpected DIs in messages is defined by the base spec -- and yes, it's
>  >> an error.
>  >
>  > I suggest adding a version of that sentence to both drafts that define
> DIs -
>  > the flow control draft and the traffic classification draft.
>
> I'm not a big fan of repeating this kind of text -- the handling of
> errors is a bit unusual in DLEP in how unforgiving it is; and it
> pervades the base document.  I wouldn't want to have to rev this
> document if someone changes/fixes the base spec.
>
>  >
>  >> > [D] Uninitialized window: The second sentence of Section 2.3.1 in the
> flow
>  >> > control draft begins with:
>  >> >
>  >> >     This Data Item SHOULD be included in any Session Initialization
> Response
>  >> >     Message that also ...
>  >> >
>  >> > What happens if that SHOULD is not adhered to? Please explain.
>  >>
>  >> There's a MAY immediately following.  Other places in the document
>  >> describe that use of an FID/credit window in a message before it is
>  >> defined/initialized are treated as errors.
>  >
>  > That MAY doesn't appear to help.  I suggest starting the sentence with:
>  >
>  >    In order to avoid errors caused by use of undefined FIDs or
> uninitialized
>  >    credit windows, this Data Item SHOULD be included ...
>  >
>
> Humm okay, even though I think this is a style comment...
>
>  >>> [E] Credit Window Size: In Section 2.3.1 of the flow control draft
> (Credit
>  >>> Window Initialization), what happens if the Credit Value exceeds the
> Credit
>  >>> Window Size?
>  >> Okay will add a sentence.
>  >
>  > Nit to fix in new sentence ...
>  >
>  >    If the resulting Credit Value results in the credit
>  >    window exceeding the represented Credit Window Max Size, the Credit
>  >    Window Size is used as the new credit window size.
>  >
>  > In last line - s/Window Size/Window Max Size/
>  >
>
> good catch, also fixed in one other place!
>
> Thanks again for all the great comments.  I'll post updated docs shortly.
>
> Lou
>
>  > Thanks, --David
>  >
>  > -----Original Message-----
>  > From: Lou Berger <[e-mail]>
>  > Sent: Tuesday, March 22, 2022 10:06 AM
>  > To: Black, David; [e-mail]
>  > Cc: MANET IETF
>  > Subject: Fwd: Tsvart early review of
> draft-ietf-manet-dlep-credit-flow-control-09
>  >
>  >
>  > [EXTERNAL EMAIL]
>  >
>  >
>  > This is a resend as it seems the first message didn't make it to David
>  > or the archive.
>  >
>  > -------- Forwarded Message --------
>  > Subject: Re: Tsvart early review of
>  > draft-ietf-manet-dlep-credit-flow-control-09
>  > Date: Thu, 24 Feb 2022 07:18:55 -0500
>  > From: Lou Berger <[e-mail]>
>  > To: David Black <[e-mail]>, [e-mail]
>  > CC: [e-mail],
> [e-mail]
>  >
>  > David,
>  >
>  > Please see (overdue) responses in-line below.
>  >
>  > On 11/19/2021 12:02 PM, David Black via Datatracker wrote:
>  >> Reviewer: David Black
>  >> Review result: On the Right Track
>  >>
>  >> TSVART Review of:
>  >>
>  >>          DLEP Credit-Based Flow Control Messages and Data Items
>  >> draft-ietf-manet-dlep-credit-flow-control-09
>  >>          DLEP DiffServ Aware Credit Window Extension
>  >> draft-ietf-manet-dlep-da-credit-extension-12
>  >>          DLEP Traffic Classification Data Item
>  >> draft-ietf-manet-dlep-traffic-classification-06
>  >>
>  >> Reviewer: David L. Black <[e-mail]>
>  >> Date: November 19, 2021
>  >>
>  >> This is a combined review of all three drafts.
>  >>
>  >> These three drafts specify a flow control mechanism between a
> "router" and a
>  >> "modem" in the sending direction (router to modem) to prevent overrun
> of
>  >> buffers in the "modem".  The mechanism is designed to provide flow
> control for
>  >> multiple queues in the "modem" that are independently protected,
> with the
>  >> "modem" in control of how much data the router is allowed to send -
> to a first
>  >> approximation, there is a separate instance of flow control, called
> a Credit
>  >> Window, for each queue.  Both Diffserv (DSCP) and Ethernet Priority
> (PCP)
>  >> values can be used to classify traffic to determine which flow
> control instance
>  >> (Credit Window) applies to each packet being sent to the "modem" by the
>  >> "router".
>  >>
>  >> Each of the drafts is reasonably well written, but there are some
> difficulties
>  >> in understanding the combination of the three drafts, which have to
> be used
>  >> together in order to implement this flow control mechanism. That's
> one of a
>  >> number of issues that deserve attention.
>  >>
>  >> ****** Major Issues:
>  >>
>  >> -- [1] -- Number of Documents
>  >>
>  >> I understand and see the merit in specifying the flow control mechanism
>  >> (-credit-flow-control draft) independently of traffic classification
>  >> (-traffic-classification draft).  In contrast, I do not see a strong
> rationale
>  >> for the separate DA credit extension draft (-da-credit-extension),
> which has
>  >> very little content (about a page of actual protocol specification
> plus one
>  >> addition to an IANA registry).
>  >>
>  >> I strongly suggest folding the DA credit extension draft into the
> traffic
>  >> classification draft, as the resulting two-draft structure will be
> easier to
>  >> explain, and more importantly, should be easier for implementers to
> understand.
>  >
>  > This was the decision of the WG made a long time ago. I defer to the
>  > chairs on this.
>  >
>  >> -- [2] -- Flow Match Criteria
>  >>
>  >> The Traffic Classification draft does not explain how traffic
> classification is
>  >> actually performed, i.e., how the applicable FID is determined for a
> packet,
>  >> particularly when more than one FID is possible. For example,
> suppose a packet
>  >> carries a DSCP that matches FID 5 and a PCP that matches FID 7 -
> does that
>  >> packet consume FID 5 credits or FID 7 credits?  If the router and
> modem make
>  >> different decisions, the result may be undesirable. This example is
> within the
>  >> same TID - the situation appears to be worse across TIDs for the same
>  >> destination, because the same match criteria could appear in
> multiple TIDs,
>  >> e.g., same DSCP could match different FIDs under different TIDs.
> Clarity is
>  >> needed here so that the router and modem agree on which FID's
> credits are
>  >> consumed by each packet, and on a related note, the DA credit
> extension draft
>  >> does not prohibit use of Ethernet traffic classification - perhaps
> it should.
>  >
>  > It took me a bit to decide how to address this (valid) comment. I
>  > propose the following change to
>  > draft-ietf-manet-dlep-traffic-classification-06:
>  >
>  > At the end of section 2.3.1, I propose adding the following text:
>  >
>  >           If a packet matches both a DSCP Field Value,see Section 2.2.
>  >           and a Priority Field value, the DSCP
>  >           associated TID MUST take precedence.
>  >
>  >> -- [3] -- Router MAY ignore flow control
>  >>
>  >> The Management Considerations section in both the traffic
> classification and DA
>  >> credit extension drafts contains this text:
>  >>
>  >>     When modem-provided credit window information exceeds the
> capabilities of a
>  >>     router, the router MAY use a subset of the provided credit windows.
>  >>
>  >> In other words, the router MAY ignore any credit windows that exceed
> the
>  >> routers capabilities ... which in practice could amount to "MAY
> ignore any
>  >> credit windows that the router implementer views as inconvenient"
> ... with the
>  >> router proceeding to overrun the corresponding modem queues.  That
> doesn't seem
>  >> right, so if this is intended, some serious rationale/explanation
> ought to be
>  >> provided.
>  >
>  > I personally have no objection to MUST, but perhaps a SHOULD is the
>  > right answer given that there is an alternative defined.
>  >
>  >
>  >> -- [4] -- Association Not Well-Specified
>  >>
>  >> The overall association structure is that flow control instances are
> identified
>  >> by FIDs which are associated with TIDs which are associated with
> destinations.
>  >> That structure is described towards the end of Section 2 of the flow
> control
>  >> draft, but the details of how it is done are mostly missing - the
> flow control
>  >> draft ought to state that:
>  >>
>  >>          i) FIDs are associated with TIDs via traffic classification
> information
>  >>          specified in the traffic classification draft. ii) TIDs
> (and their
>  >>          associated FIDs) are associated with destinations via use
> of the Credit
>  >>          Window Association data item in DLEP Destination Up and
> Destination
>  >>          Update messages.
>  >>
>  >> The first item (i) can be mostly inferred from the last few
> paragraphs in
>  >> section 2 of the flow control draft (and needs to be more explicit),
> but the
>  >> second item (ii) is missing because the flow control draft does not
> specify how
>  >> to determine the "destination" with which a TID is associated by a
> Credit
>  >> Window Association data item (in a DLEP Destination Up or
> Destination Update)
>  >> message.  I would expect that the destination is a relatively
> obvious field in
>  >> those messages, but the draft needs to specify that field.  This
> should be easy
>  >> to fix, although it made the drafts much more difficult to understand.
>  >
>  > Okay I'll try to improve the language in section 2 to cover (I) and (II)
>  >
>  >
>  >>
>  >> -- [5] -- Initialization vs. in-flight traffic
>  >>
>  >> The Credit Window Initialization data item appears to be intended to
> establish
>  >> common state for a Credit Window (e.g., size, number of credits)
> across the
>  >> router and modem ... but ... that data item appears to be allowed to
> be sent
>  >> while there's traffic in flight.  The result would be that the modem
> would
>  >> count in-flight traffic against the initialized Credit Window, but
> the router
>  >> would not.   The resulting inconsistency deserves discussion - it may be
>  >> acceptable if the amount of traffic in flight is miniscule by
> comparison to
>  >> both the Credit Window size and initial credit balance.
>  >
>  > This is the purpose of the credit window status DI.  The current text
> says:
>  >
>  >           Once the credit window is identified, the modem SHOULD
> check the
>  >           received Current Credit Window Size field value against the
> outstanding credit
>  >           window's available credits at the time the most recent
> Credit Window
>  >           Initialization or Grant Data Item associated with the
> indicated FID
>  >           was sent.  If the values significantly differ, i.e.,
> greater than can
>  >           be accounted for based on observed data frames, then the
> modem SHOULD
>  >           send a Credit Window Initialization Data Item to reset the
> associated
>  >           credit window size to the modem's current view of the available
>  >           credits.  As defined above, Credit Window Initialization
> Data Items are
>  >           sent in Session Update Messages.  When multiple Data Items
> need to be
>  >           sent, they SHOULD be combined into a single message when
> possible.
>  >           Alternatively, and also in cases where there are small
> differences,
>  >           the modem MAY adjust the values sent in Credit Window Grant
> Data Items
>  >           to account for the reported Credit Window.
>  >
>  > is this insufficient? if so, can you suggest some alternate wording.
>  >
>  >
>  >> -- [6] -- Security Considerations
>  >>
>  >> Although this is a Transport review, I found a security issue that
> would be
>  >> better dealt with now before the security directorate points it out
> ;-) - the
>  >> security considerations sections in each of the 3 drafts claims that
> adding
>  >> credit window control and flow functionality to DLEP does not
> introduce any new
>  >> security considerations (vulnerabilities). That's a nice try, but it's
>  >> incorrect.
>  >>
>  >> These drafts specify a new resource (credits in a credit window) that is
>  >> subject to resource exhaustion attacks that could cause
> denial-of-service.  For
>  >> example, suppose an attacker injects a Credit Window Initialization
> data item
>  >> that contains almost no credits and/or specifies a ridiculously tiny
> Window
>  >> (Max) Size.  I expect that the protocol contains mechanisms to
> counter this and
>  >> related attacks on credit resources (e.g., if something looks wrong,
> the modem
>  >> reinitializes the Credit Window), but the current text incorrectly
> asserts the
>  >> non-existence of such attacks.  These sorts of attacks definitely
> exist - I am
>  >> aware of a (subsequently fixed) resource exhaustion problem in another
>  >> credit-based flow control mechanism caused by an unanticipated
> environmental
>  >> "attack" on signal integrity of credit exchange messages, resulting
> in message
>  >> discard and credit loss.
>  >
>  > How about:
>  >
>  >        This document introduces credit window control and flow mechanisms
>  >       to DLEP.  These mechanisms expose vulnerabilities similar to
>  >       existing DLEP messages, e.g., Destination UP or Down message
>  >       injection attacks. The security mechanisms documented in <xref
>  >       target="RFC8175"/> can be applied equally to the mechanism defined
>  >       in this document.
>  >
>  >> ***** Minor Issues:
>  >>
>  >> [A] Packets consume credits: Section 2 of the flow control draft
> needs to say
>  >> somewhere early in the section that each packet that a router sends
> consumes a
>  >> credit for each octet in the packet.  This is to be found at the end
> of the
>  >> second paragraph in Section 2.1, but ought to be stated much
> earlier, e.g., at
>  >> the end of second paragraph in Section 2.
>  > okay
>  >>
>  >> [B] Bidirectional messages: Both the Credit Control Message and its
> response
>  >> can be sent in both directions (i.e., by both modems and routers).
> Has the
>  >> possibility of using different message types in each direction been
> considered?
>  >>   That might help out people who have to read packet traces/dumps.
> NOTE: "Yes,
>  >> that was carefully considered and the WG decided not to do that." is
> a fine
>  >> response.
>  > It was discussed, and the current approach was selected by the WG.
>  >> [C] Data item usage:  It's not clear which data items are required,
> allowed or
>  >> prohibited in which DLEP messages.  It appears that any data item
> MAY be used
>  >> in any DLEP message, which is surely not what was intended.  Both
> the flow
>  >> control and traffic classification drafts ought to spell out these
> details,
>  >> including what happens when  the rules are violated, e.g., data item
> shows up
>  >> in a message where it's not supposed to be used - is the data item
> ignored or
>  >> does it cause an error?
>  >
>  > Unexpected DIs in messages is defined by the base spec -- and yes, it's
>  > an error.  These drafts don't modify that behavior.
>  >
>  >
>  >>
>  >> [D] Uninitialized window: The second sentence of Section 2.3.1 in
> the flow
>  >> control draft begins with:
>  >>
>  >>     This Data Item SHOULD be included in any Session Initialization
> Response
>  >>     Message that also ...
>  >>
>  >> What happens if that SHOULD is not adhered to?  Please explain.
>  >
>  > There's a may immediately following.  Other places in the document
>  > describe that use of an FID/credit window in a message before it is
>  > defined/initialized are treated as errors.
>  >
>  >> [E] Credit Window Size: In Section 2.3.1 of the flow control draft
> (Credit
>  >> Window Initialization), what happens if the Credit Value exceeds the
> Credit
>  >> Window Size?
>  > Okay will add a sentence.
>  >> I also strongly suggest renaming Credit Window Size to Credit Window
> Max Size
>  >> or something similar to avoid possible confusion of this concept
> with current
>  >> credit balance (Credit Value)
>  > Sure
>  >>
>  >> [F] TID association with destination:
>  >>
>  >> Section 2.3.2 of the flow control draft (Credit Window Association)
> says that a
>  >> the traffic classification information associated with a TID "MUST be
>  >> associated with the destination" - what does that mean?
>  >>
>  >> Specifically:
>  >> - Does that traffic classification information replace any
> existing     traffic
>  >> classification information, e.g., if the info associated with that
> TID has been
>  >> updated since that TID was most recently associated with that
> destination? - Is
>  >> there any limit on the number of TIDs or amount of traffic
> classification
>  >> information that can be associated with a destination? If so, what
> happens if
>  >> that limit is exceeded? Also note that "the destination" is not
> currently
>  >> defined, see [4] above.
>  >
>  > I'll drop:
>  >
>  > That clause, the next MUST is sufficient.
>  >
>  >> [G] Bad Credit Window math: The next to last paragraph in Section
> 2.3.3 (Credit
>  >> Window Grant) of the flow control draft contains this text about
> adding credit
>  >> to a window:
>  >>
>  >>     If the increase results in a window overflow, i.e., the size of
> the credit
>  >>     window after the increase is smaller than the original credit
> window size,
>  >>     the Credit Window must be set to its maximum (0xFFFFFFFFFFFFFFFF).
>  >>
>  >> That's doubly wrong:
>  >> - consider a window max size of 10, a current window size of 3 and
> an addition
>  >> of 15 credits.  Overflow occurs, but the window size increases, as
> the new
>  >> window size is 8. - The maximum size of the credit window may be
> considerably
>  >> smaller than 0xFFFFFFFFFFFFFFFF.
>  >>
>  >> For the first item, just use the concept of overflow in modular
> arithmetic.
>  >> For the second item, remove that all-F's constant and refer back to
> the Credit
>  >> Window Initialization material for max size.
>  > WFM
>  >>
>  >> This text is also an example of why [E] strongly suggests renaming
> Credit
>  >> Window Size to Credit Window Max Size or somthing similar.
>  > done
>  >> [H] Amount of imprecision: Section 2.3.4 of the flow control draft
> discusses
>  >> modem comparison of internal values with values received from the
> router, and
>  >> contains this text:
>  >>
>  >>     If the values significantly differ, i.e., greater than can be
> accounted for
>  >>     based on observed data frames,
>  >>
>  >> That needs some guidance on what a significant difference is and how
> much of a
>  >> "greater than" difference ought to trigger the consequences,
> accompanied by a
>  >> warning against precise (== vs. !=) comparisons (e.g., courtesy of
> [5] above).
>  > I believe we (once) discussed this ad concluded it could be
>  > implementation dependent.  If you feel strongly on this, can you propose
>  > some language that we can discuss in the WG?
>  >> [I] Update means what?: The last paragraph of Section 2.1 of the traffic
>  >> classification draft (Traffic Classification Data Item) contains:
>  >>
>  >>     the router MUST update the information using the values carried
> in the Data
>  >>     Item.
>  >>
>  >> What does "update" mean, e.g., "replace", "merge"?
>  > MUST replace.
>  >>
>  >> [J] Error behavior: Section 2.1.1 of the traffic classification
> draft leaves
>  >> error behavior as a "go fish" exercise for the reader:
>  >>
>  >>     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.
>  >>
>  >> That doesn't tell an implementer what to do - this needs to cite a
> reference
>  >> that specifies the applicable error behavior.
>  > I disagree, DLEP has defined error processing in the base spec. There
>  > has been discussion about changing that behavior in an update.  I don't
>  > think repeating what is in the base spec is a good idea, particularly in
>  > light of that potential update.
>  >> [K] RFC 2474 CU field: Section 2.2 of the traffic classification
> draft defines
>  >> the DS Field as an octet and reproduces the definition from RFC 2474
> that
>  >> includes the DSCP and CU fields.  Unfortunately, this does not
> reflect updates
>  >> to RFC 2474 - those two bits are now the ECN Field, which may be
> non-zero.
>  >>
>  >> The fix for this is simple - use the 6 bit DSCP field from RFC 2474
> and replace
>  >> the CU field with two zero bits to produce an octet.
>  > okay, changed to stat that DSCP comes from 2474 and the bits "MUST be
> zero"
>  >>
>  >> [L] 802.1Q DEI field: This has a similar problem to the CU field ...
> and a
>  >> similar solution - replace the DEI field with a zero bit.
>  > Done!
>  >>
>  >> ****** Nits:
>  >>
>  >> FID paragraph in section 2.3.5 of the flow control draft:
>  >>          The FID also uniquely identifies a credit window
>  >> s/identifies/indicates/ for consistency.
>  > sure
>  >>
>  >> End of Section 2 of the traffic classification draft:
>  >>          TID and FID values is a modem-local scope.
>  >> s/is a/have/
>  >
>  > Thank you!
>  >
>  > I'll publish a matching update shortly.
>  >
>  > Lou
>  >
>  >
>  >>
>  >>
>
> _______________________________________________
> manet mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/manet