Skip to content
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

Jump to bottom

Add support for configuring loopback mode (PHY and MAC) #3095

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add support for configuring loopback mode (PHY and MAC) #3095

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions orchagent/port.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ class Port
uint32_t m_maximum_headroom = 0;
std::set<uint32_t> m_adv_speeds;
sai_port_interface_type_t m_interface_type = SAI_PORT_INTERFACE_TYPE_NONE;
sai_port_loopback_mode_t m_loopback_mode = SAI_PORT_LOOPBACK_MODE_NONE;
std::set<sai_port_interface_type_t> m_adv_interface_types;
bool m_mpls = false;
/*
Expand Down
5 changes: 5 additions & 0 deletions orchagent/port/portcnt.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,11 @@ class PortConfig final
bool is_set = false;
} pt_timestamp_template; // Port timestamp template for Path Tracing

struct {
sai_port_loopback_mode_t value;
bool is_set = false;
} loopback_mode; // Port loopback mode

struct {
sai_redis_link_event_damping_algorithm_t value;
bool is_set = false;
Expand Down
43 changes: 43 additions & 0 deletions orchagent/port/porthlpr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,14 @@ static const std::unordered_map<std::string, sai_port_path_tracing_timestamp_typ
{ PORT_PT_TIMESTAMP_TEMPLATE_4, SAI_PORT_PATH_TRACING_TIMESTAMP_TYPE_20_27 }
};

static const std::unordered_map<std::string, sai_port_loopback_mode_t> portLoopbackModeMap = {
{"none", SAI_PORT_LOOPBACK_MODE_NONE},
{"phy_local", SAI_PORT_LOOPBACK_MODE_PHY},
Copy link
Contributor

Choose a reason for hiding this comment

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

@royyi8 How is it different from SAI_PORT_INTERNAL_LOOPBACK_MODE_PHY

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same reason as above. SAI_PORT_LOOPBACK_MODE_PHY is the corresponding attribute for SAI_PORT_INTERNAL_LOOPBACK_MODE_PHY in the sai_port_loopback_mode_t enum.

{"mac_local", SAI_PORT_LOOPBACK_MODE_MAC},
Copy link
Contributor

Choose a reason for hiding this comment

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

@royyi8 how is it different from SAI_PORT_INTERNAL_LOOPBACK_MODE_MAC

Copy link
Contributor Author

Choose a reason for hiding this comment

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

SAI_PORT_LOOPBACK_MODE_MAC is the corresponding attribute for SAI_PORT_INTERNAL_LOOPBACK_MODE_MAC. We are using sai_port_loopback_mode_t enum instead of _sai_port_internal_loopback_mode_t since the later is to be deprecated in SAI.

https://github.com/opencomputeproject/SAI/blob/master/inc/saiport.h#L114

{"phy_remote", SAI_PORT_LOOPBACK_MODE_PHY_REMOTE},
{"mac_remote", SAI_PORT_LOOPBACK_MODE_MAC_REMOTE},
};

static const std::unordered_map<std::string, sai_redis_link_event_damping_algorithm_t> g_linkEventDampingAlgorithmMap =
{
{ "disabled", SAI_REDIS_LINK_EVENT_DAMPING_ALGORITHM_DISABLED },
Expand Down Expand Up @@ -253,6 +261,11 @@ std::string PortHelper::getPtTimestampTemplateStr(const PortConfig &port) const
return this->getFieldValueStr(port, PORT_PT_TIMESTAMP_TEMPLATE);
}

std::string PortHelper::getLoopbackModeStr(const PortConfig &port) const
{
return this->getFieldValueStr(port, PORT_LOOPBACK_MODE);
}

std::string PortHelper::getDampingAlgorithm(const PortConfig &port) const
{
return this->getFieldValueStr(port, PORT_DAMPING_ALGO);
Expand Down Expand Up @@ -919,6 +932,29 @@ bool PortHelper::parsePortPtTimestampTemplate(PortConfig &port, const std::strin
return true;
}

bool PortHelper::parsePortLoopbackMode(PortConfig &port, const std::string &field, const std::string &value) const
{
SWSS_LOG_ENTER();

if (value.empty())
{
SWSS_LOG_ERROR("Failed to parse field(%s): empty value is prohibited", field.c_str());
return false;
}

const auto &cit = portLoopbackModeMap.find(value);
if (cit == portLoopbackModeMap.cend())
{
SWSS_LOG_ERROR("Failed to parse field(%s): invalid value(%s)", field.c_str(), value.c_str());
return false;
}

port.loopback_mode.value = cit->second;
port.loopback_mode.is_set = true;

return true;
}

bool PortHelper::parsePortConfig(PortConfig &port) const
{
SWSS_LOG_ENTER();
Expand Down Expand Up @@ -1187,6 +1223,13 @@ bool PortHelper::parsePortConfig(PortConfig &port) const
return false;
}
}
else if (field == PORT_LOOPBACK_MODE)
{
if (!this->parsePortLoopbackMode(port, field, value))
{
return false;
}
}
else if (field == PORT_DAMPING_ALGO)
{
if (!this->parsePortLinkEventDampingAlgorithm(port, field, value))
Expand Down
2 changes: 2 additions & 0 deletions orchagent/port/porthlpr.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ class PortHelper final
std::string getLinkTrainingStr(const PortConfig &port) const;
std::string getAdminStatusStr(const PortConfig &port) const;
std::string getPtTimestampTemplateStr(const PortConfig &port) const;
std::string getLoopbackModeStr(const PortConfig &port) const;
std::string getDampingAlgorithm(const PortConfig &port) const;

bool parsePortConfig(PortConfig &port) const;
Expand Down Expand Up @@ -62,4 +63,5 @@ class PortHelper final
bool parsePortSubport(PortConfig &port, const std::string &field, const std::string &value) const;
bool parsePortPtIntfId(PortConfig &port, const std::string &field, const std::string &value) const;
bool parsePortPtTimestampTemplate(PortConfig &port, const std::string &field, const std::string &value) const;
bool parsePortLoopbackMode(PortConfig &port, const std::string &field, const std::string &value) const;
};
1 change: 1 addition & 0 deletions orchagent/port/portschema.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@
#define PORT_SUBPORT "subport"
#define PORT_PT_INTF_ID "pt_interface_id"
#define PORT_PT_TIMESTAMP_TEMPLATE "pt_timestamp_template"
#define PORT_LOOPBACK_MODE "loopback_mode"
#define PORT_DAMPING_ALGO "link_event_damping_algorithm"
#define PORT_MAX_SUPPRESS_TIME "max_suppress_time"
#define PORT_DECAY_HALF_LIFE "decay_half_life"
Expand Down
45 changes: 45 additions & 0 deletions orchagent/portsorch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4595,6 +4595,30 @@ void PortsOrch::doPortTask(Consumer &consumer)
}
}

if (pCfg.loopback_mode.is_set)
{
if (p.m_loopback_mode != pCfg.loopback_mode.value)
{
auto status = setPortLoopbackMode(p.m_port_id, pCfg.loopback_mode.value);
if (status.ok())
{
SWSS_LOG_NOTICE("Set port %s loopback mode to %s",
p.m_alias.c_str(),
m_portHlpr.getLoopbackModeStr(pCfg).c_str());
p.m_loopback_mode = pCfg.loopback_mode.value;
m_portList[p.m_alias] = p;
}
else
{
SWSS_LOG_ERROR(
"Failed to set port %s loopback mode to %s",
p.m_alias.c_str(), m_portHlpr.getLoopbackModeStr(pCfg).c_str());
it = taskMap.erase(it);
continue;
}
}
}

/* create host_tx_ready field in state-db */
initHostTxReadyState(p);

