From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Mon, 11 Sep 2023 15:49:34 +0000 (+0100) Subject: Move to smi version 10, read-only interface, no mutex, write and read each record... X-Git-Tag: 1.2.4~6 X-Git-Url: http://git.ipfire.org/gitweb/gitweb.cgi?a=commitdiff_plain;h=10969eaa0ba71287efd46252910701809b50fb36;p=thirdparty%2Fnqptp.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/Makefile.am b/Makefile.am index 8a497e6..78f36d7 100644 --- a/Makefile.am +++ b/Makefile.am @@ -19,14 +19,20 @@ endif install-exec-hook: if BUILD_FOR_LINUX +# NQPTP runs as user/group nqptp/nqptp on Linux and uses setcap to access ports 319 and 320 + setcap 'cap_net_bind_service=+ep' $(bindir)/nqptp +# no installer for System V if INSTALL_SYSTEMD_STARTUP + getent group nqptp &>/dev/null || groupadd -r nqptp &>/dev/null + getent passwd nqptp &> /dev/null || useradd -r -M -g nqptp -s /usr/sbin/nologin nqptp &>/dev/null [ -e $(DESTDIR)$(libdir)/systemd/system ] || mkdir -p $(DESTDIR)$(libdir)/systemd/system # don't replace a service file if it already exists... [ -e $(DESTDIR)$(libdir)/systemd/system/nqptp.service ] || cp nqptp.service $(DESTDIR)$(libdir)/systemd/system endif endif - # no installer for FreeBSD yet + if BUILD_FOR_FREEBSD +# NQPTP runs as root on FreeBSD to access ports 319 and 320 if INSTALL_FREEBSD_STARTUP cp nqptp.freebsd /usr/local/etc/rc.d/nqptp chmod 555 /usr/local/etc/rc.d/nqptp diff --git a/nqptp-clock-sources.c b/nqptp-clock-sources.c index 04181b6..d575b99 100644 --- a/nqptp-clock-sources.c +++ b/nqptp-clock-sources.c @@ -80,8 +80,6 @@ int get_client_id(char *client_shared_memory_interface_name) { i++; } if (response != -1) { - pthread_mutexattr_t shared; - int err; strncpy(clients[i].shm_interface_name, client_shared_memory_interface_name, sizeof(clients[i].shm_interface_name)); // create the named smi interface @@ -125,23 +123,6 @@ int get_client_id(char *client_shared_memory_interface_name) { memset(clients[i].shared_memory, 0, sizeof(struct shm_structure)); clients[i].shared_memory->version = NQPTP_SHM_STRUCTURES_VERSION; - /*create mutex attr */ - err = pthread_mutexattr_init(&shared); - if (err != 0) { - die("mutex attribute initialization failed - %s.", strerror(errno)); - } - pthread_mutexattr_setpshared(&shared, 1); - /*create a mutex */ - err = pthread_mutex_init((pthread_mutex_t *)&clients[i].shared_memory->shm_mutex, &shared); - if (err != 0) { - die("mutex initialization failed - %s.", strerror(errno)); - } - - err = pthread_mutexattr_destroy(&shared); - if (err != 0) { - die("mutex attribute destruction failed - %s.", strerror(errno)); - } - // for (i = 0; i < MAX_CLOCKS; i++) { // clocks_private[i].client_flags[response] = // 0; // turn off all client flags in every clock for this client @@ -248,26 +229,21 @@ int create_clock_source_record(char *sender_string, void update_master_clock_info(uint64_t master_clock_id, const char *ip, uint64_t local_time, uint64_t local_to_master_offset, uint64_t mastership_start_time) { - // debug(1,"update_master_clock_info clock: % " PRIx64 ", offset: %" PRIx64 ".", - // master_clock_id, local_to_master_offset); - int rc = pthread_mutex_lock(&shared_memory->shm_mutex); - if (rc != 0) - warn("Can't acquire mutex to update master clock!"); - shared_memory->master_clock_id = master_clock_id; + // to ensure that a full update has taken place, the + // reader must ensure that the main and secondary + // structures are identical + + shared_memory->main.master_clock_id = master_clock_id; if (ip != NULL) { - strncpy((char *)&shared_memory->master_clock_ip, ip, - FIELD_SIZEOF(struct shm_structure, master_clock_ip) - 1); - shared_memory->master_clock_start_time = mastership_start_time; - shared_memory->local_time = local_time; - shared_memory->local_to_master_time_offset = local_to_master_offset; + shared_memory->main.master_clock_start_time = mastership_start_time; + shared_memory->main.local_time = local_time; + shared_memory->main.local_to_master_time_offset = local_to_master_offset; } else { - shared_memory->master_clock_ip[0] = '\0'; - shared_memory->master_clock_start_time = 0; - shared_memory->local_time = 0; - shared_memory->local_to_master_time_offset = 0; + shared_memory->main.master_clock_start_time = 0; + shared_memory->main.local_time = 0; + shared_memory->main.local_to_master_time_offset = 0; } - rc = pthread_mutex_unlock(&shared_memory->shm_mutex); - if (rc != 0) - warn("Can't release mutex after updating master clock!"); - // debug(1,"update_master_clock_info done"); + __sync_synchronize(); + shared_memory->secondary = shared_memory->main; + __sync_synchronize(); } diff --git a/nqptp-shm-structures.h b/nqptp-shm-structures.h index 251d06c..85b2c9d 100644 --- a/nqptp-shm-structures.h +++ b/nqptp-shm-structures.h @@ -1,6 +1,6 @@ /* * This file is part of the nqptp distribution (https://github.com/mikebrady/nqptp). - * Copyright (c) 2021--2022 Mike Brady. + * Copyright (c) 2021--2023 Mike Brady. * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -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,29 @@ // 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 actual interface comprises a shared memory region of type struct shm_structure. +// This comprises two records of type shm_structure_set. +// The secondary record is 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. +// For safety, the secondary record should be read strictly after the first. + +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/nqptp-utilities.c b/nqptp-utilities.c index 0401069..9d6a95d 100644 --- a/nqptp-utilities.c +++ b/nqptp-utilities.c @@ -29,11 +29,11 @@ #endif #ifdef CONFIG_FOR_FREEBSD -#include -#include #include #include #include +#include +#include #endif #include // getaddrinfo etc. diff --git a/nqptp.c b/nqptp.c index ee6efe5..e5f2988 100644 --- a/nqptp.c +++ b/nqptp.c @@ -96,11 +96,13 @@ void goodbye(void) { // close off new smi // mmap cleanup if (munmap(shared_memory, sizeof(struct shm_structure)) != 0) { - debug(1, "error unmapping shared memory \"%s\": \"%s\".", NQPTP_INTERFACE_NAME, strerror(errno)); + debug(1, "error unmapping shared memory \"%s\": \"%s\".", NQPTP_INTERFACE_NAME, + strerror(errno)); } // shm_open cleanup if (shm_unlink(NQPTP_INTERFACE_NAME) == -1) { - debug(1, "error unlinking shared memory \"%s\": \"%s\".", NQPTP_INTERFACE_NAME, strerror(errno)); + debug(1, "error unlinking shared memory \"%s\": \"%s\".", NQPTP_INTERFACE_NAME, + strerror(errno)); } if (shm_fd != -1) @@ -131,8 +133,8 @@ int main(int argc, char **argv) { if (strcmp(argv[i] + 1, "V") == 0) { #ifdef CONFIG_USE_GIT_VERSION_STRING if (git_version_string[0] != '\0') - fprintf(stdout, "Version: %s. Shared Memory Interface Version: smi%u.\n", git_version_string, - NQPTP_SHM_STRUCTURES_VERSION); + fprintf(stdout, "Version: %s. Shared Memory Interface Version: smi%u.\n", + git_version_string, NQPTP_SHM_STRUCTURES_VERSION); else #endif @@ -191,13 +193,10 @@ int main(int argc, char **argv) { // open the SMI - pthread_mutexattr_t shared; - int err; - shm_fd = -1; mode_t oldumask = umask(0); - shm_fd = shm_open(NQPTP_INTERFACE_NAME, O_RDWR | O_CREAT, 0666); + shm_fd = shm_open(NQPTP_INTERFACE_NAME, O_RDWR | O_CREAT, 0644); if (shm_fd == -1) { die("cannot open shared memory \"%s\".", NQPTP_INTERFACE_NAME); } @@ -230,23 +229,6 @@ int main(int argc, char **argv) { memset(shared_memory, 0, sizeof(struct shm_structure)); shared_memory->version = NQPTP_SHM_STRUCTURES_VERSION; - /*create mutex attr */ - err = pthread_mutexattr_init(&shared); - if (err != 0) { - die("mutex attribute initialization failed - %s.", strerror(errno)); - } - pthread_mutexattr_setpshared(&shared, 1); - /*create a mutex */ - err = pthread_mutex_init((pthread_mutex_t *)&shared_memory->shm_mutex, &shared); - if (err != 0) { - die("mutex initialization failed - %s.", strerror(errno)); - } - - err = pthread_mutexattr_destroy(&shared); - if (err != 0) { - die("mutex attribute destruction failed - %s.", strerror(errno)); - } - ssize_t recv_len; char buf[BUFLEN]; diff --git a/nqptp.service.in b/nqptp.service.in index a0f619a..6f1eb0c 100644 --- a/nqptp.service.in +++ b/nqptp.service.in @@ -6,8 +6,8 @@ Before=shairport-sync.service [Service] ExecStart=@prefix@/bin/nqptp -User=root -Group=root +User=nqptp +Group=nqptp [Install] WantedBy=multi-user.target