Skip to content

Providing transformer support for sflow feature#91

Merged
kwangsuk merged 9 commits intosonic-net:masterfrom
Gokulnath-Raja:transformer_sflow_feature_support
Aug 18, 2023
Merged

Providing transformer support for sflow feature#91
kwangsuk merged 9 commits intosonic-net:masterfrom
Gokulnath-Raja:transformer_sflow_feature_support

Conversation

@Gokulnath-Raja
Copy link
Contributor

@Gokulnath-Raja Gokulnath-Raja commented May 29, 2023

What are the changes

Transformer support has been added for the sflow app.

Files Changed:

openconfig-sampling-sflow.yang: which have the parameters that can be configured via gnmi and Rest and its deviations parameters has been added in the openconfig-sampling-sflow-deviation.yang

openconfig-sampling-sflow-annot.yang: which consists of the transformer specific functions which needs to be called based on the rest/gnmi call.

xfmr_sflow.go: go file which consists of the function definitions which needs to be called based on the subscription/get/set request and for the validations.

Unit test file changes for Rest:

sflow_sonic_test.go   - To test for the parameters in the sonic-sflow.yang

sflow_openconfig_test.go – To test for the parameters in the openconfig-sampling-sflow.yang.

Unit test file changes for Gnmi:

translib/test/sflow

Why the changes are required

Currently sflow feature can be configured via click interface alone.

Transformer support has been added to support for the configuration via Rest and Gnmi

How did I verify the changes

Verified by adding sflow unit test files for restui and gnmi

Attached are the UT results and log.

gnmi_ut_for_openconfig_sflow.txt
gnmi_ut_for_sonic_sflow.txt
rest_ut_for_sflow.txt
 
Attached are the subscription UT log

target_defined_subscription.txt
On_change_subscription.txt
Sample_subscription.txt

Support added:

  1. Sflow agent, collector and interface parameters can be modified via rest and gnmi.

  2. Collector Name cannot be configured from rest_ui, it will be auto-generated with pattern using Collector-IP,Collector-Port & Collector-VRF.  For e.g. 10.1.1.1_6343_default 

  3. Port , Portchannel , Mgmt port , Vlan can be configured as an Sflow Agent Interface. So, while configuring for the sflow agent prior configurations of the interface is mandatory. Vlan leafref is not available now. 

  4. Alias support for the interface names are not available to configure for the sflow agent and for the sflow interface. 

  5. We cannot configure more than 2 collectors.

  6. We need to enable the sflow globally , which interns enable the sflow in all available interfaces and then we can disable the sflow interface individually if required. We cannot disable the sflow globally and enable the interfaces locally.

Signed-off-by: Gokulnath-Raja <Gokulnath_R@dell.com>
Co-authored-by: mohanapriya-meganathan <mohanapriya.m1@dell.com>
Copy link
Contributor

@sachinholla sachinholla left a comment

Choose a reason for hiding this comment

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

Please mention that this PR depends on #87 and #89

@Gokulnath-Raja
Copy link
Contributor Author

Following are the dependent pull request,

already updated in above comment (Following are the dependent pull request,)

Gokulnath-Raja and others added 2 commits June 1, 2023 14:07
…mments

Signed-off-by: Gokulnath-Raja <Gokulnath_R@dell.com>
Co-authored-by: mohanapriya-meganathan <mohanapriya.m1@dell.com>
…mments for sflow app test

Signed-off-by: Gokulnath-Raja <Gokulnath_R@dell.com>
Co-authored-by: mohanapriya-meganathan <mohanapriya.m1@dell.com>
Copy link
Contributor

@sachinholla sachinholla left a comment

Choose a reason for hiding this comment

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

Test binary related changes look fine. Did not review sflow app related changes.

kwangsuk
kwangsuk previously approved these changes Jun 16, 2023
@sachinholla
Copy link
Contributor

hi @kwangsuk, please do not merge this PR before sonic-net/sonic-buildimage#15226 is merged. Full image builds may fail without those buildimage makefile changes. It includes an enhancement to the main docker image makefile slave.mk, hence waiting for MSFT's approval

@sachinholla
Copy link
Contributor

Hi @Gokulnath-Raja , some tests are failing -- https://dev.azure.com/mssonic/be1b070f-be15-4154-aade-b1d3bfb17054/_apis/build/builds/334869/logs/12

