Add json output format for tools netem show
#2454
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@FloSch62 would you find some juice to add a robot test for this? Or I can do it later, both is fine |
|
Sure |
|
@hellt Done |
vista-
reviewed
Feb 8, 2025
|
agreeing to all point of @vista- |
|
IMO it should be an empty string instead of "N/A" |
|
Agree with @kaelemc, let the formatter handle nil values (and represent those in the proper data format). In this case, maybe representing the values as |
|
Agree, if someone wants to change it, feel free to do so. I would need to find some time |
hellt
reviewed
Feb 12, 2025
Comment on lines
+285
to
+302
| if qdisc.Netem.Latency64 != nil && *qdisc.Netem.Latency64 != 0 { | ||
| delay = (time.Duration(*qdisc.Netem.Latency64) * time.Nanosecond).String() | ||
| } | ||
| if qdisc.Netem.Jitter64 != nil && *qdisc.Netem.Jitter64 != 0 { | ||
| jitter = (time.Duration(*qdisc.Netem.Jitter64) * time.Nanosecond).String() | ||
| } | ||
| if qdisc.Netem.Rate != nil && int(qdisc.Netem.Rate.Rate) != 0 { | ||
| rate = int(qdisc.Netem.Rate.Rate * 8 / 1000) | ||
| } | ||
| if qdisc.Netem.Corrupt != nil && qdisc.Netem.Corrupt.Probability != 0 { | ||
| // round to 2 decimal places | ||
| corruption = math.Round((float64(qdisc.Netem.Corrupt.Probability)/ | ||
| float64(math.MaxUint32)*100)*100) / 100 | ||
| } | ||
| if qdisc.Netem.Qopt.Loss != 0 { | ||
| // round to 2 decimal places | ||
| loss = math.Round((float64(qdisc.Netem.Qopt.Loss)/float64(math.MaxUint32)*100)*100) / 100 | ||
| } |
There was a problem hiding this comment.
I have changed the output types to be float/int/string for the respecting netem values, with empty values represented by go types.
here is the output they produce.
{
"interface": "mgmt0",
"delay": "1s",
"jitter": "5ms",
"packet_loss": 0.1,
"rate": 0,
"corruption": 0.2
},Everyone OK with the change?
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2454 +/- ##
==========================================
+ Coverage 52.48% 52.60% +0.11%
==========================================
Files 172 172
Lines 17614 17677 +63
==========================================
+ Hits 9245 9299 +54
- Misses 7398 7404 +6
- Partials 971 974 +3
|
steiler
reviewed
Feb 13, 2025
| @@ -83,6 +88,11 @@ var netemShowCmd = &cobra.Command{ | |||
| } | |||
|
|
|||
| func netemSetFn(_ *cobra.Command, _ []string) error { | |||
| // Ensure that the sch_netem kernel module is loaded (for Fedora/RHEL compatibility) | |||
| if err := exec.Command("modprobe", "sch_netem").Run(); err != nil { | |||
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR brings
-format jsontocontainerlab tools netem show --node clab-vlan-srl1 --format jsonAnd we do a modprobe sch_netem first before setting an impairment to ensure rhel/fedora can directly use it.
closes #1848