diff --git a/cfgmgr/stpmgr.cpp b/cfgmgr/stpmgr.cpp index b27bb5c15af..8565579f0c9 100644 --- a/cfgmgr/stpmgr.cpp +++ b/cfgmgr/stpmgr.cpp @@ -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 &tables) : Orch(tables), @@ -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) @@ -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()); } @@ -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; } @@ -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); @@ -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; } @@ -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; @@ -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); @@ -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(vlan_ids.size()); len = sizeof(STP_MST_INST_CONFIG_MSG) + static_cast(vlan_count * sizeof(VLAN_LIST)); @@ -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) @@ -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)); diff --git a/cfgmgr/stpmgr.h b/cfgmgr/stpmgr.h index 7360dc63453..8a8b0f94cd6 100644 --- a/cfgmgr/stpmgr.h +++ b/cfgmgr/stpmgr.h @@ -17,13 +17,7 @@ #include "producerstatetable.h" #include #include - -// We remove PACKED definitions, only keep ALIGNED -#if defined(__GNUC__) -#define ALIGNED(x) __attribute__((aligned(x))) -#else -#define ALIGNED(x) -#endif +#include #define STPMGRD_SOCK_NAME "/var/run/stpmgrd.sock" @@ -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