2023-08-10T05:14:43.8672835Z --- FAIL: Test_node_openconfig_sflow_interface (20.01s)
2023-08-10T05:14:43.8673189Z     --- PASS: Test_node_openconfig_sflow_interface/Test_set_on_sflow_interface (0.00s)
2023-08-10T05:14:43.8673580Z     --- PASS: Test_node_openconfig_sflow_interface/Verify_set_on_sflow_interface (0.00s)
2023-08-10T05:14:43.8673963Z     --- FAIL: Test_node_openconfig_sflow_interface/Test_delete_on_sflow_interface (0.00s)
2023-08-10T05:14:43.8674363Z     --- FAIL: Test_node_openconfig_sflow_interface/Verify_delete_on_sflow_interface (0.00s)
2023-08-10T05:14:43.8674750Z     --- FAIL: Test_node_openconfig_sflow_interface/Test_get_on_sflow_interface (0.00s)
2023-08-10T05:14:43.8675160Z     --- FAIL: Test_node_openconfig_sflow_interface/Update_admin-state_for_sflow_interface (0.00s)
2023-08-10T05:14:43.8675561Z     --- FAIL: Test_node_openconfig_sflow_interface/Verify_admin-state_for_sflow_interface (0.00s)
2023-08-10T05:14:43.8675981Z     --- FAIL: Test_node_openconfig_sflow_interface/Update_sampling-rate_on_sflow_interface (0.00s)
2023-08-10T05:14:43.8676384Z     --- FAIL: Test_node_openconfig_sflow_interface/Verify_sampling-rate_on_sflow_interface (0.00s)
2023-08-10T05:14:43.8676788Z     --- PASS: Test_node_openconfig_sflow_interface/Test_delete_on_sflow_interfaces (0.00s)

@mohanapriya-meganathan
Copy link
Contributor

Hi @Gokulnath-Raja , some tests are failing -- https://dev.azure.com/mssonic/be1b070f-be15-4154-aade-b1d3bfb17054/_apis/build/builds/334869/logs/12

2023-08-10T05:14:43.8672835Z --- FAIL: Test_node_openconfig_sflow_interface (20.01s)
2023-08-10T05:14:43.8673189Z     --- PASS: Test_node_openconfig_sflow_interface/Test_set_on_sflow_interface (0.00s)
2023-08-10T05:14:43.8673580Z     --- PASS: Test_node_openconfig_sflow_interface/Verify_set_on_sflow_interface (0.00s)
2023-08-10T05:14:43.8673963Z     --- FAIL: Test_node_openconfig_sflow_interface/Test_delete_on_sflow_interface (0.00s)
2023-08-10T05:14:43.8674363Z     --- FAIL: Test_node_openconfig_sflow_interface/Verify_delete_on_sflow_interface (0.00s)
2023-08-10T05:14:43.8674750Z     --- FAIL: Test_node_openconfig_sflow_interface/Test_get_on_sflow_interface (0.00s)
2023-08-10T05:14:43.8675160Z     --- FAIL: Test_node_openconfig_sflow_interface/Update_admin-state_for_sflow_interface (0.00s)
2023-08-10T05:14:43.8675561Z     --- FAIL: Test_node_openconfig_sflow_interface/Verify_admin-state_for_sflow_interface (0.00s)
2023-08-10T05:14:43.8675981Z     --- FAIL: Test_node_openconfig_sflow_interface/Update_sampling-rate_on_sflow_interface (0.00s)
2023-08-10T05:14:43.8676384Z     --- FAIL: Test_node_openconfig_sflow_interface/Verify_sampling-rate_on_sflow_interface (0.00s)
2023-08-10T05:14:43.8676788Z     --- PASS: Test_node_openconfig_sflow_interface/Test_delete_on_sflow_interfaces (0.00s)

@sachinholla ,yes, we are looking into it.

Gokulnath-Raja and others added 3 commits August 10, 2023 17:16
Enabling sflow feature for the
Signed-off-by: Gokulnath-Raja <Gokulnath_R@dell.com>
Co-authored-by: mohanapriya-meganathan <mohanapriya.m1@dell.com>
@mohanapriya-meganathan
Copy link
Contributor

@sachinholla Sanity test fails as it depends on a SFLOW_SESSION_TABLE which will be created by sflow docker container. As the azure pipelines, runs the sanity test in an isolated sonic-mgmt-framework environment it fails. So, updated the changes in the unit test framework to support the interdependency table.

Updated log:
testapp.log

@Gokulnath-Raja
Copy link
Contributor Author

/AzurePipelines run Azure.sonic-common

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 91 in repo sonic-net/sonic-mgmt-common

@Gokulnath-Raja
Copy link
Contributor Author

@sachinholla please help in restarting the /AzurePipelines run Azure.sonic-common (pipeline failed due to space issue)

@sachinholla
Copy link
Contributor

hi @Gokulnath-Raja, I don't know the procedure to manually trigger the PR build. I used to amend the last commit without making any changes and force push :)

git commit --amend --no-edit
git push -f

@Gokulnath-Raja Gokulnath-Raja force-pushed the transformer_sflow_feature_support branch from bb934c9 to 25f3fcc Compare August 16, 2023 13:21
@sachinholla
Copy link
Contributor

Hi Gokul, please merge latest from master branch to solve "no space left on device" error

@Gokulnath-Raja
Copy link
Contributor Author

Hi Gokul, please merge latest from master branch to solve "no space left on device" error

@sachinholla done and pushed the code, kindly merge...

@sachinholla
Copy link
Contributor

I don't have merge permissions.. please follow up with Kwan

@kwangsuk kwangsuk merged commit 6823470 into sonic-net:master Aug 18, 2023
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