Skip to content

net: gptp: release stranded Sync on port reset #117400

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bperseghetti wants to merge 1 commit into
base: main
Choose a base branch
from
+33 −0
Open

net: gptp: release stranded Sync on port reset #117400

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions subsys/net/l2/ethernet/gptp/gptp_md.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -855,6 +855,19 @@ static void gptp_md_sync_send_state_machine(int port)
port_ds = GPTP_PORT_DS(port);

if ((!port_ds->ptt_port_enabled) || !port_ds->as_capable) {
/* A Sync waiting here for a transmit timestamp will never be
* given one: the port that was to produce it is down or no
* longer capable. Release it and reset the callback now, or
* its reference and the stale callback are leaked.
*/
if (state->sync_ptr != NULL) {
net_pkt_unref(state->sync_ptr);
state->sync_ptr = NULL;
}

gptp_sync_send_reset(port);

@jukkar jukkar Aug 26, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should have the calls other way around so

gptp_sync_timestamp_cb_unregister()
net_pkt_unref(state->sync_ptr);

This would close a window where the packet is inserted to the list after the unref.
So similar way this is done in

zephyr/subsys/net/l2/ethernet/gptp/gptp_messages.c

Line 629 in 54cdacb

net_if_unregister_timestamp_cb(&pdelay_response_timestamp_cb[port - 1]);

Also you can feed null to net_pkt_unref


state->md_sync_timestamp_avail = false;
state->rcvd_md_sync = false;
state->state = GPTP_SYNC_SEND_INITIALIZING;

Expand Down
10 changes: 10 additions & 0 deletions subsys/net/l2/ethernet/gptp/gptp_messages.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -822,6 +822,16 @@ void gptp_send_sync(int port, struct net_pkt *pkt)
net_if_queue_tx(net_pkt_iface(pkt), pkt);
}

void gptp_sync_send_reset(int port)

@jukkar jukkar Aug 26, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we call this gptp_sync_timestamp_cb_unregister() which says what it does.
gptp_sync_send_reset() sounds like "sending a reset" which is not correct.

{
if (!sync_cb_registered[port - 1]) {
return;
}

net_if_unregister_timestamp_cb(&sync_timestamp_cb[port - 1]);
sync_cb_registered[port - 1] = false;
}

void gptp_send_follow_up(int port, struct net_pkt *pkt)
{
GPTP_STATS_INC(port, tx_fup_count);
Expand Down
10 changes: 10 additions & 0 deletions subsys/net/l2/ethernet/gptp/gptp_messages.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -450,6 +450,16 @@ void gptp_handle_signaling(int port, struct net_pkt *pkt);
*/
void gptp_send_sync(int port, struct net_pkt *pkt);

/**
* @brief Forget a Sync awaiting its transmit timestamp.
*
* Releases the timestamp callback registered for a Sync that will not be
* timestamped, so that the next Sync registers one of its own.
*
* @param port gPTP port number.
*/
void gptp_sync_send_reset(int port);

/**
* @brief Send a Follow Up message.
*
Expand Down
Loading