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

SONiC Port mirroring HLD #580

Merged
merged 4 commits into from
Oct 30, 2020

Conversation

rupesh-k
Copy link
Contributor

SONiC Port Mirroring HLD

@rupesh-k rupesh-k force-pushed the port_mirroring_hld branch from c66f25d to e8c86d1 Compare March 23, 2020 17:55

2. Dynamic session management
- Allow multiple source to single destination.
- Each session supports mirroring from single port to single destination port.
Copy link
Contributor

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?

Copy link
Contributor Author

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.

Copy link
Contributor Author

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.
Copy link
Contributor

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?

Copy link
Contributor Author

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.

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

Copy link
Contributor Author

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.
Copy link
Contributor

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?

Copy link
Contributor Author

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.


;Configure SPAN/ERSPAN mirror session.
;storm control type - broadcast / unknown-unicast / unknown-multicast
key = PORT_MIRROR_TABLE:mirror_session_name ; mirror_session_name is
Copy link
Contributor

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

Copy link
Contributor Author

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.
Copy link
Contributor

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?

Copy link
Contributor Author

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.

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

Copy link
Contributor Author

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.

; 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
Copy link
Contributor

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.

Copy link
Contributor Author

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
Copy link
Contributor

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.

Copy link
Contributor Author

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.


## 3.5 CLI
### 3.5.1 Data Models
Custom Yang model will be introduced for this feature.
Copy link
Contributor

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?

Copy link
Contributor Author

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

Copy link
Contributor Author

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

SPAN Sessions
---------------------------------------------------------------------------------------------------------
Name Status DST Port SRC Port Direction
sess1 active Ethernet4 Ethernet0 rx
Copy link
Contributor

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?

Copy link
Contributor Author

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.

  1. 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.
  2. When mirroring is enabled on the source ports, then the session will become active.


## 9.1 CLI Test Cases

1. Configure ERSPAN mirror session and verify all parameters are updated properly in CONFIG_DB
Copy link
Contributor

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?

Copy link
Contributor Author

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.

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>
Copy link
Contributor

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. Policer is supported.

Copy link
Contributor

@lguohan lguohan left a comment

Choose a reason for hiding this comment

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

as comments

@rupesh-k
Copy link
Contributor Author

rupesh-k commented Jun 4, 2020

Thanks Guohan for review comments.
I have updated the document with details.
I will be updating more on unittests in next commit.

Thanks

@rupesh-k
Copy link
Contributor Author

Hi @lguohan ,

Can u please review the HLD and I have raised initial code PR along with pytest UT
Following are code PR links.
sonic-net/sonic-swss#1314
sonic-net/sonic-utilities#936

Can u please help review .

Thanks
Rupesh Kumar

@ben-gale
Copy link

@lguohan, @xinliu-seattle - can we merge this now please?



## 1.3 Scalability Requirements
- Up to max ASIC capable mirror sessions to be supported.

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.

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>

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?

Copy link
Contributor Author

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 |

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?

Copy link
Contributor Author

@rupesh-k rupesh-k Jun 26, 2020

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.

@rupesh-k rupesh-k requested a review from lguohan June 26, 2020 14:49
@xinliu-seattle xinliu-seattle merged commit aad6a1f into sonic-net:master Oct 30, 2020
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.

5 participants