ospf6d: Add debug commands for lsa all and route all #9438

ranjanyash54 · 2021-08-18T11:07:14Z

Debug commad for all lsa types and route types are not present.

Signed-off-by: Yash Ranjan ranjany@vmware.com

polychaeta

Thanks for your contribution to FRR!

Click for style suggestions

To apply these suggestions:

curl -s https://gist.githubusercontent.com/polychaeta/a3d974b5db01607e6d8ed22ea4b15d4e/raw/febb046c1dd7632db67c0436fc76acec62533d0d/cr_9438_1629284839.diff | git apply

diff --git a/ospf6d/ospf6_lsa.c b/ospf6d/ospf6_lsa.c
index ac07704d2..353706f89 100644
--- a/ospf6d/ospf6_lsa.c
+++ b/ospf6d/ospf6_lsa.c
@@ -1021,14 +1021,11 @@ static char *ospf6_lsa_handler_name(const struct ospf6_lsa_handler *h)
 	return buf;
 }
 
-DEFPY (debug_ospf6_lsa_all,
-       debug_ospf6_lsa_all_cmd,
-       "[no$no] debug ospf6 lsa all",
-       NO_STR
-       DEBUG_STR
-       OSPF6_STR
-       "Debug Link State Advertisements (LSAs)\n"
-       "Display for all types of LSAs\n")
+DEFPY(debug_ospf6_lsa_all, debug_ospf6_lsa_all_cmd,
+      "[no$no] debug ospf6 lsa all",
+      NO_STR DEBUG_STR OSPF6_STR
+      "Debug Link State Advertisements (LSAs)\n"
+      "Display for all types of LSAs\n")
 {
 	unsigned int i;
 	struct ospf6_lsa_handler *handler = NULL;
diff --git a/ospf6d/ospf6_route.c b/ospf6d/ospf6_route.c
index 92c50238a..0d02cdd11 100644
--- a/ospf6d/ospf6_route.c
+++ b/ospf6d/ospf6_route.c
@@ -1834,14 +1834,11 @@ void ospf6_brouter_show(struct vty *vty, struct ospf6_route *route)
 		OSPF6_PATH_TYPE_NAME(route->path.type), area);
 }
 
-DEFPY (debug_ospf6_route_all,
-       debug_ospf6_route_all_cmd,
-       "[no$no] debug ospf6 route all",
-       NO_STR
-       DEBUG_STR
-       OSPF6_STR
-       "Debug routes\n"
-       "Debug for all types of route calculation\n")
+DEFPY(debug_ospf6_route_all, debug_ospf6_route_all_cmd,
+      "[no$no] debug ospf6 route all",
+      NO_STR DEBUG_STR OSPF6_STR
+      "Debug routes\n"
+      "Debug for all types of route calculation\n")
 {
 	if (!no) {
 		OSPF6_DEBUG_ROUTE_ON(OSPF6_DEBUG_ROUTE_TABLE);

If you are a new contributor to FRR, please see our contributing guidelines.

After making changes, you do not need to create a new PR. You should perform an amend or interactive rebase followed by a force push.

LabN-CI · 2021-08-18T13:38:23Z

Outdated results 💚

Basic BGPD CI results: SUCCESS, 0 tests failed

_	_
Result	SUCCESS git merge/9438 `51595ac`
Date	08/18/2021
Start	09:12:09
Finish	09:38:22
Run-Time	26:13
Total	1813
Pass	1813
Fail	0
Valgrind-Errors	0
Valgrind-Loss	0
Details	vncregress-2021-08-18-09:12:09.txt
Log	autoscript-2021-08-18-09:13:17.log.bz2
Memory	503 487 423

For details, please contact louberger

NetDEF-CI · 2021-08-18T16:53:52Z

Continuous Integration Result: SUCCESSFUL

Congratulations, this patch passed basic tests

Tested-by: NetDEF / OpenSourceRouting.org CI System

CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-21178/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

polychaeta

Thanks for your contribution to FRR!

Click for style suggestions

To apply these suggestions:

curl -s https://gist.githubusercontent.com/polychaeta/ccf4d2043045c14a33f6293fb742d43f/raw/febb046c1dd7632db67c0436fc76acec62533d0d/cr_9438_1629798986.diff | git apply

diff --git a/ospf6d/ospf6_lsa.c b/ospf6d/ospf6_lsa.c
index ac07704d2..353706f89 100644
--- a/ospf6d/ospf6_lsa.c
+++ b/ospf6d/ospf6_lsa.c
@@ -1021,14 +1021,11 @@ static char *ospf6_lsa_handler_name(const struct ospf6_lsa_handler *h)
 	return buf;
 }
 
-DEFPY (debug_ospf6_lsa_all,
-       debug_ospf6_lsa_all_cmd,
-       "[no$no] debug ospf6 lsa all",
-       NO_STR
-       DEBUG_STR
-       OSPF6_STR
-       "Debug Link State Advertisements (LSAs)\n"
-       "Display for all types of LSAs\n")
+DEFPY(debug_ospf6_lsa_all, debug_ospf6_lsa_all_cmd,
+      "[no$no] debug ospf6 lsa all",
+      NO_STR DEBUG_STR OSPF6_STR
+      "Debug Link State Advertisements (LSAs)\n"
+      "Display for all types of LSAs\n")
 {
 	unsigned int i;
 	struct ospf6_lsa_handler *handler = NULL;
diff --git a/ospf6d/ospf6_route.c b/ospf6d/ospf6_route.c
index 92c50238a..0d02cdd11 100644
--- a/ospf6d/ospf6_route.c
+++ b/ospf6d/ospf6_route.c
@@ -1834,14 +1834,11 @@ void ospf6_brouter_show(struct vty *vty, struct ospf6_route *route)
 		OSPF6_PATH_TYPE_NAME(route->path.type), area);
 }
 
