diff --git a/orchagent/fdborch.cpp b/orchagent/fdborch.cpp index 5ad21f1e4f2..69ee9b9e677 100644 --- a/orchagent/fdborch.cpp +++ b/orchagent/fdborch.cpp @@ -99,7 +99,7 @@ void FdbOrch::doTask(Consumer& consumer) FdbEntry entry; entry.mac = MacAddress(keys[1]); - entry.vlan = vlan.m_vlan_id; + entry.vlan = vlan.m_vlan_info.vlan_id; if (op == SET_COMMAND) { diff --git a/orchagent/intfsorch.cpp b/orchagent/intfsorch.cpp index 7cf44ccd6d4..c326c7ee8ae 100644 --- a/orchagent/intfsorch.cpp +++ b/orchagent/intfsorch.cpp @@ -246,7 +246,7 @@ bool IntfsOrch::addRouterIntfs(Port &port) break; case Port::VLAN: attr.id = SAI_ROUTER_INTERFACE_ATTR_VLAN_ID; - attr.value.oid = port.m_vlan_oid; + attr.value.oid = port.m_vlan_info.vlan_oid; break; default: SWSS_LOG_ERROR("Unsupported port type: %d", port.m_type); @@ -287,7 +287,8 @@ bool IntfsOrch::removeRouterIntfs(Port &port) port.m_rif_id = 0; gPortsOrch->setPort(port.m_alias, port); - + /* Clean this port from default VLAN */ + gPortsOrch->removeDefaultVlanMembers(); SWSS_LOG_NOTICE("Remove router interface for port %s", port.m_alias.c_str()); return true; diff --git a/orchagent/mirrororch.cpp b/orchagent/mirrororch.cpp index 8547c4b41c7..8e8f063fbf6 100644 --- a/orchagent/mirrororch.cpp +++ b/orchagent/mirrororch.cpp @@ -327,7 +327,7 @@ bool MirrorOrch::getNeighborInfo(const string& name, MirrorEntry& session, const if (session.neighborInfo.port.m_type == Port::VLAN) { - session.neighborInfo.vlanId = session.neighborInfo.port.m_vlan_id; + session.neighborInfo.vlanId = session.neighborInfo.port.m_vlan_info.vlan_id; Port member; if (!m_fdbOrch->getPort(session.neighborInfo.mac, session.neighborInfo.vlanId, member)) diff --git a/orchagent/port.h b/orchagent/port.h index 04113654308..c331389d7b1 100644 --- a/orchagent/port.h +++ b/orchagent/port.h @@ -8,11 +8,27 @@ extern "C" { #include #include #include +#include #define DEFAULT_PORT_VLAN_ID 1 namespace swss { +struct VlanMemberEntry +{ + sai_object_id_t vlan_member_id; + sai_vlan_tagging_mode_t vlan_mode; +}; + +typedef std::map port_vlan_members_t; + +struct VlanInfo +{ + sai_object_id_t vlan_oid; + sai_vlan_id_t vlan_id; + std::string autostate; +}; + class Port { public: @@ -53,17 +69,17 @@ class Port Type m_type; int m_index = 0; // PHY_PORT: index int m_ifindex = 0; + sai_uint32_t m_mtu; sai_object_id_t m_port_id = 0; - sai_object_id_t m_vlan_oid = 0; + VlanInfo m_vlan_info; sai_object_id_t m_bridge_port_id = 0; // TODO: port could have multiple bridge port IDs - sai_vlan_id_t m_vlan_id = 0; sai_vlan_id_t m_port_vlan_id = DEFAULT_PORT_VLAN_ID; // Port VLAN ID - sai_object_id_t m_vlan_member_id = 0; sai_object_id_t m_rif_id = 0; sai_object_id_t m_hif_id = 0; sai_object_id_t m_lag_id = 0; sai_object_id_t m_lag_member_id = 0; sai_object_id_t m_acl_table_group_id = 0; + port_vlan_members_t m_vlan_members; std::set m_members; std::vector m_queue_ids; std::vector m_priority_group_ids; diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index eb96c1239ad..eb69dcfb91e 100644 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -24,6 +24,11 @@ extern sai_object_id_t gSwitchId; #define VLAN_PREFIX "Vlan" #define DEFAULT_VLAN_ID 1 +static char* hostif_vlan_tag[] = { + [SAI_HOSTIF_VLAN_TAG_STRIP] = "SAI_HOSTIF_VLAN_TAG_STRIP", + [SAI_HOSTIF_VLAN_TAG_KEEP] = "SAI_HOSTIF_VLAN_TAG_KEEP", + [SAI_HOSTIF_VLAN_TAG_ORIGINAL] = "SAI_HOSTIF_VLAN_TAG_ORIGINAL" +}; /* * Initialize PortsOrch * 0) By default, a switch has one CPU port, one 802.1Q bridge, and one default @@ -174,7 +179,7 @@ void PortsOrch::removeDefaultVlanMembers() } } - SWSS_LOG_NOTICE("Remove VLAN members from default VLAN"); + SWSS_LOG_NOTICE("Remove %d VLAN members from default VLAN", attr.value.objlist.count); } void PortsOrch::removeDefaultBridgePorts() @@ -345,6 +350,113 @@ bool PortsOrch::setPortMtu(sai_object_id_t id, sai_uint32_t mtu) return true; } +bool PortsOrch::setPortPvid (Port &port, sai_uint32_t pvid) +{ + SWSS_LOG_ENTER(); + vector portv; + + if (port.m_type == Port::PHY) + { + portv.push_back(port); + } + else if (port.m_type == Port::LAG) + { + getLagMember(port, portv); + } + else + { + SWSS_LOG_ERROR("PortsOrch::setPortPvid port type %d not supported", port.m_type); + return false; + } + + for (const auto p: portv) + { + sai_attribute_t attr; + attr.id = SAI_PORT_ATTR_PORT_VLAN_ID; + attr.value.u32 = pvid; + + sai_status_t status = sai_port_api->set_port_attribute(p.m_port_id, &attr); + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Failed to set pvid %u to port: %s", attr.value.u32, p.m_alias.c_str()); + return false; + } + SWSS_LOG_NOTICE("Set pvid %u to port: %s", attr.value.u32, p.m_alias.c_str()); + } + return true; +} + +bool PortsOrch::getPortPvid(Port &port, sai_uint32_t &pvid) +{ + /* Just return false if the router interface exists */ + if (port.m_rif_id) + { + SWSS_LOG_DEBUG("Router interface exists on %s, don't set pvid", + port.m_alias.c_str()); + return false; + } + + if (port.m_port_vlan_id != DEFAULT_PORT_VLAN_ID) + { + pvid = port.m_port_vlan_id; + return true; + } + + if (port.m_vlan_members.size() == 0) + { + pvid = DEFAULT_PORT_VLAN_ID; + return true; + } + + for (auto &vme: port.m_vlan_members) + { + if(vme.second.vlan_mode == SAI_VLAN_TAGGING_MODE_UNTAGGED) { + pvid = vme.first; + return true; + } + } + SWSS_LOG_ERROR("Failed to get pvid for port %s", port.m_alias.c_str()); + return false; +} +bool PortsOrch::setHostIntfsStripTag(Port &port, sai_hostif_vlan_tag_t strip) +{ + SWSS_LOG_ENTER(); + vector portv; + + if (port.m_type == Port::PHY) + { + portv.push_back(port); + } + else if (port.m_type == Port::LAG) + { + getLagMember(port, portv); + } + else + { + SWSS_LOG_ERROR("PortsOrch::setPortPvid port type %d not supported", port.m_type); + return false; + } + + for (const auto p: portv) + { + sai_attribute_t attr; + attr.id = SAI_HOSTIF_ATTR_VLAN_TAG; + attr.value.s32 = strip; + + sai_status_t status = sai_hostif_api->set_hostif_attribute(p.m_hif_id, &attr); + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Failed to set %s to host interface %s", + hostif_vlan_tag[strip], p.m_alias.c_str()); + return false; + } + SWSS_LOG_NOTICE("Set %s to host interface: %s", + hostif_vlan_tag[strip], p.m_alias.c_str()); + } + + return true; +} + bool PortsOrch::validatePortSpeed(sai_object_id_t port_id, sai_uint32_t speed) { sai_attribute_t attr; @@ -499,6 +611,7 @@ void PortsOrch::doPortTask(Consumer &consumer) set lane_set; string admin_status; uint32_t mtu = 0; + uint32_t pvid = 0; uint32_t speed = 0; for (auto i : kfvFieldsValues(t)) @@ -525,6 +638,10 @@ void PortsOrch::doPortTask(Consumer &consumer) if (fvField(i) == "mtu") mtu = (uint32_t)stoul(fvValue(i)); + /* Set port pvid */ + if (fvField(i) == "pvid") + pvid = (uint32_t)stoul(fvValue(i)); + /* Set port speed */ if (fvField(i) == "speed") speed = (uint32_t)stoul(fvValue(i)); @@ -636,7 +753,10 @@ void PortsOrch::doPortTask(Consumer &consumer) if (mtu != 0) { if (setPortMtu(p.m_port_id, mtu)) + { + p.m_mtu = mtu; SWSS_LOG_NOTICE("Set port %s MTU to %u", alias.c_str(), mtu); + } else { SWSS_LOG_ERROR("Failed to set port %s MTU to %u", alias.c_str(), mtu); @@ -644,6 +764,21 @@ void PortsOrch::doPortTask(Consumer &consumer) continue; } } + + if (pvid != 0) + { + if (setPortPvid(p, pvid)) + { + p.m_port_vlan_id = (sai_vlan_id_t)pvid; + SWSS_LOG_NOTICE("Set port %s pvid to %u", alias.c_str(), pvid); + } + else + { + SWSS_LOG_ERROR("Failed to set port %s pvid to %u", alias.c_str(), pvid); + it++; + continue; + } + } } } else @@ -682,24 +817,58 @@ void PortsOrch::doVlanTask(Consumer &consumer) if (op == SET_COMMAND) { - /* Duplicate entry */ - if (m_portList.find(vlan_alias) != m_portList.end()) + if (m_portList.find(vlan_alias) == m_portList.end()) { - it = consumer.m_toSync.erase(it); - continue; + if (!addVlan(vlan_alias)) + { + it++; + continue; + } } - if (addVlan(vlan_alias)) - it = consumer.m_toSync.erase(it); - else - it++; + string autostate; + uint32_t mtu = 0; + for (auto i : kfvFieldsValues(t)) + { + /* Set port autostate */ + if (fvField(i) == "autostate") + { + autostate = fvValue(i); + } + + /* Set port mtu */ + if (fvField(i) == "mtu") + { + mtu = (uint32_t)stoul(fvValue(i)); + } + + // TODO: unicast_miss_flood, multicast_miss_flood, broadcast_miss_flood + } + + if (autostate != "") + { + Port v; + getPort(vlan_alias, v); + v.m_vlan_info.autostate = autostate; + // TODO: update VLAN interface operational state based on autostate and member state. + } + + if (mtu != 0) + { + Port v; + getPort(vlan_alias, v); + v.m_mtu = mtu; + // TODO: update vlan routeif mtu + } + + it = consumer.m_toSync.erase(it); } else if (op == DEL_COMMAND) { Port vlan; getPort(vlan_alias, vlan); - if (removeVlan(vlan) && removeBridgePort(vlan)) + if (removeVlan(vlan)) it = consumer.m_toSync.erase(it); else it++; @@ -796,9 +965,6 @@ void PortsOrch::doVlanMemberTask(Consumer &consumer) continue; } - /* Assert the port doesn't belong to any VLAN */ - assert(!port.m_vlan_id && !port.m_vlan_member_id); - if (addBridgePort(port) && addVlanMember(vlan, port, tagging_mode)) it = consumer.m_toSync.erase(it); else @@ -808,10 +974,7 @@ void PortsOrch::doVlanMemberTask(Consumer &consumer) { if (vlan.m_members.find(port_alias) != vlan.m_members.end()) { - /* Assert the port belongs the a VLAN */ - assert(port.m_vlan_id && port.m_vlan_member_id); - - if (removeVlanMember(vlan, port)) + if (removeVlanMember(vlan, port) && removeBridgePort(port)) it = consumer.m_toSync.erase(it); else it++; @@ -970,6 +1133,8 @@ void PortsOrch::doLagMemberTask(Consumer &consumer) if (!port.m_lag_id || !port.m_lag_member_id) { + SWSS_LOG_WARN("Member %s not found in LAG %s lid:%lx lmid:%lx,", + port.m_alias.c_str(), lag.m_alias.c_str(), lag.m_lag_id, port.m_lag_member_id); it = consumer.m_toSync.erase(it); continue; } @@ -1083,7 +1248,7 @@ bool PortsOrch::initializePort(Port &p) initializeQueues(p); /* Create host interface */ - addHostIntfs(p.m_port_id, p.m_alias, p.m_hif_id); + addHostIntfs(p, p.m_alias, p.m_hif_id); #if 0 // TODO: Assure if_nametoindex(p.m_alias.c_str()) != 0 @@ -1111,7 +1276,7 @@ bool PortsOrch::initializePort(Port &p) return true; } -bool PortsOrch::addHostIntfs(sai_object_id_t id, string alias, sai_object_id_t &host_intfs_id) +bool PortsOrch::addHostIntfs(Port &port, string alias, sai_object_id_t &host_intfs_id) { SWSS_LOG_ENTER(); @@ -1123,7 +1288,7 @@ bool PortsOrch::addHostIntfs(sai_object_id_t id, string alias, sai_object_id_t & attrs.push_back(attr); attr.id = SAI_HOSTIF_ATTR_OBJ_ID; - attr.value.oid = id; + attr.value.oid = port.m_port_id; attrs.push_back(attr); attr.id = SAI_HOSTIF_ATTR_NAME; @@ -1137,6 +1302,11 @@ bool PortsOrch::addHostIntfs(sai_object_id_t id, string alias, sai_object_id_t & return false; } + /* + * Port has been removed from 1q bridge at PortsOrch constructor, + * also start stipping off VLAN tag. + */ + setHostIntfsStripTag(port, SAI_HOSTIF_VLAN_TAG_STRIP); SWSS_LOG_NOTICE("Create host interface for port %s", alias.c_str()); return true; @@ -1146,6 +1316,9 @@ bool PortsOrch::addBridgePort(Port &port) { SWSS_LOG_ENTER(); + if (port.m_bridge_port_id) + return true; + sai_attribute_t attr; vector attrs; @@ -1154,7 +1327,20 @@ bool PortsOrch::addBridgePort(Port &port) attrs.push_back(attr); attr.id = SAI_BRIDGE_PORT_ATTR_PORT_ID; - attr.value.oid = port.m_port_id; + if (port.m_type == Port::PHY) + { + attr.value.oid = port.m_port_id; + } + else if (port.m_type == Port::LAG) + { + attr.value.oid = port.m_lag_id; + } + else + { + SWSS_LOG_ERROR("Failed to add bridge port %s to default 1Q bridge, invalid porty type %d", + port.m_alias.c_str(), port.m_type); + return false; + } attrs.push_back(attr); /* Create a bridge port with admin status set to UP */ @@ -1170,15 +1356,32 @@ bool PortsOrch::addBridgePort(Port &port) return false; } + bool rv = setHostIntfsStripTag(port, SAI_HOSTIF_VLAN_TAG_KEEP); + if (rv != true) + { + SWSS_LOG_ERROR("Failed to set %s for hostif of port %s", + hostif_vlan_tag[SAI_HOSTIF_VLAN_TAG_KEEP], port.m_alias.c_str()); + return false; + } + m_portList[port.m_alias] = port; SWSS_LOG_NOTICE("Add bridge port %s to default 1Q bridge", port.m_alias.c_str()); return true; } -bool PortsOrch::removeBridgePort(Port port) +bool PortsOrch::removeBridgePort(Port &port) { SWSS_LOG_ENTER(); + if (port.m_vlan_members.size() > 0) + { + for (auto &vme: port.m_vlan_members) + { + SWSS_LOG_DEBUG("Port %s still in VLAN %d", port.m_alias.c_str(), vme.first); + } + return true; + } + /* Set bridge port admin status to DOWN */ sai_attribute_t attr; attr.id = SAI_BRIDGE_PORT_ATTR_ADMIN_STATE; @@ -1192,6 +1395,14 @@ bool PortsOrch::removeBridgePort(Port port) return false; } + bool rv = setHostIntfsStripTag(port, SAI_HOSTIF_VLAN_TAG_STRIP); + if (rv != true) + { + SWSS_LOG_ERROR("Failed to set %s for hostif of port %s", + hostif_vlan_tag[SAI_HOSTIF_VLAN_TAG_STRIP], port.m_alias.c_str()); + return false; + } + /* Flush FDB entries pointing to this bridge port */ // TODO: Remove all FDB entries associated with this bridge port before // removing the bridge port itself @@ -1204,9 +1415,11 @@ bool PortsOrch::removeBridgePort(Port port) port.m_alias.c_str(), status); return false; } + port.m_bridge_port_id = 0; SWSS_LOG_NOTICE("Remove bridge port %s from default 1Q bridge", port.m_alias.c_str()); + m_portList[port.m_alias] = port; return true; } @@ -1231,8 +1444,10 @@ bool PortsOrch::addVlan(string vlan_alias) SWSS_LOG_NOTICE("Create an empty VLAN %s vid:%hu", vlan_alias.c_str(), vlan_id); Port vlan(vlan_alias, Port::VLAN); - vlan.m_vlan_oid = vlan_oid; - vlan.m_vlan_id = vlan_id; + vlan.m_vlan_info.vlan_oid = vlan_oid; + vlan.m_vlan_info.vlan_id = vlan_id; + vlan.m_vlan_info.autostate = "disabled"; + vlan.m_mtu = 1500; vlan.m_members = set(); m_portList[vlan_alias] = vlan; @@ -1250,21 +1465,23 @@ bool PortsOrch::removeVlan(Port vlan) return false; } - sai_status_t status = sai_vlan_api->remove_vlan(vlan.m_vlan_oid); + sai_status_t status = sai_vlan_api->remove_vlan(vlan.m_vlan_info.vlan_oid); if (status != SAI_STATUS_SUCCESS) { - SWSS_LOG_ERROR("Failed to remove VLAN %s vid:%hu", vlan.m_alias.c_str(), vlan.m_vlan_id); + SWSS_LOG_ERROR("Failed to remove VLAN %s vid:%hu", + vlan.m_alias.c_str(), vlan.m_vlan_info.vlan_id); return false; } - SWSS_LOG_NOTICE("Remove VLAN %s vid:%hu", vlan.m_alias.c_str(), vlan.m_vlan_id); + SWSS_LOG_NOTICE("Remove VLAN %s vid:%hu", vlan.m_alias.c_str(), + vlan.m_vlan_info.vlan_id); m_portList.erase(vlan.m_alias); return true; } -bool PortsOrch::addVlanMember(Port vlan, Port port, string& tagging_mode) +bool PortsOrch::addVlanMember(Port &vlan, Port &port, string &tagging_mode) { SWSS_LOG_ENTER(); @@ -1272,21 +1489,23 @@ bool PortsOrch::addVlanMember(Port vlan, Port port, string& tagging_mode) vector attrs; attr.id = SAI_VLAN_MEMBER_ATTR_VLAN_ID; - attr.value.oid = vlan.m_vlan_oid; + attr.value.oid = vlan.m_vlan_info.vlan_oid; attrs.push_back(attr); attr.id = SAI_VLAN_MEMBER_ATTR_BRIDGE_PORT_ID; attr.value.oid = port.m_bridge_port_id; attrs.push_back(attr); + sai_vlan_tagging_mode_t sai_tagging_mode = SAI_VLAN_TAGGING_MODE_TAGGED; attr.id = SAI_VLAN_MEMBER_ATTR_VLAN_TAGGING_MODE; if (tagging_mode == "untagged") - attr.value.s32 = SAI_VLAN_TAGGING_MODE_UNTAGGED; + sai_tagging_mode = SAI_VLAN_TAGGING_MODE_UNTAGGED; else if (tagging_mode == "tagged") - attr.value.s32 = SAI_VLAN_TAGGING_MODE_TAGGED; + sai_tagging_mode = SAI_VLAN_TAGGING_MODE_TAGGED; else if (tagging_mode == "priority_tagged") - attr.value.s32 = SAI_VLAN_TAGGING_MODE_PRIORITY_TAGGED; + sai_tagging_mode = SAI_VLAN_TAGGING_MODE_PRIORITY_TAGGED; else assert(false); + attr.value.s32 = sai_tagging_mode; attrs.push_back(attr); sai_object_id_t vlan_member_id; @@ -1294,31 +1513,24 @@ bool PortsOrch::addVlanMember(Port vlan, Port port, string& tagging_mode) if (status != SAI_STATUS_SUCCESS) { SWSS_LOG_ERROR("Failed to add member %s to VLAN %s vid:%hu pid:%lx", - port.m_alias.c_str(), vlan.m_alias.c_str(), vlan.m_vlan_id, port.m_port_id); + port.m_alias.c_str(), vlan.m_alias.c_str(), vlan.m_vlan_info.vlan_id, port.m_port_id); return false; } SWSS_LOG_NOTICE("Add member %s to VLAN %s vid:%hu pid%lx", - port.m_alias.c_str(), vlan.m_alias.c_str(), vlan.m_vlan_id, port.m_port_id); + port.m_alias.c_str(), vlan.m_alias.c_str(), vlan.m_vlan_info.vlan_id, port.m_port_id); - if (tagging_mode == "untagged") // set pvlan id for untagged port only + // Use untagged VLAN as pvid of the member port when no pvid has been configured explictly + if (port.m_port_vlan_id == DEFAULT_PORT_VLAN_ID && sai_tagging_mode == SAI_VLAN_TAGGING_MODE_UNTAGGED) { - attr.id = SAI_PORT_ATTR_PORT_VLAN_ID; - attr.value.u16 = vlan.m_vlan_id; - - status = sai_port_api->set_port_attribute(port.m_port_id, &attr); - if (status != SAI_STATUS_SUCCESS) + if(!setPortPvid(port, vlan.m_vlan_info.vlan_id)) { - SWSS_LOG_ERROR("Failed to set port VLAN ID vid:%hu pid:%lx", - vlan.m_vlan_id, port.m_port_id); return false; } - - SWSS_LOG_NOTICE("Set untagged port %s VLAN ID to %hu", port.m_alias.c_str(), vlan.m_vlan_id); } - port.m_vlan_id = vlan.m_vlan_id; - port.m_port_vlan_id = vlan.m_vlan_id; - port.m_vlan_member_id = vlan_member_id; + // a physical port may join multiple vlans + VlanMemberEntry vme = {vlan_member_id, sai_tagging_mode}; + port.m_vlan_members[vlan.m_vlan_info.vlan_id] = vme; m_portList[port.m_alias] = port; vlan.m_members.insert(port.m_alias); m_portList[vlan.m_alias] = vlan; @@ -1329,37 +1541,39 @@ bool PortsOrch::addVlanMember(Port vlan, Port port, string& tagging_mode) return true; } -bool PortsOrch::removeVlanMember(Port vlan, Port port) +bool PortsOrch::removeVlanMember(Port &vlan, Port &port) { SWSS_LOG_ENTER(); - sai_status_t status = sai_vlan_api->remove_vlan_member(port.m_vlan_member_id); + sai_object_id_t vlan_member_id; + sai_vlan_tagging_mode_t sai_tagging_mode; + auto vlan_member = port.m_vlan_members.find(vlan.m_vlan_info.vlan_id); + + /* Assert the port belongs to this VLAN */ + assert (vlan_member != port.m_vlan_members.end()); + sai_tagging_mode = vlan_member->second.vlan_mode; + vlan_member_id = vlan_member->second.vlan_member_id; + sai_status_t status = sai_vlan_api->remove_vlan_member(vlan_member_id); if (status != SAI_STATUS_SUCCESS) { SWSS_LOG_ERROR("Failed to remove member %s from VLAN %s vid:%hx vmid:%lx", - port.m_alias.c_str(), vlan.m_alias.c_str(), vlan.m_vlan_id, port.m_vlan_member_id); + port.m_alias.c_str(), vlan.m_alias.c_str(), vlan.m_vlan_info.vlan_id, vlan_member_id); return false; } - + port.m_vlan_members.erase(vlan_member); SWSS_LOG_NOTICE("Remove member %s from VLAN %s lid:%hx vmid:%lx", - port.m_alias.c_str(), vlan.m_alias.c_str(), vlan.m_vlan_id, port.m_vlan_member_id); - - sai_attribute_t attr; - attr.id = SAI_PORT_ATTR_PORT_VLAN_ID; - attr.value.u16 = DEFAULT_PORT_VLAN_ID; + port.m_alias.c_str(), vlan.m_alias.c_str(), vlan.m_vlan_info.vlan_id, vlan_member_id); - status = sai_port_api->set_port_attribute(port.m_port_id, &attr); - if (status != SAI_STATUS_SUCCESS) + // Restore to default pvid if no pvid has been configured explictly and this port was in untagged mode + if (port.m_port_vlan_id == DEFAULT_PORT_VLAN_ID && sai_tagging_mode == SAI_VLAN_TAGGING_MODE_UNTAGGED) { - SWSS_LOG_ERROR("Failed to reset port VLAN ID to DEFAULT_PORT_VLAN_ID pid:%lx", - port.m_port_id); - return false; + if (!setPortPvid(port, DEFAULT_PORT_VLAN_ID)) + { + return false; + } } - port.m_vlan_id = 0; - port.m_port_vlan_id = DEFAULT_PORT_VLAN_ID; - port.m_vlan_member_id = 0; m_portList[port.m_alias] = port; vlan.m_members.erase(port.m_alias); m_portList[vlan.m_alias] = vlan; @@ -1403,6 +1617,11 @@ bool PortsOrch::removeLag(Port lag) SWSS_LOG_ERROR("Failed to remove non-empty LAG %s", lag.m_alias.c_str()); return false; } + if (lag.m_vlan_members.size() > 0) + { + SWSS_LOG_ERROR("Failed to remove LAG %s, it is still in VLAN", lag.m_alias.c_str()); + return false; + } sai_status_t status = sai_lag_api->remove_lag(lag.m_lag_id); if (status != SAI_STATUS_SUCCESS) @@ -1418,7 +1637,22 @@ bool PortsOrch::removeLag(Port lag) return true; } -bool PortsOrch::addLagMember(Port lag, Port port) +void PortsOrch::getLagMember(Port &lag, vector &portv) +{ + Port member; + + for (auto &name: lag.m_members) + { + if (!getPort(name, member)) + { + SWSS_LOG_ERROR("Failed to get port for %s alias\n", name.c_str()); + return; + } + portv.push_back(member); + } +} + +bool PortsOrch::addLagMember(Port &lag, Port &port) { SWSS_LOG_ENTER(); @@ -1453,13 +1687,27 @@ bool PortsOrch::addLagMember(Port lag, Port port) m_portList[lag.m_alias] = lag; + if (lag.m_vlan_members.size() > 0) + { + bool rv = setHostIntfsStripTag(port, SAI_HOSTIF_VLAN_TAG_KEEP); + if (rv != true) + { + SWSS_LOG_ERROR("Failed to set %s for hostif of port %s which is in LAG %s", + hostif_vlan_tag[SAI_HOSTIF_VLAN_TAG_KEEP], port.m_alias.c_str(), lag.m_alias.c_str()); + return false; + } + } + sai_uint32_t pvid = DEFAULT_PORT_VLAN_ID; + if (getPortPvid(lag, pvid)) + setPortPvid (port, pvid); + LagMemberUpdate update = { lag, port, true }; notify(SUBJECT_TYPE_LAG_MEMBER_CHANGE, static_cast(&update)); return true; } -bool PortsOrch::removeLagMember(Port lag, Port port) +bool PortsOrch::removeLagMember(Port &lag, Port &port) { sai_status_t status = sai_lag_api->remove_lag_member(port.m_lag_member_id); @@ -1479,6 +1727,16 @@ bool PortsOrch::removeLagMember(Port lag, Port port) lag.m_members.erase(port.m_alias); m_portList[lag.m_alias] = lag; + if (lag.m_vlan_members.size() > 0) + { + bool rv = setHostIntfsStripTag(port, SAI_HOSTIF_VLAN_TAG_STRIP); + if (rv != true) + { + SWSS_LOG_ERROR("Failed to set %s for hostif of port %s which is leaving LAG %s", + hostif_vlan_tag[SAI_HOSTIF_VLAN_TAG_STRIP], port.m_alias.c_str(), lag.m_alias.c_str()); + return false; + } + } LagMemberUpdate update = { lag, port, false }; notify(SUBJECT_TYPE_LAG_MEMBER_CHANGE, static_cast(&update)); diff --git a/orchagent/portsorch.h b/orchagent/portsorch.h index b5e78297e27..decb7a5f128 100644 --- a/orchagent/portsorch.h +++ b/orchagent/portsorch.h @@ -55,6 +55,7 @@ class PortsOrch : public Orch, public Subject bool setHostIntfsOperStatus(sai_object_id_t id, bool up); void updateDbPortOperStatus(sai_object_id_t id, sai_port_oper_status_t status); + void removeDefaultVlanMembers(); private: unique_ptr m_counterTable; unique_ptr
m_portTable; @@ -78,30 +79,32 @@ class PortsOrch : public Orch, public Subject void doLagTask(Consumer &consumer); void doLagMemberTask(Consumer &consumer); - void removeDefaultVlanMembers(); void removeDefaultBridgePorts(); bool initializePort(Port &port); void initializePriorityGroups(Port &port); void initializeQueues(Port &port); - bool addHostIntfs(sai_object_id_t router_intfs_id, string alias, sai_object_id_t &host_intfs_id); + bool addHostIntfs(Port &port, string alias, sai_object_id_t &host_intfs_id); + bool setHostIntfsStripTag(Port &port, sai_hostif_vlan_tag_t strip); bool addBridgePort(Port &port); - bool removeBridgePort(Port port); + bool removeBridgePort(Port &port); bool addVlan(string vlan); bool removeVlan(Port vlan); - bool addVlanMember(Port vlan, Port port, string& tagging_mode); - bool removeVlanMember(Port vlan, Port port); + bool addVlanMember(Port &vlan, Port &port, string& tagging_mode); + bool removeVlanMember(Port &vlan, Port &port); bool addLag(string lag); bool removeLag(Port lag); - bool addLagMember(Port lag, Port port); - bool removeLagMember(Port lag, Port port); - + bool addLagMember(Port &lag, Port &port); + bool removeLagMember(Port &lag, Port &port); + void getLagMember(Port &lag, vector &portv); bool setPortAdminStatus(sai_object_id_t id, bool up); bool setPortMtu(sai_object_id_t id, sai_uint32_t mtu); + bool setPortPvid (Port &port, sai_uint32_t pvid); + bool getPortPvid(Port &port, sai_uint32_t &pvid); bool setBridgePortAdminStatus(sai_object_id_t id, bool up);