diff --git a/orchagent/macsecorch.cpp b/orchagent/macsecorch.cpp index 36fbb44f896a..12c5b1ddb99c 100644 --- a/orchagent/macsecorch.cpp +++ b/orchagent/macsecorch.cpp @@ -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)) { @@ -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 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 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; } @@ -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, diff --git a/orchagent/macsecorch.h b/orchagent/macsecorch.h index 8856347118a9..6702c75cf612 100644 --- a/orchagent/macsecorch.h +++ b/orchagent/macsecorch.h @@ -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 action); bool deleteMACsecPort( const MACsecPort &macsec_port, const std::string &port_name, @@ -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, diff --git a/tests/test_macsec.py b/tests/test_macsec.py index 0f945300e385..f74f31c00876 100644 --- a/tests/test_macsec.py +++ b/tests/test_macsec.py @@ -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