-DEFPY (debug_ospf6_route_all,
-       debug_ospf6_route_all_cmd,
-       "[no$no] debug ospf6 route all",
-       NO_STR
-       DEBUG_STR
-       OSPF6_STR
-       "Debug routes\n"
-       "Debug for all types of route calculation\n")
+DEFPY(debug_ospf6_route_all, debug_ospf6_route_all_cmd,
+      "[no$no] debug ospf6 route all",
+      NO_STR DEBUG_STR OSPF6_STR
+      "Debug routes\n"
+      "Debug for all types of route calculation\n")
 {
 	if (!no) {
 		OSPF6_DEBUG_ROUTE_ON(OSPF6_DEBUG_ROUTE_TABLE);

If you are a new contributor to FRR, please see our contributing guidelines.

After making changes, you do not need to create a new PR. You should perform an amend or interactive rebase followed by a force push.

LabN-CI · 2021-08-24T10:28:36Z

Outdated results 💚

Basic BGPD CI results: SUCCESS, 0 tests failed

_	_
Result	SUCCESS git merge/9438 `69de79f`
Date	08/24/2021
Start	06:02:27
Finish	06:28:33
Run-Time	26:06
Total	1813
Pass	1813
Fail	0
Valgrind-Errors	0
Valgrind-Loss	0
Details	vncregress-2021-08-24-06:02:27.txt
Log	autoscript-2021-08-24-06:03:36.log.bz2
Memory	495 515 425

For details, please contact louberger

NetDEF-CI · 2021-08-24T13:08:52Z

Continuous Integration Result: SUCCESSFUL

Congratulations, this patch passed basic tests

Tested-by: NetDEF / OpenSourceRouting.org CI System

CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-21324/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

mjstapp · 2021-08-24T19:16:25Z

ospf6d/ospf6_route.c

@@ -1831,6 +1834,31 @@ void ospf6_brouter_show(struct vty *vty, struct ospf6_route *route)
 		OSPF6_PATH_TYPE_NAME(route->path.type), area);
 }

