Skip to content

Commit

Permalink
[macsecorch]: Fix MACsec SC creating before MACsec port enabling (son…
Browse files Browse the repository at this point in the history 
…ic-net#2087)

* Update MACsec SC attributes after MACsec SC creating

Signed-off-by: Ze Gan <ganze718@gmail.com>

* Add unittest

Signed-off-by: Ze Gan <ganze718@gmail.com>

* teardown unittest

Signed-off-by: Ze Gan <ganze718@gmail.com>

* polish name

Signed-off-by: Ze Gan <ganze718@gmail.com>
  • Loading branch information
@Pterosaur
Pterosaur authored Jan 29, 2022
1 parent 53c630b commit d49eaa2
Show file tree
Hide file tree
Showing 3 changed files with 154 additions and 23 deletions.
126 changes: 103 additions & 23 deletions orchagent/macsecorch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1183,6 +1183,18 @@ bool MACsecOrch::updateMACsecPort(MACsecPort &macsec_port, const TaskArgs &port_
if (get_value(port_attr, "enable_encrypt", alpha_boolean))
{
macsec_port.m_enable_encrypt = alpha_boolean.operator bool();
if (!updateMACsecSCs(
macsec_port,
[&macsec_port, this](MACsecOrch::MACsecSC &macsec_sc)
{
sai_attribute_t attr;
attr.id = SAI_MACSEC_SC_ATTR_ENCRYPTION_ENABLE;
attr.value.booldata = macsec_port.m_enable_encrypt;
return this->updateMACsecAttr(SAI_OBJECT_TYPE_MACSEC_SC, macsec_sc.m_sc_id, attr);
}))
{
return false;
}
}
if (get_value(port_attr, "send_sci", alpha_boolean))
{
Expand Down Expand Up @@ -1212,42 +1224,74 @@ bool MACsecOrch::updateMACsecPort(MACsecPort &macsec_port, const TaskArgs &port_
SWSS_LOG_WARN("Unknown Cipher Suite %s", cipher_suite.c_str());
return false;
}
if (!updateMACsecSCs(
macsec_port,
[&macsec_port, this](MACsecOrch::MACsecSC &macsec_sc)
{
sai_attribute_t attr;
attr.id = SAI_MACSEC_SC_ATTR_MACSEC_CIPHER_SUITE;
attr.value.s32 = macsec_port.m_cipher_suite;
return this->updateMACsecAttr(SAI_OBJECT_TYPE_MACSEC_SC, macsec_sc.m_sc_id, attr);
}))
{
return false;
}
}
swss::AlphaBoolean enable = false;
if (get_value(port_attr, "enable", enable) && enable.operator bool() != macsec_port.m_enable)
{
std::vector<MACsecOrch::MACsecSC *> macsec_scs;
macsec_port.m_enable = enable.operator bool();
for (auto &sc : macsec_port.m_egress_scs)
if (!updateMACsecSCs(
macsec_port,
[&macsec_port, &recover, this](MACsecOrch::MACsecSC &macsec_sc)
{
// Change the ACL entry action from packet action to MACsec flow
if (macsec_port.m_enable)
{
if (!this->setMACsecFlowActive(macsec_sc.m_entry_id, macsec_sc.m_flow_id, true))
{
SWSS_LOG_WARN("Cannot change the ACL entry action from packet action to MACsec flow");
return false;
}
auto entry_id = macsec_sc.m_entry_id;
auto flow_id = macsec_sc.m_flow_id;
recover.add_action([this, entry_id, flow_id]()
{ this->setMACsecFlowActive(entry_id, flow_id, false); });
}
else
{
this->setMACsecFlowActive(macsec_sc.m_entry_id, macsec_sc.m_flow_id, false);
}
return true;
}))
{
macsec_scs.push_back(&sc.second);
return false;
}
for (auto &sc : macsec_port.m_ingress_scs)
}

recover.clear();
return true;
}

bool MACsecOrch::updateMACsecSCs(MACsecPort &macsec_port, std::function<bool(MACsecOrch::MACsecSC &)> action)
{
SWSS_LOG_ENTER();

for (auto &sc : macsec_port.m_egress_scs)
{
if (!action(sc.second))
{
macsec_scs.push_back(&sc.second);
return false;
}
for (auto &macsec_sc : macsec_scs)
}
for (auto &sc : macsec_port.m_ingress_scs)
{
if (!action(sc.second))
{
// Change the ACL entry action from packet action to MACsec flow
if (macsec_port.m_enable)
{
if (!setMACsecFlowActive(macsec_sc->m_entry_id, macsec_sc->m_flow_id, true))
{
SWSS_LOG_WARN("Cannot change the ACL entry action from packet action to MACsec flow");
return false;
}
auto entry_id = macsec_sc->m_entry_id;
auto flow_id = macsec_sc->m_flow_id;
recover.add_action([this, entry_id, flow_id]() { this->setMACsecFlowActive(entry_id, flow_id, false); });
}
else
{
setMACsecFlowActive(macsec_sc->m_entry_id, macsec_sc->m_flow_id, false);
}
return false;
}
}

recover.clear();
return true;
}

Expand Down Expand Up @@ -1721,6 +1765,42 @@ bool MACsecOrch::deleteMACsecSC(sai_object_id_t sc_id)
return true;
}

