MACSec and IPSec FIPS Compliance#2167
Conversation
Signed-off-by: JaiOCP <jai.kumar@broadcom.com>
|
@karthik-krishnamurthy @dipankar-nexthop @ judyjoseph |
|
Added some minor editorial suggestions that I noticed @JaiOCP - rest of the changes look good to me as per discussions in the earlier PR. |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Signed-off-by: JaiOCP <jai.kumar@broadcom.com>
|
Thanks for the review Kartik. I have pushed a new commit fixing these two
issues.
…On Mon, May 5, 2025 at 11:04 PM kartik-arista ***@***.***> wrote:
*kartik-arista* left a comment (opencomputeproject/SAI#2167)
<#2167 (comment)>
Added some minor editorial suggestions that I noticed @JaiOCP
<https://github.com/JaiOCP> - rest of the changes look good to me as per
discussions in the earlier PR.
—
Reply to this email directly, view it on GitHub
<#2167 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AKCSHLIVCJDZQWVPN445IVT25BGG5AVCNFSM6AAAAAB4KXCLCSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDQNJTGM3TENZQGM>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
--
This electronic communication and the information and any files transmitted
with it, or attached to it, are confidential and are intended solely for
the use of the individual or entity to whom it is addressed and may contain
information that is confidential, legally privileged, protected by privacy
laws, or otherwise restricted from disclosure to anyone else. If you are
not the intended recipient or the person responsible for delivering the
e-mail to the intended recipient, you are hereby notified that any use,
copying, distributing, dissemination, forwarding, printing, or copying of
this e-mail is strictly prohibited. If you received this e-mail in error,
please return the e-mail to the sender, delete it from your computer, and
destroy any printed copy of it.
|
| SAI_SWITCH_ATTR_MACSEC_POST_STATUS_NOTIFY, | ||
|
|
||
| /** | ||
| * @brief Callback for completion status of all the IPSEC ports serviced by this IPSEC engine |
There was a problem hiding this comment.
This should be "all MACsec capable ports of the switch", not related to a specific MACsec engine.
|
|
||
| /** | ||
| * @brief MACSEC POST status | ||
| * Attribute to query the status of POST for all the MACSEC engines |
There was a problem hiding this comment.
This should be "all IPsec capable ports of the switch", not related to a specific IPsec engine.
| FIPS 140-3 compliance requires that POST is executed on each MACSec/IPSec port and only if POST completes with ‘success’, the MACSec/IPSec port must be enabled for admitting traffic. | ||
| Note that this specification will provide POST at the MACSec/IPSec single logical instance level or at the switch level. | ||
|
|
||
| As shown in Fig-1, there are two physical security engines and are represented by single logical instance in SAI. Enabling of POST happens at the logical intance level and in turn will trigger POST for all the physical instances present in a given hardware. |
There was a problem hiding this comment.
I find this confusing. There are 2 SAI_MACSEC objects (for ingress & egress) for all security engines. And there can be up to 2 SAI_IPSEC objects (for ingress & egress) for each security engine. Perhaps deleting this paragraph is best.
| SAI_OBJECT_TYPE_MACSEC and SAI_OBJECT_TYPE_ IPSEC already exists | ||
| SAI_MACSEC_ATTR_SUPPORTED_PORT_LIST and SAI_IPSEC_ATTR_SUPPORTED_PORT_LIST provides the list of MACSec/IPSec ports being service by an instance of MACSEC and IPSEC objects respectively. Each MACSec/IPSec port has a correspoding binding to the physical port SAI_MACSEC_PORT_ATTR_PORT_ID. | ||
| ``` | ||
| SAI_OBJECT_TYPE_SWITCH -> SAI_SWITCH_ATTR_MACSEC_OBJECT_LIST[x] -> SAI_MACSEC_ATTR_SUPPORTED_PORT_LIST[] -> SAI_MACSEC_PORT_ATTR_PORT_ID |
There was a problem hiding this comment.
Since the object relationship has not been changed, perhaps there is no need to mention this as enhancement.
While this description is correct, the reader misses one complexity - each of the 2 elements (for ingress & egress) of the MACSEC_OBJECT_LIST have SUPPORTED_PORT_LIST with identical membership.
| ``` | ||
| SAI_OBJECT_TYPE_SWITCH -> SAI_SWITCH_ATTR_MACSEC_OBJECT_LIST[x] -> SAI_MACSEC_ATTR_SUPPORTED_PORT_LIST[] -> SAI_MACSEC_PORT_ATTR_PORT_ID | ||
|
|
||
| SAI_OBJECT_TYPE_SWITCH -> SAI_SWITCH_ATTR_IPSEC_OBJECT_LIST[x] -> SAI_IPSEC_ATTR_SUPPORTED_PORT_LIST[] -> SAI_IPSEC_PORT_ATTR_PORT_ID |
There was a problem hiding this comment.
Similar comment as for MACSEC. However, IPsec can be a little more complex since it allows N security engines to serve N distinct subsets of ports. In that case, there would be be 2*N elements (for ingress and egress) of IPSEC_OBJECT_LIST.
|
|
||
| #### 2.2.2 IPSec Port | ||
| New READ only attribute SAI_IPSEC_PORT_ATTR_POST_STATUS is introduced to read the status of POST for a given IPSec port. | ||
| This attribute can be read after the POST is completed for the IPSec object hosting this port. |
There was a problem hiding this comment.
We should point out that this attribute provides POST status separately for ingress and egress and both have to pass POST for a port to be usable for IPsec.
| Subsequent steps are captured as an example for engine level POST. | ||
|
|
||
| **Step 2: ** | ||
| MACSec Object creation is complete |
There was a problem hiding this comment.
This should be merged with the next step. The POST is triggered at the time of object creation.
| ``` | ||
|
|
||
| **Step 4: ** | ||
| Once the POST is completed on all the ports hosted by the MACSec engine, registered post macsec callback will be called by the SAI adapter. |
There was a problem hiding this comment.
Suggest "all ingress and egress of all MACsec capable ports of the switch" instead of "ports hosted by the MACsec engine".
|
|
||
| **Step 4: ** | ||
| Once the POST is completed on all the ports hosted by the MACSec engine, registered post macsec callback will be called by the SAI adapter. | ||
| If the status is SAI_MACSEC_POST_STATUS_FAIL, NOS MUST read the post status of all the ports hosted by the engine to find out which port POST has failed. |
There was a problem hiding this comment.
Why is NOS required to read the POST status of all ports? Can't it simply disable MACsec for the whole switch?
Also, the example code below is incorrect/mispeading. SAI_MACSEC_ATTR_SUPPORTED_PORT_LIST is a list of SAI_PORT objects, not SAI_MACSEC_PORT objects.
| read(macsec_port_obj->SAI_MACSEC_PORT_ATTR_POST_STATUS) | ||
| ``` | ||
| **Step 5: ** | ||
| Set the POST enable flag in the macsec object to false. This is mainly for the hw where POST can be triggered runtime after the initialization. |
There was a problem hiding this comment.
This cannot be done for a CREATE_ONLY attribute.
|
Transferred my comments from PR #2162 |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
this PR is adding 4 new notifications is that really necessary ? do we need so many of them ? |
|
hey @JaiOCP, it seems like @dipankar-nexthop requested some changes, was that already resolved ? |
The SAI_MACSEC and SAI_IPSEC level POST is for diagnostics only. So maybe we can live without notification for those? |
|
just wanted to know what's the reason for so many notifications |
7 participants
This PR brings support for enabling POST on an IPSec and MACSec engine or at the switch level for FIPS compliance.
Old PR is closed:
#2162