Skip to content

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

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
rdhegde wants to merge 7 commits into
base: master
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
73f7dcb
stp : fix: Prevent duplication of stp ipc structures.
rdhegde Jul 9, 2026
4218f96
stp : fix : Populate "proto_mode" in STP_IPC_MSG instance.
rdhegde Jul 9, 2026
b0e906d
stp: fix: Fix parameter reinitialization and code duplication
rdhegde Jul 9, 2026
eef4646
stp : fix : Updated stpmgr daemon main loop to return success ( 0 )
rdhegde Jul 9, 2026
cbe969e
stp:fix: Replace ebtables filtering with nft
rdhegde Jul 9, 2026
4371ebf
Handled review comments in
rdhegde Jul 14, 2026
cf9ef34
Revert unnecessary stpmgrd.cpp main() return value change
rdhegde Jul 28, 2026
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
69 changes: 47 additions & 22 deletions cfgmgr/stpmgr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,42 @@
using namespace std;
using namespace swss;

namespace {

// Dedicated bridge nft table: drop PVST+ PDUs (dst 01:00:0c:cc:cc:cd) on forward.
// Table is removed on restore so add path stays idempotent.
constexpr char pvstNftBridgeTable[] = "sonic_stp_pvst";

void preventPvstPduFlooding()
{
std::string res;
const std::string tableName(pvstNftBridgeTable);
const std::string cmd =
"nft delete table bridge " + tableName + " ; "
"nft add table bridge " + tableName + " && "
"nft 'add chain bridge " + tableName +
" forward { type filter hook forward priority 0; policy accept; }' && "
"nft add rule bridge " + tableName + " forward ether daddr 01:00:0c:cc:cc:cd drop";

int ret = swss::exec(cmd, res);
if ( ret != 0 ) {
SWSS_LOG_ERROR( "Failed to install nft rule to prevent PVST PDU flooding - %d", ret );
}
}

void restorePvstPduFlooding()
{
std::string res;
const std::string cmd =
std::string("nft delete table bridge ") + pvstNftBridgeTable;
int ret = swss::exec(cmd, res);
if ( ret != 0 ) {
SWSS_LOG_ERROR( "Failed to remove nft rule which prevented PVST PDU flooding - %d", ret );
}
}

} // namespace

StpMgr::StpMgr(DBConnector *confDb, DBConnector *applDb, DBConnector *statDb,
const vector<TableConnector> &tables) :
Orch(tables),
Expand All @@ -44,8 +80,7 @@ StpMgr::StpMgr(DBConnector *confDb, DBConnector *applDb, DBConnector *statDb,
// Initialize all VLANs to Invalid instance
fill_n(m_vlanInstMap, MAX_VLANS, INVALID_INSTANCE);

int ret = system("ebtables -D FORWARD -d 01:00:0c:cc:cc:cd -j DROP");
SWSS_LOG_DEBUG("ebtables ret %d", ret);
restorePvstPduFlooding();
}

void StpMgr::doTask(Consumer &consumer)
Expand All @@ -72,8 +107,6 @@ void StpMgr::doTask(Consumer &consumer)
doStpMstInstTask(consumer);
else if (table == "STP_MST_PORT")
doStpMstInstPortTask(consumer);
else if (table == CFG_STP_PORT_TABLE_NAME)
doStpPortTask(consumer);
else
SWSS_LOG_ERROR("Invalid table %s", table.c_str());
}
Expand Down Expand Up @@ -109,14 +142,9 @@ void StpMgr::doStpGlobalTask(Consumer &consumer)
{
if (l2ProtoEnabled == L2_NONE)
{
const std::string cmd = std::string("") +
" ebtables -A FORWARD -d 01:00:0c:cc:cc:cd -j DROP";
std::string res;
int ret = swss::exec(cmd, res);
if (ret != 0)
SWSS_LOG_ERROR("ebtables add failed for PVST %d", ret);

preventPvstPduFlooding();
l2ProtoEnabled = L2_PVSTP;
SWSS_LOG_DEBUG( "Enabling PVST" );
}
msg.stp_mode = L2_PVSTP;
}
Expand All @@ -127,6 +155,7 @@ void StpMgr::doStpGlobalTask(Consumer &consumer)
l2ProtoEnabled = L2_MSTP;
}
msg.stp_mode = L2_MSTP;
SWSS_LOG_DEBUG( "Enabling MSTP" );

// Assign all VLANs to zero instance for MSTP
fill_n(m_vlanInstMap, MAX_VLANS, 0);
Expand Down Expand Up @@ -154,15 +183,9 @@ void StpMgr::doStpGlobalTask(Consumer &consumer)
// Initialize all VLANs to Invalid instance
fill_n(m_vlanInstMap, MAX_VLANS, INVALID_INSTANCE);

