Address review comments by alessandrod and tamird.

Signed-off-by: Andre Fredette <afredette@redhat.com>
reviewable/pr921/r2
Andre Fredette 7 months ago
parent b156e845b9
commit 1a83204397

@ -397,13 +397,14 @@ pub enum LinkError {
SyscallError(#[from] SyscallError),
}
/// A [`Link`] identifier, which may or may not be owned by aya.
/// A [`Link`] identifier.
pub struct LinkId(u32);
impl LinkId {
/// A wrapper for an arbitrary loaded link specified by its kernel id,
/// unsafe because there is no guarantee provided by aya that the link is
/// still loaded or even exists.
/// Create a new [`LinkId`] from its kernel id.
///
/// This method is unsafe since it doesn't check that the given `id` is a an
/// existing link id.
pub unsafe fn new(id: u32) -> Self {
Self(id)
}
@ -427,8 +428,7 @@ bitflags::bitflags! {
}
}
/// Struct defining the arguments required for interacting with the kernel's
/// multi-prog API.
/// Arguments required for interacting with the kernel's multi-prog API.
///
/// # Minimum kernel version
///
@ -466,7 +466,7 @@ impl Default for LinkOrder {
}
impl LinkOrder {
/// Ensure the link is created before all other links at a given attachment point.
/// Attach before all other links.
pub fn first() -> Self {
Self {
link_ref: LinkRef::Id(0),
@ -475,7 +475,7 @@ impl LinkOrder {
}
}
/// Ensure the link is created after all other links for a given attachment point.
/// Attach after all other links.
pub fn last() -> Self {
Self {
link_ref: LinkRef::Id(0),
@ -484,7 +484,7 @@ impl LinkOrder {
}
}
/// Ensure the link is created before the specified aya-owned link for a given attachment point.
/// Attach before the given link.
pub fn before_link<L: MultiProgLink>(link: &L) -> Result<Self, LinkError> {
Ok(Self {
link_ref: LinkRef::Fd(link.fd()?.as_raw_fd()),
@ -493,7 +493,7 @@ impl LinkOrder {
})
}
/// Ensure the link is created after the specified aya-owned link for a given attachment point.
/// Attach after the given link.
pub fn after_link<L: MultiProgLink>(link: &L) -> Result<Self, LinkError> {
Ok(Self {
link_ref: LinkRef::Fd(link.fd()?.as_raw_fd()),
@ -502,7 +502,7 @@ impl LinkOrder {
})
}
/// Ensure the link is created before a link specified by its kernel id for a given attachment point.
/// Attach before the link with the given id.
pub fn before_link_id(id: LinkId) -> Result<Self, LinkError> {
Ok(Self {
link_ref: LinkRef::Id(id.0),
@ -511,7 +511,7 @@ impl LinkOrder {
})
}
/// Ensure the link is created after a link specified by its kernel id for a given attachment point.
/// Attach after the link with the given id.
pub fn after_link_id(id: LinkId) -> Result<Self, LinkError> {
Ok(Self {
link_ref: LinkRef::Id(id.0),
@ -520,7 +520,7 @@ impl LinkOrder {
})
}
/// Ensure the link is created before the specified aya-owned program for a given attachment point.
/// Attach before the given program.
pub fn before_program<P: MultiProgProgram>(program: &P) -> Result<Self, ProgramError> {
Ok(Self {
link_ref: LinkRef::Fd(program.fd()?.as_raw_fd()),
@ -529,7 +529,7 @@ impl LinkOrder {
})
}
/// Ensure the link is created after the specified aya-owned program for a given attachment point.
/// Attach after the given program.
pub fn after_program<P: MultiProgProgram>(program: &P) -> Result<Self, ProgramError> {
Ok(Self {
link_ref: LinkRef::Fd(program.fd()?.as_raw_fd()),
@ -538,7 +538,7 @@ impl LinkOrder {
})
}
/// Ensure the link is created before a program specified by its kernel id for a given attachment point.
/// Attach before the program with the given id.
pub fn before_program_id(id: ProgramId) -> Self {
Self {
link_ref: LinkRef::Id(id.0),
@ -547,7 +547,7 @@ impl LinkOrder {
}
}
/// Ensure the link is created after a program specified by its kernel id for a given attachment point.
/// Attach after the program with the given id.
pub fn after_program_id(id: ProgramId) -> Self {
Self {
link_ref: LinkRef::Id(id.0),

@ -240,13 +240,14 @@ impl AsFd for ProgramFd {
}
/// The various eBPF programs.
/// A [`Program`] identifier, which may or may not be owned by aya.
/// A [`Program`] identifier.
pub struct ProgramId(u32);
impl ProgramId {
/// A wrapper for an arbitrary loaded program specified by its kernel id,
/// unsafe because there is no guarantee provided by aya that the program is
/// still loaded or even exists.
/// Create a new program id.
///
/// This method is unsafe since it doesn't check that the given `id` is a
/// valid program id.
pub unsafe fn new(id: u32) -> Self {
Self(id)
}
@ -811,8 +812,8 @@ impl_fd!(
CgroupDevice,
);
/// Defines which [`Program`]s support the kernel's
/// generic multi-prog API.
/// Defines the [`Program`] types which support the kernel's
/// [generic multi-prog API](https://github.com/torvalds/linux/commit/053c8e1f235dc3f69d13375b32f4209228e1cb96).
///
/// # Minimum kernel version
///
@ -838,7 +839,8 @@ macro_rules! impl_multiprog_fd {
impl_multiprog_fd!(SchedClassifier);
/// Defines the [Link] types which support the kernel's [generic multi-prog API](https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=053c8e1f235dc3f69d13375b32f4209228e1cb96).
/// Defines the [`Link`] types which support the kernel's
/// [generic multi-prog API](https://github.com/torvalds/linux/commit/053c8e1f235dc3f69d13375b32f4209228e1cb96).
///
/// # Minimum kernel version
///
@ -853,7 +855,6 @@ macro_rules! impl_multiproglink_fd {
($($struct_name:ident),+ $(,)?) => {
$(
impl MultiProgLink for $struct_name {
/// Returns the a borrowed reference file descriptor of this Program.
fn fd(&self) -> Result<BorrowedFd<'_>, LinkError> {
let link: &FdLink = self.try_into()?;
Ok(link.fd.as_fd())

@ -6,7 +6,6 @@ use std::{
path::Path,
};
use log::debug;
use thiserror::Error;
use super::FdLink;
@ -101,8 +100,8 @@ pub enum TcError {
#[error("tcx links can only be attached to ingress or egress, custom attachment: {0} is not supported")]
InvalidTcxAttach(u32),
/// program was loaded via tcx not netlink
#[error("program was loaded via tcx not netlink")]
InvalidLink,
#[error("operation not supported for programs loaded via tcx")]
InvalidLinkOperation,
}
impl TcAttachType {
@ -114,11 +113,11 @@ impl TcAttachType {
}
}
pub(crate) fn tcx_attach(&self) -> Result<bpf_attach_type, TcError> {
pub(crate) fn tcx_attach_type(&self) -> Result<bpf_attach_type, TcError> {
match self {
Self::Ingress => Ok(BPF_TCX_INGRESS),
Self::Egress => Ok(BPF_TCX_EGRESS),
Self::Custom(i) => Err(TcError::InvalidTcxAttach(i.to_owned())),
Self::Custom(i) => Err(TcError::InvalidTcxAttach(*i)),
}
}
}
@ -127,8 +126,8 @@ impl TcAttachType {
///
/// The options vary based on what is supported by the current kernel. Kernels
/// older than 6.6.0 must utilize netlink for attachments, while newer kernels
/// can utilize the modern TCX eBPF link type which support's the kernel's
/// multi-prog api.
/// can utilize the modern TCX eBPF link type which supports the kernel's
/// multi-prog API.
#[derive(Debug)]
pub enum TcAttachOptions {
/// Netlink attach options.
@ -137,7 +136,7 @@ pub enum TcAttachOptions {
TcxOrder(LinkOrder),
}
/// Options for SchedClassifier attach via netlink
/// Options for SchedClassifier attach via netlink.
#[derive(Debug, Default, Hash, Eq, PartialEq)]
pub struct NlOptions {
/// Priority assigned to tc program with lower number = higher priority.
@ -167,7 +166,7 @@ impl SchedClassifier {
/// # Errors
///
/// When attaching fails, [`ProgramError::SyscallError`] is returned for
/// kernels `>= 6.6.0`, and instead [`TcError::NetlinkError`] is returned for
/// kernels `>= 6.6.0`, and [`TcError::NetlinkError`] is returned for
/// older kernels. A common cause of netlink attachment failure is not having added
/// the `clsact` qdisc to the given interface, see [`qdisc_add_clsact`]
///
@ -177,14 +176,12 @@ impl SchedClassifier {
attach_type: TcAttachType,
) -> Result<SchedClassifierLinkId, ProgramError> {
if KernelVersion::current().unwrap() >= KernelVersion::new(6, 6, 0) {
debug!("attaching schedClassifier program via tcx link API");
self.attach_with_options(
interface,
attach_type,
TcAttachOptions::TcxOrder(LinkOrder::default()),
)
} else {
debug!("attaching SchedClassifier program via netlink API");
self.attach_with_options(
interface,
attach_type,
@ -296,7 +293,7 @@ impl SchedClassifier {
let link_fd = bpf_link_create(
prog_fd,
LinkTarget::IfIndex(if_index),
attach_type.tcx_attach()?,
attach_type.tcx_attach_type()?,
None,
options.flags.bits(),
Some(&options.link_ref),
@ -436,7 +433,6 @@ impl TryFrom<FdLink> for SchedClassifierLink {
type Error = LinkError;
fn try_from(fd_link: FdLink) -> Result<Self, Self::Error> {
// unwrap of fd_link.fd will not panic since it's only None when being dropped.
let info = bpf_link_get_info_by_fd(fd_link.fd.as_fd())?;
if info.type_ == (bpf_link_type::BPF_LINK_TYPE_TCX as u32) {
return Ok(Self::new(TcLinkInner::FdLink(fd_link)));
@ -506,7 +502,7 @@ impl SchedClassifierLink {
if let TcLinkInner::NlLink(n) = self.inner() {
Ok(n.attach_type)
} else {
Err(TcError::InvalidLink.into())
Err(TcError::InvalidLinkOperation.into())
}
}
@ -515,7 +511,7 @@ impl SchedClassifierLink {
if let TcLinkInner::NlLink(n) = self.inner() {
Ok(n.priority)
} else {
Err(TcError::InvalidLink.into())
Err(TcError::InvalidLinkOperation.into())
}
}
@ -524,7 +520,7 @@ impl SchedClassifierLink {
if let TcLinkInner::NlLink(n) = self.inner() {
Ok(n.handle)
} else {
Err(TcError::InvalidLink.into())
Err(TcError::InvalidLinkOperation.into())
}
}
}

@ -429,7 +429,7 @@ pub(crate) fn bpf_link_create(
.__bindgen_anon_1
.relative_id = id.to_owned();
}
_ => {}
None => {}
};
// SAFETY: BPF_LINK_CREATE returns a new file descriptor.

Loading…
Cancel
Save