Skip to content

Commit 86b4ede

Browse files
Junchao-MellanoxJunchao-Mellanox
authored
[portsorch] Avoid orchagent crash when set invalid interface types to port (sonic-net#1906)
What I did When setting invalid interface types to port, do not call handleSaiSetStatus to avoid orchagent exit. Why I did it Currently, when setting invalid interface types to port, orchagent would crash, this is not necessary since user still has chance to correct it
1 parent 025032f commit 86b4ede

3 files changed

Lines changed: 104 additions & 58 deletions

File tree

‎orchagent/orch.cpp‎

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -748,16 +748,36 @@ task_process_status Orch::handleSaiSetStatus(sai_api_t api, sai_status_t status,
748748
* in each orch.
749749
* 3. Take the type of sai api into consideration.
750750
*/
751-
switch (status)
751+
if (status == SAI_STATUS_SUCCESS)
752752
{
753-
case SAI_STATUS_SUCCESS:
754-
SWSS_LOG_WARN("SAI_STATUS_SUCCESS is not expected in handleSaiSetStatus");
755-
return task_success;
753+
SWSS_LOG_WARN("SAI_STATUS_SUCCESS is not expected in handleSaiSetStatus");
754+
return task_success;
755+
}
756+
757+
switch (api)
758+
{
759+
case SAI_API_PORT:
760+
switch (status)
761+
{
762+
case SAI_STATUS_INVALID_ATTR_VALUE_0:
763+
/*
764+
* If user gives an invalid attribute value, no need to retry or exit orchagent, just fail the current task
765+
* and let user correct the configuration.
766+
*/
767+
SWSS_LOG_ERROR("Encountered SAI_STATUS_INVALID_ATTR_VALUE_0 in set operation, task failed, SAI API: %s, status: %s",
768+
sai_serialize_api(api).c_str(), sai_serialize_status(status).c_str());
769+
return task_failed;
770+
default:
771+
SWSS_LOG_ERROR("Encountered failure in set operation, exiting orchagent, SAI API: %s, status: %s",
772+
sai_serialize_api(api).c_str(), sai_serialize_status(status).c_str());
773+
exit(EXIT_FAILURE);
774+
}
756775
default:
757776
SWSS_LOG_ERROR("Encountered failure in set operation, exiting orchagent, SAI API: %s, status: %s",
758777
sai_serialize_api(api).c_str(), sai_serialize_status(status).c_str());
759778
exit(EXIT_FAILURE);
760779
}
780+
761781
return task_need_retry;
762782
}
763783

‎orchagent/portsorch.cpp‎

Lines changed: 75 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1919,7 +1919,7 @@ bool PortsOrch::setGearboxPortAttr(Port &port, dest_port_type_t port_type, sai_p
19191919
return true;
19201920
}
19211921

1922-
bool PortsOrch::setPortSpeed(Port &port, sai_uint32_t speed)
1922+
task_process_status PortsOrch::setPortSpeed(Port &port, sai_uint32_t speed)
19231923
{
19241924
sai_attribute_t attr;
19251925
sai_status_t status;
@@ -1932,15 +1932,11 @@ bool PortsOrch::setPortSpeed(Port &port, sai_uint32_t speed)
19321932
status = sai_port_api->set_port_attribute(port.m_port_id, &attr);
19331933
if (status != SAI_STATUS_SUCCESS)
19341934
{
1935-
task_process_status handle_status = handleSaiSetStatus(SAI_API_PORT, status);
1936-
if (handle_status != task_success)
1937-
{
1938-
return parseHandleSaiStatusFailure(handle_status);
1939-
}
1935+
return handleSaiSetStatus(SAI_API_PORT, status);
19401936
}
19411937

19421938
setGearboxPortsAttr(port, SAI_PORT_ATTR_SPEED, &speed);
1943-
return true;
1939+
return task_success;
19441940
}
19451941

19461942
bool PortsOrch::getPortSpeed(sai_object_id_t id, sai_uint32_t &speed)
@@ -1973,7 +1969,7 @@ bool PortsOrch::getPortSpeed(sai_object_id_t id, sai_uint32_t &speed)
19731969
return true;
19741970
}
19751971

