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

Add trap flow counter support #1868

Merged
merged 4 commits into from
Nov 25, 2021

Conversation

Junchao-Mellanox
Copy link
Collaborator

What I did

Add trap flow counter CLI support. See HLD: sonic-net/SONiC#858

How I did it

Add new command:

  • counterpoll flowcnt-trap enable/disable
  • counterpoll flowcnt-trap interval
  • show flowcnt-trap stats
  • sonic-clear flowcnt-trap
  • Extend command: counterpoll show

How to verify it

Unit test/Manual test

Previous command output (if the output of a command-line utility has changed)

New command output (if the output of a command-line utility has changed)

@Junchao-Mellanox
Copy link
Collaborator Author

Hi @qiluo-msft , could you please suggest a reviewer from MSFT side? Thanks!

@Junchao-Mellanox
Copy link
Collaborator Author

Hi @qiluo-msft , kindly reminder, could you please suggest a reviewer?

@Junchao-Mellanox
Copy link
Collaborator Author

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@Junchao-Mellanox Junchao-Mellanox force-pushed the flow-counter-review branch 2 times, most recently from 8227476 to d9563bf Compare November 15, 2021 03:56
@Junchao-Mellanox
Copy link
Collaborator Author

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@liat-grozovik liat-grozovik requested review from prsunny and removed request for qiluo-msft and SuvarnaMeenakshi November 15, 2021 16:42
@Junchao-Mellanox
Copy link
Collaborator Author

/azpw run Azure.sonic-utilities

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-utilities

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@prsunny
Copy link
Contributor

prsunny commented Nov 23, 2021

@dgsudharsan, can you review this as well?

@@ -352,6 +386,8 @@ def show():
data.append([ACL, pg_drop_info.get("POLL_INTERVAL", DEFLT_10_SEC), acl_info.get("FLEX_COUNTER_STATUS", DISABLE)])
if tunnel_info:
data.append(["TUNNEL_STAT", rif_info.get("POLL_INTERVAL", DEFLT_10_SEC), rif_info.get("FLEX_COUNTER_STATUS", DISABLE)])
if trap_info:
data.append(["FLOW_CNT_TRAP_STAT", trap_info.get("POLL_INTERVAL", DEFLT_1_SEC), trap_info.get("FLEX_COUNTER_STATUS", DISABLE)])
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this be changed to 10 sec?

@@ -27,6 +26,7 @@
PG_WATERMARK_STAT 10000 enable
PG_DROP_STAT 10000 enable
ACL 10000 enable
FLOW_CNT_TRAP_STAT 1000 enable
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this be 10000?


old_values = old_stats[name]
if values[-1] != old_values[-1]:
# Counter OID not equal means the trap was removed and added again. Removing a trap would cause
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a good logic to handle. However, what if OID returned is the same as before delete? Should we rather not check if the diff is negative and consider it as a case where trap got deleted?

@Junchao-Mellanox
Copy link
Collaborator Author

/azpw run Azure.sonic-utilities

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-utilities

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@Junchao-Mellanox
Copy link
Collaborator Author

/azpw run Azure.sonic-utilities

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-utilities

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@prsunny prsunny merged commit c05845d into sonic-net:master Nov 25, 2021
abdosi pushed a commit that referenced this pull request Dec 8, 2021
Add flowcnt commands
* counterpoll flowcnt-trap enable/disable
* counterpoll flowcnt-trap interval
* show flowcnt-trap stats
@Junchao-Mellanox Junchao-Mellanox deleted the flow-counter-review branch December 10, 2021 09:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants