-
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: Modified rp-info json o/p #6025
pimd: Modified rp-info json o/p #6025
Conversation
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/db0aa3b5b821cc4c07bde51c3299ea9c/raw/0d7cac9571661c5504ced4a8df491b9e743ce926/cr_6025_1584506920.diff | git apply
diff --git a/pimd/pim_cmd.c b/pimd/pim_cmd.c
index 1c77ad1b0..13a354086 100644
--- a/pimd/pim_cmd.c
+++ b/pimd/pim_cmd.c
@@ -3419,8 +3419,7 @@ static void igmp_show_groups(struct pim_instance *pim, struct vty *vty, bool uj)
if (uj) {
json = json_object_new_object();
json_groups = json_object_new_array();
- }
- else
+ } else
vty_out(vty,
"Interface Address Group Mode Timer Srcs V Uptime \n");
@@ -3472,8 +3471,9 @@ static void igmp_show_groups(struct pim_instance *pim, struct vty *vty, bool uj)
}
json_group = json_object_new_object();
- json_object_string_add(
- json_group, "source", ifaddr_str);
+ json_object_string_add(json_group,
+ "source",
+ ifaddr_str);
json_object_string_add(
json_group, "group", group_str);
@@ -3488,15 +3488,16 @@ static void igmp_show_groups(struct pim_instance *pim, struct vty *vty, bool uj)
"timer", hhmmss);
json_object_int_add(
json_group, "sourcesCount",
- grp->group_source_list
- ? listcount(
- grp->group_source_list)
- : 0);
- json_object_int_add(json_group, "version",
+ grp->group_source_list ? listcount(
+ grp->group_source_list)
+ : 0);
+ json_object_int_add(json_group,
+ "version",
grp->igmp_version);
json_object_string_add(
json_group, "uptime", uptime);
- json_object_array_add(json_groups, json_group);
+ json_object_array_add(json_groups,
+ json_group);
} else {
vty_out(vty,
"%-16s %-15s %-15s %4s %8s %4d %d %8s\n",
diff --git a/pimd/pim_rp.c b/pimd/pim_rp.c
index 5ca5bcd57..c16e99d66 100644
--- a/pimd/pim_rp.c
+++ b/pimd/pim_rp.c
@@ -1289,9 +1289,10 @@ void pim_rp_show_information(struct pim_instance *pim, struct vty *vty, bool uj)
json_rp_rows = json_object_new_array();
json_row = json_object_new_object();
- json_object_string_add(json_row, "rp_address",
+ json_object_string_add(
+ json_row, "rp_address",
inet_ntoa(rp_info->rp.rpf_addr.u
- .prefix4));
+ .prefix4));
if (rp_info->rp.source_nexthop.interface)
json_object_string_add(
json_row, "outboundInterface",
If you are a new contributor to FRR, please see our contributing guidelines.
33ceacc
to
913ee40
Compare
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/702bebff5d412b9d15200f47e373340e/raw/7fffc3a8d06a3181a13112e108c8721420d28c5c/cr_6025_1584507301.diff | git apply
diff --git a/pimd/pim_rp.c b/pimd/pim_rp.c
index 5ca5bcd57..c16e99d66 100644
--- a/pimd/pim_rp.c
+++ b/pimd/pim_rp.c
@@ -1289,9 +1289,10 @@ void pim_rp_show_information(struct pim_instance *pim, struct vty *vty, bool uj)
json_rp_rows = json_object_new_array();
json_row = json_object_new_object();
- json_object_string_add(json_row, "rp_address",
+ json_object_string_add(
+ json_row, "rp_address",
inet_ntoa(rp_info->rp.rpf_addr.u
- .prefix4));
+ .prefix4));
if (rp_info->rp.source_nexthop.interface)
json_object_string_add(
json_row, "outboundInterface",
If you are a new contributor to FRR, please see our contributing guidelines.
913ee40
to
c10d5bf
Compare
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/2e0bc3cb44cd0f86c520b93c4fc37c5c/raw/7fffc3a8d06a3181a13112e108c8721420d28c5c/cr_6025_1584507441.diff | git apply
diff --git a/pimd/pim_rp.c b/pimd/pim_rp.c
index 5ca5bcd57..c16e99d66 100644
--- a/pimd/pim_rp.c
+++ b/pimd/pim_rp.c
@@ -1289,9 +1289,10 @@ void pim_rp_show_information(struct pim_instance *pim, struct vty *vty, bool uj)
json_rp_rows = json_object_new_array();
json_row = json_object_new_object();
- json_object_string_add(json_row, "rp_address",
+ json_object_string_add(
+ json_row, "rp_address",
inet_ntoa(rp_info->rp.rpf_addr.u
- .prefix4));
+ .prefix4));
if (rp_info->rp.source_nexthop.interface)
json_object_string_add(
json_row, "outboundInterface",
If you are a new contributor to FRR, please see our contributing guidelines.
73338f1
to
b33b6f8
Compare
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/36137768f4dd8b7149cdc4cdf00c1675/raw/cc832082832f236dc3c481887f996f55aeee895e/cr_6025_1584508598.diff | git apply
diff --git a/pimd/pim_cmd.c b/pimd/pim_cmd.c
index d159d96ed..2e69e8404 100644
--- a/pimd/pim_cmd.c
+++ b/pimd/pim_cmd.c
@@ -3521,7 +3521,8 @@ static void igmp_show_groups(struct pim_instance *pim, struct vty *vty, bool uj)
if (uj) {
if (json_iface)
- json_object_object_add(json_iface, "groups", json_groups);
+ json_object_object_add(json_iface, "groups",
+ json_groups);
vty_out(vty, "%s\n", json_object_to_json_string_ext(
json, JSON_C_TO_STRING_PRETTY));
json_object_free(json);
If you are a new contributor to FRR, please see our contributing guidelines.
Outdated results Continuous Integration Result: FAILED
CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-11245/ For questions and feedback in regards to this CI system, please feel free to email Martin Winter - mwinter (at) opensourcerouting.org. (see full Make log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-11245/artifact/FBSD12AMD64/ErrorLog/log_make.txt) |
Outdated results Continuous Integration Result: FAILED
CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-11246/ For questions and feedback in regards to this CI system, please feel free to email Martin Winter - mwinter (at) opensourcerouting.org. (see full Make log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-11246/artifact/U1804AMD64/ErrorLog/log_make.txt) |
b33b6f8
to
c98e741
Compare
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/cd13df26cb9b90aaffbc2abf2b43cfad/raw/cc832082832f236dc3c481887f996f55aeee895e/cr_6025_1584509968.diff | git apply
diff --git a/pimd/pim_cmd.c b/pimd/pim_cmd.c
index d159d96ed..2e69e8404 100644
--- a/pimd/pim_cmd.c
+++ b/pimd/pim_cmd.c
@@ -3521,7 +3521,8 @@ static void igmp_show_groups(struct pim_instance *pim, struct vty *vty, bool uj)
if (uj) {
if (json_iface)
- json_object_object_add(json_iface, "groups", json_groups);
+ json_object_object_add(json_iface, "groups",
+ json_groups);
vty_out(vty, "%s\n", json_object_to_json_string_ext(
json, JSON_C_TO_STRING_PRETTY));
json_object_free(json);
If you are a new contributor to FRR, please see our contributing guidelines.
c98e741
to
269da5d
Compare
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
Outdated results Continuous Integration Result: SUCCESSFUL
For questions and feedback in regards to this CI system, please feel free to email Martin Winter - mwinter (at) opensourcerouting.org. (see full package build log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-11241/artifact/DEB10BUILD/ErrorLog/log_lintian.txt) |
Outdated results Continuous Integration Result: SUCCESSFUL
For questions and feedback in regards to this CI system, please feel free to email Martin Winter - mwinter (at) opensourcerouting.org. (see full package build log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-11242/artifact/DEB10BUILD/ErrorLog/log_lintian.txt) |
Outdated results Continuous Integration Result: SUCCESSFUL
For questions and feedback in regards to this CI system, please feel free to email Martin Winter - mwinter (at) opensourcerouting.org. (see full package build log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-11243/artifact/DEB10BUILD/ErrorLog/log_lintian.txt) |
Outdated results Continuous Integration Result: SUCCESSFUL
For questions and feedback in regards to this CI system, please feel free to email Martin Winter - mwinter (at) opensourcerouting.org. (see full package build log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-11248/artifact/DEB10BUILD/ErrorLog/log_lintian.txt) |
Outdated results Continuous Integration Result: SUCCESSFUL
For questions and feedback in regards to this CI system, please feel free to email Martin Winter - mwinter (at) opensourcerouting.org. (see full package build log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-11249/artifact/DEB10BUILD/ErrorLog/log_lintian.txt) |
269da5d
to
bf96ef0
Compare
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: FailedCheckout code: Failed (click for details)Checkout code: No useful log found |
CI: rerun |
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: FailedCheckout code: Failed (click for details)Checkout code: No useful log found |
CI: rerun |
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: FailedCheckout code: Failed (click for details)Checkout code: No useful log found |
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
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.
Unfortunately I can't ACK json output changes. That needs the exec powers :)
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.
- Why was fix 1 needed? I am ok with the change (i personally have always preferred array of elements over dictionaries with multiple levels) but don't get why it was needed?
- fix-2 is fine.
In the command “show ip pim rp-info” json: The rp address here is the key, however the rp-address is not a field under the array. In the command: “show ip igmp groups json”, the issue I am seeing is under the interface, for every group, you have a dictionary defined. However without a fixed name, we do not have any way to identify that the group address is a dictionary. dev# show ip igmp groups json This design of the CLI is not intuitive and does not follow the unicast the same norms as the unicast commands. In the above example you can see that for the “nexthops” in “show ip route json” it is clearly indicated by keyword “nexthops” and all the next hops are defined in an array. |
CI : rerun |
You are right, the current output is not machine parse-able -
|
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.
As the current "igmp group json" output is not parse-able I don't think this change to format can actually break anything.
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-11751/ 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:
|
bf96ef0
to
b6300b2
Compare
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
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-11836/ 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:
|
@patrasar looks like you deleted your previous changes? |
That we took care in another commit |
then the commit name does not reflect what is changed, can you fix that? |
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.
@patrasar could you update what's the status of this PR?
Updated the commit. |
am, the commit message still isn't as expected. |
done |
Fix: Added a new field "rpAddress" in "show ip pim rp-info json" Before: "40.0.0.2":[ { "outboundInterface":"ens224", "group":"224.0.0.0\/4", "source":"Static" } After: "40.0.0.2":[ { "rpAddress":"40.0.0.2", "outboundInterface":"ens224", "group":"224.0.0.0\/4", "source":"Static" } Signed-off-by: Sarita Patra <saritap@vmware.com>
b6300b2
to
ac8e400
Compare
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-11994/ This is a comment from an automated CI system. Warnings Generated during build:Debian 10 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 10 amd64 build:
|
💚 Basic BGPD CI results: SUCCESS, 0 tests failedResults table
For details, please contact louberger |
Fix : Added a new field "rpAddress" in "show ip pim rp-info json "
Before:
"40.0.0.2":[
{
"outboundInterface":"ens224",
"group":"224.0.0.0/4",
"source":"Static"
}
After:
"40.0.0.2":[
{
"rpAddress":"40.0.0.2",
"outboundInterface":"ens224",
"group":"224.0.0.0/4",
"source":"Static"
}
Signed-off-by: Sarita Patra saritap@vmware.com