Skip to content

SONiC Port mirroring HLD - #580

Merged
xinliu-seattle merged 4 commits into
sonic-net:masterfrom
rupesh-k:port_mirroring_hld
Oct 30, 2020
Merged

xinliu-seattle merged 4 commits into
sonic-net:masterfrom
rupesh-k:port_mirroring_hld

Conversation

@rupesh-k

Copy link
Copy Markdown
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
Comment thread 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.

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

Copy link
Copy Markdown

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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.

Comment thread 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

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

Copy link
Copy Markdown

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
Copy Markdown
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.

Comment thread 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

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

Comment thread doc/SONiC_Port_Mirroring_HLD.md Outdated

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

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

Comment thread doc/SONiC_Port_Mirroring_HLD.md Outdated
SPAN Sessions
---------------------------------------------------------------------------------------------------------
Name Status DST Port SRC Port Direction
sess1 active Ethernet4 Ethernet0 rx

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

Comment thread 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

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

Comment thread 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>

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

@lguohan lguohan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

as comments

@rupesh-k

rupesh-k commented Jun 4, 2020

Copy link
Copy Markdown
Contributor Author

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
Copy Markdown
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
Copy Markdown

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



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

Copy link
Copy Markdown

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.

Copy link
Copy Markdown

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>

Copy link
Copy Markdown

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
Copy Markdown
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 |

Copy link
Copy Markdown

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?

@rupesh-k rupesh-k Jun 26, 2020 •

Copy link
Copy Markdown
Contributor Author

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