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.

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

ci: Add workflow to ensure Rust code is consistently formatted #97

Merged
merged 2 commits into from
Nov 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
36 changes: 36 additions & 0 deletions .github/workflows/lint-cargo-fmt.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# Make sure Rust source code is consistently formatted with rustfmt.

name: "lint-cargo-fmt.yml"

on:
pull_request: {}

concurrency:
group: "${{ github.workflow }}:${{ github.event.pull_request.number }}"
cancel-in-progress: true

jobs:
format-check:
name: "Check formatting for Rust code"
runs-on: "ubuntu-latest"
steps:
- name: "Install Rust toolchain"
uses: "dtolnay/rust-toolchain@stable"
with:
toolchain: "nightly"
components: "rustfmt"

- name: "Checkout"
uses: "actions/checkout@v4"
with:
fetch-depth: "1"
persist-credentials: "false"

- name: "Check formatting"
run: |
cargo fmt --check
- name: "How to fix"
if: ${{ failure() }}
run: |
echo "::notice::Try fixing the issue with 'cargo fmt --all', then commit the changes."
3 changes: 0 additions & 3 deletions dpdk-sys/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ fn bind(path: &Path, sysroot: &str) {
.allowlist_item("RTE.*")
.blocklist_item("__*")
.clang_macro_fallback()

// Bindgen tests seem to object to the documentation in the following
// (not the items themselves, just the documentation associated with them)
// I suspect this is a bug in bindgen, but I'm not sure.
Expand All @@ -57,13 +56,11 @@ fn bind(path: &Path, sysroot: &str) {
.blocklist_function("rte_dev_dma_unmap")
.blocklist_function("rte_pci_addr_cmp")
.blocklist_function("rte_pci_addr_parse")

// rustc doesn't like repr(packed) types which contain other repr(packed) types
.opaque_type("rte_arp_hdr")
.opaque_type("rte_arp_ipv4")
.opaque_type("rte_gtp_psc_generic_hdr")
.opaque_type("rte_l2tpv2_combined_msg_hdr")

.clang_arg(format!("-I{sysroot}/include"))
.clang_arg("-fretain-comments-from-system-headers")
.clang_arg("-fparse-all-comments")
Expand Down
2 changes: 1 addition & 1 deletion dpdk-sys/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
deprecated,
non_snake_case,
non_camel_case_types,
non_upper_case_globals,
non_upper_case_globals
)]

include!(concat!(env!("OUT_DIR"), "/generated.rs"));
17 changes: 11 additions & 6 deletions dpdk-sysroot-helper/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ use std::env;
use std::path::Path;

// from https://stackoverflow.com/questions/73595435/how-to-get-profile-from-cargo-toml-in-build-rs-or-at-runtime
#[must_use] pub fn get_profile_name() -> String {
#[must_use]
pub fn get_profile_name() -> String {
// The profile name is always the 3rd last part of the path (with 1 based indexing).
// e.g., /code/core/target/cli/build/my-build-info-9f91ba6f99d7a061/out
env::var("OUT_DIR")
Expand All @@ -16,7 +17,8 @@ use std::path::Path;
.to_string()
}

#[must_use] pub fn get_target_name() -> String {
#[must_use]
pub fn get_target_name() -> String {
// The target name is always the 4th last part of the path (with 1 based indexing).
// e.g., /code/core/target/cli/build/my-build-info-9f91ba6f99d7a061/out
env::var("OUT_DIR")
Expand All @@ -27,15 +29,18 @@ use std::path::Path;
.to_string()
}

#[must_use] pub fn get_project_root() -> String {
#[must_use]
pub fn get_project_root() -> String {
env::var("CARGO_MANIFEST_DIR").expect("CARGO_MANIFEST_DIR not set")
}

#[must_use] pub fn get_compile_env() -> String {
#[must_use]
pub fn get_compile_env() -> String {
env::var("COMPILE_ENV").expect("COMPILE_ENV not set")
}

