[RTG-DIR]Rtgdir early review of draft-ietf-manet-dlep-traffic-classification-12

[Darren Dukes via Datatracker <[e-mail]>]

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.

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?

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?

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.

5. **Dynamic Updates**
   - How should dynamic updates be handled (2.3.1). Section 2.1 notes that
   session updates can happen.

### Minor

1. **Terminology Section**:
   - A dedicated terminology section is missing. As a new reader to this space
   it would be helpful.

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…

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?

### Grammatical

Please run the document through a grammar checker to improve readability, some
examples follow but I’ll leave you to find/fix others :)

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:"