Skip to content

Commit

Permalink
Merge pull request vyos#3966 from lucasec/t6630
Browse files Browse the repository at this point in the history 
T6630: ntp: support hardware timestamp offload and other mechanisms to improve accuracy
  • Loading branch information
@dmbaturin
dmbaturin authored Sep 24, 2024
2 parents 644a91e + 917c658 commit cf55d9d
Show file tree
Hide file tree
Showing 4 changed files with 210 additions and 2 deletions.
17 changes: 16 additions & 1 deletion data/templates/chrony/chrony.conf.j2
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ user {{ user }}
{% if config.pool is vyos_defined %}
{% set association = 'pool' %}
{% endif %}
{{ association }} {{ server | replace('_', '-') }} iburst {{ 'nts' if config.nts is vyos_defined }} {{ 'noselect' if config.noselect is vyos_defined }} {{ 'prefer' if config.prefer is vyos_defined }}
{{ association }} {{ server | replace('_', '-') }} iburst {{- ' nts' if config.nts is vyos_defined }} {{- ' noselect' if config.noselect is vyos_defined }} {{- ' prefer' if config.prefer is vyos_defined }} {{- ' xleave' if config.interleave is vyos_defined }} {{- ' port ' ~ ptp.port if ptp.port is vyos_defined and config.ptp is vyos_defined }}
{% endfor %}
{% endif %}

Expand All @@ -66,3 +66,18 @@ bindaddress {{ address }}
binddevice {{ interface }}
{% endif %}
{% endif %}

{% if ptp.timestamp.interface is vyos_defined %}
# Enable hardware timestamping on the specified interfaces
{% for iface, iface_config in ptp.timestamp.interface.items() %}
{% if iface == "all" %}
{% set iface = "*" %}
{% endif %}
hwtimestamp {{ iface }} {{- ' rxfilter ' ~ iface_config.receive_filter if iface_config.receive_filter is vyos_defined }}
{% endfor %}
{% endif %}

{% if ptp.port is vyos_defined %}
# Enable sending and receiving NTP over PTP packets (PTP transport)
ptpport {{ ptp.port }}
{% endif %}
80 changes: 80 additions & 0 deletions interface-definitions/service_ntp.xml.in
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,74 @@
#include <include/generic-interface.xml.i>
#include <include/listen-address.xml.i>
#include <include/interface/vrf.xml.i>
<node name="ptp">
<properties>
<help>Enable Precision Time Protocol (PTP) transport</help>
</properties>
<children>
#include <include/port-number.xml.i>
<leafNode name="port">
<defaultValue>319</defaultValue>
</leafNode>
<node name="timestamp">
<properties>
<help>Enable timestamping of packets in the NIC hardware</help>
</properties>
<children>
<tagNode name="interface">
<properties>
<help>Interface to enable timestamping on</help>
<completionHelp>
<script>${vyos_completion_dir}/list_interfaces</script>
<list>all</list>
</completionHelp>
<valueHelp>
<format>all</format>
<description>Select all interfaces</description>
</valueHelp>
<valueHelp>
<format>txt</format>
<description>Interface name</description>
</valueHelp>
<constraint>
#include <include/constraint/interface-name.xml.i>
<regex>all</regex>
</constraint>
</properties>
<children>
<leafNode name="receive-filter">
<properties>
<help>Selects which inbound packets are timestamped by the NIC</help>
<completionHelp>
<list>all ntp ptp none</list>
</completionHelp>
<valueHelp>
<format>all</format>
<description>All packets are timestamped</description>
</valueHelp>
<valueHelp>
<format>ntp</format>
<description>Only NTP packets are timestamped</description>
</valueHelp>
<valueHelp>
<format>ptp</format>
<description>Only PTP or NTP packets using the PTP transport are timestamped</description>
</valueHelp>
<valueHelp>
<format>none</format>
<description>No packet is timestamped</description>
</valueHelp>
<constraint>
<regex>(all|ntp|ptp|none)</regex>
</constraint>
</properties>
</leafNode>
</children>
</tagNode>
</children>
</node>
</children>
</node>
<leafNode name="leap-second">
<properties>
<help>Leap second behavior</help>
Expand Down Expand Up @@ -86,6 +154,18 @@
<valueless/>
</properties>
</leafNode>
<leafNode name="ptp">
<properties>
<help>Use Precision Time Protocol (PTP) transport for the server</help>
<valueless/>
</properties>
</leafNode>
<leafNode name="interleave">
<properties>
<help>Use the interleaved mode for the server</help>
<valueless/>
</properties>
</leafNode>
</children>
</tagNode>
</children>
Expand Down
95 changes: 95 additions & 0 deletions smoketest/scripts/cli/test_service_ntp.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
from vyos.configsession import ConfigSessionError
from vyos.utils.process import cmd
from vyos.utils.process import process_named_running
from vyos.xml_ref import default_value

PROCESS_NAME = 'chronyd'
NTP_CONF = '/run/chrony/chrony.conf'
Expand Down Expand Up @@ -165,5 +166,99 @@ def test_leap_seconds(self):
self.assertIn(f'maxslewrate 1000', config)
self.assertIn(f'smoothtime 400 0.001024 leaponly', config)

