Skip to content

Commit 0227234

Browse files
daniel-nolandcathay4t
daniel-noland
authored and
cathay4t
committed
Feedback fixups
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
1 parent 65f699a commit 0227234

File tree

6 files changed

+134
-69
lines changed

6 files changed

+134
-69
lines changed

‎Cargo.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,5 @@ netlink-packet-utils = { version = "0.5.2" }
2727
name = "dump_packet_links"
2828

2929
[dev-dependencies]
30-
hex = "0.4.3"
3130
netlink-sys = { version = "0.8.5" }
3231
pretty_assertions = "0.7.2"

‎src/tc/actions/action.rs

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,10 @@ const TCA_ACT_TAB: u16 = 1;
2525
#[derive(Debug, PartialEq, Eq, Clone)]
2626
#[non_exhaustive]
2727
pub struct TcAction {
28-
/// Table id. Corresponds to the `Kind` of the action.
28+
/// Table id.
29+
/// Corresponds to the [`Kind`] of the action.
30+
///
31+
/// [`Kind`]: crate::tc::TcActionAttribute::Kind
2932
pub tab: u16,
3033
/// Attributes of the action.
3134
pub attributes: Vec<TcActionAttribute>,
@@ -203,17 +206,7 @@ impl Nla for TcActionAttribute {
203206
fn kind(&self) -> u16 {
204207
match self {
205208
Self::Kind(_) => TCA_ACT_KIND,
206-
Self::Options(opts) => {
207-
// NOTE: the kernel simply doesn't consistently use the nested
208-
// flag based on captured messages.
209-
// This is heuristically trying to match the kernel's behavior
210-
// but may not be correct.
211-
if opts.len() == 1 {
212-
TCA_ACT_OPTIONS | NLA_F_NESTED
213-
} else {
214-
TCA_ACT_OPTIONS
215-
}
216-
}
209+
Self::Options(_) => TCA_ACT_OPTIONS | NLA_F_NESTED,
217210
Self::Index(_) => TCA_ACT_INDEX,
218211
Self::Stats(_) => TCA_ACT_STATS,
219212
Self::Cookie(_) => TCA_ACT_COOKIE,
@@ -274,8 +267,8 @@ where
274267
}
275268
}
276269

277-
/// `TcActionOption` is a netlink message attribute that describes an option of
278-
/// a [tc-actions] action.
270+
/// [`TcActionOption`] is a netlink message attribute that describes an option
271+
/// of a [tc-actions] action.
279272
///
280273
/// This enum is non-exhaustive as new action types may be added to the kernel
281274
/// at any time.

‎src/tc/actions/message.rs

Lines changed: 2 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -28,27 +28,11 @@ bitflags! {
2828
#[derive(Debug, PartialEq, Eq, Clone, Copy, Default, PartialOrd, Ord, Hash)]
2929
#[non_exhaustive]
3030
pub struct TcActionMessageFlags: u32 {
31-
/// From `iproute2`'s [`rtnetlink.h`]
32-
///
33-
/// If set, this flag enables more than TCA_ACT_MAX_PRIO actions in a single
31+
/// If set, this flag enables more than `TCA_ACT_MAX_PRIO` actions in a single
3432
/// actions listing operation.
35-
///
36-
/// > TCA_ACT_FLAG_LARGE_DUMP_ON user->kernel to request for larger than
37-
/// TCA_ACT_MAX_PRIO actions in a dump.
38-
/// All dump responses will contain the number of actions being dumped
39-
/// stored in for user app's consumption in TCA_ROOT_COUNT
40-
///
41-
/// [`rtnetlink.h`]: https://github.com/iproute2/iproute2/blob/89210b9ec1c445ae963d181b5816d12a0cdafbb6/include/uapi/linux/rtnetlink.h#L803-L806
4233
const LargeDump = TCA_ACT_FLAG_LARGE_DUMP_ON;
4334
/// If set, this flag restricts an action dump to only include essential
4435
/// details.
45-
///
46-
/// From `iproute2`'s [`rtnetlink.h`]:
47-
///
48-
/// > TCA_ACT_FLAG_TERSE_DUMP user->kernel to request terse (brief) dump
49-
/// that only includes essential action info (kind, index, etc.)
50-
///
51-
/// [`rtnetlink.h`]: https://github.com/iproute2/iproute2/blob/89210b9ec1c445ae963d181b5816d12a0cdafbb6/include/uapi/linux/rtnetlink.h#L808-L809
5236
const TerseDump = TCA_ACT_FLAG_TERSE_DUMP;
5337
const _ = !0;
5438
}
@@ -147,7 +131,7 @@ const TCA_ROOT_TIME_DELTA: u16 = 4;
147131
const TCA_ROOT_EXT_WARN_MSG: u16 = 5;
148132

149133
/// This enum is used to represent the different types of attributes that can be
150-
/// part of a `TcActionMessage`.
134+
/// part of a [`TcActionMessage`].
151135
///
152136
/// This enum is non-exhaustive, additional variants may be added in the future.
153137
#[derive(Debug, PartialEq, Eq, Clone)]

‎src/tc/actions/tests/message.rs

Lines changed: 64 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -39,21 +39,75 @@ mod mirror {
3939

4040
/// Captured `TcActionMessage` examples used for testing.
4141
mod message {
42-
/// Request
42+
/// Capture of request message for
43+
///
4344
/// ```bash
4445
/// tc actions add action mirred egress redirect dev lo index 1
4546
/// ```
46-
pub(super) const CREATE1: &str = "0000000038000100340001000b0001006d69727265640000240002802000020001000000000000000400000000000000000000000100000001000000";
47-
/// Request
47+
pub(super) const CREATE1: &[u8] = &[
48+
0x00, 0x00, 0x00, 0x00, 0x38, 0x00, 0x01, 0x00, 0x34, 0x00, 0x01,
49+
0x00, 0x0b, 0x00, 0x01, 0x00, 0x6d, 0x69, 0x72, 0x72, 0x65, 0x64,
50+
0x00, 0x00, 0x24, 0x00, 0x02, 0x80, 0x20, 0x00, 0x02, 0x00, 0x01,
51+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00,
52+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00,
53+
0x00, 0x01, 0x00, 0x00, 0x00,
54+
];
55+
/// Capture of request message for
56+
///
4857
/// ```bash
4958
/// tc actions add action mirred ingress mirror dev lo index 2
5059
/// ```
51-
pub(super) const CREATE2: &str = "0000000038000100340001000b0001006d69727265640000240002802000020002000000000000000300000000000000000000000400000001000000";
52-
/// Response
60+
pub(super) const CREATE2: &[u8] = &[
61+
0x00, 0x00, 0x00, 0x00, 0x38, 0x00, 0x01, 0x00, 0x34, 0x00, 0x01,
62+
0x00, 0x0b, 0x00, 0x01, 0x00, 0x6d, 0x69, 0x72, 0x72, 0x65, 0x64,
63+
0x00, 0x00, 0x24, 0x00, 0x02, 0x80, 0x20, 0x00, 0x02, 0x00, 0x02,
64+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x03, 0x00, 0x00, 0x00,
65+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x04, 0x00, 0x00,
66+
0x00, 0x01, 0x00, 0x00, 0x00,
67+
];
68+
/// Capture of request message for
69+
///
5370
/// ```bash
5471
/// tc actions list action mirred
5572
/// ```
56-
pub(super) const LIST: &str = "00000000080003000200000064010100b00000000b0001006d6972726564000044000400140001000000000000000000000000000000000014000700000000000000000000000000000000001800030000000000000000000000000000000000000000000c000900000000000300000008000a0000000000480002002000020001000000000000000400000001000000000000000100000001000000240001000000000000000000000000000000000000000000000000000000000000000000b00001000b0001006d6972726564000044000400140001000000000000000000000000000000000014000700000000000000000000000000000000001800030000000000000000000000000000000000000000000c000900000000000300000008000a0000000000480002002000020002000000000000000300000001000000000000000400000001000000240001000000000000000000000000000000000000000000000000000000000000000000";
73+
///
74+
/// after the messages in [`CREATE1`] and [`CREATE2`] have been added.
75+
pub(super) const LIST: &[u8] = &[
76+
0x00, 0x00, 0x00, 0x00, 0x08, 0x00, 0x03, 0x00, 0x02, 0x00, 0x00,
77+
0x00, 0x64, 0x01, 0x01, 0x00, 0xb0, 0x00, 0x00, 0x00, 0x0b, 0x00,
78+
0x01, 0x00, 0x6d, 0x69, 0x72, 0x72, 0x65, 0x64, 0x00, 0x00, 0x44,
79+
0x00, 0x04, 0x00, 0x14, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00,
80+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
81+
0x00, 0x14, 0x00, 0x07, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
82+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x18,
83+
0x00, 0x03, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
84+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
85+
0x00, 0x0c, 0x00, 0x09, 0x00, 0x00, 0x00, 0x00, 0x00, 0x03, 0x00,
86+
0x00, 0x00, 0x08, 0x00, 0x0a, 0x00, 0x00, 0x00, 0x00, 0x00, 0x48,
87+
0x00, 0x02, 0x00, 0x20, 0x00, 0x02, 0x00, 0x01, 0x00, 0x00, 0x00,
88+
0x00, 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00,
89+
0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0x00,
90+
0x00, 0x00, 0x24, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
91+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
92+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
93+
0x00, 0x00, 0x00, 0x00, 0x00, 0xb0, 0x00, 0x01, 0x00, 0x0b, 0x00,
94+
0x01, 0x00, 0x6d, 0x69, 0x72, 0x72, 0x65, 0x64, 0x00, 0x00, 0x44,
95+
0x00, 0x04, 0x00, 0x14, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00,
96+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
97+
0x00, 0x14, 0x00, 0x07, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
98+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x18,
99+
0x00, 0x03, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
100+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
101+
0x00, 0x0c, 0x00, 0x09, 0x00, 0x00, 0x00, 0x00, 0x00, 0x03, 0x00,
102+
0x00, 0x00, 0x08, 0x00, 0x0a, 0x00, 0x00, 0x00, 0x00, 0x00, 0x48,
103+
0x00, 0x02, 0x00, 0x20, 0x00, 0x02, 0x00, 0x02, 0x00, 0x00, 0x00,
104+
0x00, 0x00, 0x00, 0x00, 0x03, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00,
105+
0x00, 0x00, 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x01, 0x00,
106+
0x00, 0x00, 0x24, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
107+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
108+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
109+
0x00, 0x00, 0x00, 0x00, 0x00,
110+
];
57111
}
58112

59113
#[test]
@@ -81,9 +135,8 @@ mod mirror {
81135
}])],
82136
};
83137

84-
let buf = hex::decode(message::CREATE1).unwrap();
85138
let parsed = TcActionMessage::parse(
86-
&TcActionMessageBuffer::new_checked(&buf).unwrap(),
139+
&TcActionMessageBuffer::new_checked(&message::CREATE1).unwrap(),
87140
)
88141
.unwrap();
89142
assert_eq!(parsed, expected);
@@ -114,7 +167,7 @@ mod mirror {
114167
}])],
115168
};
116169

117-
let buf = hex::decode(message::CREATE2).unwrap();
170+
let buf = message::CREATE2;
118171
let parsed = TcActionMessage::parse(
119172
&TcActionMessageBuffer::new_checked(&buf).unwrap(),
120173
)
@@ -123,6 +176,7 @@ mod mirror {
123176
}
124177

125178
#[test]
179+
#[allow(clippy::too_many_lines)]
126180
fn parse_message3_list() {
127181
let expected = TcActionMessage {
128182
header: TcActionMessageHeader {
@@ -226,9 +280,8 @@ mod mirror {
226280
]),
227281
],
228282
};
229-
let buf = hex::decode(message::LIST).unwrap();
230283
let parsed = TcActionMessage::parse(
231-
&TcActionMessageBuffer::new_checked(&buf).unwrap(),
284+
&TcActionMessageBuffer::new_checked(&message::LIST).unwrap(),
232285
)
233286
.unwrap();
234287
assert_eq!(parsed, expected);

0 commit comments

Comments
 (0)