Egress mirroring and ACL action support check via SAI HLD

[stepanblyschak]

No description provided.

[stcheng]

questions:
is it possible to support both ingress and egress actions at the same time for one rule?
according to SAI, there is currently no such prohibitions.

[stepanblyschak]

@stcheng Yes, in SAI there is no such prohibitions. To support this scenario in sonic we might need to change the schema:
'mirror_action:<stage>': '<session>'
So in this case keys will be different and it will be possible to create rule with mirror on ingress and egress stages.

Besides, we may also consider to change the redirect action, because there is no way to create rule with forward and redirect at the same time (same 'packet_action' key). So if we will change this for mirror action I would also like to have redirect as a seperate key ('redirect_action' and not part of 'packet_action') and of course preserving backwards compatibility with old schema. This will be more aligned with SAI types.
One more benefit will be that schema for ACL action capabilities will be simpler because we don't need to put there supported keys + values, but only keys:

e.g:

1) "ACL_ACTION|INGRESS"
2) "PACKET_ACTION,REDIRECT_ACTION,MIRROR_ACTION:INGRESS"

[stcheng]

my consideration is that we could use

MIRROR_INGRESS_ACTION: <mirror_session>
MIRROR_EGRESS_ACTION: <mirror_session>

In order to retain the backward compatibility, we could still have the MIRROR_ACTION serving same functionality as MIRROR_INGRESS_ACTION.

I agree that we could move redirect out of the current packet_action, we could have similar field name

REDIRECT_ACTION: <list_of_redirect_parameters>

What do you think?

[stepanblyschak]

@stcheng Good, so I'll update PRs

[stcheng]

Below is for switch capability query:

what about the following format?

ACL_ACTIONS|INGRESS
PACKET_ACTION,MIRROR_INGRESS_ACTION,MIRROR_EGRESS_ACTION,REDIRECT_ACTION
ACL_ACTIONS|EGRESS
PACKET_ACTION,MIRROR_EGRESS_ACTION

The above indicates that the switch supports ingress ACL with four different actions while egress ACL with two actions. I don't know if a secondary query for the supported PACKET_ACTION is needed or not. What's your opinion? Per SAI, the supported packet actions are many, but right now we don't have all in SONiC. My understanding is that we'd at least need to have drop/forward/copy/trap four actions at this stage.

[stcheng]

check the comments

[stepanblyschak]

@stcheng
I did not find a way how to query wich specific actions from sai_packet_action_t enum are supported, so I didn't see a reason why need to put DROP/FORWARD/TRAP/COPY/LOG, etc in DB.
From my point of view if SAI_ACL_ACTION_TYPE_PACKET_ACTION is supported at this stage then all of the sai_packet_action_t are supported (at least Mellanox supports). Do you know such query in SAI?

[stepanblyschak]

@stcheng
sai_query_attribute_enum_values_capability may return list of supported packet actions for SAI_ACL_ACTION_TYPE_PACKET_ACTION, however this type of API is not supported by sairedis yet.
We may put this in TODO

[stcheng]

Sure.
I have approved the PR. Could you help follow the procedure of signing the commits?
https://help.github.com/en/articles/about-commit-signature-verification

Also, do you have a plan to talk about this during a community meeting?

[stcheng]

change to m_aclPacketActionCapabilities?

[stepanblyschak]

@stcheng This map is not only for PACKET_ACTION, there is also ACTION_DTEL_FLOW_OP which value is enum and can be validated

[lguohan]

each acl table has binding point, egress or ingress.

Are we allowing ingress acl to have egress mirror action? or we should restrict only egress acl have egress mirror action, meanwhile, ingress acl can have ingress mirror action?

[stepanblyschak]

@lguohan
There is not restriction. EVERFLOW table in example can be either ingress stage or egress stage.
Application (acl-loader) should look in SWITCH_CAPABILITY table to check if action is supported on specific acl table stage.
The example below in the design shows the capability table example on Mellanox.
So you can see that on Mellanox egress mirror on ingress acl is not supported.

[lguohan]

it is not clear the relationship between egress mirror action v.s acl binding point. Need to make it more clear.

[stcheng]

One question here: what would be the sequence of decap and ingress ACL/mirror? will the packet get encapsulated first or match the ACL rules or get copied first?

[lguohan]

should go to master branch.

[stepanblyschak]

@lguohan changed to master