From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Mon, 11 Sep 2023 15:02:49 +0000 (+0100) Subject: Move to smi version 10, read-only interface, no mutex, write and read each record... X-Git-Tag: 4.3~1^2~6 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=dabfee5bac93e599cb2e66268bb6c9a91c048eec;p=thirdparty%2Fshairport-sync.git Move to smi version 10, read-only interface, no mutex, write and read each record twice to ensure it is not inconsistent when read. --- diff --git a/nqptp-shm-structures.h b/nqptp-shm-structures.h index 251d06c1..09f3b600 100644 --- a/nqptp-shm-structures.h +++ b/nqptp-shm-structures.h @@ -22,7 +22,7 @@ #define NQPTP_INTERFACE_NAME "/nqptp" -#define NQPTP_SHM_STRUCTURES_VERSION 9 +#define NQPTP_SHM_STRUCTURES_VERSION 10 #define NQPTP_CONTROL_PORT 9000 // The control port expects a UDP packet with the first character being a command letter @@ -50,20 +50,26 @@ // When the clock is inactive, it can stop running. This causes the offset to decrease. // NQPTP clock smoothing would treat this as a network delay, causing true sync to be lost. // To avoid this, when the clock goes from inactive to active, -// NQPTP resets clock smoothing to the new offset. - +// NQPTP resets clock smoothing to the new offset. #include #include -struct shm_structure { - pthread_mutex_t shm_mutex; // for safely accessing the structure - uint16_t version; // check this is equal to NQPTP_SHM_STRUCTURES_VERSION +typedef struct { uint64_t master_clock_id; // the current master clock - char master_clock_ip[64]; // where it's coming from uint64_t local_time; // the time when the offset was calculated uint64_t local_to_master_time_offset; // add this to the local time to get master clock time uint64_t master_clock_start_time; // this is when the master clock became master +} shm_structure_set; + +// The secondary record must be written strictly after all writes to the main record are +// complete. +// This is ensured using the __sync_synchronize() construct. +// The reader should ensure that both copies match for a read to be valid. +struct shm_structure { + uint16_t version; // check this is equal to NQPTP_SHM_STRUCTURES_VERSION + shm_structure_set main; + shm_structure_set secondary; }; #endif diff --git a/ptp-utilities.c b/ptp-utilities.c index 559b62ea..34184c14 100644 --- a/ptp-utilities.c +++ b/ptp-utilities.c @@ -1,6 +1,6 @@ /* * This file is part of Shairport Sync. - * Copyright (c) Mike Brady 2020 -- 2021 + * Copyright (c) Mike Brady 2020 -- 2023 * All rights reserved. * * Permission is hereby granted, free of charge, to any person @@ -49,23 +49,70 @@ void *mapped_addr = NULL; // returns a copy of the shared memory data from the nqptp // shared memory interface, so long as it's open. int get_nqptp_data(struct shm_structure *nqptp_data) { + // uint64_t tn = get_absolute_time_in_ns(); // if interested in timing the function... + struct shm_structure local_nqptp_data; int response = -1; // presume the worst. Fix it on success - // the first part of the shared memory is a mutex lock, so use it to get - // exclusive access while copying + + // We need to ensure that when we read the record, we are not reading it while it is partly + // updated and therefore inconsistent. To achieve this, we do the following: + + // We ensure that the secondary record is written by NQPTP _strictly after_ + // all writes to the main record are complete. + + // Here we read two copies of the entire record, the second + // _strictly after_ all reads from the first are complete. + + // (Strict write and read ordering is ensured using the __sync_synchronize() construct.) + + // We then compare the main record in the first read to the + // secondary record in the second read. + + // If they are equal, we can be sure we have not read a record that has been + // made inconsistent by an interrupted update. if ((mapped_addr != MAP_FAILED) && (mapped_addr != NULL)) { - pthread_cleanup_debug_mutex_lock((pthread_mutex_t *)mapped_addr, 10000, 1); - memcpy(nqptp_data, (char *)mapped_addr, sizeof(struct shm_structure)); - pthread_cleanup_pop(1); // release the mutex - response = 0; + int loop_count = 1; + do { + __sync_synchronize(); + memcpy(nqptp_data, (char *)mapped_addr, sizeof(struct shm_structure)); + __sync_synchronize(); + // read again strictly after a full read -- this is to read the secondary strictly after the + // primary + memcpy(&local_nqptp_data, (char *)mapped_addr, sizeof(struct shm_structure)); + // check that the main and secondary data sets match + if (memcmp(&nqptp_data->main, &local_nqptp_data.secondary, sizeof(shm_structure_set)) != 0) { + usleep(2); // microseconds + loop_count++; + } + } while ( + (memcmp(&nqptp_data->main, &local_nqptp_data.secondary, sizeof(shm_structure_set)) != 0) && + (loop_count < 100)); + if (loop_count == 10) { + debug(1, "get_nqptp_data -- main and secondary records don't match after %d attempts!", + loop_count); + response = -1; + } else { + response = 0; + } } else { if (mapped_addr == NULL) - debug(1, "get_nqptp_data because the mapped_addr is NULL"); + debug(1, "get_nqptp_data failed because the mapped_addr is NULL"); else if (mapped_addr == MAP_FAILED) - debug(1, "get_nqptp_data because the mapped_addr is MAP_FAILED"); + debug(1, "get_nqptp_data failed because the mapped_addr is MAP_FAILED"); else debug(1, "get_nqptp_data failed"); } + // int64_t et = get_absolute_time_in_ns() - tn; + // debug(1, "get_nqptp_data time: %.3f microseconds.", 0.001 * et); + return response; +} + +int ptp_get_clock_version() { + int response = 0; // no version number information available + struct shm_structure nqptp_data; + if (get_nqptp_data(&nqptp_data) == 0) { + response = nqptp_data.version; + } return response; } @@ -85,15 +132,15 @@ int ptp_get_clock_info(uint64_t *actual_clock_id, uint64_t *time_of_sample, uint if (get_nqptp_data(&nqptp_data) == 0) { if (nqptp_data.version == NQPTP_SHM_STRUCTURES_VERSION) { // assuming a clock id can not be zero - if (nqptp_data.master_clock_id != 0) { + if (nqptp_data.main.master_clock_id != 0) { if (actual_clock_id != NULL) - *actual_clock_id = nqptp_data.master_clock_id; + *actual_clock_id = nqptp_data.main.master_clock_id; if (time_of_sample != NULL) - *time_of_sample = nqptp_data.local_time; + *time_of_sample = nqptp_data.main.local_time; if (raw_offset != NULL) - *raw_offset = nqptp_data.local_to_master_time_offset; + *raw_offset = nqptp_data.main.local_to_master_time_offset; if (mastership_start_time != NULL) - *mastership_start_time = nqptp_data.master_clock_start_time; + *mastership_start_time = nqptp_data.main.master_clock_start_time; } else { response = clock_no_master; } @@ -124,12 +171,12 @@ int ptp_shm_interface_open() { if (strcmp(config.nqptp_shared_memory_interface_name, "") != 0) { response = 0; int shared_memory_file_descriptor = - shm_open(config.nqptp_shared_memory_interface_name, O_RDWR, 0); + shm_open(config.nqptp_shared_memory_interface_name, O_RDONLY, 0); if (shared_memory_file_descriptor >= 0) { mapped_addr = // needs to be PROT_READ | PROT_WRITE to allow the mapped memory to be writable for the // mutex to lock and unlock - mmap(NULL, sizeof(struct shm_structure), PROT_READ | PROT_WRITE, MAP_SHARED, + mmap(NULL, sizeof(struct shm_structure), PROT_READ, MAP_SHARED, shared_memory_file_descriptor, 0); if (mapped_addr == MAP_FAILED) { response = -1; diff --git a/ptp-utilities.h b/ptp-utilities.h index 761a5cae..738ed21f 100644 --- a/ptp-utilities.h +++ b/ptp-utilities.h @@ -1,6 +1,6 @@ /* * This file is part of Shairport Sync. - * Copyright (c) Mike Brady 2020 -- 2021 + * Copyright (c) Mike Brady 2020 -- 2023 * All rights reserved. * * Permission is hereby granted, free of charge, to any person @@ -37,6 +37,7 @@ void ptp_send_control_message_string(const char *msg); void ptp_shm_interface_init(); int ptp_shm_interface_open(); +int ptp_get_clock_version(); int ptp_shm_interface_close(); #endif /* __PTP_UTILITIES_H */ diff --git a/shairport.c b/shairport.c index 6d9fb959..66d9028a 100644 --- a/shairport.c +++ b/shairport.c @@ -61,8 +61,8 @@ #endif #ifdef CONFIG_OPENSSL -#include #include +#include #endif #if defined(CONFIG_DBUS_INTERFACE) @@ -1416,7 +1416,7 @@ int parse_options(int argc, char **argv) { for (i = 5; i >= 0; i--) { // In AirPlay 2 mode, the AP1 name prefix must be // the same as the AirPlay 2 device id less the colons. - config.ap1_prefix[i] = temporary_airplay_id & 0xFF; + config.ap1_prefix[i] = temporary_airplay_id & 0xFF; apids[i * 3 + 1] = hexchar[temporary_airplay_id & 0xF]; temporary_airplay_id = temporary_airplay_id >> 4; apids[i * 3] = hexchar[temporary_airplay_id & 0xF]; @@ -1630,7 +1630,6 @@ void exit_function() { } #endif - #ifdef CONFIG_METADATA_HUB debug(2, "Stopping metadata hub"); metadata_hub_stop(); @@ -2521,7 +2520,6 @@ int main(int argc, char **argv) { soxr_time_check_thread_started = 1; #endif - // In AirPlay 2 mode, the AP1 prefix is the same as the device ID less the colons // In AirPlay 1 mode, the AP1 prefix is calculated by hashing the service name. #ifndef CONFIG_AIRPLAY_2 @@ -2600,25 +2598,51 @@ int main(int argc, char **argv) { #endif #ifdef CONFIG_AIRPLAY_2 - ptp_send_control_message_string("T"); // get nqptp to create the named shm interface - int ptp_check_times = 0; - const int ptp_wait_interval_us = 5000; - // wait for up to ten seconds for NQPTP to come online + ptp_send_control_message_string( + "T"); // send this message to get nqptp to create the named shm interface + uint64_t nqptp_start_waiting_time = get_absolute_time_in_ns(); + int continue_waiting = 0; + int response = 0; + int64_t time_spent_waiting = 0; do { - ptp_send_control_message_string("T"); // get nqptp to create the named shm interface - usleep(ptp_wait_interval_us); - ptp_check_times++; - } while ((ptp_shm_interface_open() != 0) && - (ptp_check_times < (10000000 / ptp_wait_interval_us))); - - if (ptp_shm_interface_open() != 0) { - die("Can't access NQPTP! Is it installed and running?"); - } else { - if (ptp_check_times == 1) - debug(1, "NQPTP is online."); - else - debug(1, "NQPTP is online after %u microseconds.", ptp_check_times * ptp_wait_interval_us); + continue_waiting = 0; + response = ptp_shm_interface_open(); + if ((response == -1) && (errno == ENOENT)) { + time_spent_waiting = get_absolute_time_in_ns() - nqptp_start_waiting_time; + if (time_spent_waiting < 10000000000L) { + continue_waiting = 1; + usleep(50000); + } + } + } while (continue_waiting != 0); + + if ((response == -1) && (errno == ENOENT)) { + die("Shairport Sync can not find the nqptp service on this system. Is nqptp installed and " + "running?"); + } else if ((response == -1) && (errno == EACCES)) { + die("Shairport Sync must have read access to the nqptp shared memory file in /dev/shm/."); + } else if (response != 0) { + die("an error occurred accessing the nqptp service."); + } + + int ptp_clock_version = ptp_get_clock_version(); + if (ptp_clock_version == 0) { + die("The nqptp service on this system, which is required for Shairport Sync to operate, does " + "not seem to be initialised."); + } else if (ptp_clock_version < NQPTP_SHM_STRUCTURES_VERSION) { + die("The nqptp service (SMI Version %d) on this system is too old for this version of " + "Shairport Sync, which requires SMI Version %d. Please update.", + ptp_clock_version, NQPTP_SHM_STRUCTURES_VERSION); + } else if (ptp_clock_version > NQPTP_SHM_STRUCTURES_VERSION) { + die("This version of Shairport Sync (SMI Version %d) is too old for the version of nqptp (SMI " + "Version %d) on this system. Please update.", + NQPTP_SHM_STRUCTURES_VERSION, ptp_clock_version); } + + if (time_spent_waiting == 0) + debug(1, "NQPTP is online."); + else + debug(1, "NQPTP came online after %.3f milliseconds.", 0.000001 * time_spent_waiting); #endif #ifdef CONFIG_METADATA