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.

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

[orchagent]: Add MACsec Orchagent #1474

Merged
merged 37 commits into from
Jan 29, 2021
Merged

Conversation

Pterosaur
Copy link
Contributor

@Pterosaur Pterosaur commented Oct 22, 2020

What I did
Add MACsec orchagent for MACsec feature.
The MACsecOrch is introduced in the Orchagent to handle configuration requests. It monitors MACsec related tables in APP DB and convert those messages to SAI commands to manage the MACsec object.
The main functions are defined in class MACsecOrch as follow

    task_process_status taskUpdateMACsecPort(const std::string & port_name, const TaskArgs & port_attr);
    task_process_status taskDisableMACsecPort(const std::string & port_name, const TaskArgs & port_attr);
    task_process_status taskUpdateEgressSC(const std::string & port_sci, const TaskArgs & sc_attr);
    task_process_status taskDeleteEgressSC(const std::string & port_sci, const TaskArgs & sc_attr);
    task_process_status taskUpdateIngressSC(const std::string & port_sci, const TaskArgs & sc_attr);
    task_process_status taskDeleteIngressSC(const std::string & port_sci, const TaskArgs & sc_attr);
    task_process_status taskUpdateEgressSA(const std::string & port_sci_an, const TaskArgs & sa_attr);
    task_process_status taskDeleteEgressSA(const std::string & port_sci_an, const TaskArgs & sa_attr);
    task_process_status taskUpdateIngressSA(const std::string & port_sci_an, const TaskArgs & sa_attr);
    task_process_status taskDeleteIngressSA(const std::string & port_sci_an, const TaskArgs & sa_attr);

The HLD of MACsec orchagent is at MACsec HLD

Why I did it
MACsec orchagent is needed to transfer MACsec management message from wpa_supplicant to SAI.

How I verified it
Following the verification at #1475 . The macsec device should be created and you can obverse whether the MACsec configuration is correct by command ip macsec show

Details if related
This PR depends on :
sonic-net/sonic-wpa-supplicant#16
sonic-net/sonic-buildimage#5700
#1475
sonic-net/sonic-sairedis#691
sonic-net/sonic-swss-common#434
sonic-net/sonic-sairedis#770

@Pterosaur Pterosaur changed the title Macsecorch MACsecOrch Oct 22, 2020
@Pterosaur Pterosaur marked this pull request as draft October 22, 2020 14:30
Signed-off-by: Ze Gan <[email protected]>
@Pterosaur Pterosaur changed the title MACsecOrch [orchagent]: Add MACsec Orchagent Nov 3, 2020
@Pterosaur Pterosaur marked this pull request as ready for review November 4, 2020 01:18
orchagent/port.h Outdated
@@ -114,6 +114,8 @@ class Port

std::unordered_set<sai_object_id_t> m_ingress_acl_tables_uset;
std::unordered_set<sai_object_id_t> m_egress_acl_tables_uset;

sai_object_id_t m_line_port_id = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

remove this one since this pr does not support gearbox

void uninstallCounter(const std::string &obj_name, sai_object_id_t obj_id);

/* ACL */
bool initACLTable(
Copy link
Contributor

Choose a reason for hiding this comment

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

-> initMacsecAclTable

@Pterosaur
Copy link
Contributor Author

Add test

@Pterosaur
Copy link
Contributor Author

retest vs please

"SAI_MACSEC_SA_ATTR_MINIMUM_XPN",
};

/* Helpers */
Copy link
Contributor

Choose a reason for hiding this comment

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

can helper functions be in swss-common?

Copy link
Contributor

@qbdwlr qbdwlr left a comment

Choose a reason for hiding this comment

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

Is this going to be committed before latest saimacsec.h changes are integrated?

auto sc = scs.find(*m_sci);
if (sc == scs.end())
{
SWSS_LOG_INFO("Cannot find the MACsec SC %lu at the port %s.", *m_sci, m_port_name->c_str());
Copy link
Contributor

Choose a reason for hiding this comment

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

Use PRIx64 instead of lu for SCI for easier debugging.

auto macsec_obj = m_macsec_objs.emplace(switch_id, MACsecObject());
if (!macsec_obj.second)
{
SWSS_LOG_INFO("The MACsec has been initialized at the switch %lu", switch_id);
Copy link
Contributor

Choose a reason for hiding this comment

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

Use PRIx64 instead of lu for consistent OID display in syslog.

@@ -197,6 +199,7 @@ void initSaiApi()
sai_log_set(SAI_API_SAMPLEPACKET, SAI_LOG_LEVEL_NOTICE);
sai_log_set(SAI_API_DEBUG_COUNTER, SAI_LOG_LEVEL_NOTICE);
sai_log_set((sai_api_t)SAI_API_NAT, SAI_LOG_LEVEL_NOTICE);
sai_log_set(SAI_API_NAT, SAI_LOG_LEVEL_NOTICE);
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy-n-paste error.

extern sai_port_api_t *sai_port_api;
extern sai_switch_api_t *sai_switch_api;

static const std::vector<std::string> macsec_egress_sa_stats =
Copy link
Contributor

Choose a reason for hiding this comment

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

When you add support for MACSEC_SA_STATS, this will need to be renamed since it is not a vector of stats but attrs.

SWSS_LOG_WARN("Cannot initialize MACsec egress object at the switch %lu", switch_id);
return false;
}
recover.add_action([&]() { sai_macsec_api->remove_macsec_port(macsec_obj.first->second.m_egress_id); });
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this correct?

*ctx.get_port_id());
});
}
if (!updateMACsecPort(*ctx.get_macsec_port(), port_attr))
Copy link
Contributor

