Skip to content

net: gptp: stepsRemoved is wrongly assigned #93180

@rary21

Description

@rary21

Describe the bug

According to IEEE 802.1AS, when RM is better than the S (local clock), the stepsRemoved in masterPriority should be set to SRM + 1 .
However, in the current implementation, it appears that this value is not updated accordingly and instead retains the previous value.

static void update_bmca(int port,
			int best_port,
			struct gptp_global_ds *global_ds,
			struct gptp_default_ds *default_ds,
			struct gptp_priority_vector *gm_prio)
{
	struct gptp_port_bmca_data *bmca_data = GPTP_PORT_BMCA_DATA(port);

	/* Update masterPriorityVector for the port. */
        /* best_port == 0 means S is better than any RM */
	if (best_port == 0) {
		memcpy(&bmca_data->master_priority, gm_prio,
		       sizeof(struct gptp_priority_vector));

		bmca_data->master_priority.port_number = htons(port);
		bmca_data->master_priority.src_port_id.port_number =
			htons(port);
	} else {
                /* this block does not update bmca_data->master_priority.steps_removed */
		memcpy(&bmca_data->master_priority.root_system_id,
		       &gm_prio->root_system_id,
		       sizeof(struct gptp_root_system_identity));
		memcpy(bmca_data->master_priority.src_port_id.clk_id,
		       default_ds->clk_id, GPTP_CLOCK_ID_LEN);
		bmca_data->master_priority.port_number = htons(port);
		bmca_data->master_priority.src_port_id.port_number =
			htons(port);
	}

The suggested fix would be like this.

diff --git a/subsys/net/l2/ethernet/gptp/gptp_mi.c b/subsys/net/l2/ethernet/gptp/gptp_mi.c
index 997d2f7b9c5..d2100405183 100644
--- a/subsys/net/l2/ethernet/gptp/gptp_mi.c
+++ b/subsys/net/l2/ethernet/gptp/gptp_mi.c
@@ -1758,6 +1758,8 @@ static void update_bmca(int port,
                bmca_data->master_priority.port_number = htons(port);
                bmca_data->master_priority.src_port_id.port_number =
                        htons(port);
+               /* gm_prio->steps_removed is assigned as SRM+1 in compute_best_vector */
+               bmca_data->master_priority.steps_removed = gm_prio->steps_removed;
        }
 
        switch (bmca_data->info_is) {

Since my understanding of the specification might not be perfect, I would appreciate it if someone could confirm whether this interpretation is correct or not.

Regression

  • This is a regression.

Steps to reproduce

No response

Relevant log output

Impact

Intermittent – Occurs occasionally; hard to reproduce.

Environment

66acb36

Additional Context

No response

Metadata

Metadata

Assignees

Labels

area: gPTPbugThe issue is a bug, or the PR is fixing a bugpriority: lowLow impact/importance bug

Type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions