stp: Fix IPC layout and netlink handling between STPMGR and STP daemon by rdhegde · Pull Request #97 · sonic-net/sonic-stp

rdhegde · 2026-07-25T13:08:10Z

Summary

Unify duplicated IPC structures between STPMGR and STP daemon into a
single, packed definition with all required fields.
Add sanity checks and error logging for invalid IPC messages and
unexpected protocol modes.
Fix stp_netlink_recv() passing an invalid if_db / ifname into
stp_netlink_cb().

Motivation

PVST configuration fails when STPMGR and the STP daemon exchange
inconsistent or incomplete IPC data, or when netlink callbacks operate on
invalid interface state. This PR fixes the daemon-side IPC and netlink
handling required for PVST to work reliably.

Part of PVST fix series for #28624.

Changes

IPC structures
- Remove duplicate IPC definitions between STPMGR and STP daemon
- Pack all IPC structures and add missing structure members
Error handling
- Add sanity check in stp_intf_netlink_cb()
- Add error logs in stpmgr_process_ipc_msg() for unexpected protocol modes
Netlink
- Fix stp_netlink_recv() calling stp_netlink_cb() with invalid
  if_db / ifname

Related PRs

Repo	PR
sonic-buildimage	sonic-net/sonic-buildimage#28624
sonic-swss	sonic-net/sonic-swss#4790
sonic-utilities	sonic-net/sonic-utilities#4716

Verification

3-node PVST topology (GNS3 sonic-vs): config reached STP daemon,
PVST converged
Marvell Prestera switches: same PVST validation

Notes for reviewers

Start with shared IPC header/struct definitions, then netlink callback path
IPC packing changes affect STPMGR ↔ STP message layout; review together
with sonic-swss#4790

- Packed stp IPC structures - Added missing parameter `loop_guard` to align with swss(stpmgr) - Renamed parameters: `edge` and `max_hop` to align with swss(stpmgr) - Renamed MST_INST_CONFIG_MSG to STP_MST_INST_CONFIG_MSG for consistency Signed-off-by: Ram Hegde <rhegde@marvell.com>

- fix socket return handling - remove unused variable Signed-off-by: Ram Hegde <rhegde@marvell.com>

Signed-off-by: Ram Hegde <rhegde@marvell.com>

- uninitialized if_db.ifname is not handled before calling stp_netlink_cb() Signed-off-by: Ram Hegde <rhegde@marvell.com>

Signed-off-by: Ram Hegde <rhegde@marvell.com>

linux-foundation-easycla · 2026-07-25T13:08:15Z

The committers listed above are authorized under a signed CLA.

✅ login: rdhegde / name: Ram Hegde (1aa8c23, 2c9e26a, 6265d9f, 6a37aec, 85c4d1b, e0573a5)

mssonicbld · 2026-07-25T13:08:17Z

/azp run

azure-pipelines · 2026-07-25T13:08:18Z

Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run.

azure-pipelines · 2026-07-25T13:08:27Z

Azure Pipelines: Successfully started running 1 pipeline(s).

rdhegde · 2026-07-25T13:14:27Z

/easycla

* In stpmgr_process_rx_bpdu(), when PVST BPDUs has no VLAN get the port's untagged VLAN * Fixed typos in code comments Signed-off-by: Ram Hegde <rhegde@marvell.com>

mssonicbld · 2026-07-25T13:55:57Z

/azp run

azure-pipelines · 2026-07-25T13:56:07Z

Azure Pipelines: Successfully started running 1 pipeline(s).

rdhegde · 2026-08-20T10:53:32Z

PVST test report
pvst-testing.md

divyachandralekha · 2026-09-03T04:34:27Z

+    }
+
    // sanity checks
    if (!IS_VALID_VLAN(vlan_id))


Move if (!IS_VALID_VLAN(vlan_id)) inside else case

Move if (!IS_VALID_VLAN(vlan_id)) inside else case

Updated as suggested

divyachandralekha · 2026-09-03T04:34:41Z

 {
+    // When BPDUs arrive without VLAN get the VLAN from port's untagged vlan
+    // configuration.
+    if ( vlan_id == 0 ) {


Remove this if case. vlan decision is made inside stpmgr_rx_stp_bpdu.

Remove this if case. vlan decision is made inside stpmgr_rx_stp_bpdu.

Moved the vlan_id resolution code in stpmgr_rx_pvst_bpdu(). The current code accessed vlan_id even before it was populated.

divyachandralekha · 2026-09-03T04:49:20Z

+                                   if_db.ifname, if_db.kif_index, if_db.oper_state,
+                                   if_db.is_bond, if_db.is_member, if_db.master_ifindex);
+
+                    stp_netlink_cb( &if_db,


if rt_list[IFLA_IFNAME] is not set , how RTM_NEWLINK is handled ?
This portion of code changes are not required. I dnt see any issue with RTM_DELLINK when IFLA_IFNAME is not set. stp_netlink_cb is done outside of if(rt_list[IFLA_IFNAME] ) condition. please check.

if rt_list[IFLA_IFNAME] is not set , how RTM_NEWLINK is handled ? This portion of code changes are not required. I dnt see any issue with RTM_DELLINK when IFLA_IFNAME is not set. stp_netlink_cb is done outside of if(rt_list[IFLA_IFNAME] ) condition. please check.

The current call to stp_netlink_cb() is outside nlmsg_type check which means it would be called for anything other than add and del also. I just re-arragned the code for correctness such that add is called only with valid if_name while del can be called without it.

divyachandralekha · 2026-09-03T04:54:07Z

-} PORT_ATTR;
+    // Add padding to align to 4 bytes
+    uint16_t padding;
+} __attribute__((packed)) PORT_ATTR;