Expand Down Expand Up @@ -8528,6 +8552,27 @@ void PortsOrch::getPortSerdesVal(const std::string& val_str,
}
}

ReturnCode PortsOrch::setPortLoopbackMode(sai_object_id_t id, sai_port_loopback_mode_t loopback_mode)
{
SWSS_LOG_ENTER();

sai_attribute_t attr;
attr.id = SAI_PORT_ATTR_LOOPBACK_MODE;
attr.value.u32 = loopback_mode;

sai_status_t status = sai_port_api->set_port_attribute(id, &attr);
if (status != SAI_STATUS_SUCCESS)
{
LOG_ERROR_AND_RETURN(ReturnCode(status)
Copy link
Contributor

Choose a reason for hiding this comment

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

@royyi8 this won't crash OA right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that is correct. It logs the error and returns from the current function.

https://github.com/sonic-net/sonic-swss/blob/master/orchagent/return_code.h#L50

Copy link
Contributor

Choose a reason for hiding this comment

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

@royyi8 can we avoid crash. Why crash OA if one port fails?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@prgeor To clarify, the code will not crash OA. It will simply log an error and return status failed in the setPortLoopbackMode function. OA will proceed in the else block on line 4319.

<< "Failed to set loopback mode " << attr.value.u32
<< " to port pid 0x" << std::hex << id
<< ", rv: " << sai_serialize_status(status));
}

SWSS_LOG_INFO("Set loopback mode %u on port pid: %" PRIx64, attr.value.u32, id);
return ReturnCode();
}

