Skip to content

[portsorch] fix errors when moving port from one lag to another. #1797

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

Merged
liat-grozovik merged 11 commits into from
Jul 8, 2021
Merged

[portsorch] fix errors when moving port from one lag to another. #1797

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
eee6941
[portsorch] fix errors when moving port from one lag to another.
stepanblyschak Jun 17, 2021
89cbcbd
add ut
Jun 22, 2021
9ffcc88
Merge branch 'master' of github.com:azure/sonic-swss into lag-mem-add…
stepanblyschak Jun 22, 2021
2b14813
Merge branch 'master' of https://github.com/Azure/sonic-swss into HEAD
stepanblyschak Jun 30, 2021
a084a19
remove assert
stepanblyschak Jul 2, 2021
480b576
Merge branch 'master' of https://github.com/Azure/sonic-swss into HEAD
stepanblyschak Jul 2, 2021
820714f
add test description
stepanblyschak Jul 2, 2021
a7e44f0
change log levl to INFO
stepanblyschak Jul 2, 2021
460d4b0
fix comment
stepanblyschak Jul 7, 2021
681c822
Merge branch 'master' of github.com:azure/sonic-swss into lag-mem-add…
stepanblyschak Jul 7, 2021
736f739
remove changes added during merge resolving
stepanblyschak Jul 7, 2021
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
53 changes: 36 additions & 17 deletions orchagent/portsorch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,12 @@ static char* hostif_vlan_tag[] = {
[SAI_HOSTIF_VLAN_TAG_KEEP] = "SAI_HOSTIF_VLAN_TAG_KEEP",
[SAI_HOSTIF_VLAN_TAG_ORIGINAL] = "SAI_HOSTIF_VLAN_TAG_ORIGINAL"
};

static bool isValidPortTypeForLagMember(const Port& port)
{
return (port.m_type == Port::Type::PHY || port.m_type == Port::Type::SYSTEM);
}

/*
* Initialize PortsOrch
* 0) If Gearbox is enabled, then initialize the external PHYs as defined in
Expand Down Expand Up @@ -1722,7 +1728,7 @@ void PortsOrch::getPortSupportedSpeeds(const std::string& alias, sai_object_id_t
}

// if our guess was wrong, retry with the correct value
speeds.resize(attr.value.u32list.count);
speeds.resize(attr.value.u32list.count);
}

if (status == SAI_STATUS_SUCCESS)
Expand Down Expand Up @@ -2193,7 +2199,7 @@ sai_status_t PortsOrch::removePort(sai_object_id_t port_id)

Port port;

/*
/*
* Make sure to bring down admin state.
* SET would have replaced with DEL
*/
Expand Down Expand Up @@ -2775,7 +2781,7 @@ void PortsOrch::doPortTask(Consumer &consumer)
it = consumer.m_toSync.erase(it);
continue;
}

an = autoneg_mode_map[an_str];
if (an != p.m_autoneg)
{
Expand Down Expand Up @@ -2841,7 +2847,7 @@ void PortsOrch::doPortTask(Consumer &consumer)
it++;
continue;
}

SWSS_LOG_NOTICE("Set port %s speed from %u to %u", alias.c_str(), p.m_speed, speed);
p.m_speed = speed;
m_portList[alias] = p;
Expand Down Expand Up @@ -2882,7 +2888,7 @@ void PortsOrch::doPortTask(Consumer &consumer)
auto ori_adv_speeds = swss::join(',', p.m_adv_speeds.begin(), p.m_adv_speeds.end());
if (!setPortAdvSpeeds(p.m_port_id, adv_speeds))
{

SWSS_LOG_ERROR("Failed to set port %s advertised speed from %s to %s", alias.c_str(),
ori_adv_speeds.c_str(),
adv_speeds_str.c_str());
Expand Down Expand Up @@ -3676,6 +3682,15 @@ void PortsOrch::doLagMemberTask(Consumer &consumer)
continue;
}

