[manet] Re: Rtgdir early review of draft-ietf-manet-dlep-traffic-classification-12

[Darren Dukes <[e-mail]>]

Thanks for the responses Don, no problem with the delay. I’ll read the
latest draft to see the closures and reply by the weekend.

Darren

On Tue, Nov 19, 2024 at 3:59 PM Don Fedyk <[e-mail]> wrote:

> Hi Darren
>
> Thanks for your comments. Apologies for the tardy reply.  I have taken an
> editorship role on this document to get closure. Responses inline and new
> draft posted. [Don]
>
> Regards,
> Don
>
> -----Original Message-----
> From: Darren Dukes via Datatracker <[e-mail]>
> Sent: Tuesday, August 6, 2024 12:24 PM
> To: [e-mail]
> Cc: [e-mail];
> [e-mail]
> Subject: Rtgdir early review of
> draft-ietf-manet-dlep-traffic-classification-12
>
> Reviewer: Darren Dukes
> Review result: Has Issues
>
> # Review of draft-ietf-manet-dlep-traffic-classification-12
>
> ## Overview
> The document defines a new Data Item for the Dynamic Link Exchange Protocol
> (DLEP) (RFC8175) to be used by other documents. Data items and sub data
> items are defined for DiffServ and Ethernet classifications. Overall I
> found the document clear enough to interpret as an implementor, I have a
> few questions/suggestions that should be easily dispatched by the authors
> and/or working group
>
> ## Major
>
> 1. **Section 2.1 - Credit-Based Flow Control**:
>    - Can you please describe how Traffic Classification Data Item interacts
>    with the credit-based flow control mechanisms [defined in
>    draft-ietf-manet-dlep-credit-flow-control](
> https://datatracker.ietf.org/doc/html/draft-ietf-manet-dlep-credit-flow-control
> ).
>     I don’t see this defined in the specification, yet it’s referenced as a
>    MUST.
>
> [Don] Another reviewer commented that an example would help.  After a bit
> of work we added a diagram and the example to the companion draft
> https://datatracker.ietf.org/doc/draft-ietf-manet-dlep-credit-flow-control/
> and conferring with Lou on this point, the Traffic Classification draft was
> split from the Credit Flow control to keep it independent and be able to
> work with other schemes that defined FIDs. That is why it is not normative.
> Traffic classification is one way that the flow control draft can be
> associated with traffic.  The example and description belongs in the credit
> flow draft because this draft is about a single data item.
>
> 2. **Flow Match Criteria**:
>    - I see no explanation how traffic classification is actually performed,
>    particularly when multiple Flow Identification Data Items could match a
>    single packet. Eg how does a router know which FID to use?
>
> [Don] There is only one match outcome possible per packet.  We have
> clarified the text on this. Typically, it will be on an Ethernet Priority
> Code Point (PCP) or a DiffServ Codepoint (DSCP).  The actual mechanism a
> router uses to classify the flows based on the DSCP or PCP is not defined
> here.
>
> 3. **RFC 8175 backward compatibility**
>    - The draft introduces new uses for existing DLEP messages -
> Destination Up
>    and Session Update. - RFC8175 says
> > If a received Message contains unrecognized, invalid, or disallowed
> > duplicate Data Items, 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.
>
>     - How does a sending implementation know what a receiving
> implementation
>     can consume and does this data item break existing receiver
> implementations?
>
> [Don] DLEP RFC has been designed to be extensible. The DLEP RFC 8175 has
> an extension negotiation mechanism.
>
>
> 4. **DSCP to Credit Mapping**
>    - How does Traffic Classification Data Item integrates with the DSCP to
>    Credit Mapping feature described in
>    draft-ietf-manet-dlep-da-credit-extension, does it? - I see references
> but
>    nothing normative.
>
> [Don] The Traffic Classification Data Item maps one or more code point to
> a FID. There may be multiple FIDs. There are two types of code points
> defined in these drafts: DiffServ Code Points (DSCPs) and Ethernet Priority
> Code Points (PCPs)  The draft-ietf-manet-dlep-da-credit-extension defines
> the IANA assigned DLEP Extension type value for DSCPs.  These documents
> were structured this way to allow vendors to support IP DSCPs (only),
> Ethernet PCPs (only) or both or any other future types and maintain
> compliance with the RFCs.
>
>
>
> 5. **Dynamic Updates**
>    - How should dynamic updates be handled (2.3.1). Section 2.1 notes that
>    session updates can happen.
> [Don] The Credit Window Flow Control document describes the dynamic
> updates for the credits.
>
> ### Minor
>
> 1. **Terminology Section**:
>    - A dedicated terminology section is missing. As a new reader to this
> space
>    it would be helpful.
> [Don] The base document RFC 8175 defines the terminology.
>
> 2. **Security Considerations**:
>    - The security considerations section should be expanded to discuss
>    potential risks associated with traffic classification data items, such
> as
>    the possibility of misclassification or malicious manipulation of
> traffic
>    classes. This is important for ensuring that implementers and operators
> are
>    aware of and can mitigate risks. I don’t think this is covered in
> RFC8175…
>
> [Don] We added some clarifying text in the security sections on both this
> draft and the credit flow control draft.
>
> 3. **Scalability**
>    - There is no discussion on scalability in devices producing this DI or
>    consuming it.  I assume there is some policy that would be implemented
> based
>    on classification. If appropriate this may be a manageability concern
> worth
>    documenting i.e. what is recommended when a receiver cannot maintain
> state,
>    and is that up to documents using this DI to specify or can some
> guidance be
>    given here?
>
> [Don] The Traffic Classification draft should not have scalability issues
> - routers can classify on DSCPs of PCPs today. The Credit Window Flow
> control does discuss some scalability aspects.
>
> ### Grammatical
>
> Please run the document through a grammar checker to improve readability,
> some examples follow but I’ll leave you to find/fix others :)
>
> [Don] Done Thank you.
>
> 1. **Abstract**:
>    - Current: "This document defines a new Dynamic Link Exchange Protocol
>    (DLEP) Data Item that is used to support traffic classification." -
>    Suggested: "This document defines a new Data Item for the Dynamic Link
>    Exchange Protocol (DLEP) to support traffic classification."
>
> 2. **Section 3.1**:
>    - Current: "The Traffic Classification Data Item is used to indicate..."
>    - Suggested: "The Traffic Classification Data Item indicates..."
>
> 3. **Section 4.2**:
>    - Current: "The following fields are defined for the Traffic
> Classification
>    Data Item:" - Suggested: "The Traffic Classification Data Item defines
> the
>    following fields:"
>
>
>
>