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

Correct the sflow default sample rate #1623

Merged
merged 4 commits into from
Apr 1, 2021

Conversation

fredyu190011
Copy link
Contributor

What I did
Correct the sflow default sample rate to "ifSpeed / 1e6".

Why I did it
According the PR: SONiC#746, the sample rate should be "ifSpeed / 1e6".

How I verified it

Details if related
Signed-off-by: Fred Yu fred_yu@edge-core.com

- What I did
Correct the sflow default sample rate to "ifSpeed / 1e6".

- Why I did it
According the PR: SONiC#746, the sample rate should be "ifSpeed / 1e6".

- How I verified it

- Details if related

Signed-off-by: Fred Yu fred_yu@edge-core.com
@prsunny
Copy link
Collaborator

prsunny commented Feb 3, 2021

@dgsudharsan , @padmanarayana to review

dgsudharsan
dgsudharsan previously approved these changes Feb 3, 2021
padmanarayana
padmanarayana previously approved these changes Feb 3, 2021
dgsudharsan
dgsudharsan previously approved these changes Feb 3, 2021
padmanarayana
padmanarayana previously approved these changes Feb 3, 2021
@prsunny
Copy link
Collaborator

prsunny commented Mar 26, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@prsunny
Copy link
Collaborator

prsunny commented Mar 26, 2021

@fredyu190011 , can you please do a force-push?

@fredyu190011
Copy link
Contributor Author

Hi @prsunny , I solve the conflict through merge. Is it okay?

@liat-grozovik
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@dgsudharsan
Copy link
Collaborator

@fredyu190011 Please address my latest comments.

@fredyu190011
Copy link
Contributor Author

Hi @dgsudharsan , I cannot see your latest comment.
I fixed some merging mistake to pass the auto test.

#define SFLOW_SAMPLE_RATE_VALUE_10G "1000"
#define SFLOW_SAMPLE_RATE_VALUE_1G "100"
#define SFLOW_SAMPLE_RATE_VALUE_400G "400000"
#define SFLOW_SAMPLE_RATE_VALUE_100G "100000"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add 200G speed sample rate value. It got missed in the merge I assume

@@ -1,12 +1,12 @@
class TestSflow:
speed_rate_table = {
"400000": "40000",
"100000": "10000",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you please add 200G here too?

@dgsudharsan
Copy link
Collaborator

@prsunny Can we merge this?

@dgsudharsan
Copy link
Collaborator

Please add it for 202012 branch.

@prsunny prsunny merged commit cba6576 into sonic-net:master Apr 1, 2021
yxieca pushed a commit that referenced this pull request Apr 8, 2021
According the PR: SONiC#746, the sample rate should be "ifSpeed / 1e6".

Signed-off-by: Fred Yu fred_yu@edge-core.com
raphaelt-nvidia pushed a commit to raphaelt-nvidia/sonic-swss that referenced this pull request Oct 5, 2021
According the PR: SONiC#746, the sample rate should be "ifSpeed / 1e6".

Signed-off-by: Fred Yu fred_yu@edge-core.com
EdenGri pushed a commit to EdenGri/sonic-swss that referenced this pull request Feb 28, 2022
Restored cfggen sort method of json data and used it to
sort config db before saving.

signed-off-by: Tamer Ahmed <tamer.ahmed@microsoft.com>
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