/* Bring up/down Vlan interface associated with L3 VNI*/
bool PortsOrch::updateL3VniStatus(uint16_t vlan_id, bool isUp)
{
Expand Down
1 change: 1 addition & 0 deletions orchagent/portsorch.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,7 @@ class PortsOrch : public Orch, public Subject

ReturnCode addSendToIngressHostIf(const std::string &send_to_ingress_name);
ReturnCode removeSendToIngressHostIf();
ReturnCode setPortLoopbackMode(sai_object_id_t id, sai_port_loopback_mode_t loopback_mode);
void initGearbox();
bool initGearboxPort(Port &port);
bool getPortOperFec(const Port& port, sai_port_fec_mode_t &fec_mode) const;
Expand Down
102 changes: 102 additions & 0 deletions tests/test_port.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,108 @@ def test_PortPathTracing(self, dvs, testlog):
for key, queue in buffer_queues.items():
dvs.get_config_db().update_entry("BUFFER_QUEUE", key, queue)

def test_InvalidPortLoopbackMode(self, dvs, testlog):
pdb = swsscommon.DBConnector(0, dvs.redis_sock, 0)
adb = swsscommon.DBConnector(1, dvs.redis_sock, 0)

table_name = "PORT_TABLE"
port_name = "Ethernet0"
attribute_name = "loopback_mode"

for attribute_value in ["", "invalid"]:
# Set invalid loopback mode.
tbl = swsscommon.ProducerStateTable(pdb, table_name)
fvs = swsscommon.FieldValuePairs([(attribute_name, attribute_value)])
tbl.set(port_name, fvs)
time.sleep(1)

# Check application database.
tbl = swsscommon.Table(pdb, table_name)
(status, fvs) = tbl.get(port_name)
assert status == True
attribute_found = False
for fv in fvs:
if fv[0] == attribute_name:
attribute_found = True
assert fv[1] == attribute_value
assert attribute_found == True

# Check attribute not present in asic database.
tbl = swsscommon.Table(adb, "ASIC_STATE:SAI_OBJECT_TYPE_PORT")
(status, fvs) = tbl.get(dvs.asicdb.portnamemap[port_name])
assert status == True
attribute_found = False
for fv in fvs:
if fv[0] == "SAI_PORT_ATTR_LOOPBACK_MODE":
attribute_found = True
assert attribute_found == False

def test_PortLoopbackMode(self, dvs, testlog):
pdb = swsscommon.DBConnector(0, dvs.redis_sock, 0)
adb = swsscommon.DBConnector(1, dvs.redis_sock, 0)

table_name = "PORT_TABLE"
port_name = "Ethernet0"
attribute_name = "loopback_mode"
attribute_value = "mac_local"

# Enable loopback mode.
tbl = swsscommon.ProducerStateTable(pdb, table_name)
fvs = swsscommon.FieldValuePairs([(attribute_name, attribute_value)])
tbl.set(port_name, fvs)
time.sleep(1)

# Check application database.
tbl = swsscommon.Table(pdb, table_name)
(status, fvs) = tbl.get(port_name)
assert status == True
attribute_found = False
for fv in fvs:
if fv[0] == attribute_name:
attribute_found = True
assert fv[1] == attribute_value
assert attribute_found == True

# Check asic database.
tbl = swsscommon.Table(adb, "ASIC_STATE:SAI_OBJECT_TYPE_PORT")
(status, fvs) = tbl.get(dvs.asicdb.portnamemap[port_name])
assert status == True
attribute_found = False
for fv in fvs:
if fv[0] == "SAI_PORT_ATTR_LOOPBACK_MODE":
attribute_found = True
assert fv[1] == "SAI_PORT_LOOPBACK_MODE_MAC"
assert attribute_found == True

# Disable loopback mode.
attribute_value = "none"
tbl = swsscommon.ProducerStateTable(pdb, table_name)
fvs = swsscommon.FieldValuePairs([(attribute_name, attribute_value)])
tbl.set(port_name, fvs)
time.sleep(1)

# Check application database.
tbl = swsscommon.Table(pdb, table_name)
(status, fvs) = tbl.get(port_name)
assert status == True
attribute_found = False
for fv in fvs:
if fv[0] == attribute_name:
attribute_found = True
assert fv[1] == attribute_value
assert attribute_found == True

# Check asic database.
tbl = swsscommon.Table(adb, "ASIC_STATE:SAI_OBJECT_TYPE_PORT")
(status, fvs) = tbl.get(dvs.asicdb.portnamemap[port_name])
assert status == True
attribute_found = False
for fv in fvs:
if fv[0] == "SAI_PORT_ATTR_LOOPBACK_MODE":
attribute_found = True
assert fv[1] == "SAI_PORT_LOOPBACK_MODE_NONE"
assert attribute_found == True

def test_PortLinkEventDamping(self, dvs, testlog):
cdb = swsscommon.DBConnector(4, dvs.redis_sock, 0)
pdb = swsscommon.DBConnector(0, dvs.redis_sock, 0)
Expand Down
Loading