// Remove ebtables rule based on protocol mode
if (l2ProtoEnabled == L2_PVSTP)
{
const std::string pvst_cmd =
"ebtables -D FORWARD -d 01:00:0c:cc:cc:cd -j DROP";
std::string res_pvst;
int ret_pvst = swss::exec(pvst_cmd, res_pvst);
if (ret_pvst != 0)
SWSS_LOG_ERROR("ebtables del failed for PVST %d", ret_pvst);
restorePvstPduFlooding();
}
l2ProtoEnabled = L2_NONE;
}
Expand Down Expand Up @@ -689,6 +712,7 @@ void StpMgr::doVlanMemUpdateTask(Consumer &consumer)
{
STP_VLAN_MEM_CONFIG_MSG msg;
memset(&msg, 0, sizeof(STP_VLAN_MEM_CONFIG_MSG));
msg.priority = -1;

KeyOpFieldsValuesTuple t = it->second;

Expand Down Expand Up @@ -749,8 +773,6 @@ void StpMgr::doVlanMemUpdateTask(Consumer &consumer)
msg.vlan_id = vlan_id;
msg.inst_id = m_vlanInstMap[vlan_id];
msg.mode = tagging_mode;
msg.priority = -1;
msg.path_cost = 0;

strncpy(msg.intf_name, intfName.c_str(), IFNAMSIZ-1);

Expand Down Expand Up @@ -1068,9 +1090,10 @@ void StpMgr::doStpMstInstTask(Consumer &consumer)
vlan_list_str = fvValue(i);
vlan_ids = parseVlanList(vlan_list_str);
}
updateVlanInstanceMap(instance_id, vlan_ids, true);
}

updateVlanInstanceMap(instance_id, vlan_ids, true);

uint32_t vlan_count = static_cast<uint32_t>(vlan_ids.size());
len = sizeof(STP_MST_INST_CONFIG_MSG) + static_cast<uint32_t>(vlan_count * sizeof(VLAN_LIST));

Expand Down Expand Up @@ -1227,7 +1250,8 @@ int StpMgr::sendMsgStpd(STP_MSG_TYPE msgType, uint32_t msgLen, void *data)
int rc;

len = msgLen + (offsetof(struct STP_IPC_MSG, data));
SWSS_LOG_INFO("tx_msg len %d msglen %d", (int)len, msgLen);
SWSS_LOG_INFO( "tx_msg len %d msglen %u for proto_mode %d",
(int)len, msgLen, l2ProtoEnabled );

tx_msg = (STP_IPC_MSG *)calloc(1, len);
if (tx_msg == NULL)
Expand All @@ -1238,6 +1262,7 @@ int StpMgr::sendMsgStpd(STP_MSG_TYPE msgType, uint32_t msgLen, void *data)

tx_msg->msg_type = msgType;
tx_msg->msg_len = msgLen;
tx_msg->proto_mode = l2ProtoEnabled;
memcpy(tx_msg->data, data, msgLen);

bzero(&addr, sizeof(addr));
Expand Down
188 changes: 1 addition & 187 deletions cfgmgr/stpmgr.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,7 @@
#include "producerstatetable.h"
#include <stddef.h>
#include <algorithm>

// We remove PACKED definitions, only keep ALIGNED
#if defined(__GNUC__)
#define ALIGNED(x) __attribute__((aligned(x)))
#else
#define ALIGNED(x)
#endif
#include <stp_ipc.h>

#define STPMGRD_SOCK_NAME "/var/run/stpmgrd.sock"

Expand All @@ -46,186 +40,6 @@
#define FREE_ALL_INST_ID() l2InstPool.reset()
#define IS_INST_ID_AVAILABLE() (l2InstPool.count() < max_stp_instances)

#define STPD_SOCK_NAME "/var/run/stpipc.sock"

// Enumerations must match stp_ipc.h
typedef enum L2_PROTO_MODE {
L2_NONE,
L2_PVSTP,
L2_MSTP
}L2_PROTO_MODE;


typedef enum LinkType {
AUTO = 0, // Auto
POINT_TO_POINT = 1, // Point-to-point
SHARED = 2 // Shared
} LinkType;

typedef enum STP_MSG_TYPE {
STP_INVALID_MSG,
STP_INIT_READY,
STP_BRIDGE_CONFIG,
STP_VLAN_CONFIG,
STP_VLAN_PORT_CONFIG,
STP_PORT_CONFIG,
STP_VLAN_MEM_CONFIG,
STP_STPCTL_MSG,
STP_MST_GLOBAL_CONFIG,
STP_MST_INST_CONFIG,
STP_MST_INST_PORT_CONFIG,
STP_MAX_MSG
} STP_MSG_TYPE;

