ionic: Add PTP Hardware Clock (PHC) timestamping support#1724
ionic: Add PTP Hardware Clock (PHC) timestamping support#1724abhijitG-xlnx wants to merge 4 commits into
Conversation
To commit: ("net: ionic: Add PHC state page for user space access").
Signed-off-by: Abhijit Gangurde <abhijit.gangurde@amd.com>
map the PHC page to allow applications to access hardware timestamp information. Signed-off-by: Allen Hubbe <allen.hubbe@amd.com> Signed-off-by: Abhijit Gangurde <abhijit.gangurde@amd.com>
Support IBV_WC_EX_WITH_COMPLETION_TIMESTAMP and IBV_WC_EX_WITH_COMPLETION_TIMESTAMP_WALLCLOCK. Modify the polling path to extract timestamps from the CQE. For Send Queue completions, cache the timestamp in the request metadata (`sq_meta`) which allows the driver to propagate the correct hardware timestamp to all Work Completions covered by a coalesced (MSN) completion event. Update `start_poll` to process one completion at a time and store the current timestamp in the `vcq` context. This ensures that the `read_completion_ts` and `read_completion_wallclock_ns` accessors return the correct value for the specific Work Completion currently being inspected. Signed-off-by: Allen Hubbe <allen.hubbe@amd.com> Signed-off-by: Abhijit Gangurde <abhijit.gangurde@amd.com>
Add a test for completion time stamp. Validate the wall clock time by opening the PTP hardware clock device node to read the current time and compare it to the wall clock completion time stamps. Expect that completion time stamps are within the past one second of the current PHC time at the end of the test. Signed-off-by: Allen Hubbe <allen.hubbe@amd.com> Signed-off-by: Abhijit Gangurde <abhijit.gangurde@amd.com>
ShacharKagan
left a comment
ShacharKagan
left a comment
There was a problem hiding this comment.
Reviewing only the tests commit (tests: test PHC completion time stamp). The three verbs/provider commits are being reviewed separately.
Since this new test file is based on tests/test_mlx5_timestamp.py but drops some of its scenarios, a few leftovers can be cleaned up. I'll add per-line comments with the specifics.
| import os | ||
| import errno | ||
|
|
||
| from pyverbs.libibverbs_enums import IBV_WC_EX_WITH_COMPLETION_TIMESTAMP as FREE_RUNNING, \ |
There was a problem hiding this comment.
I'd skip the as FREE_RUNNING / as REAL_TIME renames. Using the original enum names inline makes it clearer which ibverbs flag is being exercised, without having to look back at the imports.
|
|
||
| from pyverbs.libibverbs_enums import IBV_WC_EX_WITH_COMPLETION_TIMESTAMP as FREE_RUNNING, \ | ||
| IBV_WC_EX_WITH_COMPLETION_TIMESTAMP_WALLCLOCK as REAL_TIME | ||
| from tests.base import RCResources, RDMATestCase, PyverbsAPITestCase |
There was a problem hiding this comment.
PyverbsAPITestCase not in use
| from tests.base import RCResources, RDMATestCase, PyverbsAPITestCase | ||
| from pyverbs.pyverbs_error import PyverbsRDMAError | ||
| from pyverbs.cq import CqInitAttrEx, CQEX | ||
| from tests.test_flow import FlowRes |
| self.open_phc_dev(dev_name) | ||
| super().__init__(dev_name=dev_name, ib_port=ib_port, gid_index=gid_index) | ||
|
|
||
| def __del__(self): |
There was a problem hiding this comment.
The cleanup logic here needs to move out of __del__ and into an explicit close() (or close_files()) called from tearDown. del is GC-timed and swallows exceptions silently, so the /dev/ptpN fd can leak across tests; explicit tearDown makes cleanup deterministic and aligned with the rest of tests/.
| try: | ||
| phc_name = os.listdir(f'/sys/class/infiniband/{dev_name}/device/ptp')[0] | ||
| self.phc_file = open(f'/dev/{phc_name}', 'rb') | ||
| self.phc_clkid = (~self.phc_file.fileno() << 3) | 3 |
There was a problem hiding this comment.
Move the magic numbers to meaningful parameters. suggestion: CLOCKID_TYPE_BITS, CLOCKFD.
| phc_name = os.listdir(f'/sys/class/infiniband/{dev_name}/device/ptp')[0] | ||
| self.phc_file = open(f'/dev/{phc_name}', 'rb') | ||
| self.phc_clkid = (~self.phc_file.fileno() << 3) | 3 | ||
| except: |
There was a problem hiding this comment.
Please narrow this 'except:'. It currently swallows KeyboardInterrupt/SystemExit and reports the same skip for both "no PHC" and real open failures.
| """ | ||
| s_recv_wr = u.get_recv_wr(self.server) | ||
| u.post_recv(self.server, s_recv_wr) | ||
| if self.qp_type == e.IBV_QPT_RAW_PACKET: |
There was a problem hiding this comment.
None of the tests in this file set qp_type = IBV_QPT_RAW_PACKET, so it's unreachable. The if/else can collapse to a single call to get_send_elements.
| if ret != 0: | ||
| raise PyverbsRDMAError('Failed to poll CQEX') | ||
| if cqex.status != e.IBV_WC_SUCCESS: | ||
| raise PyverbsRDMAError('Completion status is {cqex.status}') |
There was a problem hiding this comment.
Missing f prefix. This currently raises with the literal text Completion status is {cqex.status} instead of the actual status, which makes failures undebuggable.
|
|
||
| def test_timestamp_free_running_rc_traffic(self): | ||
| """ | ||
| Test free running timestamp on RC traffic. |
There was a problem hiding this comment.
Please expand: free-running returns raw HW PHC ticks, so only polling success is checked here.
Suggestion:
Test free-running timestamp on RC traffic.
The timestamp is returned as a raw HW PHC tick value, so this test
only verifies that polling succeeds; value validation is done by the
real-time variant.
|
|
||
| def test_timestamp_real_time_rc_traffic(self): | ||
| """ | ||
| Test real time timestamp on RC traffic. |
There was a problem hiding this comment.
Please expand: real-time returns ns and is validated against the live PHC from /dev/ptpN.
Suggestion:
Test real-time timestamp on RC traffic.
The timestamp is returned in nanoseconds and validated against the
live PHC read from /dev/ptpN.
4 participants
This series adds PTP Hardware Clock (PHC) support to the ionic RDMA provider,
enabling applications to access hardware timestamps for send and receive
completions.
Key features:
IBV_WC_EX_WITH_COMPLETION_TIMESTAMP_WALLCLOCK work completion flags
to work completions
coalesced completions