Skip to content

Commit

Permalink
Feedback fixups
Browse files Browse the repository at this point in the history 
Summary of changes:

1. adjust use of `NLA_F_NESTED`
   flag in both tests and emitted messages
   to reflect the kernel's stated usage
   (i.e. that `NLA_F_NESTED` be set for the tc-actions options).

   This required
   adjusting the tests which seem to have captured iproute2's
   incorrectly formed messages
   (iproute2 does not always set the `NLA_F_NESTED` flag)

2. Remove the use of hex as a dev dependency
   and use a const slice of `u8` instead as
   per @cathay4t request

3. remove links to iproute2's github as per
   @cathay4t request

4. trivial documentation cleanup

5. restore previously deleted test
  • Loading branch information
@daniel-noland
daniel-noland committed May 14, 2024
1 parent 8f2d6ec commit 6a28989
Show file tree
Hide file tree
Showing 6 changed files with 134 additions and 69 deletions.
1 change: 0 additions & 1 deletion Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,5 @@ netlink-packet-utils = { version = "0.5.2" }
name = "dump_packet_links"

[dev-dependencies]
hex = "0.4.3"
netlink-sys = { version = "0.8.5" }
pretty_assertions = "0.7.2"
21 changes: 7 additions & 14 deletions src/tc/actions/action.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,10 @@ const TCA_ACT_TAB: u16 = 1;
#[derive(Debug, PartialEq, Eq, Clone)]
#[non_exhaustive]
pub struct TcAction {
/// Table id. Corresponds to the `Kind` of the action.
/// Table id.
/// Corresponds to the [`Kind`] of the action.
///
/// [`Kind`]: crate::tc::TcActionAttribute::Kind
pub tab: u16,
/// Attributes of the action.
pub attributes: Vec<TcActionAttribute>,
Expand Down Expand Up @@ -203,17 +206,7 @@ impl Nla for TcActionAttribute {
fn kind(&self) -> u16 {
match self {
Self::Kind(_) => TCA_ACT_KIND,
Self::Options(opts) => {
// NOTE: the kernel simply doesn't consistently use the nested
// flag based on captured messages.
// This is heuristically trying to match the kernel's behavior
// but may not be correct.
if opts.len() == 1 {
TCA_ACT_OPTIONS | NLA_F_NESTED
} else {
TCA_ACT_OPTIONS
}
}
Self::Options(_) => TCA_ACT_OPTIONS | NLA_F_NESTED,
Self::Index(_) => TCA_ACT_INDEX,
Self::Stats(_) => TCA_ACT_STATS,
Self::Cookie(_) => TCA_ACT_COOKIE,
Expand Down Expand Up @@ -274,8 +267,8 @@ where
}
}

/// `TcActionOption` is a netlink message attribute that describes an option of
/// a [tc-actions] action.
/// [`TcActionOption`] is a netlink message attribute that describes an option
/// of a [tc-actions] action.
///
/// This enum is non-exhaustive as new action types may be added to the kernel
/// at any time.
Expand Down
20 changes: 2 additions & 18 deletions src/tc/actions/message.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,27 +28,11 @@ bitflags! {
#[derive(Debug, PartialEq, Eq, Clone, Copy, Default, PartialOrd, Ord, Hash)]
#[non_exhaustive]
pub struct TcActionMessageFlags: u32 {
/// From `iproute2`'s [`rtnetlink.h`]
///
/// If set, this flag enables more than TCA_ACT_MAX_PRIO actions in a single
/// If set, this flag enables more than `TCA_ACT_MAX_PRIO` actions in a single
/// actions listing operation.
///
/// > TCA_ACT_FLAG_LARGE_DUMP_ON user->kernel to request for larger than
/// TCA_ACT_MAX_PRIO actions in a dump.
/// All dump responses will contain the number of actions being dumped
/// stored in for user app's consumption in TCA_ROOT_COUNT
///
/// [`rtnetlink.h`]: https://github.com/iproute2/iproute2/blob/89210b9ec1c445ae963d181b5816d12a0cdafbb6/include/uapi/linux/rtnetlink.h#L803-L806
const LargeDump = TCA_ACT_FLAG_LARGE_DUMP_ON;
/// If set, this flag restricts an action dump to only include essential
/// details.
///
/// From `iproute2`'s [`rtnetlink.h`]:
///
/// > TCA_ACT_FLAG_TERSE_DUMP user->kernel to request terse (brief) dump
/// that only includes essential action info (kind, index, etc.)
///
/// [`rtnetlink.h`]: https://github.com/iproute2/iproute2/blob/89210b9ec1c445ae963d181b5816d12a0cdafbb6/include/uapi/linux/rtnetlink.h#L808-L809
const TerseDump = TCA_ACT_FLAG_TERSE_DUMP;
const _ = !0;
}
Expand Down Expand Up @@ -147,7 +131,7 @@ const TCA_ROOT_TIME_DELTA: u16 = 4;
const TCA_ROOT_EXT_WARN_MSG: u16 = 5;

/// This enum is used to represent the different types of attributes that can be
/// part of a `TcActionMessage`.
/// part of a [`TcActionMessage`].
///
/// This enum is non-exhaustive, additional variants may be added in the future.
#[derive(Debug, PartialEq, Eq, Clone)]
Expand Down
75 changes: 64 additions & 11 deletions src/tc/actions/tests/message.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,21 +39,75 @@ mod mirror {

/// Captured `TcActionMessage` examples used for testing.
mod message {
/// Request
/// Capture of request message for
///
/// ```bash
/// tc actions add action mirred egress redirect dev lo index 1
/// ```
pub(super) const CREATE1: &str = "0000000038000100340001000b0001006d69727265640000240002802000020001000000000000000400000000000000000000000100000001000000";
/// Request
pub(super) const CREATE1: &[u8] = &[
0x00, 0x00, 0x00, 0x00, 0x38, 0x00, 0x01, 0x00, 0x34, 0x00, 0x01,
0x00, 0x0b, 0x00, 0x01, 0x00, 0x6d, 0x69, 0x72, 0x72, 0x65, 0x64,
0x00, 0x00, 0x24, 0x00, 0x02, 0x80, 0x20, 0x00, 0x02, 0x00, 0x01,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00,
0x00, 0x01, 0x00, 0x00, 0x00,
];
/// Capture of request message for
///
/// ```bash
/// tc actions add action mirred ingress mirror dev lo index 2
/// ```
pub(super) const CREATE2: &str = "0000000038000100340001000b0001006d69727265640000240002802000020002000000000000000300000000000000000000000400000001000000";
/// Response
pub(super) const CREATE2: &[u8] = &[
0x00, 0x00, 0x00, 0x00, 0x38, 0x00, 0x01, 0x00, 0x34, 0x00, 0x01,
0x00, 0x0b, 0x00, 0x01, 0x00, 0x6d, 0x69, 0x72, 0x72, 0x65, 0x64,
0x00, 0x00, 0x24, 0x00, 0x02, 0x80, 0x20, 0x00, 0x02, 0x00, 0x02,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x03, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x04, 0x00, 0x00,
0x00, 0x01, 0x00, 0x00, 0x00,
];
/// Capture of request message for
///
/// ```bash
/// tc actions list action mirred
/// ```
pub(super) const LIST: &str = "00000000080003000200000064010100b00000000b0001006d6972726564000044000400140001000000000000000000000000000000000014000700000000000000000000000000000000001800030000000000000000000000000000000000000000000c000900000000000300000008000a0000000000480002002000020001000000000000000400000001000000000000000100000001000000240001000000000000000000000000000000000000000000000000000000000000000000b00001000b0001006d6972726564000044000400140001000000000000000000000000000000000014000700000000000000000000000000000000001800030000000000000000000000000000000000000000000c000900000000000300000008000a0000000000480002002000020002000000000000000300000001000000000000000400000001000000240001000000000000000000000000000000000000000000000000000000000000000000";
///
/// after the messages in [`CREATE1`] and [`CREATE2`] have been added.
pub(super) const LIST: &[u8] = &[
0x00, 0x00, 0x00, 0x00, 0x08, 0x00, 0x03, 0x00, 0x02, 0x00, 0x00,
0x00, 0x64, 0x01, 0x01, 0x00, 0xb0, 0x00, 0x00, 0x00, 0x0b, 0x00,
0x01, 0x00, 0x6d, 0x69, 0x72, 0x72, 0x65, 0x64, 0x00, 0x00, 0x44,
0x00, 0x04, 0x00, 0x14, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x14, 0x00, 0x07, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x18,
0x00, 0x03, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x0c, 0x00, 0x09, 0x00, 0x00, 0x00, 0x00, 0x00, 0x03, 0x00,
0x00, 0x00, 0x08, 0x00, 0x0a, 0x00, 0x00, 0x00, 0x00, 0x00, 0x48,
0x00, 0x02, 0x00, 0x20, 0x00, 0x02, 0x00, 0x01, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0x00,
0x00, 0x00, 0x24, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0xb0, 0x00, 0x01, 0x00, 0x0b, 0x00,
0x01, 0x00, 0x6d, 0x69, 0x72, 0x72, 0x65, 0x64, 0x00, 0x00, 0x44,
0x00, 0x04, 0x00, 0x14, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x14, 0x00, 0x07, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x18,
0x00, 0x03, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x0c, 0x00, 0x09, 0x00, 0x00, 0x00, 0x00, 0x00, 0x03, 0x00,
0x00, 0x00, 0x08, 0x00, 0x0a, 0x00, 0x00, 0x00, 0x00, 0x00, 0x48,
0x00, 0x02, 0x00, 0x20, 0x00, 0x02, 0x00, 0x02, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x03, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x01, 0x00,
0x00, 0x00, 0x24, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00,
];
}

#[test]
Expand Down Expand Up @@ -81,9 +135,8 @@ mod mirror {
}])],
};

