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

Added configurable minor version number. #546

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
3 changes: 3 additions & 0 deletions docs/man/statime.toml.5.md
Original file line number Diff line number Diff line change
Expand Up @@ -84,3 +84,6 @@ will be indicated by each configuration setting shown.
A clock identity is encoded as a 16-character hexadecimal string, for example
`acceptable-master-list = ["00FFFFFFFFFFFFFB"]`.
The default is to accept all clock identities.

`minor-ptp-version` = *version number* (**1**)
: Set a different minor ptp version. Should be either 1 or 0, intended to work around misbehaving ptp 2.0 hardware
2 changes: 1 addition & 1 deletion docs/precompiled/man/statime.8
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
.\" Automatically generated by Pandoc 3.4
.\" Automatically generated by Pandoc 3.5
.\"
.TH "STATIME" "8" "" "statime 0.2.2" "statime"
.SH NAME
Expand Down
7 changes: 6 additions & 1 deletion docs/precompiled/man/statime.toml.5
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
.\" Automatically generated by Pandoc 3.4
.\" Automatically generated by Pandoc 3.5
.\"
.TH "STATIME.TOML" "5" "" "statime 0.2.2" "statime"
.SH NAME
Expand Down Expand Up @@ -102,3 +102,8 @@ A clock identity is encoded as a 16\-character hexadecimal string, for
example
\f[CR]acceptable\-master\-list = [\[dq]00FFFFFFFFFFFFFB\[dq]]\f[R].
The default is to accept all clock identities.
.TP
\f[CR]minor\-ptp\-version\f[R] = \f[I]version number\f[R] (\f[B]1\f[R])
Set a different minor ptp version.
Should be either 1 or 0, intended to work around misbehaving ptp 2.0
hardware
8 changes: 8 additions & 0 deletions statime-linux/src/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ pub struct PortConfig {
pub delay_mechanism: DelayType,
#[serde(default = "default_delay_interval")]
pub delay_interval: i8,
#[serde(default = "default_minor_ptp_version")]
pub minor_ptp_version: u8,
Copy link
Member

Choose a reason for hiding this comment

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

I guess this technically right now allows you to specify version 2.255, which feels like something we really shouldn't allow. Maybe use an enum MinorVersion { Zero, One } with a From<MinorVersion> for u8, TryFrom<u8> for MinorVersion and equivalent serde implementations?

}

fn deserialize_acceptable_master_list<'de, D>(
Expand Down Expand Up @@ -116,6 +118,7 @@ impl From<PortConfig> for statime::config::PortConfig<Option<Vec<ClockIdentity>>
interval: Interval::from_log_2(pc.delay_interval),
},
},
minor_ptp_version: pc.minor_ptp_version,
}
}
}
Expand Down Expand Up @@ -218,6 +221,10 @@ fn default_delay_interval() -> i8 {
0
}

fn default_minor_ptp_version() -> u8 {
1
}

#[derive(Deserialize, Debug, Clone, PartialEq, Eq)]
#[serde(rename_all = "kebab-case", deny_unknown_fields)]
pub struct ObservabilityConfig {
Expand Down Expand Up @@ -275,6 +282,7 @@ interface = "enp0s31f6"
delay_asymmetry: 0,
delay_mechanism: crate::config::DelayType::E2E,
delay_interval: 0,
minor_ptp_version: 1,
};