bool MACsecOrch::updateMACsecAttr(sai_object_type_t object_type, sai_object_id_t object_id, const sai_attribute_t &attr)
{
SWSS_LOG_ENTER();

sai_status_t status = SAI_STATUS_SUCCESS;

if (object_type == SAI_OBJECT_TYPE_MACSEC_PORT)
{
status = sai_macsec_api->set_macsec_port_attribute(object_id, &attr);
}
else if (object_type == SAI_OBJECT_TYPE_MACSEC_SC)
{
status = sai_macsec_api->set_macsec_sc_attribute(object_id, &attr);
}
else if (object_type == SAI_OBJECT_TYPE_MACSEC_SA)
{
status = sai_macsec_api->set_macsec_sa_attribute(object_id, &attr);
}
else
{
SWSS_LOG_ERROR("Wrong type %s", sai_serialize_object_type(object_type).c_str());
return false;
}

if (status != SAI_STATUS_SUCCESS)
{
task_process_status handle_status = handleSaiSetStatus(SAI_API_MACSEC, status);
if (handle_status != task_success)
{
return parseHandleSaiStatusFailure(handle_status);
}
}

return true;
}

task_process_status MACsecOrch::createMACsecSA(
const std::string &port_sci_an,
const TaskArgs &sa_attr,
Expand Down
3 changes: 3 additions & 0 deletions orchagent/macsecorch.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ class MACsecOrch : public Orch
sai_object_id_t switch_id,
sai_macsec_direction_t direction);
bool updateMACsecPort(MACsecPort &macsec_port, const TaskArgs & port_attr);
bool updateMACsecSCs(MACsecPort &macsec_port, std::function<bool(MACsecOrch::MACsecSC &)> action);
bool deleteMACsecPort(
const MACsecPort &macsec_port,
const std::string &port_name,
Expand Down Expand Up @@ -179,6 +180,8 @@ class MACsecOrch : public Orch
sai_macsec_direction_t direction);
bool deleteMACsecSC(sai_object_id_t sc_id);

bool updateMACsecAttr(sai_object_type_t object_type, sai_object_id_t object_id, const sai_attribute_t &attr);

/* MACsec SA */
task_process_status createMACsecSA(
const std::string &port_sci_an,
Expand Down
48 changes: 48 additions & 0 deletions tests/test_macsec.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -699,6 +699,54 @@ def test_macsec_term_orch(self, dvs: conftest.DockerVirtualSwitch, testlog):
1)
assert(not inspector.get_macsec_port(macsec_port))

def test_macsec_attribute_change(self, dvs: conftest.DockerVirtualSwitch, testlog):
port_name = "Ethernet0"
local_mac_address = "00-15-5D-78-FF-C1"
peer_mac_address = "00-15-5D-78-FF-C2"
macsec_port_identifier = 1
macsec_port = "macsec_eth1"
sak = "0" * 32
auth_key = "0" * 32
packet_number = 1
ssci = 1
salt = "0" * 24

wpa = WPASupplicantMock(dvs)
inspector = MACsecInspector(dvs)

self.init_macsec(
wpa,
port_name,
local_mac_address,
macsec_port_identifier)
wpa.set_macsec_control(port_name, True)
wpa.config_macsec_port(port_name, {"enable_encrypt": False})
wpa.config_macsec_port(port_name, {"cipher_suite": "GCM-AES-256"})
self.establish_macsec(
wpa,
port_name,
local_mac_address,
peer_mac_address,
macsec_port_identifier,
0,
sak,
packet_number,
auth_key,
ssci,
salt)
macsec_info = inspector.get_macsec_port(macsec_port)
assert("encrypt off" in macsec_info)
assert("GCM-AES-256" in macsec_info)
self.deinit_macsec(
wpa,
inspector,
port_name,
macsec_port,
local_mac_address,
peer_mac_address,
macsec_port_identifier,
0)


# Add Dummy always-pass test at end as workaroud
# for issue when Flaky fail on final test it invokes module tear-down
Expand Down

0 comments on commit d49eaa2

Please sign in to comment.