Enabling/fixing debug/undebug feature for quagga/frr#254
Enabling/fixing debug/undebug feature for quagga/frr#254rodnymolina wants to merge 1 commit intosonic-net:masterfrom
Conversation
With this PR I'm taking care of the following issues:
* Fixed debug/undebug cli commands for bgp and zebra processes.
* Added a new cli command to activate/deactivate the generation of debuging info.
* Added two separated command structures for Quagga and FRR routing-stacks -- notice that they don't fully match.
* Also notice that FRR's approach doesn't require the explicit definition of all subcommands, as we're passing this information to FRR's vtysh daemon.
New debug/undebug stanzas now look like this:
Quagga example
==============
admin@lnos-x1-a-csw01:~$ debug ?
Usage: debug [OPTIONS] COMMAND [ARGS]...
SONiC command line - 'debug' command
Options:
-?, -h, --help Show this message and exit.
Commands:
bgp debug bgp events
disable disable debugging for routing events
enable enable debugging for routing events
zebra debug bgp events
admin@lnos-x1-a-csw01:~$ debug bgp ?
Usage: debug bgp [OPTIONS] COMMAND [ARGS]...
debug bgp events
Options:
-?, -h, --help Show this message and exit.
Commands:
as4 debug bgp AS4 actions
events debug bgp events
filters debug bgp filters
fsm debug bgp fsm
keepalives debug bgp keepalives
updates debug bgp updates
zebra debug bgp zebra messages
admin@lnos-x1-a-csw01:~$ debug zebra ?
Usage: debug zebra [OPTIONS] COMMAND [ARGS]...
debug bgp events
Options:
-?, -h, --help Show this message and exit.
Commands:
events debug zebra events
fpm debug zebra fpm events
kernel debug zebra's kernel-interface events
packet debug zebra packets
rib debug zebra RIB events
FRR example
============
admin@lnos-x1-a-csw02:~$ debug ?
Usage: debug [OPTIONS] COMMAND [ARGS]...
SONiC debugging commands for routing events
Options:
-?, -h, --help Show this message and exit.
Commands:
bgp Debug BGP information
disable Disable debugging for routing events
enable Enable debugging for routing events
zebra Debug Zebra information
admin@lnos-x1-a-csw02:~$ debug bgp ?
Command: sudo vtysh -c "debug bgp ?"
allow-martians BGP allow martian next hops
as4 BGP AS4 actions
bestpath BGP bestpath
keepalives BGP keepalives
neighbor-events BGP Neighbor Events
nht BGP nexthop tracking events
update-groups BGP update-groups
updates BGP updates
zebra BGP Zebra messages
admin@lnos-x1-a-csw02:~$ debug zebra ?
Command: sudo vtysh -c "debug zebra ?"
events Debug option set for zebra events
fpm Debug zebra FPM events
kernel Debug option set for zebra between kernel interface
mpls Debug option set for zebra MPLS LSPs
nht Debug option set for zebra next hop tracking
packet Debug option set for zebra packet
pseudowires Debug option set for zebra pseudowires
rib Debug RIB events
UT's
====
admin@lnos-x1-a-csw02:~$ debug enable
Command: sudo vtysh -c "configure terminal" -c "log syslog debugging"
admin@lnos-x1-a-csw02:~$ debug bgp ?
Command: sudo vtysh -c "debug bgp ?"
allow-martians BGP allow martian next hops
as4 BGP AS4 actions
bestpath BGP bestpath
keepalives BGP keepalives
neighbor-events BGP Neighbor Events
nht BGP nexthop tracking events
update-groups BGP update-groups
updates BGP updates
zebra BGP Zebra messages
admin@lnos-x1-a-csw02:~$ debug bgp keepalive
Command: sudo vtysh -c "debug bgp keepalive"
BGP keepalives debugging is on
admin@lnos-x1-a-csw02:~$ sudo tail -f /var/log/syslog [ destination log file is as per /etc/rsyslog.d/00-sonic.conf ]
...
May 9 23:51:48.525961 lnos-x1-a-csw02 DEBUG bgpd[65]: fc00:2:2::2 sending KEEPALIVE
May 9 23:51:48.526019 lnos-x1-a-csw02 DEBUG bgpd[65]: 10.2.2.2 sending KEEPALIVE
May 9 23:51:48.526054 lnos-x1-a-csw02 DEBUG bgpd[65]: fc00:2:2::2 KEEPALIVE rcvd
May 9 23:51:48.526123 lnos-x1-a-csw02 DEBUG bgpd[65]: 10.2.2.2 KEEPALIVE rcvd
...
admin@lnos-x1-a-csw02:~$ undebug bgp keepalive
Command: sudo vtysh -c "no debug bgp keepalive"
BGP keepalives debugging is off
nikos-github
left a comment
There was a problem hiding this comment.
The comments in #222 which you haven't followed, still apply.
|
I did took #222 into account, and that's one of the reasons i removed all FRR related debug commands. Only Quagga debug commands are exposed, as this one is not using our knob to pass cli-commands transparently to the backend. Concerning the 'undebug' vs 'no debug', we can discuss if we want to migrate in the future towards the 'no debug' stanza, but what we have today is 'undebug', and this one is broken, so this PR is simply fixing what is already out there. |
|
Right, my goal is to extend my FRR changes to Quagga too, but that will require more changes in more repos. This current patch is already involving changes in three different repos. Quagga changes can come later. We should fix 'debug/undebug' cli now. If the cli implemented by the backend services change, we can later decide to update our front-end cli for end-to-end consistency, or simply leave it the way it is today as we wouldn't be breaking any functionality thanks to Click wrappers. |
Description Fixed typo FW_AUTO_ERR_UKNOWN --> FW_AUTO_ERR_UNKNOWN Motivation and Context This is a typo and in the interest of a readable code base should not remain. There are also very few references to this as of now so changing it now before more references are added is a priority. How Has This Been Tested? Unit tests passing
…et#254) ***Important***: Please review each commit (including commit message) individually. Looking at the patch-set as a whole may cause confusion. #### What I did Generic Configuration Updater is extremely slow, using the python profiler it was possible to determine the worst offenders where changes could be made without affecting the overall algorithm and HLD design documentation. Brief overview of changes: * Prevent copy.deepcopy() calls where possible * Don't run validation twice back to back * Move configdb path <> xpath conversion logic to sonic-yang-mgmt where it belongs and enhance it to support schema conversion (not just data) and add caching. * Sort table keys by the number of schema backlinks and must statements for the node to try better guess the right order of the patches to generate rather than doing it in alphabetical order which is likely to cause validation failures. * Add ability to Group patches together in some commits where its known they will not cause issues, these are things like grouping parameter updates under the same key. * When applying changes, do not re-read the configuration from redis twice between each applied patch (this is **extremely** slow, and actually hid a race condition). We are mutating the configuration and a lock is held so we know the expected before and after. There is still a final validation to ensure something didn't go sideways. Dependencies: failure_prs.log sonic-yang-mgmt enhancements: sonic-net/sonic-buildimage#22254 failure_prs.log sonic-yang-mgmt parse uses/grouping: sonic-net/sonic-buildimage#21907 failure_prs.log sonic-utilities rely on sonic-yang-mgmt uses/grouping handling: sonic-net#3814 Stats below ... (stats need both this and the sonic-utilities PR to be relevant)... <ins>**Original Performance:**</ins> Dry Run: ``` time sudo config replace -d ./config_db.json ... real 2m51.588s user 2m23.777s sys 0m25.300s ``` Full: ``` time sudo config replace ./config_db.json ... real 14m53.772s user 12m2.376s sys 2m8.908s ``` <ins>**With Patch**:</ins> Dry Run: ``` time sudo config replace -d ./config_db.json ... real 0m59.602s user 0m56.434s sys 0m2.110s ``` Full: ``` time sudo config replace ./config_db.json ... real 1m54.303s user 0m58.482s sys 0m2.545s ``` So that's roughly 3x improvement for dry-run, and 7.5x improvement for full commit. There is room for improvement on the full commit due to a `sleep(1)` being used between each patch because of a race condition found in the prior code (that was hidden due to a costly sanity check that has been removed). #### How I did it Profiling via cProfiler #### How to verify it Run sonic-utilities test cases, they still pass #### Previous command output (if the output of a command-line utility has changed) N/A #### New command output (if the output of a command-line utility has changed) N/A Fixes sonic-net/sonic-buildimage#22372
Note: PR depends on PR/1714
With this PR I'm taking care of the following issues:
New debug/undebug stanzas now look like this:
Quagga example
FRR example