1976-
bool PortsOrch::setPortAdvSpeeds(sai_object_id_t port_id, std::vector<sai_uint32_t>& speed_list)
1972+
task_process_status PortsOrch::setPortAdvSpeeds(sai_object_id_t port_id, std::vector<sai_uint32_t>& speed_list)
19771973
{
19781974
SWSS_LOG_ENTER();
19791975
sai_attribute_t attr;
@@ -1984,11 +1980,15 @@ bool PortsOrch::setPortAdvSpeeds(sai_object_id_t port_id, std::vector<sai_uint32
19841980
attr.value.u32list.count = static_cast<uint32_t>(speed_list.size());
19851981

19861982
status = sai_port_api->set_port_attribute(port_id, &attr);
1983+
if (status != SAI_STATUS_SUCCESS)
1984+
{
1985+
return handleSaiSetStatus(SAI_API_PORT, status);
1986+
}
19871987

1988-
return status == SAI_STATUS_SUCCESS;
1988+
return task_success;
19891989
}
19901990

1991-
bool PortsOrch::setPortInterfaceType(sai_object_id_t port_id, sai_port_interface_type_t interface_type)
1991+
task_process_status PortsOrch::setPortInterfaceType(sai_object_id_t port_id, sai_port_interface_type_t interface_type)
19921992
{
19931993
SWSS_LOG_ENTER();
19941994
sai_attribute_t attr;
@@ -1998,11 +1998,15 @@ bool PortsOrch::setPortInterfaceType(sai_object_id_t port_id, sai_port_interface
19981998
attr.value.u32 = static_cast<uint32_t>(interface_type);
19991999

20002000
status = sai_port_api->set_port_attribute(port_id, &attr);
2001+
if (status != SAI_STATUS_SUCCESS)
2002+
{
2003+
return handleSaiSetStatus(SAI_API_PORT, status);
2004+
}
20012005

2002-
return status == SAI_STATUS_SUCCESS;
2006+
return task_success;
20032007
}
20042008

2005-
bool PortsOrch::setPortAdvInterfaceTypes(sai_object_id_t port_id, std::vector<uint32_t> &interface_types)
2009+
task_process_status PortsOrch::setPortAdvInterfaceTypes(sai_object_id_t port_id, std::vector<uint32_t> &interface_types)
20062010
{
20072011
SWSS_LOG_ENTER();
20082012
sai_attribute_t attr;
@@ -2015,14 +2019,10 @@ bool PortsOrch::setPortAdvInterfaceTypes(sai_object_id_t port_id, std::vector<ui
20152019
status = sai_port_api->set_port_attribute(port_id, &attr);
20162020
if (status != SAI_STATUS_SUCCESS)
20172021
{
2018-
task_process_status handle_status = handleSaiSetStatus(SAI_API_PORT, status);
2019-
if (handle_status != task_success)
2020-
{
2021-
return parseHandleSaiStatusFailure(handle_status);
2022-
}
2022+
return handleSaiSetStatus(SAI_API_PORT, status);
20232023
}
20242024

2025-
return true;
2025+
return task_success;
20262026
}
20272027

20282028
bool PortsOrch::getQueueTypeAndIndex(sai_object_id_t queue_id, string &type, uint8_t &index)
@@ -2065,33 +2065,22 @@ bool PortsOrch::getQueueTypeAndIndex(sai_object_id_t queue_id, string &type, uin
20652065
return true;
20662066
}
20672067

2068-
bool PortsOrch::setPortAutoNeg(sai_object_id_t id, int an)
2068+
task_process_status PortsOrch::setPortAutoNeg(sai_object_id_t id, int an)
20692069
{
20702070
SWSS_LOG_ENTER();
20712071

20722072
sai_attribute_t attr;
20732073
attr.id = SAI_PORT_ATTR_AUTO_NEG_MODE;
2074-
switch(an) {
2075-
case 1:
2076-
attr.value.booldata = true;
2077-
break;
2078-
default:
2079-
attr.value.booldata = false;
2080-
break;
2081-
}
2074+
attr.value.booldata = (an == 1 ? true : false);
20822075

20832076
sai_status_t status = sai_port_api->set_port_attribute(id, &attr);
20842077
if (status != SAI_STATUS_SUCCESS)
20852078
{
20862079
SWSS_LOG_ERROR("Failed to set AutoNeg %u to port pid:%" PRIx64, attr.value.booldata, id);
2087-
task_process_status handle_status = handleSaiSetStatus(SAI_API_PORT, status);
2088-
if (handle_status != task_success)
2089-
{
2090-
return parseHandleSaiStatusFailure(handle_status);
2091-
}
2080+
return handleSaiSetStatus(SAI_API_PORT, status);
20922081
}
20932082
SWSS_LOG_INFO("Set AutoNeg %u to port pid:%" PRIx64, attr.value.booldata, id);
2094-
return true;
2083+
return task_success;
20952084
}
20962085

20972086
bool PortsOrch::setHostIntfsOperStatus(const Port& port, bool isUp) const
@@ -2827,18 +2816,23 @@ void PortsOrch::doPortTask(Consumer &consumer)
28272816
m_portList[alias] = p;
28282817
}
28292818

2830-
if (setPortAutoNeg(p.m_port_id, an))
2831-
{
2832-
SWSS_LOG_NOTICE("Set port %s AutoNeg from %d to %d", alias.c_str(), p.m_autoneg, an);
2833-
p.m_autoneg = an;
2834-
m_portList[alias] = p;
2835-
}
2836-
else
2819+
auto status = setPortAutoNeg(p.m_port_id, an);
2820+
if (status != task_success)
28372821
{
28382822
SWSS_LOG_ERROR("Failed to set port %s AN from %d to %d", alias.c_str(), p.m_autoneg, an);
2839-
it++;
2823+
if (status == task_need_retry)
2824+
{
2825+
it++;
2826+
}
2827+
else
2828+
{
2829+
it = consumer.m_toSync.erase(it);
2830+
}
28402831
continue;
28412832
}
2833+
SWSS_LOG_NOTICE("Set port %s AutoNeg from %d to %d", alias.c_str(), p.m_autoneg, an);
2834+
p.m_autoneg = an;
2835+
m_portList[alias] = p;
28422836
}
28432837
}
28442838