def test_interleave_option(self):
# "interleave" option differs from some others in that the
# name is not a 1:1 mapping from VyOS config
servers = ['192.0.2.1', '192.0.2.2']
options = ['prefer']

for server in servers:
for option in options:
self.cli_set(base_path + ['server', server, option])
self.cli_set(base_path + ['server', server, 'interleave'])

# commit changes
self.cli_commit()

# Check generated configuration
# this file must be read with higher permissions
config = cmd(f'sudo cat {NTP_CONF}')
self.assertIn('driftfile /run/chrony/drift', config)
self.assertIn('dumpdir /run/chrony', config)
self.assertIn('ntsdumpdir /run/chrony', config)
self.assertIn('clientloglimit 1048576', config)
self.assertIn('rtcsync', config)
self.assertIn('makestep 1.0 3', config)
self.assertIn('leapsectz right/UTC', config)

for server in servers:
self.assertIn(f'server {server} iburst ' + ' '.join(options) + ' xleave', config)

def test_offload_timestamp_default(self):
# Test offloading of NIC timestamp
servers = ['192.0.2.1', '192.0.2.2']
ptp_port = '8319'

for server in servers:
self.cli_set(base_path + ['server', server, 'ptp'])

self.cli_set(base_path + ['ptp', 'port', ptp_port])
self.cli_set(base_path + ['ptp', 'timestamp', 'interface', 'all'])

# commit changes
self.cli_commit()

# Check generated configuration
# this file must be read with higher permissions
config = cmd(f'sudo cat {NTP_CONF}')
self.assertIn('driftfile /run/chrony/drift', config)
self.assertIn('dumpdir /run/chrony', config)
self.assertIn('ntsdumpdir /run/chrony', config)
self.assertIn('clientloglimit 1048576', config)
self.assertIn('rtcsync', config)
self.assertIn('makestep 1.0 3', config)
self.assertIn('leapsectz right/UTC', config)

for server in servers:
self.assertIn(f'server {server} iburst port {ptp_port}', config)

self.assertIn('hwtimestamp *', config)

def test_ptp_transport(self):
# Test offloading of NIC timestamp
servers = ['192.0.2.1', '192.0.2.2']
options = ['prefer']

default_ptp_port = default_value(base_path + ['ptp', 'port'])

for server in servers:
for option in options:
self.cli_set(base_path + ['server', server, option])
self.cli_set(base_path + ['server', server, 'ptp'])

# commit changes (expected to fail)
with self.assertRaises(ConfigSessionError):
self.cli_commit()

# add the required top-level option and commit
self.cli_set(base_path + ['ptp'])
self.cli_commit()

# Check generated configuration
# this file must be read with higher permissions
config = cmd(f'sudo cat {NTP_CONF}')
self.assertIn('driftfile /run/chrony/drift', config)
self.assertIn('dumpdir /run/chrony', config)
self.assertIn('ntsdumpdir /run/chrony', config)
self.assertIn('clientloglimit 1048576', config)
self.assertIn('rtcsync', config)
self.assertIn('makestep 1.0 3', config)
self.assertIn('leapsectz right/UTC', config)

for server in servers:
self.assertIn(f'server {server} iburst ' + ' '.join(options) + f' port {default_ptp_port}', config)

self.assertIn(f'ptpport {default_ptp_port}', config)

if __name__ == '__main__':
unittest.main(verbosity=2)
20 changes: 19 additions & 1 deletion src/conf_mode/service_ntp.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import os

from vyos.config import Config
from vyos.config import config_dict_merge
from vyos.configdict import is_node_changed
from vyos.configverify import verify_vrf
from vyos.configverify import verify_interface_exists
Expand All @@ -42,13 +43,21 @@ def get_config(config=None):
if not conf.exists(base):
return None

ntp = conf.get_config_dict(base, key_mangling=('-', '_'), get_first_key=True, with_defaults=True)
ntp = conf.get_config_dict(base, key_mangling=('-', '_'), get_first_key=True)
ntp['config_file'] = config_file
ntp['user'] = user_group

tmp = is_node_changed(conf, base + ['vrf'])
if tmp: ntp.update({'restart_required': {}})

# We have gathered the dict representation of the CLI, but there are default
# options which we need to update into the dictionary retrived.
default_values = conf.get_config_defaults(**ntp.kwargs, recursive=True)
# Only defined PTP default port, if PTP feature is in use
if 'ptp' not in ntp:
del default_values['ptp']

ntp = config_dict_merge(default_values, ntp)
return ntp

def verify(ntp):
Expand Down Expand Up @@ -87,6 +96,15 @@ def verify(ntp):
if ipv6_addresses > 1:
raise ConfigError(f'NTP Only admits one ipv6 value for listen-address parameter ')

if 'server' in ntp:
for host, server in ntp['server'].items():
if 'ptp' in server:
if 'ptp' not in ntp:
raise ConfigError('PTP must be enabled for the NTP service '\
f'before it can be used for server "{host}"')
else:
break

return None

def generate(ntp):
Expand Down

0 comments on commit cf55d9d

Please sign in to comment.