Choose a reason for hiding this comment

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

what's this doing?

macsec_port.m_ingress_port_id = SAI_NULL_OBJECT_ID;
});

macsec_port.m_enable_encrypt = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

are these always fixed? There might be integrity only mode?

});

macsec_port.m_enable_encrypt = true;
macsec_port.m_sci_in_sectag = true;
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 it's false by default.

SWSS_LOG_WARN("Cannot initialize MACsec ingress object at the switch 0x%" PRIx64, switch_id);
return false;
}
recover.add_action([&]() { sai_macsec_api->remove_macsec_port(macsec_obj.first->second.m_ingress_id); });
Copy link
Contributor Author

Choose a reason for hiding this comment

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

remove macsec()

const std::string &,
const TaskArgs &);
const static std::map<TaskType, TaskFunc> TaskMap = {
{{APP_MACSEC_PORT_TABLE_NAME, SET_COMMAND},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rename these functions' names

@Pterosaur
Copy link
Contributor Author

retest vs please

Signed-off-by: Ze Gan <[email protected]>
@lguohan lguohan merged commit 832815e into sonic-net:master Jan 29, 2021
DavidZagury pushed a commit to DavidZagury/sonic-swss that referenced this pull request Mar 4, 2021
Add MACsec orchagent for MACsec feature.
The MACsecOrch is introduced in the Orchagent to handle configuration requests. It monitors MACsec related tables in APP DB and convert those messages to SAI commands to manage the MACsec object. 
The main functions are defined in class MACsecOrch as follow
```
    task_process_status taskUpdateMACsecPort(const std::string & port_name, const TaskArgs & port_attr);
    task_process_status taskDisableMACsecPort(const std::string & port_name, const TaskArgs & port_attr);
    task_process_status taskUpdateEgressSC(const std::string & port_sci, const TaskArgs & sc_attr);
    task_process_status taskDeleteEgressSC(const std::string & port_sci, const TaskArgs & sc_attr);
    task_process_status taskUpdateIngressSC(const std::string & port_sci, const TaskArgs & sc_attr);
    task_process_status taskDeleteIngressSC(const std::string & port_sci, const TaskArgs & sc_attr);
    task_process_status taskUpdateEgressSA(const std::string & port_sci_an, const TaskArgs & sa_attr);
    task_process_status taskDeleteEgressSA(const std::string & port_sci_an, const TaskArgs & sa_attr);
    task_process_status taskUpdateIngressSA(const std::string & port_sci_an, const TaskArgs & sa_attr);
    task_process_status taskDeleteIngressSA(const std::string & port_sci_an, const TaskArgs & sa_attr);
```
 The HLD of MACsec orchagent is at [MACsec HLD](https://github.com/Azure/SONiC/blob/master/doc/macsec/MACsec_hld.md#344-macsec-orch)

Signed-off-by: Ze Gan <[email protected]>
raphaelt-nvidia pushed a commit to raphaelt-nvidia/sonic-swss that referenced this pull request Oct 5, 2021
Add MACsec orchagent for MACsec feature.
The MACsecOrch is introduced in the Orchagent to handle configuration requests. It monitors MACsec related tables in APP DB and convert those messages to SAI commands to manage the MACsec object. 
The main functions are defined in class MACsecOrch as follow
```
    task_process_status taskUpdateMACsecPort(const std::string & port_name, const TaskArgs & port_attr);
    task_process_status taskDisableMACsecPort(const std::string & port_name, const TaskArgs & port_attr);
    task_process_status taskUpdateEgressSC(const std::string & port_sci, const TaskArgs & sc_attr);
    task_process_status taskDeleteEgressSC(const std::string & port_sci, const TaskArgs & sc_attr);
    task_process_status taskUpdateIngressSC(const std::string & port_sci, const TaskArgs & sc_attr);
    task_process_status taskDeleteIngressSC(const std::string & port_sci, const TaskArgs & sc_attr);
    task_process_status taskUpdateEgressSA(const std::string & port_sci_an, const TaskArgs & sa_attr);
    task_process_status taskDeleteEgressSA(const std::string & port_sci_an, const TaskArgs & sa_attr);
    task_process_status taskUpdateIngressSA(const std::string & port_sci_an, const TaskArgs & sa_attr);
    task_process_status taskDeleteIngressSA(const std::string & port_sci_an, const TaskArgs & sa_attr);
```
 The HLD of MACsec orchagent is at [MACsec HLD](https://github.com/Azure/SONiC/blob/master/doc/macsec/MACsec_hld.md#344-macsec-orch)

Signed-off-by: Ze Gan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants