Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tc actions support #122

Merged
merged 3 commits into from
Jun 29, 2024
Merged

tc actions support #122

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
186747e
Docs for existing tc-actions
daniel-noland May 9, 2024
8f2d6ec
Support for tc-actions
daniel-noland May 9, 2024
6a28989
Feedback fixups
daniel-noland May 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 52 additions & 9 deletions src/message.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
// SPDX-License-Identifier: MIT

use anyhow::Context;
use netlink_packet_utils::{
DecodeError, Emitable, Parseable, ParseableParametrized,
};

use netlink_packet_core::{
NetlinkDeserializable, NetlinkHeader, NetlinkPayload, NetlinkSerializable,
};
use netlink_packet_utils::{
DecodeError, Emitable, Parseable, ParseableParametrized,
};

use crate::tc::{TcActionMessage, TcActionMessageBuffer};
use crate::{
address::{AddressHeader, AddressMessage, AddressMessageBuffer},
link::{LinkMessage, LinkMessageBuffer},
Expand Down Expand Up @@ -46,9 +46,9 @@ const RTM_GETTCLASS: u16 = 42;
const RTM_NEWTFILTER: u16 = 44;
const RTM_DELTFILTER: u16 = 45;
const RTM_GETTFILTER: u16 = 46;
// const RTM_NEWACTION: u16 = 48;
// const RTM_DELACTION: u16 = 49;
// const RTM_GETACTION: u16 = 50;
const RTM_NEWACTION: u16 = 48;
const RTM_DELACTION: u16 = 49;
const RTM_GETACTION: u16 = 50;
const RTM_NEWPREFIX: u16 = 52;
// const RTM_GETMULTICAST: u16 = 58;
// const RTM_GETANYCAST: u16 = 62;
Expand Down Expand Up @@ -291,6 +291,21 @@ impl<'a, T: AsRef<[u8]> + ?Sized>
}
}

RTM_NEWACTION | RTM_DELACTION | RTM_GETACTION => {
let err = "invalid tc action message";
let msg = TcActionMessage::parse(
&TcActionMessageBuffer::new_checked(&buf.inner())
.context(err)?,
)
.context(err)?;
Copy link
Contributor

@wllenyj wllenyj May 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is context required, if only return errors to caller. Or write some context message?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch. No idea how that got there. Will fix

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like I was just following an existing pattern when I did this?

If we change this pattern here we should likely change this pattern for all the other arms to remain consistent.

Thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's ok to keep origin pattern.
But the context is used to trace error path. I haven't tried what context(err) behaves like.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. Then I suggest we leave it as is for now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The use of anyhow need to be debated in another PR or issue. I don't like anyhow, but let's follow the precedent

match message_type {
RTM_NEWACTION => RouteNetlinkMessage::NewTrafficAction(msg),
RTM_DELACTION => RouteNetlinkMessage::DelTrafficAction(msg),
RTM_GETACTION => RouteNetlinkMessage::GetTrafficAction(msg),
_ => unreachable!(),
}
}

// ND ID Messages
RTM_NEWNSID | RTM_GETNSID | RTM_DELNSID => {
let err = "invalid nsid message";
Expand Down Expand Up @@ -348,6 +363,9 @@ pub enum RouteNetlinkMessage {
NewTrafficFilter(TcMessage),
DelTrafficFilter(TcMessage),
GetTrafficFilter(TcMessage),
NewTrafficAction(TcActionMessage),
DelTrafficAction(TcActionMessage),
GetTrafficAction(TcActionMessage),
NewTrafficChain(TcMessage),
DelTrafficChain(TcMessage),
GetTrafficChain(TcMessage),
Expand Down Expand Up @@ -460,6 +478,18 @@ impl RouteNetlinkMessage {
matches!(self, RouteNetlinkMessage::GetTrafficFilter(_))
}

pub fn is_new_action(&self) -> bool {
matches!(self, RouteNetlinkMessage::NewTrafficAction(_))
}

pub fn is_del_action(&self) -> bool {
matches!(self, RouteNetlinkMessage::DelTrafficAction(_))
}

pub fn is_get_action(&self) -> bool {
matches!(self, RouteNetlinkMessage::GetTrafficAction(_))
}

pub fn is_new_chain(&self) -> bool {
matches!(self, RouteNetlinkMessage::NewTrafficChain(_))
}
Expand Down Expand Up @@ -528,6 +558,9 @@ impl RouteNetlinkMessage {
NewTrafficFilter(_) => RTM_NEWTFILTER,
DelTrafficFilter(_) => RTM_DELTFILTER,
GetTrafficFilter(_) => RTM_GETTFILTER,
NewTrafficAction(_) => RTM_NEWACTION,
DelTrafficAction(_) => RTM_DELACTION,
GetTrafficAction(_) => RTM_GETACTION,
NewTrafficChain(_) => RTM_NEWCHAIN,
DelTrafficChain(_) => RTM_DELCHAIN,
GetTrafficChain(_) => RTM_GETCHAIN,
Expand Down Expand Up @@ -598,7 +631,12 @@ impl Emitable for RouteNetlinkMessage {
| NewRule(ref msg)
| DelRule(ref msg)
| GetRule(ref msg)
=> msg.buffer_len()
=> msg.buffer_len(),

| NewTrafficAction(ref msg)
| DelTrafficAction(ref msg)
| GetTrafficAction(ref msg)
=> msg.buffer_len(),
}
}

Expand Down Expand Up @@ -658,7 +696,12 @@ impl Emitable for RouteNetlinkMessage {
| NewRule(ref msg)
| DelRule(ref msg)
| GetRule(ref msg)
=> msg.emit(buffer)
=> msg.emit(buffer),

| NewTrafficAction(ref msg)
| DelTrafficAction(ref msg)
| GetTrafficAction(ref msg)
=> msg.emit(buffer),
}
}
}
Expand Down
Loading
Loading