@@ -2869,10 +2863,18 @@ void PortsOrch::doPortTask(Consumer &consumer)
28692863
m_portList[alias] = p;
28702864
}
28712865

2872-
if (!setPortSpeed(p, speed))
2866+
auto status = setPortSpeed(p, speed);
2867+
if (status != task_success)
28732868
{
28742869
SWSS_LOG_ERROR("Failed to set port %s speed from %u to %u", alias.c_str(), p.m_speed, speed);
2875-
it++;
2870+
if (status == task_need_retry)
2871+
{
2872+
it++;
2873+
}
2874+
else
2875+
{
2876+
it = consumer.m_toSync.erase(it);
2877+
}
28762878
continue;
28772879
}
28782880

@@ -2914,13 +2916,21 @@ void PortsOrch::doPortTask(Consumer &consumer)
29142916
}
29152917

29162918
auto ori_adv_speeds = swss::join(',', p.m_adv_speeds.begin(), p.m_adv_speeds.end());
2917-
if (!setPortAdvSpeeds(p.m_port_id, adv_speeds))
2919+
auto status = setPortAdvSpeeds(p.m_port_id, adv_speeds);
2920+
if (status != task_success)
29182921
{
29192922

29202923
SWSS_LOG_ERROR("Failed to set port %s advertised speed from %s to %s", alias.c_str(),
29212924
ori_adv_speeds.c_str(),
29222925
adv_speeds_str.c_str());
2923-
it++;
2926+
if (status == task_need_retry)
2927+
{
2928+
it++;
2929+
}
2930+
else
2931+
{
2932+
it = consumer.m_toSync.erase(it);
2933+
}
29242934
continue;
29252935
}
29262936
SWSS_LOG_NOTICE("Set port %s advertised speed from %s to %s", alias.c_str(),
@@ -2957,10 +2967,18 @@ void PortsOrch::doPortTask(Consumer &consumer)
29572967
m_portList[alias] = p;
29582968
}
29592969

