Skip to content

Commit e329820

Browse files
authored
Add Send + Sync bounds to resolve Arc clippy warnings (#883)
- Add Send + Sync bounds to LogProvider trait - Add Send + Sync bounds to ExpiryPolicy trait - Add Send + Sync bounds to action traits: MetaAction, StaticAction, StatefulAction, HairpinAction, ActionDesc - Add Send + Sync bounds to SNAT implementation trait bounds - Add unsafe Send + Sync implementations for KStatNamed Fixes #785: Port<N> should require Send + Sync bounds
1 parent be27e2f commit e329820

File tree

4 files changed

+27
-11
lines changed

4 files changed

+27
-11
lines changed

lib/opte/src/ddi/kstat.rs

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ cfg_if! {
6161
/// ```
6262
///
6363
/// To register a provider see [`KStatNamed`].
64-
pub trait KStatProvider {
64+
pub trait KStatProvider: Send + Sync {
6565
const NUM_FIELDS: u32;
6666
type Snap;
6767

@@ -306,3 +306,19 @@ impl From<alloc::ffi::NulError> for Error {
306306
Self::NulChar
307307
}
308308
}
309+
310+
// SAFETY: KStatNamed<T> is safe for Send + Sync because:
311+
//
312+
// 1. The underlying T must itself be Send + Sync because it is fully visible to any
313+
// user of the struct. We meet this through the new type bound on KStatProvider.
314+
//
315+
// 2. ksp is itself safe to move between threads, since KSTAT(9S) imposes no MT
316+
// constraints on callers.
317+
//
318+
// 3. ksp is never exposed via a &ref (nor is it used by any
319+
// methods taking &self), and is only used during drop.
320+
//
321+
#[cfg(all(not(feature = "std"), not(test)))]
322+
unsafe impl<T: KStatProvider> Send for KStatNamed<T> {}
323+
#[cfg(all(not(feature = "std"), not(test)))]
324+
unsafe impl<T: KStatProvider> Sync for KStatNamed<T> {}

lib/opte/src/engine/flow_table.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ impl Ttl {
6464
}
6565

6666
/// A policy for expiring flow table entries over time.
67-
pub trait ExpiryPolicy<S: Dump>: fmt::Debug {
67+
pub trait ExpiryPolicy<S: Dump>: fmt::Debug + Send + Sync {
6868
/// Returns whether the given flow should be removed, given current flow
6969
/// state, the time a packet was last received, and the current time.
7070
fn is_expired(&self, entry: &FlowEntry<S>, now: Moment) -> bool;

lib/opte/src/engine/rule.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ where
173173
/// An Action Descriptor holds the information needed to create the
174174
/// [`HdrTransform`] which implements the desired action. An
175175
/// ActionDesc is created by a [`StatefulAction`] implementation.
176-
pub trait ActionDesc {
176+
pub trait ActionDesc: Send + Sync {
177177
/// Generate the [`HdrTransform`] which implements this descriptor.
178178
fn gen_ht(&self, dir: Direction) -> HdrTransform;
179179

@@ -742,7 +742,7 @@ pub enum GenDescError {
742742

743743
pub type GenDescResult = ActionResult<Arc<dyn ActionDesc>, GenDescError>;
744744

745-
pub trait StatefulAction: Display {
745+
pub trait StatefulAction: Display + Send + Sync {
746746
/// Generate a an [`ActionDesc`] based on the [`InnerFlowId`] and
747747
/// [`ActionMeta`]. This action may also add, remove, or modify
748748
/// metadata to communicate data to downstream actions.
@@ -772,7 +772,7 @@ pub enum GenHtError {
772772

773773
pub type GenHtResult = ActionResult<HdrTransform, GenHtError>;
774774

775-
pub trait StaticAction: Display {
775+
pub trait StaticAction: Display + Send + Sync {
776776
fn gen_ht(
777777
&self,
778778
dir: Direction,
@@ -795,7 +795,7 @@ pub type ModMetaResult = ActionResult<(), String>;
795795
/// metadata in some way. That is, it has no transformation to make on
796796
/// the packet, only add/modify/remove metadata for use by later
797797
/// layers.
798-
pub trait MetaAction: Display {
798+
pub trait MetaAction: Display + Send + Sync {
799799
/// Return the predicates implicit to this action.
800800
///
801801
/// Return both the header [`Predicate`] list and
@@ -843,7 +843,7 @@ impl From<smoltcp::wire::Error> for GenBtError {
843843
///
844844
/// For example, you could use this to hairpin an ARP Reply in response
845845
/// to a guest's ARP request.
846-
pub trait HairpinAction: Display {
846+
pub trait HairpinAction: Display + Send + Sync {
847847
/// Generate a [`Packet`] to hairpin back to the source. The
848848
/// `meta` argument holds the packet metadata, including any
849849
/// modifications made by previous layers up to this point.

lib/opte/src/engine/snat.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ impl From<GenIcmpErr> for GenDescError {
218218
}
219219
}
220220

221-
impl<T: ConcreteIpAddr + 'static> SNat<T> {
221+
impl<T: ConcreteIpAddr + 'static + Send + Sync> SNat<T> {
222222
pub fn new(addr: T) -> Self {
223223
SNat {
224224
priv_ip: addr,
@@ -299,7 +299,7 @@ impl Display for SNat<Ipv6Addr> {
299299
}
300300
}
301301

302-
impl<T: ConcreteIpAddr + 'static> StatefulAction for SNat<T>
302+
impl<T: ConcreteIpAddr + 'static + Send + Sync> StatefulAction for SNat<T>
303303
where
304304
SNat<T>: Display,
305305
{
@@ -367,7 +367,7 @@ pub struct SNatDesc<T: ConcreteIpAddr> {
367367

368368
pub const SNAT_NAME: &str = "SNAT";
369369

370-
impl<T: ConcreteIpAddr> ActionDesc for SNatDesc<T> {
370+
impl<T: ConcreteIpAddr + Send + Sync> ActionDesc for SNatDesc<T> {
371371
fn gen_ht(&self, dir: Direction) -> HdrTransform {
372372
match dir {
373373
// Outbound traffic needs its source IP and source port
@@ -425,7 +425,7 @@ pub struct SNatIcmpEchoDesc<T: ConcreteIpAddr> {
425425

426426
pub const SNAT_ICMP_ECHO_NAME: &str = "SNAT_ICMP_ECHO";
427427

428-
impl<T: ConcreteIpAddr> ActionDesc for SNatIcmpEchoDesc<T> {
428+
impl<T: ConcreteIpAddr + Send + Sync> ActionDesc for SNatIcmpEchoDesc<T> {
429429
// SNAT needs to generate an additional transform for ICMP traffic in
430430
// order to treat the Echo Identifier as a psuedo ULP port.
431431
fn gen_ht(&self, dir: Direction) -> HdrTransform {

0 commit comments

Comments
 (0)