Skip to content

Commit

Permalink
Change flows_per_sec to flows in tc module (#8237)
Browse files Browse the repository at this point in the history
Summary:
`flows` is a static value. As mentioned in [man page](https://man7.org/linux/man-pages/man8/tc-fq_codel.8.html):

> **flows**
       is the number of flows into which the incoming packets are
       classified. Due to the stochastic nature of hashing, multiple
       flows may end up being hashed into the same slot. Newer flows
       have priority over older ones. This parameter can be set only at
       load time since memory has to be allocated for the hash table.
       Default value is 1024.

Thus, converting this to a rate will always give zero. Change `flows_per_sec` to `flows` which is basically the last collected value.

Pull Request resolved: #8237

Reviewed By: antonis-m

Differential Revision: D59769678

Pulled By: brianc118

fbshipit-source-id: 6a4fafbef35e48e95acf3dc31c0d8acc971b7a1e
  • Loading branch information
mmynk authored and facebook-github-bot committed Jul 15, 2024
1 parent df308ac commit b0e79f4
Show file tree
Hide file tree
Showing 5 changed files with 13 additions and 27 deletions.
6 changes: 3 additions & 3 deletions below/dump/src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1270,7 +1270,7 @@ fn test_tc_titles() {
"qdisc.fq_codel.ce_threshold",
"qdisc.fq_codel.drop_batch_size",
"qdisc.fq_codel.memory_limit",
"qdisc.fq_codel.flows_per_sec",
"qdisc.fq_codel.flows",
];
assert_eq!(titles, expected_titles);
}
Expand Down Expand Up @@ -1315,7 +1315,7 @@ fn test_dump_tc_content() {
ce_threshold: 101,
drop_batch_size: 9000,
memory_limit: 123456,
flows_per_sec: Some(31415),
flows: 1024,
}),
}),
xstats: Some(model::XStatsModel {
Expand Down Expand Up @@ -1424,7 +1424,7 @@ fn test_dump_tc_content() {
"CeThreshold": "101",
"DropBatchSize": "9000",
"MemoryLimit": "123456",
"Flows": "31415/s",
"Flows": "1024",
"MaxPacket": "8675309",
"EcnMark": "299792458",
"NewFlowsLen": "314",
Expand Down
2 changes: 1 addition & 1 deletion below/model/src/common_field_ids.rs
Original file line number Diff line number Diff line change
Expand Up @@ -485,7 +485,7 @@ pub const COMMON_MODEL_FIELD_IDS: [&str; 478] = [
"tc.tc.<idx>.qdisc.fq_codel.ce_threshold",
"tc.tc.<idx>.qdisc.fq_codel.drop_batch_size",
"tc.tc.<idx>.qdisc.fq_codel.ecn",
"tc.tc.<idx>.qdisc.fq_codel.flows_per_sec",
"tc.tc.<idx>.qdisc.fq_codel.flows",
"tc.tc.<idx>.qdisc.fq_codel.interval",
"tc.tc.<idx>.qdisc.fq_codel.limit",
"tc.tc.<idx>.qdisc.fq_codel.memory_limit",
Expand Down
2 changes: 1 addition & 1 deletion below/model/src/sample_model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -977,7 +977,7 @@ pub const SAMPLE_MODEL_JSON: &str = r#"
"ce_threshold": 101,
"drop_batch_size": 9000,
"memory_limit": 123456,
"flows_per_sec": 31415
"flows": 1024
}
},
"xstats": {
Expand Down
26 changes: 6 additions & 20 deletions below/model/src/tc_model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,7 @@ impl SingleTcModel {
}

if let Some(sample) = sample.qdisc.as_ref() {
let last = last.and_then(|(last, d)| last.qdisc.as_ref().map(|l| (l, d)));

tc_model.qdisc = Some(QDiscModel::new(sample, last));
tc_model.qdisc = Some(QDiscModel::new(sample));
}

tc_model
Expand All @@ -123,17 +121,10 @@ pub struct QDiscModel {
}

impl QDiscModel {
fn new(sample: &QDisc, last: Option<(&QDisc, Duration)>) -> Self {
fn new(sample: &QDisc) -> Self {
match sample {
QDisc::FqCodel(sample) => Self {
fq_codel: {
last.map(|(l, d)| match l {
QDisc::FqCodel(last) => {
let last = Some((last, d));
FqCodelQDiscModel::new(sample, last)
}
})
},
fq_codel: Some(FqCodelQDiscModel::new(sample)),
},
}
}
Expand All @@ -149,11 +140,11 @@ pub struct FqCodelQDiscModel {
pub ce_threshold: u32,
pub drop_batch_size: u32,
pub memory_limit: u32,
pub flows_per_sec: Option<u32>,
pub flows: u32,
}

impl FqCodelQDiscModel {
fn new(sample: &tc::FqCodelQDisc, last: Option<(&tc::FqCodelQDisc, Duration)>) -> Self {
fn new(sample: &tc::FqCodelQDisc) -> Self {
Self {
target: sample.target,
limit: sample.limit,
Expand All @@ -163,12 +154,7 @@ impl FqCodelQDiscModel {
ce_threshold: sample.ce_threshold,
drop_batch_size: sample.drop_batch_size,
memory_limit: sample.memory_limit,
flows_per_sec: {
last.and_then(|l| {
let last = Some(l);
rate!(flows, sample, last, u32)
})
},
flows: sample.flows,
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions below/render/src/default_configs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1749,7 +1749,7 @@ impl HasRenderConfig for model::FqCodelQDiscModel {
CeThreshold => rc.title("CeThreshold"),
DropBatchSize => rc.title("DropBatchSize"),
MemoryLimit => rc.title("MemoryLimit"),
FlowsPerSec => rc.title("Flows").suffix("/s"),
Flows => rc.title("Flows"),
}
}
}
Expand Down Expand Up @@ -1793,7 +1793,7 @@ impl HasRenderConfigForDump for model::FqCodelQDiscModel {
CeThreshold => Some(gauge()),
DropBatchSize => Some(gauge()),
MemoryLimit => Some(gauge()),
FlowsPerSec => Some(gauge()),
Flows => Some(gauge()),
}
}
}
Expand Down

0 comments on commit b0e79f4

Please sign in to comment.