let expected = crate::config::Config {
Expand Down
5 changes: 4 additions & 1 deletion statime/src/config/port.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,12 @@ pub struct PortConfig<A> {

/// The estimated asymmetry in the link connected to this [`Port`]
pub delay_asymmetry: Duration,

/// Minor version number to use.
pub minor_ptp_version: u8,
// Notes:
// Fields specific for delay mechanism are kept as part of [DelayMechanism].
// Version is always 2.1, so not stored (versionNumber, minorVersionNumber)
// Major version is always 2.1, so not stored (versionNumber)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// Major version is always 2.1, so not stored (versionNumber)
// Major version is always 2, so not stored (versionNumber)

}

impl<A> PortConfig<A> {
Expand Down
5 changes: 2 additions & 3 deletions statime/src/datastructures/messages/announce.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ mod tests {
0x80, 0x80,
],
AnnounceMessage {
header: Header::default(),
header: Header::new(1),
origin_timestamp: WireTimestamp {
seconds: 1169232218,
nanos: 175326816,
Expand Down Expand Up @@ -132,8 +132,7 @@ mod tests {

// Test the deserialization output
let deserialized_data =
AnnounceMessage::deserialize_content(Header::default(), &byte_representation)
.unwrap();
AnnounceMessage::deserialize_content(Header::new(1), &byte_representation).unwrap();
assert_eq!(deserialized_data, object_representation);
}
}
Expand Down
39 changes: 18 additions & 21 deletions statime/src/datastructures/messages/header.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,13 @@ pub(crate) struct DeserializedHeader {
}

impl Header {
pub(super) fn new() -> Self {
pub(crate) fn new(minor_ptp_version: u8) -> Self {
Self {
sdo_id: SdoId(0),
version: PtpVersion { major: 2, minor: 1 },
version: PtpVersion {
major: 2,
minor: minor_ptp_version,
},
domain_number: 0,
alternate_master_flag: false,
two_step_flag: false,
Expand Down Expand Up @@ -134,12 +137,6 @@ impl Header {
}
}

impl Default for Header {
fn default() -> Self {
Self::new()
}
}

/// A wrapper type for PTP Sdo Identifiers.
///
/// Because `SdoId`s are 12 bit values they always lie within `0..=0xFFF`.
Expand Down Expand Up @@ -277,19 +274,19 @@ mod tests {
fn flagfield_wireformat() {
#[rustfmt::skip]
let representations = [
([0x00, 0x00u8], Header::default()),
([0x01, 0x00u8], Header { alternate_master_flag: true, ..Default::default() }),
([0x02, 0x00u8], Header { two_step_flag: true, ..Default::default() }),
([0x04, 0x00u8], Header { unicast_flag: true, ..Default::default() }),
([0x20, 0x00u8], Header { ptp_profile_specific_1: true, ..Default::default() }),
([0x40, 0x00u8], Header { ptp_profile_specific_2: true, ..Default::default() }),
([0x00, 0x01u8], Header { leap61: true, ..Default::default() }),
([0x00, 0x02u8], Header { leap59: true, ..Default::default() }),
([0x00, 0x04u8], Header { current_utc_offset_valid: true, ..Default::default() }),
([0x00, 0x08u8], Header { ptp_timescale: true, ..Default::default() }),
([0x00, 0x10u8], Header { time_tracable: true, ..Default::default() }),
([0x00, 0x20u8], Header { frequency_tracable: true, ..Default::default() }),
([0x00, 0x40u8], Header { synchronization_uncertain: true, ..Default::default() }),
([0x00, 0x00u8], Header::new(1)),
([0x01, 0x00u8], Header { alternate_master_flag: true, ..Header::new(1) }),
([0x02, 0x00u8], Header { two_step_flag: true, ..Header::new(1) }),
([0x04, 0x00u8], Header { unicast_flag: true, ..Header::new(1) }),
([0x20, 0x00u8], Header { ptp_profile_specific_1: true, ..Header::new(1) }),
([0x40, 0x00u8], Header { ptp_profile_specific_2: true, ..Header::new(1) }),
([0x00, 0x01u8], Header { leap61: true, ..Header::new(1) }),
([0x00, 0x02u8], Header { leap59: true, ..Header::new(1) }),
([0x00, 0x04u8], Header { current_utc_offset_valid: true, ..Header::new(1) }),
([0x00, 0x08u8], Header { ptp_timescale: true, ..Header::new(1) }),
([0x00, 0x10u8], Header { time_tracable: true, ..Header::new(1) }),
([0x00, 0x20u8], Header { frequency_tracable: true, ..Header::new(1) }),
([0x00, 0x40u8], Header { synchronization_uncertain: true, ..Header::new(1) }),
];

for (i, (byte_representation, flag_representation)) in
Expand Down
39 changes: 29 additions & 10 deletions statime/src/datastructures/messages/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,32 +227,35 @@ fn base_header(
default_ds: &InternalDefaultDS,
port_identity: PortIdentity,
sequence_id: u16,
minor_ptp_version: u8,
) -> Header {
Header {
sdo_id: default_ds.sdo_id,
domain_number: default_ds.domain_number,
source_port_identity: port_identity,
sequence_id,
..Default::default()
..Header::new(minor_ptp_version)
}
}

/// Checks whether message is of PTP revision compatible with Statime
pub fn is_compatible(buffer: &[u8]) -> bool {
// this ensures that versionPTP in the header is 2
// it will never happen in PTPv1 packets because this octet is the LSB of versionPTP there
(buffer.len() >= 2) && (buffer[1] & 0xF) == 2
// it will never happen in PTPv1 packets because this octet is the LSB of
// versionPTP there
(buffer.len() >= 2) && (buffer[1] & 0xf) == 2
}

impl Message<'_> {
pub(crate) fn sync(
default_ds: &InternalDefaultDS,
port_identity: PortIdentity,
sequence_id: u16,
minor_ptp_version: u8,
) -> Self {
let header = Header {
two_step_flag: true,
..base_header(default_ds, port_identity, sequence_id)
..base_header(default_ds, port_identity, sequence_id, minor_ptp_version)
};

Message {
Expand All @@ -269,10 +272,11 @@ impl Message<'_> {
port_identity: PortIdentity,
sequence_id: u16,
timestamp: Time,
minor_ptp_version: u8,
) -> Self {
let header = Header {
correction_field: timestamp.subnano(),
..base_header(default_ds, port_identity, sequence_id)
..base_header(default_ds, port_identity, sequence_id, minor_ptp_version)
};

Message {
Expand All @@ -288,6 +292,7 @@ impl Message<'_> {
global: &PtpInstanceState,
port_identity: PortIdentity,
sequence_id: u16,
minor_ptp_version: u8,
) -> Self {
let time_properties_ds = &global.time_properties_ds;

Expand All @@ -298,7 +303,12 @@ impl Message<'_> {
ptp_timescale: time_properties_ds.ptp_timescale,
time_tracable: time_properties_ds.time_traceable,
frequency_tracable: time_properties_ds.frequency_traceable,
..base_header(&global.default_ds, port_identity, sequence_id)
..base_header(
&global.default_ds,
port_identity,
sequence_id,
minor_ptp_version,
)
};

let body = MessageBody::Announce(AnnounceMessage {
Expand All @@ -324,10 +334,11 @@ impl Message<'_> {
default_ds: &InternalDefaultDS,
port_identity: PortIdentity,
sequence_id: u16,
minor_ptp_version: u8,
) -> Self {
let header = Header {
log_message_interval: 0x7f,
..base_header(default_ds, port_identity, sequence_id)
..base_header(default_ds, port_identity, sequence_id, minor_ptp_version)
};

Message {
Expand Down Expand Up @@ -375,9 +386,10 @@ impl Message<'_> {
default_ds: &InternalDefaultDS,
port_identity: PortIdentity,
sequence_id: u16,
minor_ptp_version: u8,
) -> Self {
Message {
header: base_header(default_ds, port_identity, sequence_id),
header: base_header(default_ds, port_identity, sequence_id, minor_ptp_version),
body: MessageBody::PDelayReq(PDelayReqMessage {
origin_timestamp: WireTimestamp::default(),
}),
Expand All @@ -390,13 +402,19 @@ impl Message<'_> {
port_identity: PortIdentity,
request_header: Header,
timestamp: Time,
minor_ptp_version: u8,
) -> Self {
// We implement Option B from IEEE 1588-2019 page 202
Message {
header: Header {
two_step_flag: true,
correction_field: request_header.correction_field,
..base_header(default_ds, port_identity, request_header.sequence_id)
..base_header(
default_ds,
port_identity,
request_header.sequence_id,
minor_ptp_version,
)
},
body: MessageBody::PDelayResp(PDelayRespMessage {
request_receive_timestamp: timestamp.into(),
Expand All @@ -412,9 +430,10 @@ impl Message<'_> {
requestor_identity: PortIdentity,
sequence_id: u16,
timestamp: Time,
minor_ptp_version: u8,
) -> Self {
Message {
header: base_header(default_ds, port_identity, sequence_id),
header: base_header(default_ds, port_identity, sequence_id, minor_ptp_version),
body: MessageBody::PDelayRespFollowUp(PDelayRespFollowUpMessage {
response_origin_timestamp: timestamp.into(),
requesting_port_identity: requestor_identity,
Expand Down
12 changes: 8 additions & 4 deletions statime/src/overlay_clock.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
use crate::{time::Duration, time::Time, Clock};
use crate::{
time::{Duration, Time},
Clock,
};

/// An overlay over other, read-only clock, frequency-locked to it.
/// In other words, a virtual clock which can be tuned in software without affecting
/// the underlying system or hardware clock.
/// In other words, a virtual clock which can be tuned in software without
/// affecting the underlying system or hardware clock.
#[derive(Debug)]
pub struct OverlayClock<C: Clock> {
roclock: C,
Expand All @@ -23,7 +26,8 @@ impl<C: Clock> OverlayClock<C> {
}
}

/// Converts (shifts and scales) `Time` in underlying clock's timescale to overlay clock timescale
/// Converts (shifts and scales) `Time` in underlying clock's timescale to
/// overlay clock timescale
pub fn time_from_underlying(&self, roclock_time: Time) -> Time {
let elapsed = roclock_time - self.last_sync;
let corr = elapsed * self.freq_scale_ppm_diff / 1_000_000;
Expand Down
Loading
Loading