+DEFPY (debug_ospf6_route_all,


why not just add the 'all' keyword to the existing debug route command? that would eliminate some of the other changes.

mjstapp · 2021-08-24T19:17:47Z

ospf6d/ospf6_route.c

+		OSPF6_DEBUG_ROUTE_ON(OSPF6_DEBUG_ROUTE_TABLE);
+		OSPF6_DEBUG_ROUTE_ON(OSPF6_DEBUG_ROUTE_INTRA);
+		OSPF6_DEBUG_ROUTE_ON(OSPF6_DEBUG_ROUTE_INTER);
+		OSPF6_DEBUG_ROUTE_ON(OSPF6_DEBUG_ROUTE_MEMORY);


for the lsa debug change, you added a "_ALL" macro, combining all of the existing flags values. why not do that for the DEBUG_ROUTE flags too, instead of hard-coding each individual test?

Debug commad for all lsa types and route types are not present. Signed-off-by: Yash Ranjan <ranjany@vmware.com>

polychaeta

Thanks for your contribution to FRR!

Style checking failed; check logs

If you are a new contributor to FRR, please see our contributing guidelines.

After making changes, you do not need to create a new PR. You should perform an amend or interactive rebase followed by a force push.

LabN-CI · 2021-08-25T13:50:27Z

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

Results table

_	_
Result	SUCCESS git merge/9438 `d5cb350`
Date	08/25/2021
Start	09:24:13
Finish	09:50:25
Run-Time	26:12
Total	1813
Pass	1813
Fail	0
Valgrind-Errors	0
Valgrind-Loss	0
Details	vncregress-2021-08-25-09:24:13.txt
Log	autoscript-2021-08-25-09:25:21.log.bz2
Memory	505 513 426

For details, please contact louberger

NetDEF-CI · 2021-08-25T15:04:41Z

Continuous Integration Result: SUCCESSFUL

Congratulations, this patch passed basic tests

Tested-by: NetDEF / OpenSourceRouting.org CI System

CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-21375/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

mjstapp · 2021-08-25T15:13:03Z

ospf6d/ospf6_lsa.c

+		handler = vector_slot(ospf6_lsa_handler_vector, i);
+		if (handler == NULL)
+			continue;
+		if (CHECK_FLAG(handler->lh_debug, OSPF6_LSA_DEBUG_ALL)


Do we really need to do this loop twice? Could the check for ALL be added to the existing loop, with a continue instead of falling through to the individual categories?

Yeah, I think we do. For example if we apply
debug ospf6 lsa all
followed by
no debug ospf6 lsa inter-prefix
the output should be

do show running-config Building configuration... Current configuration: ! frr version 8.1-dev-MyOwnFRRVersion frr defaults traditional hostname frr no ip forwarding no ipv6 forwarding service integrated-vtysh-config ! debug ospf6 lsa unknown debug ospf6 lsa unknown originate debug ospf6 lsa unknown examine debug ospf6 lsa unknown flooding debug ospf6 lsa router debug ospf6 lsa router originate debug ospf6 lsa router examine debug ospf6 lsa router flooding debug ospf6 lsa network debug ospf6 lsa network originate debug ospf6 lsa network examine debug ospf6 lsa network flooding debug ospf6 lsa inter-prefix debug ospf6 lsa inter-prefix originate debug ospf6 lsa inter-prefix flooding debug ospf6 lsa inter-router debug ospf6 lsa inter-router originate debug ospf6 lsa inter-router examine debug ospf6 lsa inter-router flooding debug ospf6 lsa as-external debug ospf6 lsa as-external originate debug ospf6 lsa as-external examine debug ospf6 lsa as-external flooding debug ospf6 lsa nssa debug ospf6 lsa nssa originate debug ospf6 lsa nssa examine debug ospf6 lsa nssa flooding debug ospf6 lsa link debug ospf6 lsa link originate debug ospf6 lsa link examine debug ospf6 lsa link flooding debug ospf6 lsa intra-prefix debug ospf6 lsa intra-prefix originate debug ospf6 lsa intra-prefix examine debug ospf6 lsa intra-prefix flooding debug ospf6 lsa grace debug ospf6 lsa grace originate debug ospf6 lsa grace examine debug ospf6 lsa grace flooding

whereas if we keep the check for ALL inside the loop then the output will be (because of the way the debugs are being printed)

debug ospf6 lsa inter-prefix debug ospf6 lsa inter-prefix originate debug ospf6 lsa inter-prefix flooding

interesting - that's not what I would expect - maybe that's naive?
If I configure "debug foo", then I don't expect to get "show runn" output that's a lot different from what I configured; I wouldn't expect "debug foo all" to be so different from "debug foo lsa", for example. I was expecting that if you want to configure lots of specific debugs, you ... configure them explicitly, and your config inputs are largely the same as the output of "show running". Do we have other cases where there is an "all" keyword, and it behaves so differently?

Even I can't find an example of "all" that is handled this way. My idea behind this was that each LSA type has 3 subtypes (originate|examine|flooding) so we set them all for the "all" case. And in the case when one or more subtypes are unset then we print the remaining subtypes (baring the ones we unset). Similarly, we can unset all using the "no" statement and set particular subtypes explicitly. That was my understanding of the "all" case. Do you have anything else in mind?

This is an interesting question ... my inclination is that the operator isn't going to keep a "mental list" of all the debugs turned on or off ... on the other hand, I could see a situation where an operator is troubleshooting, turns on "all," and then sees they're getting too much information, so they start turning off debugs to cut the amount of information they're getting down, so I can see the utility of having this behavior.

I'm inclined to think this is a useful way of building the debugs.

riw777

code looks okay, need to resolve the issue about how debug all should work

mjstapp · 2021-09-14T15:37:36Z

we had some discussion about this "all" question during the weekly meeting, and there was agreement that this is a fine approach.

polychaeta added the ospfv3 label Aug 18, 2021

polychaeta reviewed Aug 18, 2021

View reviewed changes

ranjanyash54 force-pushed the debug_comm branch from 51595ac to 69de79f Compare August 24, 2021 09:56

polychaeta reviewed Aug 24, 2021

View reviewed changes

mjstapp self-requested a review August 24, 2021 15:47

mjstapp reviewed Aug 24, 2021

View reviewed changes

ospf6d: Add debug commands for lsa all and route all

d5cb350

Debug commad for all lsa types and route types are not present. Signed-off-by: Yash Ranjan <ranjany@vmware.com>

ranjanyash54 force-pushed the debug_comm branch from 69de79f to d5cb350 Compare August 25, 2021 11:55

polychaeta reviewed Aug 25, 2021

View reviewed changes

mjstapp reviewed Aug 25, 2021

View reviewed changes

riw777 reviewed Sep 7, 2021

View reviewed changes

mjstapp approved these changes Sep 14, 2021

View reviewed changes

mjstapp merged commit 5fbbd15 into FRRouting:master Sep 14, 2021

ranjanyash54 deleted the debug_comm branch September 26, 2021 17:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ospf6d: Add debug commands for lsa all and route all #9438

ospf6d: Add debug commands for lsa all and route all #9438

ranjanyash54 commented Aug 18, 2021

polychaeta left a comment

LabN-CI commented Aug 18, 2021 •

edited

Loading

Basic BGPD CI results: SUCCESS, 0 tests failed

NetDEF-CI commented Aug 18, 2021 •

edited

Loading

Continuous Integration Result: SUCCESSFUL

polychaeta left a comment

LabN-CI commented Aug 24, 2021 •

edited

Loading

Basic BGPD CI results: SUCCESS, 0 tests failed

NetDEF-CI commented Aug 24, 2021 •

edited

Loading

Continuous Integration Result: SUCCESSFUL

mjstapp Aug 24, 2021

mjstapp Aug 24, 2021

polychaeta left a comment

LabN-CI commented Aug 25, 2021

NetDEF-CI commented Aug 25, 2021

mjstapp Aug 25, 2021

ranjanyash54 Aug 29, 2021

mjstapp Aug 30, 2021

ranjanyash54 Aug 31, 2021 •

edited

Loading

riw777 Sep 7, 2021

riw777 left a comment

mjstapp commented Sep 14, 2021

ospf6d: Add debug commands for lsa all and route all #9438

ospf6d: Add debug commands for lsa all and route all #9438

Conversation

ranjanyash54 commented Aug 18, 2021

polychaeta left a comment

Choose a reason for hiding this comment

LabN-CI commented Aug 18, 2021 • edited Loading

Basic BGPD CI results: SUCCESS, 0 tests failed

NetDEF-CI commented Aug 18, 2021 • edited Loading

Continuous Integration Result: SUCCESSFUL

polychaeta left a comment

Choose a reason for hiding this comment

LabN-CI commented Aug 24, 2021 • edited Loading

Basic BGPD CI results: SUCCESS, 0 tests failed

NetDEF-CI commented Aug 24, 2021 • edited Loading

Continuous Integration Result: SUCCESSFUL

mjstapp Aug 24, 2021

Choose a reason for hiding this comment

mjstapp Aug 24, 2021

Choose a reason for hiding this comment

polychaeta left a comment

Choose a reason for hiding this comment

LabN-CI commented Aug 25, 2021

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

NetDEF-CI commented Aug 25, 2021

Continuous Integration Result: SUCCESSFUL

mjstapp Aug 25, 2021

Choose a reason for hiding this comment

ranjanyash54 Aug 29, 2021

Choose a reason for hiding this comment

mjstapp Aug 30, 2021

Choose a reason for hiding this comment

ranjanyash54 Aug 31, 2021 • edited Loading

Choose a reason for hiding this comment

riw777 Sep 7, 2021

Choose a reason for hiding this comment

riw777 left a comment

Choose a reason for hiding this comment

mjstapp commented Sep 14, 2021

LabN-CI commented Aug 18, 2021 •

edited

Loading

NetDEF-CI commented Aug 18, 2021 •

edited

Loading

LabN-CI commented Aug 24, 2021 •

edited

Loading

NetDEF-CI commented Aug 24, 2021 •

edited

Loading

ranjanyash54 Aug 31, 2021 •

edited

Loading