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

Configure hardware clock and hardware based timestamping automatically when available by default #549

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
43 changes: 38 additions & 5 deletions statime-linux/src/config/mod.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
use std::{
fs::read_to_string,
net::SocketAddr,
os::unix::fs::PermissionsExt,
path::{Path, PathBuf},
fs::read_to_string, net::SocketAddr, os::unix::fs::PermissionsExt, path::{Path, PathBuf}
};

use log::warn;
Expand Down Expand Up @@ -40,14 +37,50 @@ pub struct Config {
pub virtual_system_clock: bool,
}

#[derive(Debug, Clone, PartialEq, Eq, Default)]
pub enum HardwareClock {
/// Automatically use the (default) hardware clock for the interface specified if available
#[default]
Auto,
/// Require the use of the (default) hardware clock for the interface specified.
/// If a hardware clock is not available, statime will refuse to start.
Required,
/// Use the specified hardware clock
Specific(u32),
/// Do not use a hardware clock
None,
}

impl<'de> Deserialize<'de> for HardwareClock {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: Deserializer<'de>,
{
use serde::de::Error;

let raw: String = Deserialize::deserialize(deserializer)?;

if raw == "auto" {
Ok(HardwareClock::Auto)
} else if raw == "required" {
Ok(HardwareClock::Required)
} else if raw == "none" {
Ok(HardwareClock::None)
} else {
let clock = raw.parse().map_err(|e| D::Error::custom(format!("Invalid hardware clock: {}", e)))?;
Ok(HardwareClock::Specific(clock))
}
}
}

#[derive(Deserialize, Debug, Clone, PartialEq, Eq)]
#[serde(rename_all = "kebab-case", deny_unknown_fields)]
pub struct PortConfig {
pub interface: InterfaceName,
#[serde(default, deserialize_with = "deserialize_acceptable_master_list")]
pub acceptable_master_list: Option<Vec<ClockIdentity>>,
#[serde(default)]
pub hardware_clock: Option<u32>,
pub hardware_clock: HardwareClock,
#[serde(default)]
pub network_mode: NetworkMode,
#[serde(default = "default_announce_interval")]
Expand Down
85 changes: 54 additions & 31 deletions statime-linux/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,10 @@ use statime::{
Clock, OverlayClock, PtpInstance, PtpInstanceState, SharedClock,
};
use statime_linux::{
clock::{LinuxClock, PortTimestampToTime},
initialize_logging_parse_config,
observer::ObservableInstanceState,
socket::{
clock::{LinuxClock, PortTimestampToTime}, config::HardwareClock, initialize_logging_parse_config, observer::ObservableInstanceState, socket::{
open_ethernet_socket, open_ipv4_event_socket, open_ipv4_general_socket,
open_ipv6_event_socket, open_ipv6_general_socket, PtpTargetAddress,
},
tlvforwarder::TlvForwarder,
}, tlvforwarder::TlvForwarder
};
use timestamped_socket::{
interface::interfaces,
Expand Down Expand Up @@ -314,38 +310,65 @@ async fn actual_main() {

let tlv_forwarder = TlvForwarder::new();

macro_rules! add_hw {
($idx:expr) => {{
let idx = $idx;
let mut clock = LinuxClock::open_idx(idx).expect("Unable to open clock");
if let Some(id) = clock_name_map.get(&idx) {
clock_port_map.push(Some(*id));
} else {
clock.init().expect("Unable to initialize clock");
let id = internal_sync_senders.len();
clock_port_map.push(Some(id));
clock_name_map.insert(idx, id);
internal_sync_senders
.push(start_clock_task(clock.clone(), system_clock.clone()));
}
(
Some(idx),
Box::new(clock) as BoxedClock,
InterfaceTimestampMode::HardwarePTPAll,
)
}};
}

macro_rules! add_sw {
() => {{
clock_port_map.push(None);
(
None,
system_clock.clone_boxed(),
InterfaceTimestampMode::SoftwareAll,
)
}};
}

Comment on lines +313 to +345
Copy link
Member

Choose a reason for hiding this comment

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

I am a bit uncomfortable with these being macros. What is stopping us from making these just outright functions (and exposing all the dependencies on names in the surrounding code)?

for port_config in config.ports {
let interface = port_config.interface;
let network_mode = port_config.network_mode;
let (port_clock, timestamping) = match port_config.hardware_clock {
Some(idx) => {
let mut clock = LinuxClock::open_idx(idx).expect("Unable to open clock");
if let Some(id) = clock_name_map.get(&idx) {
clock_port_map.push(Some(*id));
} else {
clock.init().expect("Unable to initialize clock");
let id = internal_sync_senders.len();
clock_port_map.push(Some(id));
clock_name_map.insert(idx, id);
internal_sync_senders
.push(start_clock_task(clock.clone(), system_clock.clone()));
let (bind_phc, port_clock, timestamping) = match port_config.hardware_clock {
HardwareClock::Auto => {
match interface.lookup_phc() {
Some(idx) => add_hw!(idx),
None => {
log::info!("No hardware clock found, falling back to software timestamping");
add_sw!()
},
}
(
Box::new(clock) as BoxedClock,
InterfaceTimestampMode::HardwarePTPAll,
)
}
None => {
clock_port_map.push(None);
(
system_clock.clone_boxed(),
InterfaceTimestampMode::SoftwareAll,
)
}
},
HardwareClock::Required => {
let idx = interface.lookup_phc().expect("No hardware clock found");
add_hw!(idx)
},
HardwareClock::Specific(idx) => {
add_hw!(idx)
},
HardwareClock::None => {
add_sw!()
},
};

let rng = StdRng::from_entropy();
let bind_phc = port_config.hardware_clock;
let port = instance.add_port(
port_config.into(),
KalmanConfiguration::default(),
Expand Down
Loading