2960-
if (!setPortInterfaceType(p.m_port_id, interface_type))
2970+
auto status = setPortInterfaceType(p.m_port_id, interface_type);
2971+
if (status != task_success)
29612972
{
29622973
SWSS_LOG_ERROR("Failed to set port %s interface type to %s", alias.c_str(), interface_type_str.c_str());
2963-
it++;
2974+
if (status == task_need_retry)
2975+
{
2976+
it++;
2977+
}
2978+
else
2979+
{
2980+
it = consumer.m_toSync.erase(it);
2981+
}
29642982
continue;
29652983
}
29662984

@@ -2996,10 +3014,18 @@ void PortsOrch::doPortTask(Consumer &consumer)
29963014
m_portList[alias] = p;
29973015
}
29983016

2999-
if (!setPortAdvInterfaceTypes(p.m_port_id, adv_interface_types))
3017+
auto status = setPortAdvInterfaceTypes(p.m_port_id, adv_interface_types);
3018+
if (status != task_success)
30003019
{
30013020
SWSS_LOG_ERROR("Failed to set port %s advertised interface type to %s", alias.c_str(), adv_interface_types_str.c_str());
3002-
it++;
3021+
if (status == task_need_retry)
3022+
{
3023+
it++;
3024+
}
3025+
else
3026+
{
3027+
it = consumer.m_toSync.erase(it);
3028+
}
30033029
continue;
30043030
}
30053031

‎orchagent/portsorch.h‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -278,12 +278,12 @@ class PortsOrch : public Orch, public Subject
278278
bool isSpeedSupported(const std::string& alias, sai_object_id_t port_id, sai_uint32_t speed);
279279
void getPortSupportedSpeeds(const std::string& alias, sai_object_id_t port_id, PortSupportedSpeeds &supported_speeds);
280280
void initPortSupportedSpeeds(const std::string& alias, sai_object_id_t port_id);
281-
bool setPortSpeed(Port &port, sai_uint32_t speed);
281+
task_process_status setPortSpeed(Port &port, sai_uint32_t speed);
282282
bool getPortSpeed(sai_object_id_t id, sai_uint32_t &speed);
283283
bool setGearboxPortsAttr(Port &port, sai_port_attr_t id, void *value);
284284
bool setGearboxPortAttr(Port &port, dest_port_type_t port_type, sai_port_attr_t id, void *value);
285285

286-
bool setPortAdvSpeeds(sai_object_id_t port_id, std::vector<sai_uint32_t>& speed_list);
286+
task_process_status setPortAdvSpeeds(sai_object_id_t port_id, std::vector<sai_uint32_t>& speed_list);
287287

288288
bool getQueueTypeAndIndex(sai_object_id_t queue_id, string &type, uint8_t &index);
289289

@@ -296,10 +296,10 @@ class PortsOrch : public Orch, public Subject
296296
bool m_isPortCounterMapGenerated = false;
297297
bool m_isPortBufferDropCounterMapGenerated = false;
298298

299-
bool setPortAutoNeg(sai_object_id_t id, int an);
299+
task_process_status setPortAutoNeg(sai_object_id_t id, int an);
300300
bool setPortFecMode(sai_object_id_t id, int fec);
301-
bool setPortInterfaceType(sai_object_id_t id, sai_port_interface_type_t interface_type);
302-
bool setPortAdvInterfaceTypes(sai_object_id_t id, std::vector<uint32_t> &interface_types);
301+
task_process_status setPortInterfaceType(sai_object_id_t id, sai_port_interface_type_t interface_type);
302+
task_process_status setPortAdvInterfaceTypes(sai_object_id_t id, std::vector<uint32_t> &interface_types);
303303

304304
bool getPortOperStatus(const Port& port, sai_port_oper_status_t& status) const;
305305
void updatePortOperStatus(Port &port, sai_port_oper_status_t status);

0 commit comments

Comments
 (0)