From 19f14357d15f5d38ff47cc9bef9d28bdb7469420 Mon Sep 17 00:00:00 2001
From: Gris Ge <fge@redhat.com>
Date: Tue, 24 Oct 2023 14:11:17 +0800
Subject: [PATCH] tc: Fix DecodeError on sfq scheduler

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 <fge@redhat.com>
---
 src/rtnl/tc/message.rs      | 19 +++++++-----------
 src/rtnl/tc/nlas/mod.rs     |  1 +
 src/rtnl/tc/nlas/options.rs | 40 ++++++++++++++++++++++++++++++++++++-
 src/rtnl/tc/test.rs         |  4 ++--
 4 files changed, 49 insertions(+), 15 deletions(-)

diff --git a/src/rtnl/tc/message.rs b/src/rtnl/tc/message.rs
index 70b29b92..b2aa2b9d 100644
--- a/src/rtnl/tc/message.rs
+++ b/src/rtnl/tc/message.rs
@@ -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,
 };
 
@@ -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)
diff --git a/src/rtnl/tc/nlas/mod.rs b/src/rtnl/tc/nlas/mod.rs
index 47ac2adc..b30eef4f 100644
--- a/src/rtnl/tc/nlas/mod.rs
+++ b/src/rtnl/tc/nlas/mod.rs
@@ -10,6 +10,7 @@ mod stats_basic;
 pub use self::stats_basic::*;
 
 mod options;
+pub(crate) use self::options::VecTcOpt;
 pub use self::options::*;
 
 mod qdisc;
diff --git a/src/rtnl/tc/nlas/options.rs b/src/rtnl/tc/nlas/options.rs
index 5fc9a727..15cc2394 100644
--- a/src/rtnl/tc/nlas/options.rs
+++ b/src/rtnl/tc/nlas/options.rs
@@ -2,7 +2,7 @@
 
 use anyhow::Context;
 use netlink_packet_utils::{
-    nla::{self, DefaultNla, NlaBuffer},
+    nla::{self, DefaultNla, NlaBuffer, NlasIterator},
     traits::{Parseable, ParseableParametrized},
     DecodeError,
 };
@@ -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)?)]),
+        })
+    }
+}
diff --git a/src/rtnl/tc/test.rs b/src/rtnl/tc/test.rs
index 003857f5..8316fcca 100644
--- a/src/rtnl/tc/test.rs
+++ b/src/rtnl/tc/test.rs
@@ -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,
 };
 
@@ -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));