/* Fast failure if a port type is not a valid type for beeing a LAG member port.
Copy link
Collaborator

@prsunny prsunny Jul 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please revisit the comment? What is fast failure? Also beeing -> typo

Copy link
Contributor Author

@stepanblyschak stepanblyschak Jul 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

* Erase invalid entry, no need to retry in this case. */
if (!isValidPortTypeForLagMember(port))
{
SWSS_LOG_ERROR("LAG member port has to be of type PHY or SYSTEM");
it = consumer.m_toSync.erase(it);
continue;
}

if (table_name == CHASSIS_APP_LAG_MEMBER_TABLE_NAME)
{
int32_t lag_switch_id = lag.m_system_lag_info.switch_id;
Expand Down Expand Up @@ -3709,8 +3724,12 @@ void PortsOrch::doLagMemberTask(Consumer &consumer)

if (lag.m_members.find(port_alias) == lag.m_members.end())
{
/* Assert the port doesn't belong to any LAG already */
assert(!port.m_lag_id && !port.m_lag_member_id);
if (port.m_lag_member_id != SAI_NULL_OBJECT_ID)
{
SWSS_LOG_INFO("Port %s is already a LAG member", port.m_alias.c_str());
it++;
continue;
}

if (!addLagMember(lag, port, (status == "enabled")))
{
Expand Down Expand Up @@ -5510,7 +5529,7 @@ void PortsOrch::getPortSerdesVal(const std::string& val_str,
}
}

bool PortsOrch::getPortAdvSpeedsVal(const std::string &val_str,
bool PortsOrch::getPortAdvSpeedsVal(const std::string &val_str,
std::vector<uint32_t> &speed_values)
{
SWSS_LOG_ENTER();
Expand All @@ -5524,7 +5543,7 @@ bool PortsOrch::getPortAdvSpeedsVal(const std::string &val_str,
std::string speed_str;
std::istringstream iss(val_str);

try
try
{
while (std::getline(iss, speed_str, ','))
{
Expand All @@ -5541,31 +5560,31 @@ bool PortsOrch::getPortAdvSpeedsVal(const std::string &val_str,
return true;
}

bool PortsOrch::getPortInterfaceTypeVal(const std::string &s,
bool PortsOrch::getPortInterfaceTypeVal(const std::string &s,
sai_port_interface_type_t &interface_type)
{
SWSS_LOG_ENTER();

auto iter = interface_type_map_for_an.find(s);
if (iter != interface_type_map_for_an.end())
if (iter != interface_type_map_for_an.end())
{
interface_type = interface_type_map_for_an[s];
return true;
}
else
else
{
const std::string &validInterfaceTypes = getValidInterfaceTypes();
SWSS_LOG_ERROR("Failed to parse interface_type value %s, valid interface type includes: %s",
SWSS_LOG_ERROR("Failed to parse interface_type value %s, valid interface type includes: %s",
s.c_str(), validInterfaceTypes.c_str());
return false;
}
}

bool PortsOrch::getPortAdvInterfaceTypesVal(const std::string &val_str,
bool PortsOrch::getPortAdvInterfaceTypesVal(const std::string &val_str,
std::vector<uint32_t> &type_values)
{
SWSS_LOG_ENTER();
if (val_str == "all")
if (val_str == "all")
{
return true;
}
Expand All @@ -5580,7 +5599,7 @@ bool PortsOrch::getPortAdvInterfaceTypesVal(const std::string &val_str,
valid = getPortInterfaceTypeVal(type_str, interface_type);
if (!valid) {
const std::string &validInterfaceTypes = getValidInterfaceTypes();
SWSS_LOG_ERROR("Failed to parse adv_interface_types value %s, valid interface type includes: %s",
SWSS_LOG_ERROR("Failed to parse adv_interface_types value %s, valid interface type includes: %s",
val_str.c_str(), validInterfaceTypes.c_str());
return false;
}
Expand Down Expand Up @@ -5967,7 +5986,7 @@ bool PortsOrch::doProcessRecircPort(string alias, string role, set<int> lane_set
return true;
}

/* Find pid of recirc port */
/* Find pid of recirc port */
sai_object_id_t port_id = SAI_NULL_OBJECT_ID;
if (m_portListLaneMap.find(lane_set) != m_portListLaneMap.end())
{
Expand Down
Loading