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

[Don Fedyk <[e-mail]>]

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