-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
SONiC Port mirroring HLD #580
Conversation
c66f25d
to
e8c86d1
Compare
doc/SONiC_Port_Mirroring_HLD.md
Outdated
|
||
2. Dynamic session management | ||
- Allow multiple source to single destination. | ||
- Each session supports mirroring from single port to single destination port. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we allow one mirror session from multiple source port to single dest port?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. we do allow multiple source to single destination with multiple sessions. This is currently planned as multiple sessions, we can also do it in single session with list of source interfaces. Please suggest.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. we allow one mirror session from multiple source port to single destination port.
|
||
|
||
## 1.3 Scalability Requirements | ||
- Up to max ASIC capable mirror sessions to be supported. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how do we know how many mirror session can be supported in the asic?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The SAI Attribute SAI_SWITCH_ATTR_MAX_MIRROR_SESSION can be used to support the max mirror sessinons. This is for both ERSPAN and SPAN and we don't have any mechanism in orchagent to retrieve this now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have a number based on TD3 and TH series?
Also how many sessions can be active at the same time. Is there any limitation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For TD3 and TH series, max 4 sessions can be active at same time. single session can be shared across multiple source ports.
|
||
## 1.3 Scalability Requirements | ||
- Up to max ASIC capable mirror sessions to be supported. | ||
- Once max mirror sessions are created and user attempts to create new session, error will be logged in syslog. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is not discussed in the design document how this is implemented. can you add a section to discuss how you are going to implement this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When the user exceeds max mirror sessions then SAI returns SAI_STATUS_INSUFFICIENT_RESOURCES, This is similar behaviour as what is currently present in ERSPAN also. The error is logged to syslog from SyncD. OrchAgent treats the error as fatal, similar to the existing ERPSAN code.
doc/SONiC_Port_Mirroring_HLD.md
Outdated
|
||
;Configure SPAN/ERSPAN mirror session. | ||
;storm control type - broadcast / unknown-unicast / unknown-multicast | ||
key = PORT_MIRROR_TABLE:mirror_session_name ; mirror_session_name is |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PORT_MIRROR_TABLE -> PORT_MIRROR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Updated the doc.
; unique session | ||
; identifier | ||
;field = value | ||
destination_port = PORT_TABLE:ifname ; ifname must be unique across PORT TABLE. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can dest port be lag?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No. Destination port cant be LAG. We are not supporting this now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you update the Source-ip and destination-ip fields to the table
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ERSPAN is already supported in community and we captured only new additions which are done as part of this PR.
doc/SONiC_Port_Mirroring_HLD.md
Outdated
; identifier | ||
;field = value | ||
destination_port = PORT_TABLE:ifname ; ifname must be unique across PORT TABLE. | ||
source_port = PORT_TABLE:ifname ; ifname must be unique across PORT,INTF,LAG TABLES |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what do you mean by INTF? INTF is layer 3 concept.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doc is already updated. this is only port for destination port, port/LAG for src port.
|
||
mirror_session_name = 1*255VCHAR | ||
|
||
### 3.2.2 APP_DB |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is plan to support this feature in virtual switch, like sflow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We dont have plan for this now. If u mean to handle specific flow to mirror, then ACL mirroring can be used. Can u please clarify on sflow part here.
doc/SONiC_Port_Mirroring_HLD.md
Outdated
|
||
## 3.5 CLI | ||
### 3.5.1 Data Models | ||
Custom Yang model will be introduced for this feature. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add description for the yang model?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. I will add and update the doc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated doc with both SONiC yang and openconfig extension model also
doc/SONiC_Port_Mirroring_HLD.md
Outdated
SPAN Sessions | ||
--------------------------------------------------------------------------------------------------------- | ||
Name Status DST Port SRC Port Direction | ||
sess1 active Ethernet4 Ethernet0 rx |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is the definition of active status? what is the criteria. can you make it clear in the document?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Port mirror session will be active in below cases.
- When destination port only session is created, then once the session is created from SAI. the session becomes active. These sessions can be used in ACL mirroring.
- When mirroring is enabled on the source ports, then the session will become active.
doc/SONiC_Port_Mirroring_HLD.md
Outdated
|
||
## 9.1 CLI Test Cases | ||
|
||
1. Configure ERSPAN mirror session and verify all parameters are updated properly in CONFIG_DB |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please describe where the tests are going to be contributed to? which repo?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some of the tests will be contributed to swss and others will be contributed to spytest. I will update in the doc.
doc/SONiC_Port_Mirroring_HLD.md
Outdated
config mirror_session add erspan <session-name> <src_ip> <dst_ip> <gre> <dscp> [ttl] [queue] [src_port] [rx/tx/both] | ||
|
||
#Configure Port mirror span mirror session. | ||
config mirror_session add span <session-name> <destination_ifName> <source_ifName> <rx/tx/both> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is policer going to supported in span session?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Policer is supported.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as comments
Thanks Guohan for review comments. Thanks |
Hi @lguohan , Can u please review the HLD and I have raised initial code PR along with pytest UT Can u please help review . Thanks |
@lguohan, @xinliu-seattle - can we merge this now please? |
|
||
|
||
## 1.3 Scalability Requirements | ||
- Up to max ASIC capable mirror sessions to be supported. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have a number based on TD3 and TH series?
Also how many sessions can be active at the same time. Is there any limitation
; unique session | ||
; identifier | ||
;field = value | ||
destination_port = PORT_TABLE:ifname ; ifname must be unique across PORT TABLE. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you update the Source-ip and destination-ip fields to the table
|
||
# Modify existing ERSPAN configuration to accept source port and direction | ||
config mirror_session add erspan <session-name> <src_ip> <dst_ip> <gre> <dscp> [ttl] [queue] [src_port] [rx/tx/both] --policer <policer> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we have dscp here? Is it for any prioritization of ERSPAN traffic mirrored across the devices?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes ERSPAN tunnel header will have this dscp, This is already supported in current community code and this PR doesnt modify any behaviour of this field.
|Name | Scaling value | | ||
|--------------------------|--------------------| | ||
| Max mirror sessions | silicon specific | | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have any scaling numbers?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For TD3, TH series max 4 sessions can be supported.
SONiC Port Mirroring HLD