From 9c50b8a25df8ae6ee8684148d01d83b98d458e56 Mon Sep 17 00:00:00 2001 From: Florian Pose Date: Tue, 14 Jul 2026 11:36:19 +0200 Subject: [PATCH 1/6] Test for re-ordering problem during synchrony check. --- master/fsm_slave_config.c | 2 +- master/master.c | 10 ++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/master/fsm_slave_config.c b/master/fsm_slave_config.c index 47e750f4..993d3d28 100644 --- a/master/fsm_slave_config.c +++ b/master/fsm_slave_config.c @@ -1464,7 +1464,7 @@ void ec_fsm_slave_config_state_dc_sync_check( return; } - if (datagram->working_counter != 1) { + if (smp_load_acquire(&datagram->working_counter) != 1) { slave->error_flag = 1; fsm->state = ec_fsm_slave_config_state_error; EC_SLAVE_ERR(slave, "Failed to check DC synchrony: "); diff --git a/master/master.c b/master/master.c index c0c89ce0..c39df445 100644 --- a/master/master.c +++ b/master/master.c @@ -1,6 +1,6 @@ /***************************************************************************** * - * Copyright (C) 2006-2020 Florian Pose, Ingenieurgemeinschaft IgH + * Copyright (C) 2006-2026 Florian Pose, Ingenieurgemeinschaft IgH * * This file is part of the IgH EtherCAT Master. * @@ -1260,17 +1260,19 @@ void ec_master_receive_datagrams( cur_data += data_size; // set the datagram's working counter - datagram->working_counter = EC_READ_U16(cur_data); + smp_store_release(&datagram->working_counter, EC_READ_U16(cur_data)); cur_data += EC_DATAGRAM_FOOTER_SIZE; - // dequeue the received datagram - datagram->state = EC_DATAGRAM_RECEIVED; + // set the state and receive time + smp_store_release(&datagram->state, EC_DATAGRAM_RECEIVED); #ifdef EC_HAVE_CYCLES datagram->cycles_received = master->devices[EC_DEVICE_MAIN].cycles_poll; #endif datagram->jiffies_received = master->devices[EC_DEVICE_MAIN].jiffies_poll; + + // dequeue the received datagram list_del_init(&datagram->queue); } } From 439787373800e68c58c20330431739274329b974 Mon Sep 17 00:00:00 2001 From: Florian Pose Date: Wed, 15 Jul 2026 12:46:26 +0200 Subject: [PATCH 2/6] General re-odering barrier for datagrams. --- master/fsm_master.c | 32 ++++++++++++++------------------ master/fsm_slave_config.c | 9 ++++----- master/master.c | 22 ++++++++++++---------- 3 files changed, 30 insertions(+), 33 deletions(-) diff --git a/master/fsm_master.c b/master/fsm_master.c index b4f8f2ad..23ef6bdb 100644 --- a/master/fsm_master.c +++ b/master/fsm_master.c @@ -1,6 +1,6 @@ /***************************************************************************** * - * Copyright (C) 2006-2023 Florian Pose, Ingenieurgemeinschaft IgH + * Copyright (C) 2006-2026 Florian Pose, Ingenieurgemeinschaft IgH * * This file is part of the IgH EtherCAT Master. * @@ -174,8 +174,9 @@ int ec_fsm_master_exec( ec_fsm_master_t *fsm /**< Master state machine. */ ) { - if (fsm->datagram->state == EC_DATAGRAM_SENT - || fsm->datagram->state == EC_DATAGRAM_QUEUED) { + ec_datagram_state_t state = smp_load_acquire(&datagram->state); + + if (state == EC_DATAGRAM_SENT || state == EC_DATAGRAM_QUEUED) { // datagram was not sent or received yet. return 0; } @@ -315,8 +316,8 @@ void ec_fsm_master_state_broadcast( for (dev_idx = EC_DEVICE_MAIN; dev_idx < ec_master_num_devices(master); dev_idx++) { fsm->slave_states[dev_idx] = 0x00; - fsm->slaves_responding[dev_idx] = 0; /* Reset to trigger rescan on - next link up. */ + /* Reset to trigger rescan on next link up. */ + fsm->slaves_responding[dev_idx] = 0; } } fsm->link_state[fsm->dev_idx] = master->devices[fsm->dev_idx].link_state; @@ -429,7 +430,6 @@ void ec_fsm_master_state_broadcast( } if (master->slave_count) { - // application applied configurations if (master->config_changed) { master->config_changed = 0; @@ -438,8 +438,8 @@ void ec_fsm_master_state_broadcast( fsm->slave = master->slaves; // begin with first slave ec_fsm_master_enter_write_system_times(fsm); - - } else { + } + else { // fetch state from first slave fsm->slave = master->slaves; ec_datagram_fprd(fsm->datagram, fsm->slave->station_address, @@ -535,14 +535,12 @@ int ec_fsm_master_action_process_int_request( for (slave = master->slaves; slave < master->slaves + master->slave_count; slave++) { - if (!slave->config) { continue; } list_for_each_entry(sdo_req, &slave->config->sdo_requests, list) { if (sdo_req->state == EC_INT_REQUEST_QUEUED) { - if (ec_sdo_request_timed_out(sdo_req)) { sdo_req->state = EC_INT_REQUEST_FAILURE; EC_SLAVE_DBG(slave, 1, "Internal SDO request" @@ -570,7 +568,6 @@ int ec_fsm_master_action_process_int_request( list_for_each_entry(soe_req, &slave->config->soe_requests, list) { if (soe_req->state == EC_INT_REQUEST_QUEUED) { - if (ec_soe_request_timed_out(soe_req)) { soe_req->state = EC_INT_REQUEST_FAILURE; EC_SLAVE_DBG(slave, 1, "Internal SoE request" @@ -634,8 +631,9 @@ void ec_fsm_master_action_idle( || slave->sdo_dictionary_fetched || slave->current_state == EC_SLAVE_STATE_INIT || slave->current_state == EC_SLAVE_STATE_UNKNOWN - || jiffies - slave->jiffies_preop < EC_WAIT_SDO_DICT * HZ - ) continue; + || jiffies - slave->jiffies_preop < EC_WAIT_SDO_DICT * HZ) { + continue; + } EC_SLAVE_DBG(slave, 1, "Fetching SDO dictionary.\n"); @@ -715,7 +713,6 @@ void ec_fsm_master_action_configure( // Does the slave have to be configured? if ((slave->current_state != slave->requested_state || slave->force_config) && !slave->error_flag) { - // Start slave configuration down(&master->config_sem); master->config_busy = 1; @@ -1033,7 +1030,7 @@ void ec_fsm_master_state_configure_slave( wake_up_interruptible(&master->config_queue); if (!ec_fsm_slave_config_success(&fsm->fsm_slave_config)) { - // TODO: mark slave_config as failed. + // TODO(fp): mark slave_config as failed. } fsm->idle = 1; @@ -1051,7 +1048,6 @@ void ec_fsm_master_enter_write_system_times( ec_master_t *master = fsm->master; if (master->dc_ref_time) { - while (fsm->slave < master->slaves + master->slave_count) { if (!fsm->slave->base_dc_supported || !fsm->slave->has_dc_system_time) { @@ -1334,10 +1330,10 @@ void ec_fsm_master_state_write_sii( if (request->offset <= 4 && request->offset + request->nwords > 4) { // alias was written slave->sii.alias = EC_READ_U16(request->words + 4); - // TODO: read alias from register 0x0012 + // TODO(fp): read alias from register 0x0012 slave->effective_alias = slave->sii.alias; } - // TODO: Evaluate other SII contents! + // TODO(fp): Evaluate other SII contents! request->state = EC_INT_REQUEST_SUCCESS; wake_up_all(&master->request_queue); diff --git a/master/fsm_slave_config.c b/master/fsm_slave_config.c index 993d3d28..5ce95dbf 100644 --- a/master/fsm_slave_config.c +++ b/master/fsm_slave_config.c @@ -121,10 +121,10 @@ void ec_fsm_slave_config_reconfigure(ec_fsm_slave_config_t *); void ec_fsm_slave_config_init( ec_fsm_slave_config_t *fsm, /**< slave state machine */ ec_datagram_t *datagram, /**< datagram structure to use */ - ec_fsm_change_t *fsm_change, /**< State change state machine to use. */ + ec_fsm_change_t *fsm_change, /**< State machine to use. */ ec_fsm_coe_t *fsm_coe, /**< CoE state machine to use. */ ec_fsm_soe_t *fsm_soe, /**< SoE state machine to use. */ - ec_fsm_pdo_t *fsm_pdo, /**< PDO configuration state machine to use. */ + ec_fsm_pdo_t *fsm_pdo, /**< PDO config. state machine to use. */ ec_fsm_eoe_t *fsm_eoe /**< EoE state machine to use. */ ) { @@ -1030,7 +1030,7 @@ void ec_fsm_slave_config_state_pdo_conf( ec_fsm_slave_config_t *fsm /**< slave state machine */ ) { - // TODO check for config here + // TODO(fp) check for config here if (ec_fsm_pdo_exec(fsm->fsm_pdo, fsm->datagram)) { return; @@ -1464,7 +1464,7 @@ void ec_fsm_slave_config_state_dc_sync_check( return; } - if (smp_load_acquire(&datagram->working_counter) != 1) { + if (datagram->working_counter != 1) { slave->error_flag = 1; fsm->state = ec_fsm_slave_config_state_error; EC_SLAVE_ERR(slave, "Failed to check DC synchrony: "); @@ -1476,7 +1476,6 @@ void ec_fsm_slave_config_state_dc_sync_check( diff_ms = (datagram->jiffies_received - fsm->jiffies_start) * 1000 / HZ; if (abs_sync_diff > EC_DC_MAX_SYNC_DIFF_NS) { - if (diff_ms >= EC_DC_SYNC_WAIT_MS) { EC_SLAVE_WARN(slave, "Slave did not sync after %lu ms.\n", diff_ms); diff --git a/master/master.c b/master/master.c index 83a63f58..cbed0043 100644 --- a/master/master.c +++ b/master/master.c @@ -863,7 +863,7 @@ void ec_master_inject_external_datagrams( queue_size = new_queue_size; } else if (datagram->data_size > master->max_queue_size) { - datagram->state = EC_DATAGRAM_ERROR; + smp_store_release(&datagram->state, EC_DATAGRAM_ERROR); EC_MASTER_ERR(master, "External datagram %s is too large," " size=%zu, max_queue_size=%zu\n", datagram->name, datagram->data_size, @@ -884,7 +884,7 @@ void ec_master_inject_external_datagrams( unsigned int time_us; #endif - datagram->state = EC_DATAGRAM_ERROR; + smp_store_release(&datagram->state, EC_DATAGRAM_ERROR); #if defined EC_RT_SYSLOG || DEBUG_INJECT #ifdef EC_HAVE_CYCLES @@ -981,13 +981,13 @@ void ec_master_queue_datagram( EC_MASTER_DBG(master, 1, "Datagram %p already queued (skipping).\n", datagram); #endif - datagram->state = EC_DATAGRAM_QUEUED; + smp_store_release(&datagram->state, EC_DATAGRAM_QUEUED); return; } } list_add_tail(&datagram->queue, &master->datagram_queue); - datagram->state = EC_DATAGRAM_QUEUED; + smp_store_release(&datagram->state, EC_DATAGRAM_QUEUED); } /****************************************************************************/ @@ -1116,7 +1116,7 @@ void ec_master_send_datagrams( // set datagram states and sending timestamps list_for_each_entry_safe(datagram, next, &sent_datagrams, sent) { - datagram->state = EC_DATAGRAM_SENT; + smp_store_release(&datagram->state, EC_DATAGRAM_SENT); #ifdef EC_HAVE_CYCLES datagram->cycles_sent = cycles_sent; #endif @@ -1262,11 +1262,10 @@ void ec_master_receive_datagrams( cur_data += data_size; // set the datagram's working counter - smp_store_release(&datagram->working_counter, EC_READ_U16(cur_data)); + datagram->working_counter = EC_READ_U16(cur_data); cur_data += EC_DATAGRAM_FOOTER_SIZE; - // set the state and receive time - smp_store_release(&datagram->state, EC_DATAGRAM_RECEIVED); + // set the receive time #ifdef EC_HAVE_CYCLES datagram->cycles_received = master->devices[EC_DEVICE_MAIN].cycles_poll; @@ -1274,6 +1273,9 @@ void ec_master_receive_datagrams( datagram->jiffies_received = master->devices[EC_DEVICE_MAIN].jiffies_poll; + // set the state with a barrier + smp_store_release(&datagram->state, EC_DATAGRAM_RECEIVED); + // dequeue the received datagram list_del_init(&datagram->queue); } @@ -2480,7 +2482,7 @@ int ecrt_master_send(ec_master_t *master) list_for_each_entry_safe(datagram, n, &master->datagram_queue, queue) { if (datagram->device_index == dev_idx) { - datagram->state = EC_DATAGRAM_ERROR; + smp_store_release(&datagram->state, EC_DATAGRAM_ERROR); list_del_init(&datagram->queue); } } @@ -2529,7 +2531,7 @@ int ecrt_master_receive(ec_master_t *master) datagram->jiffies_sent > timeout_jiffies) { #endif list_del_init(&datagram->queue); - datagram->state = EC_DATAGRAM_TIMED_OUT; + smp_store_release(&datagram->state, EC_DATAGRAM_TIMED_OUT); master->stats.timeouts++; #ifdef EC_RT_SYSLOG From ef425519db29812024c52ca19fd431a002a96c92 Mon Sep 17 00:00:00 2001 From: Florian Pose Date: Wed, 15 Jul 2026 12:54:34 +0200 Subject: [PATCH 3/6] Moved definition of smp macros to globals. --- master/fsm_master.c | 2 +- master/globals.h | 24 ++++++++++++++++++++++++ master/master.c | 17 ----------------- 3 files changed, 25 insertions(+), 18 deletions(-) diff --git a/master/fsm_master.c b/master/fsm_master.c index 23ef6bdb..36a29e2f 100644 --- a/master/fsm_master.c +++ b/master/fsm_master.c @@ -174,7 +174,7 @@ int ec_fsm_master_exec( ec_fsm_master_t *fsm /**< Master state machine. */ ) { - ec_datagram_state_t state = smp_load_acquire(&datagram->state); + ec_datagram_state_t state = smp_load_acquire(&fsm->datagram->state); if (state == EC_DATAGRAM_SENT || state == EC_DATAGRAM_QUEUED) { // datagram was not sent or received yet. diff --git a/master/globals.h b/master/globals.h index da738e8b..4343a9ed 100644 --- a/master/globals.h +++ b/master/globals.h @@ -30,6 +30,8 @@ #include "../globals.h" #include "../include/ecrt.h" +#include + /***************************************************************************** * EtherCAT master ****************************************************************************/ @@ -252,6 +254,28 @@ extern const char *ec_device_names[2]; // only main and backup! /****************************************************************************/ +/* Define SMP macros on kernel versions where they did not exist yet. + */ + +#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 12, 47) + +#define smp_store_release(p, v) \ +do { \ + smp_mb(); \ + ACCESS_ONCE(*p) = (v); \ +} while (0) + +#define smp_load_acquire(p) \ +({ \ + typeof(*p) ___p1 = ACCESS_ONCE(*p); \ + smp_mb(); \ + ___p1; \ +}) + +#endif + +/****************************************************************************/ + extern char *ec_master_version_str; /****************************************************************************/ diff --git a/master/master.c b/master/master.c index cbed0043..f92f1869 100644 --- a/master/master.c +++ b/master/master.c @@ -62,23 +62,6 @@ rt_mutex_lock_interruptible(lock, 0) #endif -#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 12, 47) - -#define smp_store_release(p, v) \ -do { \ - smp_mb(); \ - ACCESS_ONCE(*p) = (v); \ -} while (0) - -#define smp_load_acquire(p) \ -({ \ - typeof(*p) ___p1 = ACCESS_ONCE(*p); \ - smp_mb(); \ - ___p1; \ -}) - -#endif - #include "master.h" /****************************************************************************/ From b4217e93b0aa2f369fbba4b28951da509523a8e8 Mon Sep 17 00:00:00 2001 From: Florian Pose Date: Wed, 15 Jul 2026 13:41:25 +0200 Subject: [PATCH 4/6] Moved SMP macros to smp.h --- master/Makefile.am | 1 + master/fsm_master.c | 1 + master/globals.h | 33 ++++---------------------- master/master.c | 1 + master/smp.h | 56 +++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 63 insertions(+), 29 deletions(-) create mode 100644 master/smp.h diff --git a/master/Makefile.am b/master/Makefile.am index 1a0ef30a..2b8bbf77 100644 --- a/master/Makefile.am +++ b/master/Makefile.am @@ -67,6 +67,7 @@ noinst_HEADERS = \ sdo_request.c sdo_request.h \ slave.c slave.h \ slave_config.c slave_config.h \ + smp.h \ soe_errors.c \ soe_request.c soe_request.h \ sync.c sync.h \ diff --git a/master/fsm_master.c b/master/fsm_master.c index 36a29e2f..11e1e58e 100644 --- a/master/fsm_master.c +++ b/master/fsm_master.c @@ -32,6 +32,7 @@ #ifdef EC_EOE #include "ethernet.h" #endif +#include "smp.h" #include "fsm_master.h" #include "fsm_foe.h" diff --git a/master/globals.h b/master/globals.h index 4343a9ed..afa2a808 100644 --- a/master/globals.h +++ b/master/globals.h @@ -24,14 +24,12 @@ /****************************************************************************/ -#ifndef __EC_MASTER_GLOBALS_H__ -#define __EC_MASTER_GLOBALS_H__ +#ifndef MASTER_GLOBALS_H_ +#define MASTER_GLOBALS_H_ #include "../globals.h" #include "../include/ecrt.h" -#include - /***************************************************************************** * EtherCAT master ****************************************************************************/ @@ -173,8 +171,7 @@ typedef struct { */ typedef enum { EC_DC_32, /**< 32 bit. */ - EC_DC_64 /*< 64 bit for system time, system time offset and - port 0 receive time. */ + EC_DC_64 /*< 64 bit for system time, time offset and port receive time. */ } ec_slave_dc_range_t; /** EtherCAT slave sync signal configuration. @@ -254,28 +251,6 @@ extern const char *ec_device_names[2]; // only main and backup! /****************************************************************************/ -/* Define SMP macros on kernel versions where they did not exist yet. - */ - -#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 12, 47) - -#define smp_store_release(p, v) \ -do { \ - smp_mb(); \ - ACCESS_ONCE(*p) = (v); \ -} while (0) - -#define smp_load_acquire(p) \ -({ \ - typeof(*p) ___p1 = ACCESS_ONCE(*p); \ - smp_mb(); \ - ___p1; \ -}) - -#endif - -/****************************************************************************/ - extern char *ec_master_version_str; /****************************************************************************/ @@ -335,4 +310,4 @@ typedef struct ec_slave ec_slave_t; /**< \see ec_slave. */ /****************************************************************************/ -#endif +#endif // MASTER_GLOBALS_H_ diff --git a/master/master.c b/master/master.c index f92f1869..0c5e653b 100644 --- a/master/master.c +++ b/master/master.c @@ -43,6 +43,7 @@ #include "slave_config.h" #include "device.h" #include "datagram.h" +#include "smp.h" #ifdef EC_EOE #if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 11, 0) diff --git a/master/smp.h b/master/smp.h new file mode 100644 index 00000000..f8c8fe35 --- /dev/null +++ b/master/smp.h @@ -0,0 +1,56 @@ +/***************************************************************************** + * + * Copyright (C) 2006-2026 Florian Pose, Ingenieurgemeinschaft IgH + * + * This file is part of the IgH EtherCAT master. + * + * The file is free software; you can redistribute it and/or modify it under + * the terms of the GNU Lesser General Public License as published by the + * Free Software Foundation; version 2.1 of the License. + * + * This file 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 Lesser General Public + * License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this file. If not, see . + * + ****************************************************************************/ + +/** \file + * Definitions of Kernel SMP macros. + */ + +/****************************************************************************/ + +#ifndef MASTER_SMP_H_ +#define MASTER_SMP_H_ + +#include + +/****************************************************************************/ + +/* Define SMP macros on kernel versions where they did not exist yet. + */ + +#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 12, 47) + +#define smp_store_release(p, v) \ +do { \ + smp_mb(); \ + ACCESS_ONCE(*p) = (v); \ +} while (0) + +#define smp_load_acquire(p) \ +({ \ + typeof(*p) ___p1 = ACCESS_ONCE(*p); \ + smp_mb(); \ + ___p1; \ +}) + +#endif + +/****************************************************************************/ + +#endif // MASTER_SMP_H_ From 754d0e548ad4d5ea6bb6e2531a4bff874bfca7b6 Mon Sep 17 00:00:00 2001 From: Florian Pose Date: Wed, 15 Jul 2026 13:45:50 +0200 Subject: [PATCH 5/6] Improved smp_store positions. --- master/master.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/master/master.c b/master/master.c index 0c5e653b..27158eb8 100644 --- a/master/master.c +++ b/master/master.c @@ -1100,12 +1100,12 @@ void ec_master_send_datagrams( // set datagram states and sending timestamps list_for_each_entry_safe(datagram, next, &sent_datagrams, sent) { - smp_store_release(&datagram->state, EC_DATAGRAM_SENT); #ifdef EC_HAVE_CYCLES datagram->cycles_sent = cycles_sent; #endif datagram->jiffies_sent = jiffies_sent; - list_del_init(&datagram->sent); // empty list of sent datagrams + list_del_init(&datagram->sent); // remove from sent queue + smp_store_release(&datagram->state, EC_DATAGRAM_SENT); } frame_count++; @@ -1257,11 +1257,11 @@ void ec_master_receive_datagrams( datagram->jiffies_received = master->devices[EC_DEVICE_MAIN].jiffies_poll; - // set the state with a barrier - smp_store_release(&datagram->state, EC_DATAGRAM_RECEIVED); - // dequeue the received datagram list_del_init(&datagram->queue); + + // set the state (with a barrier) + smp_store_release(&datagram->state, EC_DATAGRAM_RECEIVED); } } @@ -2466,8 +2466,8 @@ int ecrt_master_send(ec_master_t *master) list_for_each_entry_safe(datagram, n, &master->datagram_queue, queue) { if (datagram->device_index == dev_idx) { - smp_store_release(&datagram->state, EC_DATAGRAM_ERROR); list_del_init(&datagram->queue); + smp_store_release(&datagram->state, EC_DATAGRAM_ERROR); } } From c569080e2f4b2979255c96a790ac90a8cb31605e Mon Sep 17 00:00:00 2001 From: Florian Pose Date: Wed, 15 Jul 2026 14:04:46 +0200 Subject: [PATCH 6/6] NEWS entry. --- NEWS.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEWS.md b/NEWS.md index 539b75d4..00cc7cd4 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,7 @@ # Version History +- Protect datagram receiving mechanism against re-ordering. + ## Version 1.6.10 - Added RasPi 5 macb (Cadence GEM / RP1) driver for kernel 6.18.