-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
pimd: test command to stop/allow ageing of sg entries #6206
Conversation
Decription: There are requirements where sg ageing is disabled. A hidden CLI is introduced to test enable/disable sg-ageing. This is used for testing purpose. This will be used to test all sg entries of the vrf at once. We will have topotest using this CLI. Signed-off-by: Saravanan K <saravanank@vmware.com>
…ow>]" Signed-off-by: Saravanan K <saravanank@vmware.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution to FRR!
Click for style suggestions
To apply these suggestions:
curl -s https://gist.githubusercontent.com/polychaeta/8f150a2ba4163fa41c5a36dcc81c1060/raw/cfc130babc023320ece9e260d335b421e9238f06/cr_6206_1586762081.diff | git apply
diff --git a/pimd/pim_cmd.c b/pimd/pim_cmd.c
index 0bebf1337..b2fd61a98 100644
--- a/pimd/pim_cmd.c
+++ b/pimd/pim_cmd.c
@@ -8071,15 +8071,12 @@ static int pim_cmd_interface_add(struct vty *vty, struct interface *ifp)
return 1;
}
-DEFPY_HIDDEN (pim_test_sg_stop_ageing,
- pim_test_sg_stop_ageing_cmd,
- "test pim [vrf NAME$name] sg-ageing [<stop|allow>]",
- "Test code\n"
- PIM_STR
- VRF_CMD_HELP_STR
- "control ageing sg entries\n"
- "stop ageing sg entries\n"
- "allow ageing sg entries\n")
+DEFPY_HIDDEN(pim_test_sg_stop_ageing, pim_test_sg_stop_ageing_cmd,
+ "test pim [vrf NAME$name] sg-ageing [<stop|allow>]",
+ "Test code\n" PIM_STR VRF_CMD_HELP_STR
+ "control ageing sg entries\n"
+ "stop ageing sg entries\n"
+ "allow ageing sg entries\n")
{
struct pim_upstream *up;
struct pim_instance *pim;
If you are a new contributor to FRR, please see our contributing guidelines.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution to FRR!
Click for style suggestions
To apply these suggestions:
curl -s https://gist.githubusercontent.com/polychaeta/2d27ed3a8cbb77cc9dc134680ba81e6d/raw/cfc130babc023320ece9e260d335b421e9238f06/cr_6206_1586763103.diff | git apply
diff --git a/pimd/pim_cmd.c b/pimd/pim_cmd.c
index 0bebf1337..b2fd61a98 100644
--- a/pimd/pim_cmd.c
+++ b/pimd/pim_cmd.c
@@ -8071,15 +8071,12 @@ static int pim_cmd_interface_add(struct vty *vty, struct interface *ifp)
return 1;
}
-DEFPY_HIDDEN (pim_test_sg_stop_ageing,
- pim_test_sg_stop_ageing_cmd,
- "test pim [vrf NAME$name] sg-ageing [<stop|allow>]",
- "Test code\n"
- PIM_STR
- VRF_CMD_HELP_STR
- "control ageing sg entries\n"
- "stop ageing sg entries\n"
- "allow ageing sg entries\n")
+DEFPY_HIDDEN(pim_test_sg_stop_ageing, pim_test_sg_stop_ageing_cmd,
+ "test pim [vrf NAME$name] sg-ageing [<stop|allow>]",
+ "Test code\n" PIM_STR VRF_CMD_HELP_STR
+ "control ageing sg entries\n"
+ "stop ageing sg entries\n"
+ "allow ageing sg entries\n")
{
struct pim_upstream *up;
struct pim_instance *pim;
If you are a new contributor to FRR, please see our contributing guidelines.
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
💚 Basic BGPD CI results: SUCCESS, 0 tests failedResults table
For details, please contact louberger |
Continuous Integration Result: SUCCESSFULContinuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-11798/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warnings
Warnings Generated during build:Debian 10 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 10 amd64 build:
|
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: FailedCentOS 7 rpm pkg check: Failed (click for details)CentOS 7 rpm pkg check: No useful log foundSuccessful on other platforms/tests
Warnings Generated during build:Checkout code: Successful with additional warningsCentOS 7 rpm pkg check: Failed (click for details)CentOS 7 rpm pkg check: No useful log found
Warnings Generated during build:Debian 10 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 10 amd64 build:
|
CI: rerun |
Continuous Integration Result: SUCCESSFULContinuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-11802/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warnings
Warnings Generated during build:Debian 10 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 10 amd64 build:
|
We already have this command:
|
There is some difference in this implementation. The command you mentioned restart/refresh the keep alive timer once. This implementation will avoid ageing of sg entries until it's allowed to age by same CLI. |
Continuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-11799/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warnings
Warnings Generated during build:Debian 10 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 10 amd64 build:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nack from me. I see no reason to have a slightly different test command from what is already implemented in FRR.
'stop ageing' and 'delaying the ageing once' is not same. Infact the vxlan BUM traffic implementation do not age the entry which is same as this CLI implementation. There are implementations where we do not use linux forwarding for datapath. They may have a hardware based or dpdk based elsewhere outside kernel. kat timer inside pim would lack the capability of sensing the actual traffic in datapath. Those scenarios need to stop ageing like this and have a external trigger to age out. These commands will be used to mimic that scenario of ageout. Topotests are built around these CLI. If there are any suggestions to make it a config command, we shall make it so. |
This implies you need this behaviour in production / regular operation, rather than a test command. Is that the case? |
@eqvinox Yes. We need this behavior in production. |
Additionally: The data plane must be able to either be queried or report stream flowing state. |
The current ageing mechanism in the PIM code is polling for traffic status kernel.
We will decide on the approach later after discussing with Donald and rework using a different PR. |
Decription: There are requirements where sg ageing is disabled.
A hidden CLI is introduced to test enable/disable sg-ageing. This is
used for testing purpose. This will be used to test all sg entries of the vrf
at once. We will have topotest using this CLI.
Signed-off-by: Saravanan K saravanank@vmware.com