let buf = hex::decode(message::CREATE1).unwrap();
let parsed = TcActionMessage::parse(
&TcActionMessageBuffer::new_checked(&buf).unwrap(),
&TcActionMessageBuffer::new_checked(&message::CREATE1).unwrap(),
)
.unwrap();
assert_eq!(parsed, expected);
Expand Down Expand Up @@ -114,7 +167,7 @@ mod mirror {
}])],
};

let buf = hex::decode(message::CREATE2).unwrap();
let buf = message::CREATE2;
let parsed = TcActionMessage::parse(
&TcActionMessageBuffer::new_checked(&buf).unwrap(),
)
Expand All @@ -123,6 +176,7 @@ mod mirror {
}

#[test]
#[allow(clippy::too_many_lines)]
fn parse_message3_list() {
let expected = TcActionMessage {
header: TcActionMessageHeader {
Expand Down Expand Up @@ -226,9 +280,8 @@ mod mirror {
]),
],
};
let buf = hex::decode(message::LIST).unwrap();
let parsed = TcActionMessage::parse(
&TcActionMessageBuffer::new_checked(&buf).unwrap(),
&TcActionMessageBuffer::new_checked(&message::LIST).unwrap(),
)
.unwrap();
assert_eq!(parsed, expected);
Expand Down
Loading

0 comments on commit 6a28989

Please sign in to comment.