Merge pull request #3966 from lucasec/t6630

T6630: ntp: support hardware timestamp offload and other mechanisms to improve accuracy

4 files changed, 210 insertions, 2 deletions

diff --git a/data/templates/chrony/chrony.conf.j2 b/data/templates/chrony/chrony.conf.j2
index e3f078fdc..2838f5524 100644
--- a/data/templates/chrony/chrony.conf.j2
+++ b/data/templates/chrony/chrony.conf.j2
@@ -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 %}
 
@@ -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 %}
diff --git a/interface-definitions/service_ntp.xml.in b/interface-definitions/service_ntp.xml.in
index c057b62b5..5dc0cd295 100644
--- a/interface-definitions/service_ntp.xml.in
+++ b/interface-definitions/service_ntp.xml.in
@@ -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>
@@ -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>
diff --git a/smoketest/scripts/cli/test_service_ntp.py b/smoketest/scripts/cli/test_service_ntp.py
index ae45fe2f4..07af4f5eb 100755
--- a/smoketest/scripts/cli/test_service_ntp.py
+++ b/smoketest/scripts/cli/test_service_ntp.py
@@ -21,6 +21,7 @@ from base_vyostest_shim import VyOSUnitTestSHIM
 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'
@@ -165,5 +166,99 @@ class TestSystemNTP(VyOSUnitTestSHIM.TestCase):
                 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)
diff --git a/src/conf_mode/service_ntp.py b/src/conf_mode/service_ntp.py
index 83880fd72..32563aa0e 100755
--- a/src/conf_mode/service_ntp.py
+++ b/src/conf_mode/service_ntp.py
@@ -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
@@ -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):
@@ -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):