Skip to content

Commit

Permalink
tc: Fix DecodeError on sfq scheduler
Browse files Browse the repository at this point in the history
When running `GetQueueDiscipline` against `sfq` scheduler, we will get
DecodeError.

This is because different scheduler has different layout of
`TCA_OPTIONS`.

For `fq_codel`, it is a list of netlink attribute. For kernel code,
`fq_codel_dump()` has `nla_nest_start_noflag(skb, TCA_OPTIONS)`
For `sfq`, it is a single netlink attribute, trying to decode it as
`NlasIterator` will generate DecodeError as expected. For kernel code:
`sfq_dump()` only has `nla_put(skb, TCA_OPTIONS, sizeof(opt), &opt)`.

This patch created `pub(crate) struct VecTcOpt` to handle this
difference correctly.

Signed-off-by: Gris Ge <[email protected]>
  • Loading branch information
cathay4t committed Oct 24, 2023
1 parent 2457bdf commit 467b1c6
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 15 deletions.
19 changes: 7 additions & 12 deletions src/rtnl/tc/message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@ use netlink_packet_utils::{
DecodeError,
};

use super::nlas::VecTcOpt;
use crate::{
constants::*,
nlas::tc::{Nla, Stats, Stats2, StatsBuffer, TcOpt},
nlas::tc::{Nla, Stats, Stats2, StatsBuffer},
TcMessageBuffer, TC_HEADER_LEN,
};

Expand Down Expand Up @@ -119,17 +120,11 @@ impl<'a, T: AsRef<[u8]> + 'a> Parseable<TcMessageBuffer<&'a T>> for Vec<Nla> {
kind = parse_string(payload).context("invalid TCA_KIND")?;
Nla::Kind(kind.clone())
}
TCA_OPTIONS => {
let mut nlas = vec![];
for nla in NlasIterator::new(payload) {
let nla = nla.context("invalid TCA_OPTIONS")?;
nlas.push(
TcOpt::parse_with_param(&nla, &kind)
.context("failed to parse TCA_OPTIONS")?,
)
}
Nla::Options(nlas)
}
TCA_OPTIONS => Nla::Options(
VecTcOpt::parse_with_param(&buf, &kind)
.context(format!("Invalid TCA_OPTIONS for {kind}"))?
.0,
),
TCA_STATS => Nla::Stats(
Stats::parse(
&StatsBuffer::new_checked(payload)
Expand Down
1 change: 1 addition & 0 deletions src/rtnl/tc/nlas/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ pub use self::stats_basic::*;

mod options;
pub use self::options::*;
pub(crate) use self::options::VecTcOpt;

mod qdisc;
pub use self::qdisc::*;
Expand Down
40 changes: 39 additions & 1 deletion src/rtnl/tc/nlas/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

use anyhow::Context;
use netlink_packet_utils::{
nla::{self, DefaultNla, NlaBuffer},
nla::{self, DefaultNla, NlaBuffer, NlasIterator},
traits::{Parseable, ParseableParametrized},
DecodeError,
};
Expand Down Expand Up @@ -73,3 +73,41 @@ where
})
}
}

pub(crate) struct VecTcOpt(pub(crate) Vec<TcOpt>);

impl<'a, T, S> ParseableParametrized<NlaBuffer<&'a T>, S> for VecTcOpt
where
T: AsRef<[u8]> + ?Sized,
S: AsRef<str>,
{
fn parse_with_param(
buf: &NlaBuffer<&'a T>,
kind: S,
) -> Result<VecTcOpt, DecodeError> {
Ok(match kind.as_ref() {
ingress::KIND => {
Self(vec![TcOpt::parse_with_param(&buf, &kind).context(
format!("Failed to pase TCA_OPTIONS for {}", ingress::KIND),
)?])
}
u32::KIND | matchall::KIND => {
let mut nlas = vec![];
for nla in NlasIterator::new(buf.value()) {
let nla = nla.context(format!(
"invalid TCA_OPTIONS for {}",
kind.as_ref()
))?;
nlas.push(TcOpt::parse_with_param(&nla, &kind).context(
format!(
"failed to parse TCA_OPTIONS for {}",
kind.as_ref()
),
)?)
}
Self(nlas)
}
_ => Self(vec![TcOpt::Other(DefaultNla::parse(buf)?)]),

Check warning on line 110 in src/rtnl/tc/nlas/options.rs

View check run for this annotation

Codecov / codecov/patch

src/rtnl/tc/nlas/options.rs#L110

Added line #L110 was not covered by tests
})
}
}
4 changes: 2 additions & 2 deletions src/rtnl/tc/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

use crate::{
constants::*,
tc::{ingress, Nla, Stats, Stats2, StatsBuffer, TC_HEADER_LEN},
tc::{ingress, Nla, Stats, Stats2, StatsBuffer, TcOpt, TC_HEADER_LEN},
TcHeader, TcMessage, TcMessageBuffer,
};

Expand Down Expand Up @@ -167,7 +167,7 @@ fn tc_qdisc_ingress_read() {
assert_eq!(nla, &Nla::Kind(String::from(ingress::KIND)));

let nla = iter.next().unwrap();
assert_eq!(nla, &Nla::Options(vec![]));
assert_eq!(nla, &Nla::Options(vec![TcOpt::Ingress]));

let nla = iter.next().unwrap();
assert_eq!(nla, &Nla::HwOffload(0));
Expand Down

0 comments on commit 467b1c6

Please sign in to comment.