opencomputeproject
Updating saimacsec.h to add macsec_port_id to flow obj
In the current implementation (without the proposed changes), there is no relation between secure flow, secure channel and the macsec_port_id/port. MACsec user is not able create a flow or secure channel for a particular core. This is an optional attiribute so this change won't break current macsec applications and the new attribute should only be used when there is a need to have a relation between flow, SC, SA and a port.
In the current implementation (without the proposed changes), there is no relation between secure flow, secure channel and the macsec_port_id/port. MACsec user is not able create a flow or secure channel for a particular core. This is an optional attiribute so this change won't break current macsec applications and the new attribute should only be used when there is a need to have a relation between flow, SC, SA and a port.
please add PR description
I have added comment about the description:
In the current implementation (without the proposed changes), there is no relation between secure flow, secure channel and the macsec_port_id/port. MACsec user is not able create a flow or secure channel for a particular core. This is an optional attiribute so this change won't break current macsec applications and the new attribute should only be used when there is a need to have a relation between flow, SC, SA and a port.
This change does not allow using a MACsec flow for multiple MACsec ports. Instead of binding a MACsec port to a MACsec flow, can a MACsec port be bound to a MACsec SC? That allows MACsec flows to identify common policies (stored in per-flow entries) to be applied to SCs that serve different ports. An alternative could be to allow a list of ports to be associated to each flow.
Yes. we can move the attribute from MACsec flow to MACsec SC.
MACSEC rules are bound to a port. These rules have MACSEC flow as action. The MACSEC flow can have multiple SCs. Adding Port to SC association at MACSEC SC, could break the association of MACSEC rule to port.
This change does not allow using a MACsec flow for multiple MACsec ports. Instead of binding a MACsec port to a MACsec flow, can a MACsec port be bound to a MACsec SC? That allows MACsec flows to identify common policies (stored in per-flow entries) to be applied to SCs that serve different ports. An alternative could be to allow a list of ports to be associated to each flow.
Yes. we can move the attribute from MACsec flow to MACsec SC.
Why would you want to share a MACSEC flow across MACSEC ports ? Does hardware engine allow it ?
Hi Anandhi,
Honestly I do not know about all MACsec implementations out there. This object relationship is accepted and used for years - so I didn't want to risk breaking that unless it is absolutely necessary.
The SAI MACsec clearly allows multiple MACsec ingress SC to be bound to one MACsec ingress flow. Years back, I came across an implementation that makes use of it. However I do not remember if all those SCs (sharing a flow) had to be for 1 port.
- Dipankar
On Thu, Apr 25, 2024 at 9:10 AM Anandhi Dhanabalan @.***> wrote:
This change does not allow using a MACsec flow for multiple MACsec ports. Instead of binding a MACsec port to a MACsec flow, can a MACsec port be bound to a MACsec SC? That allows MACsec flows to identify common policies (stored in per-flow entries) to be applied to SCs that serve different ports. An alternative could be to allow a list of ports to be associated to each flow.
Yes. we can move the attribute from MACsec flow to MACsec SC.
Why would you want to share a MACSEC flow across MACSEC ports ? Does hardware engine allow it ?
— Reply to this email directly, view it on GitHub https://github.com/opencomputeproject/SAI/pull/2003#issuecomment-2077661670, or unsubscribe https://github.com/notifications/unsubscribe-auth/AN3GZXMJEAD5NBKALTJH2JTY7ETJFAVCNFSM6AAAAABGVZKDYOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANZXGY3DCNRXGA . You are receiving this because you were mentioned.Message ID: @.***>
So are we moving back the MACsec port id from MACsec SC to Flow. There is already an implicit relationship between MACsec port -> ACL table -> MACsec flow -> MACsec SC but the relation doesn't establish until it binds to an ACL entry. Instead of bind point, can we make an explicit relation between MACsec flow and MACsec port id? Looks like based on the current design, we cannot assign a flow to multiple MACsec port ids.
There is no implicit relationship limiting a flow from being used for multiple ports. Multiple ACL entries can be bound to a MACsec flow, and those can belong to different ACL tables and different ports.
- Dipankar
On Thu, Apr 25, 2024 at 10:32 AM SidharajU @.***> wrote:
So are we moving back the MACsec port id from MACsec SC to Flow. There is already an implicit relationship between MACsec port -> ACL table -> MACsec flow -> MACsec SC but the relation doesn't establish until it binds to an ACL entry. Instead of bind point, can we make an explicit relation between MACsec flow and MACsec port id? Looks like based on the current design, we cannot assign a flow to multiple MACsec port ids.
— Reply to this email directly, view it on GitHub https://github.com/opencomputeproject/SAI/pull/2003#issuecomment-2077806082, or unsubscribe https://github.com/notifications/unsubscribe-auth/AN3GZXJI4FI5MA63GRKVFSLY7E42TAVCNFSM6AAAAABGVZKDYOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANZXHAYDMMBYGI . You are receiving this because you were mentioned.Message ID: @.***>
Thanks Dipankar. @helloanandhi Are you okay with the current changes based on Dipankar's explanation?
If MACsec port is allowed to be bound to SC, that would break the existing semantics
For example, In illustration below MACSEC port P1 would point to a MACsec flow (F1) with secure channel (SC2) associated to P2, this would be incorrect. MACsec port (say P1) -> ACL table (say T1)-> MACsec flow (say F1) -> MACsec SC (SC1 {mapped to P1} , SC2 {mapped to P2} )
There is no implicit relationship limiting a flow from being used for multiple ports. Multiple ACL entries can be bound to a MACsec flow, and those can belong to different ACL tables and different ports. - Dipankar … On Thu, Apr 25, 2024 at 10:32 AM SidharajU @.> wrote: So are we moving back the MACsec port id from MACsec SC to Flow. There is already an implicit relationship between MACsec port -> ACL table -> MACsec flow -> MACsec SC but the relation doesn't establish until it binds to an ACL entry. Instead of bind point, can we make an explicit relation between MACsec flow and MACsec port id? Looks like based on the current design, we cannot assign a flow to multiple MACsec port ids. — Reply to this email directly, view it on GitHub <#2003 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AN3GZXJI4FI5MA63GRKVFSLY7E42TAVCNFSM6AAAAABGVZKDYOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANZXHAYDMMBYGI . You are receiving this because you were mentioned.Message ID: @.>
Please note, multiple secure channels (SCs) can be associated with a single MACsec port. To allow this, MACsec flow - which is a logical encapsulation has list of SC's.
There is no implicit relationship limiting a flow from being used for multiple ports. Multiple ACL entries can be bound to a MACsec flow, and those can belong to different ACL tables and different ports. - Dipankar … On Thu, Apr 25, 2024 at 10:32 AM SidharajU @.> wrote: So are we moving back the MACsec port id from MACsec SC to Flow. There is already an implicit relationship between MACsec port -> ACL table -> MACsec flow -> MACsec SC but the relation doesn't establish until it binds to an ACL entry. Instead of bind point, can we make an explicit relation between MACsec flow and MACsec port id? Looks like based on the current design, we cannot assign a flow to multiple MACsec port ids. — Reply to this email directly, view it on GitHub <#2003 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AN3GZXJI4FI5MA63GRKVFSLY7E42TAVCNFSM6AAAAABGVZKDYOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANZXHAYDMMBYGI . You are receiving this because you were mentioned.Message ID: _@**.**_>
Please note, multiple secure channels (SCs) can be associated with a single MACsec port. To allow this, MACsec flow - which is a logical encapsulation has list of SC's.
When I defined the API, I did not intend a flow to be used for multiple ports. In fact the only intended use case for multiple SCs attached to a flow was for a single ingress port as described in Fig-4 and the text below it in https://github.com/opencomputeproject/SAI/blob/master/doc/macsec-gearbox/SAI_MACsec_API_Proposal-v1.4.docx
But while considering this PR, I realized nothing prevented someone from binding the ACL and SC for 2 ports to 1 MACsec flow. That could allow the same flow (aka vPort) policy to be applied to multiple ports (if the hardware permitted it). And the safer approach was to attach a port to a flow, because nobody should attach the same SC to 2 ports of a device. If you think nobody has shared a flow for multiple ports, and there an imperative to attach the port to a flow, please go ahead and do it. I would not have done it, because attaching a port to an SC works equally well and seems safer.
@SidharajU , AFAIK, It not prudent to share a flow across macsec ports in hardware though current SAI software modelling does not prevent it explicitly. As @dipankar-ba stated, if your hardware supports it, could you please add a 'md' file with work-flow, to clearly explain the intention and use case of this new attribute ? Please note the ACL binding happens at physical port level, which can have one or more macsec ports created. I believe the SC is at per macsec port level. A pictorial diagram explaining all this would be helpful.
@SidharajU DCO error please use this command to signoff using your email. git commit -s -m “commit message”
Hi Anandhi,
Perhaps you are right and nobody in their right mind would have shared a flow across ports. But in Sid's defense it is not him :-)
Forgive the repetition - but in case you missed my main point. I do not want to break somebody's existing driver (including people other than Broadcom or Marvell) - and that's why I advocated attaching port to SC. But if you and Sid think attaching it to flow is worth the risk, try it as long as you are willing to fix any fallout.
- Dipankar
On Sun, Apr 28, 2024 at 10:25 PM Anandhi Dhanabalan < @.***> wrote:
@SidharajU https://github.com/SidharajU , AFAIK, It not prudent to share a flow across macsec ports in hardware though current SAI software modelling does not prevent it explicitly. As @dipankar-ba https://github.com/dipankar-ba stated, if your hardware supports it could you please add a 'md' file with work flow as well to clearly explain the intention and use case of this attribute ? Please note the ACL binding happens at physical port level, which can have one or more macsec ports created. I believe the SC is at per macsec port level. A pictorial diagram explaining all this would be helpful.
— Reply to this email directly, view it on GitHub https://github.com/opencomputeproject/SAI/pull/2003#issuecomment-2081911709, or unsubscribe https://github.com/notifications/unsubscribe-auth/AN3GZXLQZXSCKMNWRZQBVALY7XKUHAVCNFSM6AAAAABGVZKDYOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOBRHEYTCNZQHE . You are receiving this because you were mentioned.Message ID: @.***>
@dipankar-ba , Attaching port to SC or Flow, will have same limitation as multiple ACL tables bound to different physical ports can point to these. Hence a work flow from @SidharajU would clarify the use case.
Attaching a port to SC will not cause trouble for existing drivers because a MACsec SC cannot support 2 ports. If a new driver needs to program per-port memory for a flow, such programming can be deferred until the flow is associated with a SC.
Attaching a port to a flow can have problems because, as discussed earlier, an existing driver could have attached a flow to have multiple ports. However, I do not know of anyone who has done it. So as I said before, you can try it out - but be ready to revert if it breaks.
Sid and you can make that call - IMO there is no point rehashing this topic further.
- Dipankar
On Mon, May 6, 2024 at 12:09 AM Anandhi Dhanabalan @.***> wrote:
@dipankar-ba https://github.com/dipankar-ba , Attaching port to SC or Flow, will have same limitation as multiple ACL tables bound to different physical ports can point to these. Hence a work flow from @SidharajU https://github.com/SidharajU would clarify the use case.
— Reply to this email directly, view it on GitHub https://github.com/opencomputeproject/SAI/pull/2003#issuecomment-2095325259, or unsubscribe https://github.com/notifications/unsubscribe-auth/AN3GZXJZVO5QWNAKDGVPHJDZA4UCDAVCNFSM6AAAAABGVZKDYOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOJVGMZDKMRVHE . You are receiving this because you were mentioned.Message ID: @.***>
