Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inconsistent description in qos-interface-classifier-top #1201

Open
dliu2223 opened this issue Oct 19, 2024 · 3 comments
Open

Inconsistent description in qos-interface-classifier-top #1201

dliu2223 opened this issue Oct 19, 2024 · 3 comments

Comments

@dliu2223
Copy link

grouping qos-interface-classifier-top {
description
"Top-level grouping for a QoS classifier associated with an
interface";

container classifiers {
  description
    "Classifiers to be applied to the interface.";

  list classifier {
    key "type";

    description
      "A list of classifiers that should be applied to the interface";

    leaf type {
      type leafref {
        path "../config/type";
      }
      description
        "Reference to the classifier name.";
    }

The key type has description that references the classifier name which makes more sense. In our experience, each qos interface may have multiple classifiers with the same type, but with different name, so the list should be using the name as the key as defined in
grouping qos-interface-classifiers-config {
description
"Configuration parameters for the list of classifiers";

leaf name {
  type leafref {
    // current loc: /qos/interfaces/interface/input/classifiers/
    // classifier/config/name
    path "../../../../../../../classifiers/classifier/config/name";
  }
  description
    "Reference to the classifier to be applied to ingress traffic on
    the interface";
}
@dliu2223 dliu2223 changed the title Inconsistent description is qos-interface-classifier-top Inconsistent description in qos-interface-classifier-top Oct 19, 2024
@dliu2223
Copy link
Author

dliu2223 commented Oct 20, 2024

example use case:

  • Divide ipv4 classifier type in 2 groups, group 1 classifies packets based on specific transport destination port + ipv4 source and group 2 classifies packets based on ipv4 source. This grouping can be defined in /qos/classifiers/classifier[name]
  • 2 qos interfaces, each uses 1 group of ipv4 classifier, in current model, the /qos/interfaces/interface[interface-id=group1]/classifiers/classifier[type], when type is the key, we cannot separate 2 ipv4 classifiers to 2 qos interfaces.

@dplore
Copy link
Member

dplore commented Nov 1, 2024

Thanks for this @dliu2223. This would be a significant breaking change to the model. But I see the operational use case to want to create multiple classifiers and then apply them to interfaces in a composable way.

Would you like to send a PR for this change?

@LimeHat
Copy link

LimeHat commented Nov 1, 2024

Divide ipv4 classifier type in 2 groups, group 1 classifies packets based on specific transport destination port + ipv4 source and group 2 classifies packets based on ipv4 source. This grouping can be defined in /qos/classifiers/classifier[name]

Why is there a need for 2 classifiers (of the same type) in this scenario? 1 seems sufficient. Can you provide a few references (>1) to existing implementations of this? How classifier ordering or hierarchy is expected to work (if there are multiple matches)?

All of this seems very unusual and not backed by a real use case.

2 qos interfaces, each uses 1 group of ipv4 classifier, in current model, the /qos/interfaces/interface[interface-id=group1]/classifiers/classifier[type], when type is the key, we cannot separate 2 ipv4 classifiers to 2 qos interfaces.

I don't understand this. Each interface (in fact, each subinterface) can have its own classifier in the existing implementation. You listed only 1 interface, named group1 in your example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

No branches or pull requests

3 participants