typedef enum STP_CTL_TYPE {
STP_CTL_HELP,
STP_CTL_DUMP_ALL,
STP_CTL_DUMP_GLOBAL,
STP_CTL_DUMP_VLAN_ALL,
STP_CTL_DUMP_VLAN,
STP_CTL_DUMP_INTF,
STP_CTL_SET_LOG_LVL,
STP_CTL_DUMP_NL_DB,
STP_CTL_DUMP_NL_DB_INTF,
STP_CTL_DUMP_LIBEV_STATS,
STP_CTL_SET_DBG,
STP_CTL_CLEAR_ALL,
STP_CTL_CLEAR_VLAN,
STP_CTL_CLEAR_INTF,
STP_CTL_CLEAR_VLAN_INTF,
STP_CTL_MAX
} STP_CTL_TYPE;

// Remove PACKED, add ALIGNED(4)
typedef struct STP_IPC_MSG {
int msg_type;
unsigned int msg_len;
L2_PROTO_MODE proto_mode;
char data[0];
} ALIGNED(4) STP_IPC_MSG;

#define STP_SET_COMMAND 1
#define STP_DEL_COMMAND 0

// Add padding for alignment if needed (compare to stp_ipc.h)
typedef struct STP_INIT_READY_MSG {
uint8_t opcode; // enable/disable
uint16_t max_stp_instances;
// Example: potential extra padding if alignment warnings arise
// uint8_t padding[1];
} ALIGNED(4) STP_INIT_READY_MSG;

// Add padding for alignment if needed
typedef struct STP_BRIDGE_CONFIG_MSG {
uint8_t opcode; // enable/disable
uint8_t stp_mode;
int rootguard_timeout;
uint8_t base_mac_addr[6];
// Potential padding for alignment:
// uint8_t padding[2];
} ALIGNED(4) STP_BRIDGE_CONFIG_MSG;

// Must match the version in stp_ipc.h exactly
typedef struct PORT_ATTR {
char intf_name[IFNAMSIZ]; // 16 bytes typically
int8_t mode;
uint8_t enabled;
// Add padding to align to 4 bytes
uint16_t padding;
} ALIGNED(4) PORT_ATTR;

// Must match the version in stp_ipc.h exactly
typedef struct STP_VLAN_CONFIG_MSG {
uint8_t opcode; // enable/disable
uint8_t newInstance;
int vlan_id;
int inst_id;
int forward_delay;
int hello_time;
int max_age;
int priority;
int count;
PORT_ATTR port_list[0];
} ALIGNED(4) STP_VLAN_CONFIG_MSG;

typedef struct STP_VLAN_PORT_CONFIG_MSG {
uint8_t opcode; // enable/disable
int vlan_id;
char intf_name[IFNAMSIZ];
int inst_id;
int path_cost;
int priority;
} ALIGNED(4) STP_VLAN_PORT_CONFIG_MSG;

typedef struct VLAN_ATTR {
int inst_id;
int vlan_id;
int8_t mode;
// Add padding to align to 4 bytes
uint8_t padding[3];
} ALIGNED(4) VLAN_ATTR;

typedef struct VLAN_LIST{
uint16_t vlan_id;
}VLAN_LIST;

typedef struct STP_PORT_CONFIG_MSG {
uint8_t opcode; // enable/disable
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; // PVST only
uint8_t uplink_fast; // PVST only
uint8_t edge_port; // MSTP only
LinkType link_type; // MSTP only
int path_cost;
int priority;
int count;
VLAN_ATTR vlan_list[0];
} STP_PORT_CONFIG_MSG;

typedef struct STP_VLAN_MEM_CONFIG_MSG {
uint8_t opcode; // enable/disable
int vlan_id;
int inst_id;
char intf_name[IFNAMSIZ];
uint8_t enabled;
int8_t mode;
// Add 1 byte padding
uint8_t padding;
int path_cost;
int priority;
} ALIGNED(4) STP_VLAN_MEM_CONFIG_MSG;

typedef struct STP_MST_GLOBAL_CONFIG_MSG {
uint8_t opcode; // enable/disable
uint32_t revision_number;
char name[32];
uint8_t forward_delay;
uint8_t hello_time;
uint8_t max_age;
uint8_t max_hops;
}__attribute__ ((packed))STP_MST_GLOBAL_CONFIG_MSG;

typedef struct STP_MST_INST_CONFIG_MSG {
uint8_t opcode; // enable/disable
uint16_t mst_id; // MST instance ID
int priority; // Bridge priority
uint16_t vlan_count; // Number of VLANs in this instance
VLAN_LIST vlan_list[0]; // Flexible array for VLAN IDs
}__attribute__((packed)) STP_MST_INST_CONFIG_MSG;

typedef struct STP_MST_INST_PORT_CONFIG_MSG {
uint8_t opcode; // enable/disable
char intf_name[IFNAMSIZ]; // Interface name
uint16_t mst_id; // MST instance ID
int path_cost; // Path cost
int priority; // Port priority
} __attribute__((packed)) STP_MST_INST_PORT_CONFIG_MSG;

namespace swss {

class StpMgr : public Orch
Expand Down
Loading