#[must_use] pub fn get_sysroot() -> String {
#[must_use]
pub fn get_sysroot() -> String {
let compile_env = env::var("COMPILE_ENV").expect("COMPILE_ENV not set");
let sysroot_env = format!("{compile_env}/sysroot");
let target = get_target_name();
Expand All @@ -50,7 +55,7 @@ use std::path::Path;
if fallback_sysroot_path.exists() {
fallback_sysroot
} else {
panic!("sysroot not found at {expected_sysroot} or {fallback_sysroot}")
panic!("sysroot not found at {expected_sysroot} or {fallback_sysroot}")
}
}
}
21 changes: 8 additions & 13 deletions dpdk/src/dev.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@ impl DevIndex {
pub const MAX: u16 = RTE_MAX_ETHPORTS as u16;

/// The index of the port represented as a `u16`.
#[must_use] pub fn as_u16(&self) -> u16 {
#[must_use]
pub fn as_u16(&self) -> u16 {
self.0
}

Expand Down Expand Up @@ -179,9 +180,7 @@ impl DevIndex {
// However, DPDK has a depressing number of sign and bit-width errors in its API, so we
// need to check for nonsense values to make a properly safe wrapper.
// Better to panic than malfunction.
Eal::fatal_error(format!(
"SocketId for port {self} is negative? {socket_id}"
));
Eal::fatal_error(format!("SocketId for port {self} is negative? {socket_id}"));
}

Ok(SocketId(socket_id as c_uint))
Expand Down Expand Up @@ -252,12 +251,7 @@ impl DevConfig {
let nb_tx_queues = self.num_tx_queues + self.num_hairpin_queues;

let ret = unsafe {
rte_eth_dev_configure(
dev.index().as_u16(),
nb_rx_queues,
nb_tx_queues,
&eth_conf,
)
rte_eth_dev_configure(dev.index().as_u16(), nb_rx_queues, nb_tx_queues, &eth_conf)
};

if ret != 0 {
Expand Down Expand Up @@ -661,14 +655,16 @@ impl Manager {

impl DevInfo {
/// Get the port index of the device.
#[must_use] pub fn index(&self) -> DevIndex {
#[must_use]
pub fn index(&self) -> DevIndex {
self.index
}

/// Get the device `if_index`.
///
/// This is the Linux interface index of the device.
#[must_use] pub fn if_index(&self) -> u32 {
#[must_use]
pub fn if_index(&self) -> u32 {
self.inner.if_index
}

Expand Down Expand Up @@ -847,7 +843,6 @@ impl Drop for Dev {
}
}


#[derive(Debug, thiserror::Error)]
pub enum SocketIdLookupError {
#[error("Invalid port ID")]
Expand Down
6 changes: 3 additions & 3 deletions dpdk/src/eal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@
// Copyright Open Network Fabric Authors

//! DPDK Environment Abstraction Layer (EAL)
use crate::{dev, mem, socket};
use alloc::ffi::CString;
use alloc::format;
use alloc::string::{String, ToString};
use crate::{dev, mem, socket};
use alloc::vec::Vec;
use core::ffi::c_int;
use core::fmt::{Debug, Display};
use alloc::ffi::CString;
use alloc::vec::Vec;
use dpdk_sys::*;
use tracing::{error, info};

Expand Down
3 changes: 2 additions & 1 deletion dpdk/src/flow/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -876,7 +876,8 @@ pub struct SetFieldAction {

impl SetFlowField {
/// Converts the `SetFlowField` into a `SetFieldAction`.
#[must_use] pub fn to_flow_rule(&self) -> SetFieldAction {
#[must_use]
pub fn to_flow_rule(&self) -> SetFieldAction {
let conf = match self {
SetFlowField::MacDst(mac_addr) => {
let mut value = [0u8; 16];
Expand Down
6 changes: 2 additions & 4 deletions dpdk/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,11 @@
//! This crate uses lints to discourage casual use of `unwrap`, `expect`, and `panic` to help
//! encourage this practice.
#![cfg_attr(not(test), no_std)]
#![warn(
clippy::all,
)]
#![warn(clippy::all)]
#![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic)]
#![allow(private_bounds)]
extern crate core;
extern crate alloc;
extern crate core;

pub mod dev;
pub mod eal;
Expand Down
18 changes: 11 additions & 7 deletions dpdk/src/mem.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,15 @@

//! DPDK memory management wrappers.

use crate::socket::SocketId;
use alloc::format;
use alloc::string::String;
use crate::socket::SocketId;
use dpdk_sys::*;
use core::cell::UnsafeCell;
use core::ffi::{c_char, c_int, CStr};
use core::fmt::{Debug, Display};
use core::marker::PhantomData;
use core::ptr::NonNull;
use dpdk_sys::*;
use tracing::{debug, error, warn};

use alloc::sync::Arc;
Expand Down Expand Up @@ -121,12 +121,14 @@ impl PoolHandle {
}

/// Get the name of the memory pool.
#[must_use] pub fn name(&self) -> &str {
#[must_use]
pub fn name(&self) -> &str {
self.config().name()
}

/// Get the configuration of the memory pool.
#[must_use] pub fn config(&self) -> &PoolConfig {
#[must_use]
pub fn config(&self) -> &PoolConfig {
&self.0.config
}
}
Expand Down Expand Up @@ -349,7 +351,7 @@ impl Drop for PoolInner {
}

/// A DPDK Mbuf (memory buffer)
///
///
/// Usually used to hold an ethernet frame.
#[repr(transparent)]
pub struct Mbuf {
Expand Down Expand Up @@ -396,7 +398,8 @@ impl Mbuf {
}

/// Get an immutable ref to the raw data of an Mbuf
#[must_use] pub fn raw_data(&self) -> &[u8] {
#[must_use]
pub fn raw_data(&self) -> &[u8] {
let pkt_data_start = unsafe {
(self.raw.as_ref().buf_addr as *const u8)
.offset(self.raw.as_ref().annon1.annon1.data_off as isize)
Expand All @@ -416,7 +419,8 @@ impl Mbuf {
.raw
.as_mut()
.buf_addr
.offset(self.raw.as_ref().annon1.annon1.data_off as isize).cast::<u8>();
.offset(self.raw.as_ref().annon1.annon1.data_off as isize)
.cast::<u8>();
core::slice::from_raw_parts_mut(
data_start,
self.raw.as_ref().annon2.annon1.data_len as usize,
Expand Down
34 changes: 18 additions & 16 deletions dpdk/src/queue/hairpin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@
use super::{rx, tx};
use crate::dev::{Dev, DevInfo};
use crate::queue::rx::RxQueue;
use crate::queue::tx::{TxQueue};
use crate::queue::tx::TxQueue;
use dpdk_sys::*;
use tracing::debug;
use errno::ErrorCode;
use tracing::debug;

/// A stopped DPDK hairpin queue.
#[derive(Debug)]
Expand Down Expand Up @@ -59,11 +59,7 @@ impl HairpinQueue {
///
/// This design ensures that the hairpin queue is correctly tracked in the list of queues
/// associated with the device.
pub(crate) fn new(
dev: &Dev,
rx: RxQueue,
tx: TxQueue,
) -> Result<Self, HairpinConfigFailure> {
pub(crate) fn new(dev: &Dev, rx: RxQueue, tx: TxQueue) -> Result<Self, HairpinConfigFailure> {
let peering = HairpinPeering::define(&dev.info, &rx, &tx);
// configure the rx queue

Expand All @@ -77,9 +73,9 @@ impl HairpinQueue {
};

if ret < 0 {
return Err(HairpinConfigFailure::CreationFailed(
ErrorCode::parse_i32(ret)
));
return Err(HairpinConfigFailure::CreationFailed(ErrorCode::parse_i32(
ret,
)));
}
debug!("RX hairpin queue configured");

Expand All @@ -93,16 +89,17 @@ impl HairpinQueue {
};

if ret < 0 {
return Err(HairpinConfigFailure::CreationFailed(
ErrorCode::parse_i32(ret)
));
return Err(HairpinConfigFailure::CreationFailed(ErrorCode::parse_i32(
ret,
)));
}
debug!("TX hairpin queue configured");

Ok(HairpinQueue { rx, tx, peering })
}

#[must_use] pub fn start(self) -> HairpinQueue {
#[must_use]
pub fn start(self) -> HairpinQueue {
let rx = match self.rx.start() {
Ok(rx) => rx,
Err(_) => todo!(),
Expand All @@ -111,13 +108,18 @@ impl HairpinQueue {
Ok(tx) => tx,
Err(_) => todo!(),
};
HairpinQueue { rx, tx, peering: self.peering }
HairpinQueue {
rx,
tx,
peering: self.peering,
}
}
}

impl HairpinQueue {
/// Stop the hairpin queue.
#[must_use] pub fn stop(self) -> HairpinQueue {
#[must_use]
pub fn stop(self) -> HairpinQueue {
let rx = match self.rx.stop() {
Ok(rx) => rx,
Err(_) => todo!(),
Expand Down
3 changes: 1 addition & 2 deletions dpdk/src/queue/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@
// Copyright Open Network Fabric Authors

//! DPDK queue abstractions.
pub mod hairpin;
pub mod rx;
pub mod tx;
pub mod hairpin;

/// The possible states of a DPDK queue
#[derive(Debug)]
Expand All @@ -16,4 +16,3 @@ pub enum QueueState {
/// A started queue
Started,
}

3 changes: 2 additions & 1 deletion dpdk/src/queue/rx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ impl RxQueueIndex {
/// The index of the rx queue represented as a `u16`.
///
/// This function is mostly useful for interfacing with [`dpdk_sys`].
#[must_use] pub fn as_u16(&self) -> u16 {
#[must_use]
pub fn as_u16(&self) -> u16 {
self.0
}
}
Expand Down
Loading