From a7d7cc32f1cb83f6318d33e38f86efdc22d065e9 Mon Sep 17 00:00:00 2001 From: Zhongning Li <60045212+tomli380576@users.noreply.github.com> Date: Thu, 16 Jul 2026 14:45:01 +0800 Subject: [PATCH 01/13] feat: implement native ts_info getter in checkbox support --- .../checkbox_support/ethtool/__init__.py | 0 .../checkbox_support/ethtool/ts_info.py | 138 ++++++++++++++++++ 2 files changed, 138 insertions(+) create mode 100644 checkbox-support/checkbox_support/ethtool/__init__.py create mode 100644 checkbox-support/checkbox_support/ethtool/ts_info.py diff --git a/checkbox-support/checkbox_support/ethtool/__init__.py b/checkbox-support/checkbox_support/ethtool/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/checkbox-support/checkbox_support/ethtool/ts_info.py b/checkbox-support/checkbox_support/ethtool/ts_info.py new file mode 100644 index 0000000000..6cc18f17f2 --- /dev/null +++ b/checkbox-support/checkbox_support/ethtool/ts_info.py @@ -0,0 +1,138 @@ +#!/usr/bin/env python3 +# This file is part of Checkbox. +# +# Copyright 2026 Canonical Ltd. +# Authors: +# Zhongning Li +# +# Checkbox is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License version 3, +# as published by the Free Software Foundation. +# +# Checkbox is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Checkbox. If not, see . + +""" +Source code of ethtool: +https://git.kernel.org/pub/scm/network/ethtool/ethtool.git +This python program re-implements this function: +https://git.kernel.org/pub/scm/network/ethtool/ethtool.git/tree/ethtool.c#n4986 +""" + +import ctypes +import fcntl +import os +import socket +import logging +from pathlib import Path + +logger = logging.getLogger() +# these 2 are used to make same ioctl request as ethtool +# https://github.com/torvalds/linux/blob/3b029c035b34bbc693405ddf759f0e9b920c27f1/include/uapi/linux/sockios.h#L102 +SIOCETHTOOL = 0x8946 +# https://github.com/torvalds/linux/blob/3b029c035b34bbc693405ddf759f0e9b920c27f1/include/uapi/linux/ethtool.h#L1940 +ETHTOOL_GET_TS_INFO = 0x00000041 + +# yes this is actually how it's spelled +# https://github.com/torvalds/linux/blob/3b029c035b34bbc693405ddf759f0e9b920c27f1/include/uapi/linux/if.h#L33 +IFNAMSIZ = 16 + + +# the main struct of interest +# for the purpose of finding PTP capable interfaces we only need phc_index +# we also need to specify .cmd to indicate that we want to do ETHTOOL_GET_TS_INFO +# https://github.com/torvalds/linux/blob/3b029c035b34bbc693405ddf759f0e9b920c27f1/include/uapi/linux/ethtool.h#L1739 +class ethtool_ts_info(ctypes.Structure): + _fields_ = [ + ("cmd", ctypes.c_uint32), + ("so_timestamping", ctypes.c_uint32), + ("phc_index", ctypes.c_int32), + ("tx_types", ctypes.c_uint32), + ("tx_reserved", ctypes.c_uint32 * 3), + ("rx_filters", ctypes.c_uint32), + ("rx_reserved", ctypes.c_uint32 * 3), + ] + + +# this is only used to make the SIOCETHTOOL ioctl request +# we don't need any of the field values in this struct +# https://github.com/torvalds/linux/blob/3b029c035b34bbc693405ddf759f0e9b920c27f1/include/uapi/linux/if.h#L234-L256 +class ifreq(ctypes.Structure): + _fields_ = [ + ("ifr_name", ctypes.c_char * IFNAMSIZ), + ("ifr_data", ctypes.c_void_p), + ] + + +def _is_ethernet_interface(interface: str) -> bool: + """Check whether the given interface is a physical ethernet device + + :param interface: interface name like enp1s1 or wlp194s0 + :return: True if the interface is a real device + (not wifi, not loopback, not vpn, etc.) + """ + + try: + sys_class_net_interface = Path("/sys/class/net/") / interface + + # check for ARPHRD_ETHER + if not (sys_class_net_interface / "type").read_text().strip() == "1": + return False + # skip everything not associated with a physical device + if not (sys_class_net_interface / "device").exists(): + return False + # wifi interfaces (16.04+) + if (sys_class_net_interface / "phy80211").exists(): + return False + + return True + except Exception: + # assume false if failed to read any of the paths above + return False + + +def get_ts_info(interface: str) -> ethtool_ts_info: + """ + The main entry point of getting ts_info for the given interface + + :param interface: interface name like enp1s1 + :return: the ethtool_ts_info struct. The main attr we need is the phc_index + because it directly maps to /dev/ptpX + """ + if not _is_ethernet_interface(interface): + logger.warning( + "{} is not a physical ethernet interface".format(interface) + ) + + info = ethtool_ts_info() + info.cmd = ETHTOOL_GET_TS_INFO + + ifr = ifreq() + ifr.ifr_name = interface.encode() + ifr.ifr_data = ctypes.cast(ctypes.pointer(info), ctypes.c_void_p) + + with socket.socket(socket.AF_INET, socket.SOCK_DGRAM) as sock: + fcntl.ioctl(sock.fileno(), SIOCETHTOOL, ifr, True) + + return info + + +def is_ptp_capable(interface: str) -> bool: + """Checks if supports PTP + + This is equivalent to checking the "PTP Hardware Clock" line in ethtool + and testing if the corresponding device node exists + + :param interface: _description_ + :return: _description_ + """ + info = get_ts_info(interface) + phc_index = int(info.phc_index) + expected_ptp_device_path = "/dev/ptp{}".format(phc_index) + + return phc_index >= 0 and os.path.exists(expected_ptp_device_path) \ No newline at end of file From 4f7eed5454c7bb6978f5056804be2734a02fe15b Mon Sep 17 00:00:00 2001 From: Zhongning Li <60045212+tomli380576@users.noreply.github.com> Date: Thu, 16 Jul 2026 14:57:28 +0800 Subject: [PATCH 02/13] feat: add pretty print --- checkbox-support/checkbox_support/ethtool/ts_info.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/checkbox-support/checkbox_support/ethtool/ts_info.py b/checkbox-support/checkbox_support/ethtool/ts_info.py index 6cc18f17f2..31e7df5355 100644 --- a/checkbox-support/checkbox_support/ethtool/ts_info.py +++ b/checkbox-support/checkbox_support/ethtool/ts_info.py @@ -58,6 +58,16 @@ class ethtool_ts_info(ctypes.Structure): ("rx_reserved", ctypes.c_uint32 * 3), ] + def __str__(self) -> str: + lines = [] # type: list[str] + for field in self._fields_: + name = field[0] + value = getattr(self, name) + if isinstance(value, ctypes.Array): + value = list(value) + lines.append("{}={}".format(name, value)) + return "ethtool_ts_info({})".format(", ".join(lines)) + # this is only used to make the SIOCETHTOOL ioctl request # we don't need any of the field values in this struct @@ -135,4 +145,4 @@ def is_ptp_capable(interface: str) -> bool: phc_index = int(info.phc_index) expected_ptp_device_path = "/dev/ptp{}".format(phc_index) - return phc_index >= 0 and os.path.exists(expected_ptp_device_path) \ No newline at end of file + return phc_index >= 0 and os.path.exists(expected_ptp_device_path) From efbe124bc3b965c545514c14288ac4faf829ea35 Mon Sep 17 00:00:00 2001 From: Zhongning Li <60045212+tomli380576@users.noreply.github.com> Date: Thu, 16 Jul 2026 14:59:13 +0800 Subject: [PATCH 03/13] doc: docstring --- checkbox-support/checkbox_support/ethtool/ts_info.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/checkbox-support/checkbox_support/ethtool/ts_info.py b/checkbox-support/checkbox_support/ethtool/ts_info.py index 31e7df5355..dab287c8d2 100644 --- a/checkbox-support/checkbox_support/ethtool/ts_info.py +++ b/checkbox-support/checkbox_support/ethtool/ts_info.py @@ -138,8 +138,9 @@ def is_ptp_capable(interface: str) -> bool: This is equivalent to checking the "PTP Hardware Clock" line in ethtool and testing if the corresponding device node exists - :param interface: _description_ - :return: _description_ + :param interface: the interface to check + :return: if kernel says the interface supports ptp + AND The /dev/ptpX device exists """ info = get_ts_info(interface) phc_index = int(info.phc_index) From dfaa005fb4445bf931cf8f425ce4c81537075799 Mon Sep 17 00:00:00 2001 From: Zhongning Li <60045212+tomli380576@users.noreply.github.com> Date: Thu, 16 Jul 2026 15:09:34 +0800 Subject: [PATCH 04/13] test: copilot unittests --- .../ethtool/tests/__init__.py | 0 .../ethtool/tests/test_ts_info.py | 217 ++++++++++++++++++ 2 files changed, 217 insertions(+) create mode 100644 checkbox-support/checkbox_support/ethtool/tests/__init__.py create mode 100644 checkbox-support/checkbox_support/ethtool/tests/test_ts_info.py diff --git a/checkbox-support/checkbox_support/ethtool/tests/__init__.py b/checkbox-support/checkbox_support/ethtool/tests/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/checkbox-support/checkbox_support/ethtool/tests/test_ts_info.py b/checkbox-support/checkbox_support/ethtool/tests/test_ts_info.py new file mode 100644 index 0000000000..a044b55702 --- /dev/null +++ b/checkbox-support/checkbox_support/ethtool/tests/test_ts_info.py @@ -0,0 +1,217 @@ +#!/usr/bin/env python3 +# This file is part of Checkbox. +# +# Copyright 2026 Canonical Ltd. +# Authors: +# Zhongning Li +# +# Checkbox is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License version 3, +# as published by the Free Software Foundation. +# +# Checkbox is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Checkbox. If not, see . + +""" +Unit tests for checkbox_support.ethtool.ts_info + +These tests never touch real hardware or the kernel: every ioctl() call +and every filesystem read under /sys/class/net is mocked so the suite +can run unmodified on a headless CI runner (e.g. GitHub Actions). +""" + +import ctypes +import tempfile +from pathlib import Path +from unittest import TestCase +import unittest +from unittest.mock import patch + +from checkbox_support.ethtool.ts_info import ( + ETHTOOL_GET_TS_INFO, + SIOCETHTOOL, + ethtool_ts_info, + get_ts_info, + is_ptp_capable, + _is_ethernet_interface, +) + + +class TestEthtoolTsInfoStr(TestCase): + def test_str_pretty_prints_scalars_and_arrays(self): + info = ethtool_ts_info() + info.cmd = ETHTOOL_GET_TS_INFO + info.so_timestamping = 0x1F + info.phc_index = 2 + info.tx_types = 7 + info.tx_reserved[0] = 1 + info.tx_reserved[1] = 2 + info.tx_reserved[2] = 3 + info.rx_filters = 9 + info.rx_reserved[0] = 4 + info.rx_reserved[1] = 5 + info.rx_reserved[2] = 6 + + expected = ( + "ethtool_ts_info(cmd={}, so_timestamping=31, phc_index=2, " + "tx_types=7, tx_reserved=[1, 2, 3], rx_filters=9, " + "rx_reserved=[4, 5, 6])" + ).format(ETHTOOL_GET_TS_INFO) + self.assertEqual(str(info), expected) + + def test_str_default_values(self): + info = ethtool_ts_info() + expected = ( + "ethtool_ts_info(cmd=0, so_timestamping=0, phc_index=0, " + "tx_types=0, tx_reserved=[0, 0, 0], rx_filters=0, " + "rx_reserved=[0, 0, 0])" + ) + self.assertEqual(str(info), expected) + + +class TestIsEthernetInterface(TestCase): + def _make_sys_class_net( + self, tmp_dir, has_type=True, is_ether=True, has_device=True, + is_wifi=False, + ): + iface_dir = Path(tmp_dir) / "enp1s0" + iface_dir.mkdir(parents=True) + if has_type: + (iface_dir / "type").write_text("1\n" if is_ether else "772\n") + if has_device: + (iface_dir / "device").mkdir() + if is_wifi: + (iface_dir / "phy80211").mkdir() + return iface_dir.parent + + def test_physical_ethernet_interface(self): + with tempfile.TemporaryDirectory() as tmp_dir: + base = self._make_sys_class_net(tmp_dir) + with patch( + "checkbox_support.ethtool.ts_info.Path", + side_effect=lambda p: base if "/sys/class/net" in p + else Path(p), + ): + self.assertTrue(_is_ethernet_interface("enp1s0")) + + def test_not_ether_type(self): + with tempfile.TemporaryDirectory() as tmp_dir: + base = self._make_sys_class_net(tmp_dir, is_ether=False) + with patch( + "checkbox_support.ethtool.ts_info.Path", + side_effect=lambda p: base if "/sys/class/net" in p + else Path(p), + ): + self.assertFalse(_is_ethernet_interface("enp1s0")) + + def test_no_device_dir(self): + with tempfile.TemporaryDirectory() as tmp_dir: + base = self._make_sys_class_net(tmp_dir, has_device=False) + with patch( + "checkbox_support.ethtool.ts_info.Path", + side_effect=lambda p: base if "/sys/class/net" in p + else Path(p), + ): + self.assertFalse(_is_ethernet_interface("enp1s0")) + + def test_wifi_interface(self): + with tempfile.TemporaryDirectory() as tmp_dir: + base = self._make_sys_class_net(tmp_dir, is_wifi=True) + with patch( + "checkbox_support.ethtool.ts_info.Path", + side_effect=lambda p: base if "/sys/class/net" in p + else Path(p), + ): + self.assertFalse(_is_ethernet_interface("enp1s0")) + + def test_missing_interface_returns_false(self): + with tempfile.TemporaryDirectory() as tmp_dir: + base = Path(tmp_dir) + with patch( + "checkbox_support.ethtool.ts_info.Path", + side_effect=lambda p: base if "/sys/class/net" in p + else Path(p), + ): + self.assertFalse(_is_ethernet_interface("does-not-exist")) + + +class TestGetTsInfo(TestCase): + def _fake_ioctl(self, fd, request, arg, mutate_flag=False): + """Simulate the kernel writing into the ethtool_ts_info struct + + pointed to by arg.ifr_data, the same way SIOCETHTOOL would. + """ + self.assertEqual(request, SIOCETHTOOL) + info_ptr = ctypes.cast( + arg.ifr_data, ctypes.POINTER(ethtool_ts_info) + ) + info_ptr.contents.so_timestamping = 0x1F + info_ptr.contents.phc_index = 3 + info_ptr.contents.tx_types = 7 + info_ptr.contents.rx_filters = 9 + return 0 + + @patch("checkbox_support.ethtool.ts_info._is_ethernet_interface") + @patch("checkbox_support.ethtool.ts_info.fcntl.ioctl") + def test_get_ts_info_populates_struct( + self, ioctl_mock, is_ethernet_mock + ): + is_ethernet_mock.return_value = True + ioctl_mock.side_effect = self._fake_ioctl + + info = get_ts_info("enp1s0") + + ioctl_mock.assert_called_once() + self.assertEqual(info.cmd, ETHTOOL_GET_TS_INFO) + self.assertEqual(info.phc_index, 3) + self.assertEqual(info.so_timestamping, 0x1F) + self.assertEqual(info.tx_types, 7) + self.assertEqual(info.rx_filters, 9) + + @patch("checkbox_support.ethtool.ts_info._is_ethernet_interface") + @patch("checkbox_support.ethtool.ts_info.fcntl.ioctl") + def test_get_ts_info_warns_on_non_ethernet( + self, ioctl_mock, is_ethernet_mock + ): + is_ethernet_mock.return_value = False + ioctl_mock.side_effect = self._fake_ioctl + + with self.assertLogs(level="WARNING"): + get_ts_info("wlp1s0") + + +class TestIsPtpCapable(TestCase): + @patch("checkbox_support.ethtool.ts_info.os.path.exists") + @patch("checkbox_support.ethtool.ts_info.get_ts_info") + def test_ptp_capable(self, get_ts_info_mock, exists_mock): + get_ts_info_mock.return_value = ethtool_ts_info(phc_index=0) + exists_mock.return_value = True + + self.assertTrue(is_ptp_capable("enp1s0")) + exists_mock.assert_called_once_with("/dev/ptp0") + + @patch("checkbox_support.ethtool.ts_info.os.path.exists") + @patch("checkbox_support.ethtool.ts_info.get_ts_info") + def test_no_phc_device(self, get_ts_info_mock, exists_mock): + get_ts_info_mock.return_value = ethtool_ts_info(phc_index=-1) + exists_mock.return_value = False + + self.assertFalse(is_ptp_capable("enp1s0")) + + @patch("checkbox_support.ethtool.ts_info.os.path.exists") + @patch("checkbox_support.ethtool.ts_info.get_ts_info") + def test_phc_index_valid_but_device_missing( + self, get_ts_info_mock, exists_mock + ): + get_ts_info_mock.return_value = ethtool_ts_info(phc_index=1) + exists_mock.return_value = False + + self.assertFalse(is_ptp_capable("enp1s0")) + +if __name__ == "__main__": + unittest.main() \ No newline at end of file From 7283c837ddd83672325962f8595701d930211991 Mon Sep 17 00:00:00 2001 From: Zhongning Li <60045212+tomli380576@users.noreply.github.com> Date: Thu, 16 Jul 2026 15:09:55 +0800 Subject: [PATCH 05/13] style: formatting --- .../ethtool/tests/test_ts_info.py | 42 +++++++++++-------- 1 file changed, 24 insertions(+), 18 deletions(-) diff --git a/checkbox-support/checkbox_support/ethtool/tests/test_ts_info.py b/checkbox-support/checkbox_support/ethtool/tests/test_ts_info.py index a044b55702..78b4dae0fd 100644 --- a/checkbox-support/checkbox_support/ethtool/tests/test_ts_info.py +++ b/checkbox-support/checkbox_support/ethtool/tests/test_ts_info.py @@ -76,7 +76,11 @@ def test_str_default_values(self): class TestIsEthernetInterface(TestCase): def _make_sys_class_net( - self, tmp_dir, has_type=True, is_ether=True, has_device=True, + self, + tmp_dir, + has_type=True, + is_ether=True, + has_device=True, is_wifi=False, ): iface_dir = Path(tmp_dir) / "enp1s0" @@ -94,8 +98,9 @@ def test_physical_ethernet_interface(self): base = self._make_sys_class_net(tmp_dir) with patch( "checkbox_support.ethtool.ts_info.Path", - side_effect=lambda p: base if "/sys/class/net" in p - else Path(p), + side_effect=lambda p: ( + base if "/sys/class/net" in p else Path(p) + ), ): self.assertTrue(_is_ethernet_interface("enp1s0")) @@ -104,8 +109,9 @@ def test_not_ether_type(self): base = self._make_sys_class_net(tmp_dir, is_ether=False) with patch( "checkbox_support.ethtool.ts_info.Path", - side_effect=lambda p: base if "/sys/class/net" in p - else Path(p), + side_effect=lambda p: ( + base if "/sys/class/net" in p else Path(p) + ), ): self.assertFalse(_is_ethernet_interface("enp1s0")) @@ -114,8 +120,9 @@ def test_no_device_dir(self): base = self._make_sys_class_net(tmp_dir, has_device=False) with patch( "checkbox_support.ethtool.ts_info.Path", - side_effect=lambda p: base if "/sys/class/net" in p - else Path(p), + side_effect=lambda p: ( + base if "/sys/class/net" in p else Path(p) + ), ): self.assertFalse(_is_ethernet_interface("enp1s0")) @@ -124,8 +131,9 @@ def test_wifi_interface(self): base = self._make_sys_class_net(tmp_dir, is_wifi=True) with patch( "checkbox_support.ethtool.ts_info.Path", - side_effect=lambda p: base if "/sys/class/net" in p - else Path(p), + side_effect=lambda p: ( + base if "/sys/class/net" in p else Path(p) + ), ): self.assertFalse(_is_ethernet_interface("enp1s0")) @@ -134,8 +142,9 @@ def test_missing_interface_returns_false(self): base = Path(tmp_dir) with patch( "checkbox_support.ethtool.ts_info.Path", - side_effect=lambda p: base if "/sys/class/net" in p - else Path(p), + side_effect=lambda p: ( + base if "/sys/class/net" in p else Path(p) + ), ): self.assertFalse(_is_ethernet_interface("does-not-exist")) @@ -147,9 +156,7 @@ def _fake_ioctl(self, fd, request, arg, mutate_flag=False): pointed to by arg.ifr_data, the same way SIOCETHTOOL would. """ self.assertEqual(request, SIOCETHTOOL) - info_ptr = ctypes.cast( - arg.ifr_data, ctypes.POINTER(ethtool_ts_info) - ) + info_ptr = ctypes.cast(arg.ifr_data, ctypes.POINTER(ethtool_ts_info)) info_ptr.contents.so_timestamping = 0x1F info_ptr.contents.phc_index = 3 info_ptr.contents.tx_types = 7 @@ -158,9 +165,7 @@ def _fake_ioctl(self, fd, request, arg, mutate_flag=False): @patch("checkbox_support.ethtool.ts_info._is_ethernet_interface") @patch("checkbox_support.ethtool.ts_info.fcntl.ioctl") - def test_get_ts_info_populates_struct( - self, ioctl_mock, is_ethernet_mock - ): + def test_get_ts_info_populates_struct(self, ioctl_mock, is_ethernet_mock): is_ethernet_mock.return_value = True ioctl_mock.side_effect = self._fake_ioctl @@ -213,5 +218,6 @@ def test_phc_index_valid_but_device_missing( self.assertFalse(is_ptp_capable("enp1s0")) + if __name__ == "__main__": - unittest.main() \ No newline at end of file + unittest.main() From d8e23d60a3958066cce3efae7a7c815344e67686 Mon Sep 17 00:00:00 2001 From: Zhongning Li <60045212+tomli380576@users.noreply.github.com> Date: Thu, 16 Jul 2026 15:20:15 +0800 Subject: [PATCH 06/13] fix: missing basicConfig --- checkbox-support/checkbox_support/ethtool/ts_info.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/checkbox-support/checkbox_support/ethtool/ts_info.py b/checkbox-support/checkbox_support/ethtool/ts_info.py index dab287c8d2..3ec052e8d6 100644 --- a/checkbox-support/checkbox_support/ethtool/ts_info.py +++ b/checkbox-support/checkbox_support/ethtool/ts_info.py @@ -31,6 +31,7 @@ import logging from pathlib import Path +logging.basicConfig() logger = logging.getLogger() # these 2 are used to make same ioctl request as ethtool # https://github.com/torvalds/linux/blob/3b029c035b34bbc693405ddf759f0e9b920c27f1/include/uapi/linux/sockios.h#L102 @@ -139,7 +140,7 @@ def is_ptp_capable(interface: str) -> bool: and testing if the corresponding device node exists :param interface: the interface to check - :return: if kernel says the interface supports ptp + :return: if kernel says the interface supports ptp AND The /dev/ptpX device exists """ info = get_ts_info(interface) From 2b8d080d40065a8162bbd9a1460c4af794001dec Mon Sep 17 00:00:00 2001 From: Zhongning Li <60045212+tomli380576@users.noreply.github.com> Date: Thu, 16 Jul 2026 15:39:05 +0800 Subject: [PATCH 07/13] fix: 3.5 --- .../checkbox_support/ethtool/tests/test_ts_info.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/checkbox-support/checkbox_support/ethtool/tests/test_ts_info.py b/checkbox-support/checkbox_support/ethtool/tests/test_ts_info.py index 78b4dae0fd..ce7074c73b 100644 --- a/checkbox-support/checkbox_support/ethtool/tests/test_ts_info.py +++ b/checkbox-support/checkbox_support/ethtool/tests/test_ts_info.py @@ -30,7 +30,7 @@ from pathlib import Path from unittest import TestCase import unittest -from unittest.mock import patch +from unittest.mock import MagicMock, patch from checkbox_support.ethtool.ts_info import ( ETHTOOL_GET_TS_INFO, @@ -165,13 +165,15 @@ def _fake_ioctl(self, fd, request, arg, mutate_flag=False): @patch("checkbox_support.ethtool.ts_info._is_ethernet_interface") @patch("checkbox_support.ethtool.ts_info.fcntl.ioctl") - def test_get_ts_info_populates_struct(self, ioctl_mock, is_ethernet_mock): + def test_get_ts_info_populates_struct( + self, ioctl_mock: MagicMock, is_ethernet_mock: MagicMock + ): is_ethernet_mock.return_value = True ioctl_mock.side_effect = self._fake_ioctl info = get_ts_info("enp1s0") - ioctl_mock.assert_called_once() + self.assertGreaterEqual(len(ioctl_mock.call_args_list), 1) self.assertEqual(info.cmd, ETHTOOL_GET_TS_INFO) self.assertEqual(info.phc_index, 3) self.assertEqual(info.so_timestamping, 0x1F) From 32c3bf165efea60bc01f02315f25a1b4a75a515b Mon Sep 17 00:00:00 2001 From: Zhongning Li <60045212+tomli380576@users.noreply.github.com> Date: Thu, 16 Jul 2026 16:46:37 +0800 Subject: [PATCH 08/13] fix: add padding to avoid mem corruption --- .../checkbox_support/ethtool/ts_info.py | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/checkbox-support/checkbox_support/ethtool/ts_info.py b/checkbox-support/checkbox_support/ethtool/ts_info.py index 3ec052e8d6..21e392ef7a 100644 --- a/checkbox-support/checkbox_support/ethtool/ts_info.py +++ b/checkbox-support/checkbox_support/ethtool/ts_info.py @@ -77,6 +77,23 @@ class ifreq(ctypes.Structure): _fields_ = [ ("ifr_name", ctypes.c_char * IFNAMSIZ), ("ifr_data", ctypes.c_void_p), + # https://github.com/torvalds/linux/blob/3b029c035b34bbc693405ddf759f0e9b920c27f1/include/uapi/linux/if.h#L234-L256 + # the rest of the ifreq struct is a 24 byte union + # because the largest member is the ifmap struct + # which has (long*2 + shot + char*3) + # therefore this padding should be 16 bytes + ( + "_ifr_padding", + ctypes.c_byte + * ( + ctypes.sizeof(ctypes.c_long) * 2 + + ctypes.sizeof(ctypes.c_short) + + ctypes.sizeof(ctypes.c_char) * 3 + + 3 # in the ifmap struct, it says "3 bytes to spare" + # make sure to account for ifr_data + - ctypes.sizeof(ctypes.c_void_p) + ), + ), ] @@ -148,3 +165,7 @@ def is_ptp_capable(interface: str) -> bool: expected_ptp_device_path = "/dev/ptp{}".format(phc_index) return phc_index >= 0 and os.path.exists(expected_ptp_device_path) + + +if __name__ == "__main__": + print(get_ts_info("enx5c925ed71416")) From 9c258a3b32d951414c2d9a3b4e6eb63653a3a2bc Mon Sep 17 00:00:00 2001 From: Zhongning Li <60045212+tomli380576@users.noreply.github.com> Date: Thu, 16 Jul 2026 16:50:25 +0800 Subject: [PATCH 09/13] fix: copilot suggestions --- .../checkbox_support/ethtool/ts_info.py | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/checkbox-support/checkbox_support/ethtool/ts_info.py b/checkbox-support/checkbox_support/ethtool/ts_info.py index 21e392ef7a..48a626c8f7 100644 --- a/checkbox-support/checkbox_support/ethtool/ts_info.py +++ b/checkbox-support/checkbox_support/ethtool/ts_info.py @@ -32,7 +32,7 @@ from pathlib import Path logging.basicConfig() -logger = logging.getLogger() +logger = logging.getLogger(__name__) # these 2 are used to make same ioctl request as ethtool # https://github.com/torvalds/linux/blob/3b029c035b34bbc693405ddf759f0e9b920c27f1/include/uapi/linux/sockios.h#L102 SIOCETHTOOL = 0x8946 @@ -60,14 +60,14 @@ class ethtool_ts_info(ctypes.Structure): ] def __str__(self) -> str: - lines = [] # type: list[str] + words = [] # type: list[str] for field in self._fields_: name = field[0] value = getattr(self, name) if isinstance(value, ctypes.Array): value = list(value) - lines.append("{}={}".format(name, value)) - return "ethtool_ts_info({})".format(", ".join(lines)) + words.append("{}={}".format(name, value)) + return "ethtool_ts_info({})".format(", ".join(words)) # this is only used to make the SIOCETHTOOL ioctl request @@ -119,7 +119,7 @@ def _is_ethernet_interface(interface: str) -> bool: return False return True - except Exception: + except OSError: # assume false if failed to read any of the paths above return False @@ -160,11 +160,14 @@ def is_ptp_capable(interface: str) -> bool: :return: if kernel says the interface supports ptp AND The /dev/ptpX device exists """ - info = get_ts_info(interface) - phc_index = int(info.phc_index) - expected_ptp_device_path = "/dev/ptp{}".format(phc_index) + try: + info = get_ts_info(interface) + phc_index = int(info.phc_index) + expected_ptp_device_path = "/dev/ptp{}".format(phc_index) - return phc_index >= 0 and os.path.exists(expected_ptp_device_path) + return phc_index >= 0 and os.path.exists(expected_ptp_device_path) + except (OSError, ValueError): + return False if __name__ == "__main__": From 5d2ec3ece52e135e445516b3a99eadb277d5e079 Mon Sep 17 00:00:00 2001 From: Zhongning Li <60045212+tomli380576@users.noreply.github.com> Date: Thu, 16 Jul 2026 16:57:47 +0800 Subject: [PATCH 10/13] fix: copilot suggestions --- checkbox-support/checkbox_support/ethtool/ts_info.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/checkbox-support/checkbox_support/ethtool/ts_info.py b/checkbox-support/checkbox_support/ethtool/ts_info.py index 48a626c8f7..8a6d578577 100644 --- a/checkbox-support/checkbox_support/ethtool/ts_info.py +++ b/checkbox-support/checkbox_support/ethtool/ts_info.py @@ -31,7 +31,6 @@ import logging from pathlib import Path -logging.basicConfig() logger = logging.getLogger(__name__) # these 2 are used to make same ioctl request as ethtool # https://github.com/torvalds/linux/blob/3b029c035b34bbc693405ddf759f0e9b920c27f1/include/uapi/linux/sockios.h#L102 @@ -80,7 +79,7 @@ class ifreq(ctypes.Structure): # https://github.com/torvalds/linux/blob/3b029c035b34bbc693405ddf759f0e9b920c27f1/include/uapi/linux/if.h#L234-L256 # the rest of the ifreq struct is a 24 byte union # because the largest member is the ifmap struct - # which has (long*2 + shot + char*3) + # which has (long*2 + short + char*3) # therefore this padding should be 16 bytes ( "_ifr_padding", From 200a2963331841a0d2b5bbc83fc52918bf4d3df1 Mon Sep 17 00:00:00 2001 From: Zhongning Li <60045212+tomli380576@users.noreply.github.com> Date: Thu, 16 Jul 2026 17:14:00 +0800 Subject: [PATCH 11/13] fix: excessive copilot comments --- .../checkbox_support/ethtool/tests/test_ts_info.py | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/checkbox-support/checkbox_support/ethtool/tests/test_ts_info.py b/checkbox-support/checkbox_support/ethtool/tests/test_ts_info.py index ce7074c73b..99857ce29e 100644 --- a/checkbox-support/checkbox_support/ethtool/tests/test_ts_info.py +++ b/checkbox-support/checkbox_support/ethtool/tests/test_ts_info.py @@ -17,14 +17,6 @@ # You should have received a copy of the GNU General Public License # along with Checkbox. If not, see . -""" -Unit tests for checkbox_support.ethtool.ts_info - -These tests never touch real hardware or the kernel: every ioctl() call -and every filesystem read under /sys/class/net is mocked so the suite -can run unmodified on a headless CI runner (e.g. GitHub Actions). -""" - import ctypes import tempfile from pathlib import Path @@ -151,10 +143,7 @@ def test_missing_interface_returns_false(self): class TestGetTsInfo(TestCase): def _fake_ioctl(self, fd, request, arg, mutate_flag=False): - """Simulate the kernel writing into the ethtool_ts_info struct - - pointed to by arg.ifr_data, the same way SIOCETHTOOL would. - """ + # mock kernel response to the SIOCETHTOOL ioctl self.assertEqual(request, SIOCETHTOOL) info_ptr = ctypes.cast(arg.ifr_data, ctypes.POINTER(ethtool_ts_info)) info_ptr.contents.so_timestamping = 0x1F From f6afe28fa15a2ed228f606fc43e7014f72096d25 Mon Sep 17 00:00:00 2001 From: Zhongning Li <60045212+tomli380576@users.noreply.github.com> Date: Fri, 17 Jul 2026 11:04:50 +0800 Subject: [PATCH 12/13] fix: use a less brittle ifr_ifru impl --- .../ethtool/tests/test_ts_info.py | 4 +- .../checkbox_support/ethtool/ts_info.py | 66 +++++++++++-------- 2 files changed, 43 insertions(+), 27 deletions(-) diff --git a/checkbox-support/checkbox_support/ethtool/tests/test_ts_info.py b/checkbox-support/checkbox_support/ethtool/tests/test_ts_info.py index 99857ce29e..28a2079c38 100644 --- a/checkbox-support/checkbox_support/ethtool/tests/test_ts_info.py +++ b/checkbox-support/checkbox_support/ethtool/tests/test_ts_info.py @@ -145,7 +145,9 @@ class TestGetTsInfo(TestCase): def _fake_ioctl(self, fd, request, arg, mutate_flag=False): # mock kernel response to the SIOCETHTOOL ioctl self.assertEqual(request, SIOCETHTOOL) - info_ptr = ctypes.cast(arg.ifr_data, ctypes.POINTER(ethtool_ts_info)) + info_ptr = ctypes.cast( + arg.ifr_ifru.ifr_data, ctypes.POINTER(ethtool_ts_info) + ) info_ptr.contents.so_timestamping = 0x1F info_ptr.contents.phc_index = 3 info_ptr.contents.tx_types = 7 diff --git a/checkbox-support/checkbox_support/ethtool/ts_info.py b/checkbox-support/checkbox_support/ethtool/ts_info.py index 8a6d578577..f766469662 100644 --- a/checkbox-support/checkbox_support/ethtool/ts_info.py +++ b/checkbox-support/checkbox_support/ethtool/ts_info.py @@ -69,33 +69,51 @@ def __str__(self) -> str: return "ethtool_ts_info({})".format(", ".join(words)) -# this is only used to make the SIOCETHTOOL ioctl request -# we don't need any of the field values in this struct -# https://github.com/torvalds/linux/blob/3b029c035b34bbc693405ddf759f0e9b920c27f1/include/uapi/linux/if.h#L234-L256 +# ------ these structs are only used to make the ioctl request +class sockaddr(ctypes.Structure): + _fields_ = [ + # unsigned short int + ("sa_family", ctypes.c_ushort), + # char sa_data[14]; + # note that ipv6 doesn't use this + ("sa_data", ctypes.c_char * 14), + ] + + +class ifmap(ctypes.Structure): + _fields_ = [ + ("mem_start", ctypes.c_ulong), + ("mem_end", ctypes.c_ulong), + ("base_addr", ctypes.c_ushort), + ("irq", ctypes.c_ubyte), # ubyte is unsigned char + ("dma", ctypes.c_ubyte), + ("port", ctypes.c_ubyte), + ] + + +class ifr_ifru(ctypes.Union): + _fields_ = [ + # there are more union members in the source code + # we are not really using them here so they are omitted + ("ifr_data", ctypes.c_void_p), + # ifmap is included for the padding + # because it's the largest union member + # it has to be here to avoid this field going out of bounds + ("ifr_map", ifmap), + ] + + +# https://github.com/torvalds/linux/blob/fce2dfa773ced15f27dd27cd0b482a7473cdcf2a/include/uapi/linux/if.h#L234-L256 class ifreq(ctypes.Structure): _fields_ = [ ("ifr_name", ctypes.c_char * IFNAMSIZ), - ("ifr_data", ctypes.c_void_p), - # https://github.com/torvalds/linux/blob/3b029c035b34bbc693405ddf759f0e9b920c27f1/include/uapi/linux/if.h#L234-L256 - # the rest of the ifreq struct is a 24 byte union - # because the largest member is the ifmap struct - # which has (long*2 + short + char*3) - # therefore this padding should be 16 bytes - ( - "_ifr_padding", - ctypes.c_byte - * ( - ctypes.sizeof(ctypes.c_long) * 2 - + ctypes.sizeof(ctypes.c_short) - + ctypes.sizeof(ctypes.c_char) * 3 - + 3 # in the ifmap struct, it says "3 bytes to spare" - # make sure to account for ifr_data - - ctypes.sizeof(ctypes.c_void_p) - ), - ), + ("ifr_ifru", ifr_ifru), ] +# ------------------------------------------------ + + def _is_ethernet_interface(interface: str) -> bool: """Check whether the given interface is a physical ethernet device @@ -141,7 +159,7 @@ def get_ts_info(interface: str) -> ethtool_ts_info: ifr = ifreq() ifr.ifr_name = interface.encode() - ifr.ifr_data = ctypes.cast(ctypes.pointer(info), ctypes.c_void_p) + ifr.ifr_ifru.ifr_data = ctypes.cast(ctypes.pointer(info), ctypes.c_void_p) with socket.socket(socket.AF_INET, socket.SOCK_DGRAM) as sock: fcntl.ioctl(sock.fileno(), SIOCETHTOOL, ifr, True) @@ -167,7 +185,3 @@ def is_ptp_capable(interface: str) -> bool: return phc_index >= 0 and os.path.exists(expected_ptp_device_path) except (OSError, ValueError): return False - - -if __name__ == "__main__": - print(get_ts_info("enx5c925ed71416")) From ab06bb92e31a48f40c0d4df31e17d183431ca776 Mon Sep 17 00:00:00 2001 From: Zhongning Li <60045212+tomli380576@users.noreply.github.com> Date: Fri, 17 Jul 2026 11:08:40 +0800 Subject: [PATCH 13/13] fix: unused sockaddr --- checkbox-support/checkbox_support/ethtool/ts_info.py | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/checkbox-support/checkbox_support/ethtool/ts_info.py b/checkbox-support/checkbox_support/ethtool/ts_info.py index f766469662..f2b5040164 100644 --- a/checkbox-support/checkbox_support/ethtool/ts_info.py +++ b/checkbox-support/checkbox_support/ethtool/ts_info.py @@ -70,16 +70,6 @@ def __str__(self) -> str: # ------ these structs are only used to make the ioctl request -class sockaddr(ctypes.Structure): - _fields_ = [ - # unsigned short int - ("sa_family", ctypes.c_ushort), - # char sa_data[14]; - # note that ipv6 doesn't use this - ("sa_data", ctypes.c_char * 14), - ] - - class ifmap(ctypes.Structure): _fields_ = [ ("mem_start", ctypes.c_ulong),