From 85c4d1b6132e075544d54cc8e306507fb3a132fd Mon Sep 17 00:00:00 2001 From: Ram Hegde Date: Thu, 9 Jul 2026 09:47:15 +0000 Subject: [PATCH 1/6] stp: fix: pack stp_ipc structs for consistent ipc - 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 --- include/stp_ipc.h | 33 ++++++++++++++++++--------------- mstp/mstp_mgr.c | 18 +++++++++--------- 2 files changed, 27 insertions(+), 24 deletions(-) diff --git a/include/stp_ipc.h b/include/stp_ipc.h index 6585329..e663680 100644 --- a/include/stp_ipc.h +++ b/include/stp_ipc.h @@ -74,7 +74,7 @@ typedef struct STP_IPC_MSG unsigned int msg_len; L2_PROTO_MODE proto_mode; char data[0]; -} __attribute__((aligned(4))) STP_IPC_MSG; +} __attribute__((packed)) STP_IPC_MSG; #define STP_SET_COMMAND 1 #define STP_DEL_COMMAND 0 @@ -98,7 +98,9 @@ typedef struct PORT_ATTR char intf_name[IFNAMSIZ]; int8_t mode; uint8_t enabled; -} PORT_ATTR; + // Add padding to align to 4 bytes + uint16_t padding; +} __attribute__((packed)) PORT_ATTR; typedef struct STP_VLAN_CONFIG_MSG { @@ -126,11 +128,11 @@ typedef struct STP_VLAN_PORT_CONFIG_MSG typedef struct VLAN_ATTR { - int8_t mode; - uint8_t padding[3]; // Explicit padding for alignment int inst_id; int vlan_id; -} VLAN_ATTR; + int8_t mode; + uint8_t padding[3]; // Explicit padding for alignment +} __attribute__((packed)) VLAN_ATTR; typedef struct STP_PORT_CONFIG_MSG { @@ -138,17 +140,18 @@ typedef struct STP_PORT_CONFIG_MSG char intf_name[IFNAMSIZ]; uint8_t enabled; uint8_t root_guard; + uint8_t loop_guard; uint8_t bpdu_guard; uint8_t bpdu_guard_do_disable; uint8_t portfast; uint8_t uplink_fast; - uint8_t edge; + uint8_t edge_port; LinkType link_type; // MSTP only int path_cost; int priority; int count; VLAN_ATTR vlan_list[0]; -} STP_PORT_CONFIG_MSG; +} __attribute__((packed)) STP_PORT_CONFIG_MSG; typedef struct STP_VLAN_MEM_CONFIG_MSG { @@ -161,7 +164,7 @@ typedef struct STP_VLAN_MEM_CONFIG_MSG char intf_name[IFNAMSIZ]; int path_cost; int priority; -} STP_VLAN_MEM_CONFIG_MSG; +} __attribute__((packed)) STP_VLAN_MEM_CONFIG_MSG; typedef struct STP_MST_GLOBAL_CONFIG_MSG { uint8_t opcode; // enable/disable @@ -170,26 +173,26 @@ typedef struct STP_MST_GLOBAL_CONFIG_MSG { uint8_t forward_delay; uint8_t hello_time; uint8_t max_age; - uint8_t max_hop; + uint8_t max_hops; }__attribute__((packed)) STP_MST_GLOBAL_CONFIG_MSG; typedef struct VLAN_LIST{ uint16_t vlan_id; -}VLAN_LIST; +}__attribute__((packed)) VLAN_LIST; -typedef struct MST_INST_CONFIG_MSG +typedef struct STP_MST_INST_CONFIG_MSG { uint8_t opcode; // enable/disable uint16_t mst_id; int priority; uint16_t vlan_count; VLAN_LIST vlan_list[0]; -} __attribute__((packed)) MST_INST_CONFIG_MSG; +} __attribute__((packed)) STP_MST_INST_CONFIG_MSG; typedef struct STP_MST_INSTANCE_CONFIG_MSG { uint8_t mst_count; - MST_INST_CONFIG_MSG mst_list[0]; + STP_MST_INST_CONFIG_MSG mst_list[0]; } __attribute__((packed)) STP_MST_INSTANCE_CONFIG_MSG; typedef struct STP_MST_INST_PORT_CONFIG_MSG { @@ -204,7 +207,7 @@ typedef struct PORT_LIST { char intf_name[IFNAMSIZ]; int8_t tagging_mode; -}PORT_LIST; +}__attribute__((packed)) PORT_LIST; typedef struct STP_MST_VLAN_PORT_MAP { @@ -213,7 +216,7 @@ typedef struct STP_MST_VLAN_PORT_MAP int8_t stp_mode; uint8_t add; PORT_LIST port_list[0]; -}STP_MST_VLAN_PORT_MAP; +}__attribute__((packed)) STP_MST_VLAN_PORT_MAP; typedef struct MSTP_INST_VLAN_LIST { diff --git a/mstp/mstp_mgr.c b/mstp/mstp_mgr.c index fa78f5d..b914518 100755 --- a/mstp/mstp_mgr.c +++ b/mstp/mstp_mgr.c @@ -2211,7 +2211,7 @@ void mstpmgr_process_intf_config_msg(void *msg) "do_dis:%d, cost:%d, pri:%d, edge:%d, count:%d", pmsg->intf_name, pmsg->opcode, pmsg->enabled, pmsg->root_guard, pmsg->bpdu_guard, pmsg->bpdu_guard_do_disable, pmsg->path_cost, pmsg->priority, - pmsg->edge, pmsg->count); + pmsg->edge_port, pmsg->count); port_id = stp_intf_get_port_id_by_name(pmsg->intf_name); @@ -2286,7 +2286,7 @@ void mstpmgr_process_intf_config_msg(void *msg) { mstpmgr_add_control_port(port_id); - mstpmgr_config_port_admin_edge(port_id, pmsg->edge); + mstpmgr_config_port_admin_edge(port_id, pmsg->edge_port); mstpmgr_config_root_protect(port_id, pmsg->root_guard); @@ -2520,8 +2520,8 @@ void mstpmgr_process_mstp_global_config_msg(void *msg) else mstpmgr_config_max_age(MSTP_DFLT_MAX_AGE); - if(pmsg->max_hop) - mstpmgr_config_max_hops(pmsg->max_hop); + if(pmsg->max_hops) + mstpmgr_config_max_hops(pmsg->max_hops); else mstpmgr_config_max_hops(MSTP_DFLT_MAX_HOPS); } @@ -2545,7 +2545,7 @@ void mstpmgr_process_mstp_global_config_msg(void *msg) if(!pmsg->max_age) mstpmgr_config_max_age(MSTP_DFLT_MAX_AGE); - if(!pmsg->max_hop) + if(!pmsg->max_hops) mstpmgr_config_max_hops(MSTP_DFLT_MAX_HOPS); } @@ -2566,7 +2566,7 @@ void mstpmgr_process_inst_vlan_config_msg(void *msg) PORT_MASK *mem_port_mask; PORT_MASK *cist_port_mask = portmask_local_init(&cist_mask); PORT_MASK *msti_port_mask = portmask_local_init(&msti_mask); - MST_INST_CONFIG_MSG *mst_list; + STP_MST_INST_CONFIG_MSG *mst_list; MSTP_MSTID mst_id; VLAN_ID vlan_id = VLAN_ID_INVALID; UINT8 vlanmask_string[500] = {0,}; @@ -2598,7 +2598,7 @@ void mstpmgr_process_inst_vlan_config_msg(void *msg) { mstpmgr_config_cist_priority(mst_list->priority); - mst_list = (MST_INST_CONFIG_MSG *)((char *)mst_list + (sizeof(MST_INST_CONFIG_MSG))); + mst_list = (STP_MST_INST_CONFIG_MSG *)((char *)mst_list + (sizeof(STP_MST_INST_CONFIG_MSG))); continue; } else @@ -2607,7 +2607,7 @@ void mstpmgr_process_inst_vlan_config_msg(void *msg) * Ignore it, will update priority once the instance to vlan mapping comes */ if(mst_list->opcode == STP_SET_COMMAND && mst_list->vlan_count == 0) { - mst_list = (MST_INST_CONFIG_MSG *)((char *)mst_list + (sizeof(MST_INST_CONFIG_MSG))); + mst_list = (STP_MST_INST_CONFIG_MSG *)((char *)mst_list + (sizeof(STP_MST_INST_CONFIG_MSG))); continue; } } @@ -2813,7 +2813,7 @@ void mstpmgr_process_inst_vlan_config_msg(void *msg) } } - mst_list = (MST_INST_CONFIG_MSG *)((char *)mst_list + (sizeof(MST_INST_CONFIG_MSG)) + (mst_list->vlan_count * sizeof(VLAN_LIST))); + mst_list = (STP_MST_INST_CONFIG_MSG *)((char *)mst_list + (sizeof(STP_MST_INST_CONFIG_MSG)) + (mst_list->vlan_count * sizeof(VLAN_LIST))); } if(restart) From e0573a5f61e8bd08e9b1475bb562da672dd4b715 Mon Sep 17 00:00:00 2001 From: Ram Hegde Date: Thu, 9 Jul 2026 10:01:26 +0000 Subject: [PATCH 2/6] stp: fix socket return error - fix socket return handling - remove unused variable Signed-off-by: Ram Hegde --- stp/stp_main.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/stp/stp_main.c b/stp/stp_main.c index 497b240..cd5921e 100644 --- a/stp/stp_main.c +++ b/stp/stp_main.c @@ -27,7 +27,7 @@ int stpd_ipc_init() unlink(STPD_SOCK_NAME); g_stpd_ipc_handle = socket(AF_UNIX, SOCK_DGRAM, 0); - if (!g_stpd_ipc_handle) + if (g_stpd_ipc_handle == -1 ) { STP_LOG_ERR("ipc socket error %s", strerror(errno)); return -1; @@ -62,6 +62,7 @@ int stpd_ipc_init() void stpd_log_init() { STP_LOG_INIT(); + if (fopen("/stpd_dbg_reload", "r")) { STP_LOG_SET_LEVEL(STP_LOG_LEVEL_DEBUG); @@ -78,7 +79,6 @@ int stpd_main() struct event *evtimer_100ms = 0; struct event *evpkt = 0; struct event_config *cfg = 0; - int8_t ret = 0; signal(SIGPIPE, SIG_IGN); From 6265d9f6ced7d3b21620b5a367f4367ce63f3721 Mon Sep 17 00:00:00 2001 From: Ram Hegde Date: Thu, 9 Jul 2026 10:04:34 +0000 Subject: [PATCH 3/6] stp: add error handling Signed-off-by: Ram Hegde --- stp/stp_mgr.c | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/stp/stp_mgr.c b/stp/stp_mgr.c index 1cfcc6b..14a620b 100755 --- a/stp/stp_mgr.c +++ b/stp/stp_mgr.c @@ -1866,7 +1866,8 @@ static void stpmgr_process_vlan_mem_config_msg(void *msg) static void stpmgr_process_ipc_msg(STP_IPC_MSG *msg, int len, struct sockaddr_un client_addr) { int ret; - STP_LOG_INFO("rcvd %s msg type", msgtype_str[msg->msg_type]); + + STP_LOG_INFO("rcvd %s msg type proto_mode %d", msgtype_str[msg->msg_type], msg->proto_mode); /* Temp code until warm boot is handled */ if(msg->msg_type != STP_INIT_READY && msg->msg_type != STP_STPCTL_MSG) @@ -1906,6 +1907,9 @@ static void stpmgr_process_ipc_msg(STP_IPC_MSG *msg, int len, struct sockaddr_un else if(msg->proto_mode == L2_MSTP) { mstpmgr_process_bridge_config_msg(msg->data); + } else + { + STP_LOG_ERR("Invalid proto_mode %d", msg->proto_mode); } break; } @@ -1914,6 +1918,9 @@ static void stpmgr_process_ipc_msg(STP_IPC_MSG *msg, int len, struct sockaddr_un if(msg->proto_mode == L2_PVSTP) { stpmgr_process_vlan_config_msg(msg->data); + } else + { + STP_LOG_ERR("Invalid proto_mode %d", msg->proto_mode); } break; } @@ -1922,6 +1929,9 @@ static void stpmgr_process_ipc_msg(STP_IPC_MSG *msg, int len, struct sockaddr_un if(msg->proto_mode == L2_PVSTP) { stpmgr_process_vlan_intf_config_msg(msg->data); + } else + { + STP_LOG_ERR("Invalid proto_mode %d", msg->proto_mode); } break; } @@ -1934,6 +1944,9 @@ static void stpmgr_process_ipc_msg(STP_IPC_MSG *msg, int len, struct sockaddr_un else if(msg->proto_mode == L2_MSTP) { mstpmgr_process_intf_config_msg(msg->data); + } else + { + STP_LOG_ERR("Invalid proto_mode %d", msg->proto_mode); } break; } @@ -1942,6 +1955,9 @@ static void stpmgr_process_ipc_msg(STP_IPC_MSG *msg, int len, struct sockaddr_un if(msg->proto_mode == L2_PVSTP) { stpmgr_process_vlan_mem_config_msg(msg->data); + } else + { + STP_LOG_ERR("Invalid proto_mode %d", msg->proto_mode); } break; } @@ -1955,6 +1971,9 @@ static void stpmgr_process_ipc_msg(STP_IPC_MSG *msg, int len, struct sockaddr_un else if (STP_IS_PROTOCOL_ENABLED(L2_MSTP)) { mstpdbg_process_ctl_msg(msg->data); + } else + { + STP_LOG_ERR("Invalid proto_mode %d", msg->proto_mode); } stpmgr_send_reply(client_addr, (void *)msg, len); break; From 2c9e26a50c7170f49177b246939f733fd51836f2 Mon Sep 17 00:00:00 2001 From: Ram Hegde Date: Thu, 9 Jul 2026 10:05:19 +0000 Subject: [PATCH 4/6] stp: fix: call stp_netlink_cb() with valid ifname - uninitialized if_db.ifname is not handled before calling stp_netlink_cb() Signed-off-by: Ram Hegde --- stp/stp_netlink.c | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/stp/stp_netlink.c b/stp/stp_netlink.c index 51dde3a..d44f7bc 100644 --- a/stp/stp_netlink.c +++ b/stp/stp_netlink.c @@ -290,8 +290,7 @@ static int stp_netlink_recv(int nl_fd, bool read_all) nh_len = nh->nlmsg_len; stp_netlink_parse_rtattr(rt_list, IFLA_MAX, IFLA_RTA(ifi), nh_len); - if (rt_list[IFLA_IFNAME]) - { + if ( rt_list[IFLA_IFNAME] ) { ptr = rt_list[IFLA_IFNAME]; strncpy(if_db.ifname, (char *)RTA_DATA(ptr), IFNAMSIZ - 1); if_db.ifname[IFNAMSIZ - 1] = '\0'; @@ -355,15 +354,23 @@ static int stp_netlink_recv(int nl_fd, bool read_all) if_db.master_ifindex = *(uint32_t *)RTA_DATA(ptr); } } - STP_LOG_INFO("RTM-%s IF:%s KIF:%u Oper:%d Bond:%d Mem:%d Master:%u", (nh->nlmsg_type == RTM_NEWLINK)?"UPDATE":"DELETE", - if_db.ifname, if_db.kif_index, if_db.oper_state, if_db.is_bond, if_db.is_member, if_db.master_ifindex); - } - else - { - STP_LOG_DEBUG("No ifname for kif_index :%d ", if_db.kif_index); + STP_LOG_INFO( "RTM-%s IF:%s KIF:%u Oper:%d Bond:%d Mem:%d Master:%u", + (nh->nlmsg_type == RTM_NEWLINK)?"UPDATE":"DELETE", + 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, + ( ( nh->nlmsg_type == RTM_NEWLINK ) ? 1 : 0 ), + read_all ); + } else { + // RTM_DELLINK may omit IFLA_IFNAME; delete path uses kif_index only. + if (nh->nlmsg_type == RTM_DELLINK) { + stp_netlink_cb( &if_db, 0, read_all ); + } else { + STP_LOG_DEBUG("No ifname for kif_index :%d ", if_db.kif_index); + } } } - stp_netlink_cb(&if_db, (nh->nlmsg_type == RTM_NEWLINK)?1:0, read_all); } //end for loop From 6a37aec0576c526284cbc1802dbb4b40212b6726 Mon Sep 17 00:00:00 2001 From: Ram Hegde Date: Thu, 9 Jul 2026 10:13:19 +0000 Subject: [PATCH 5/6] stp: fix: handle NULL ifname interface add Signed-off-by: Ram Hegde --- stp/stp_intf.c | 20 ++++++++++++++------ stpctl/stpctl.c | 2 ++ 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/stp/stp_intf.c b/stp/stp_intf.c index cd016bb..e2122de 100755 --- a/stp/stp_intf.c +++ b/stp/stp_intf.c @@ -485,12 +485,20 @@ void stp_intf_netlink_cb(netlink_db_t *if_db, uint8_t is_add, bool init_in_prog) bool eth_if; g_stpd_stats_libev_netlink++; - if(STP_IS_ETH_PORT(if_db->ifname)) - eth_if = true; - else if(STP_IS_PO_PORT(if_db->ifname)) - eth_if = false; - else - return; + if ( STP_IS_ETH_PORT( if_db->ifname ) ) { + eth_if = true; + } else if ( STP_IS_PO_PORT( if_db->ifname ) ) { + eth_if = false; + } else { + // If it is e delete then if_db->ifname can be NULL since the + // delete path relies on if_db->kif_index. For add we need to + // have if_db->ifname + if ( is_add ) { + STP_LOG_ERR( "Interface '%s' neither port nor portchannel", + if_db->ifname ); + return; + } + } node = stp_intf_update_intf_db(if_db, is_add, init_in_prog, eth_if); diff --git a/stpctl/stpctl.c b/stpctl/stpctl.c index 7381101..9f2d6d2 100644 --- a/stpctl/stpctl.c +++ b/stpctl/stpctl.c @@ -83,6 +83,8 @@ int connect_server() close(stpd_fd); return -1; } + + return 0; } int send_msg_stpd(STP_MSG_TYPE msgType, uint32_t msgLen, void *data) From 334e7629972caa230069ed2a916d2e81dfba6fe6 Mon Sep 17 00:00:00 2001 From: Ram Hegde Date: Fri, 24 Jul 2026 17:59:16 +0530 Subject: [PATCH 6/6] stp: fix: Pick the right VLAN for untagged PVST BPDUs * 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 --- include/stp_externs.h | 1 + stp/stp_intf.c | 2 +- stp/stp_mgr.c | 6 ++++++ stp/stp_netlink.c | 8 ++++---- 4 files changed, 12 insertions(+), 5 deletions(-) diff --git a/include/stp_externs.h b/include/stp_externs.h index 87ed966..db525c2 100755 --- a/include/stp_externs.h +++ b/include/stp_externs.h @@ -105,6 +105,7 @@ extern bool stpmgr_config_root_protect(PORT_ID port_id, bool enable); /* stp_util.c */ extern bool stputil_is_protocol_enabled(L2_PROTO_MODE proto_mode); extern bool stputil_is_port_untag(VLAN_ID vlan_id, PORT_ID port_id); +extern VLAN_ID stputil_get_untag_vlan(PORT_ID port_id); extern void stputil_bridge_to_string(BRIDGE_IDENTIFIER *bridge_id, UINT8 *buffer, UINT16 size); extern UINT32 stputil_get_default_path_cost(PORT_ID port_number, bool extend); extern UINT32 stputil_get_path_cost(STP_PORT_SPEED port_speed, bool extend); diff --git a/stp/stp_intf.c b/stp/stp_intf.c index e2122de..b0cc090 100755 --- a/stp/stp_intf.c +++ b/stp/stp_intf.c @@ -490,7 +490,7 @@ void stp_intf_netlink_cb(netlink_db_t *if_db, uint8_t is_add, bool init_in_prog) } else if ( STP_IS_PO_PORT( if_db->ifname ) ) { eth_if = false; } else { - // If it is e delete then if_db->ifname can be NULL since the + // If it is delete then if_db->ifname can be NULL since the // delete path relies on if_db->kif_index. For add we need to // have if_db->ifname if ( is_add ) { diff --git a/stp/stp_mgr.c b/stp/stp_mgr.c index 14a620b..172205a 100755 --- a/stp/stp_mgr.c +++ b/stp/stp_mgr.c @@ -1505,6 +1505,12 @@ void stpmgr_rx_pvst_bpdu(uint16_t vlan_id, uint32_t port_id, void *pkt) void stpmgr_process_rx_bpdu(uint16_t vlan_id, uint32_t port_id, unsigned char *pkt) { + // When BPDUs arrive without VLAN get the VLAN from port's untagged vlan + // configuration. + if ( vlan_id == 0 ) { + vlan_id = stputil_get_untag_vlan(port_id); + } + // sanity checks if (!IS_VALID_VLAN(vlan_id)) { diff --git a/stp/stp_netlink.c b/stp/stp_netlink.c index d44f7bc..0cbbb22 100644 --- a/stp/stp_netlink.c +++ b/stp/stp_netlink.c @@ -354,10 +354,10 @@ static int stp_netlink_recv(int nl_fd, bool read_all) if_db.master_ifindex = *(uint32_t *)RTA_DATA(ptr); } } - STP_LOG_INFO( "RTM-%s IF:%s KIF:%u Oper:%d Bond:%d Mem:%d Master:%u", - (nh->nlmsg_type == RTM_NEWLINK)?"UPDATE":"DELETE", - if_db.ifname, if_db.kif_index, if_db.oper_state, - if_db.is_bond, if_db.is_member, if_db.master_ifindex); + STP_LOG_DEBUG( "RTM-%s IF:%s KIF:%u Oper:%d Bond:%d Mem:%d Master:%u", + (nh->nlmsg_type == RTM_NEWLINK)?"UPDATE":"DELETE", + 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, ( ( nh->nlmsg_type == RTM_NEWLINK ) ? 1 : 0 ),