if there is a similar header file in swss repo, please change that also.

I have removed the duplicate header definition from swss and made it use stp_ipc.h. Please look at the changes from sonic-net/sonic-swss#4790

mssonicbld · 2026-09-07T10:43:22Z

/azp run

azure-pipelines · 2026-09-07T10:43:32Z

Azure Pipelines: Successfully started running 1 pipeline(s).

mssonicbld · 2026-09-07T10:55:50Z

/azp run

azure-pipelines · 2026-09-07T10:56:00Z

Azure Pipelines: Successfully started running 1 pipeline(s).

Signed-off-by: Ram Hegde <rhegde@marvell.com>

mssonicbld · 2026-09-07T11:00:43Z

/azp run

azure-pipelines · 2026-09-07T11:00:53Z

Azure Pipelines: Successfully started running 1 pipeline(s).

rdhegde added 5 commits July 9, 2026 10:00

stp: fix socket return error

e0573a5

- fix socket return handling - remove unused variable Signed-off-by: Ram Hegde <rhegde@marvell.com>

stp: add error handling

6265d9f

Signed-off-by: Ram Hegde <rhegde@marvell.com>

stp: fix: call stp_netlink_cb() with valid ifname

2c9e26a

- uninitialized if_db.ifname is not handled before calling stp_netlink_cb() Signed-off-by: Ram Hegde <rhegde@marvell.com>

stp: fix: handle NULL ifname interface add

6a37aec

Signed-off-by: Ram Hegde <rhegde@marvell.com>

rdhegde mentioned this pull request Jul 25, 2026

stp: Fix docker-stp image and build templates for PVST sonic-net/sonic-buildimage#28624

Open

stp: fix: Pick the right VLAN for untagged PVST BPDUs

334e762

* In stpmgr_process_rx_bpdu(), when PVST BPDUs has no VLAN get the port's untagged VLAN * Fixed typos in code comments Signed-off-by: Ram Hegde <rhegde@marvell.com>

rdhegde force-pushed the rdhegde-pvst-stp branch from 1aa8c23 to 334e762 Compare July 25, 2026 13:55

rdhegde changed the title ~~PVST bug fixes~~ stp: Fix IPC layout and netlink handling between STPMGR and STP daemon Jul 28, 2026

rdhegde mentioned this pull request Jul 28, 2026

stpmgr: Unify IPC with sonic-stp and replace ebtables with nftables sonic-net/sonic-swss#4790

Open

rdhegde requested a review from divyachandralekha August 4, 2026 05:51

divyachandralekha reviewed Sep 3, 2026

View reviewed changes

divyachandralekha requested changes Sep 3, 2026

View reviewed changes

rdhegde force-pushed the rdhegde-pvst-stp branch from e33347c to d278f50 Compare September 7, 2026 10:55

stp:fix: Moves untagged vlan resolution to stpmgr_rx_pvst_bpdu()

ac07371

Signed-off-by: Ram Hegde <rhegde@marvell.com>

rdhegde force-pushed the rdhegde-pvst-stp branch from d278f50 to ac07371 Compare September 7, 2026 11:00

Uh oh!

Conversation

rdhegde commented Jul 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Motivation

Changes

Related PRs

Verification

Notes for reviewers

Uh oh!

linux-foundation-easycla Bot commented Jul 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mssonicbld commented Jul 25, 2026

Uh oh!

azure-pipelines Bot commented Jul 25, 2026

Uh oh!

azure-pipelines Bot commented Jul 25, 2026

Uh oh!

rdhegde commented Jul 25, 2026

Uh oh!

mssonicbld commented Jul 25, 2026

Uh oh!

azure-pipelines Bot commented Jul 25, 2026

Uh oh!

rdhegde commented Aug 20, 2026

Uh oh!

divyachandralekha Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

rdhegde Sep 7, 2026

Choose a reason for hiding this comment

Uh oh!

divyachandralekha Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rdhegde Sep 7, 2026

Choose a reason for hiding this comment

Uh oh!

divyachandralekha Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

rdhegde Sep 7, 2026

Choose a reason for hiding this comment

Uh oh!

divyachandralekha Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

rdhegde Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

mssonicbld commented Sep 7, 2026

Uh oh!

azure-pipelines Bot commented Sep 7, 2026

Uh oh!

mssonicbld commented Sep 7, 2026

Uh oh!

azure-pipelines Bot commented Sep 7, 2026

Uh oh!

mssonicbld commented Sep 7, 2026

Uh oh!

azure-pipelines Bot commented Sep 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rdhegde commented Jul 25, 2026 •

edited

Loading

linux-foundation-easycla Bot commented Jul 25, 2026 •

edited

Loading

divyachandralekha Sep 3, 2026 •

edited

Loading