Skip to content
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.

Sign up for GitHub

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

Fix ping crash on Enimga2 linux distribution (mipsle) #3877

Merged
merged 3 commits into from
May 1, 2018
Merged

Fix ping crash on Enimga2 linux distribution (mipsle) #3877

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
36354c6
Fix ping crash on BusyBox linux distribution (Enigma2 mipsle)
Mar 9, 2018
2852757
Change default PingInterval to 1.0
Apr 17, 2018
bc8d684
Merge remote-tracking branch 'remotes/upstream/master' into enigma2_ping
Apr 17, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 14 additions & 11 deletions plugins/inputs/ping/ping.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,17 +171,17 @@ func (p *Ping) args(url string) []string {
// Build the ping command args based on toml config
args := []string{"-c", strconv.Itoa(p.Count), "-n", "-s", "16"}
if p.PingInterval > 0 {
args = append(args, "-i", strconv.FormatFloat(p.PingInterval, 'f', 1, 64))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can just change this to use the -1 precision:

args = append(args, "-i", strconv.FormatFloat(p.PingInterval, 'f', -1, 64))

It still will require users to specify an even value, but I think it is better than switching on arch.

args = append(args, "-i", strconv.FormatFloat(p.PingInterval, 'f', -1, 64))
}
if p.Timeout > 0 {
switch runtime.GOOS {
case "darwin":
args = append(args, "-W", strconv.FormatFloat(p.Timeout*1000, 'f', 1, 64))
args = append(args, "-W", strconv.FormatFloat(p.Timeout*1000, 'f', -1, 64))
case "linux":
args = append(args, "-W", strconv.FormatFloat(p.Timeout, 'f', 1, 64))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can use -1 precision here too

args = append(args, "-W", strconv.FormatFloat(p.Timeout, 'f', -1, 64))
default:
// Not sure the best option here, just assume GNU ping?
args = append(args, "-W", strconv.FormatFloat(p.Timeout, 'f', 1, 64))
args = append(args, "-W", strconv.FormatFloat(p.Timeout, 'f', -1, 64))
}
}
if p.Deadline > 0 {
Expand Down Expand Up @@ -243,21 +243,24 @@ func processPingOutput(out string) (int, int, float64, float64, float64, float64
}
} else if strings.Contains(line, "min/avg/max") {
stats := strings.Split(line, " ")[3]
min, err = strconv.ParseFloat(strings.Split(stats, "/")[0], 64)
data := strings.Split(stats, "/")
min, err = strconv.ParseFloat(data[0], 64)
if err != nil {
return trans, recv, min, avg, max, stddev, err
}
avg, err = strconv.ParseFloat(strings.Split(stats, "/")[1], 64)
avg, err = strconv.ParseFloat(data[1], 64)
if err != nil {
return trans, recv, min, avg, max, stddev, err
}
max, err = strconv.ParseFloat(strings.Split(stats, "/")[2], 64)
max, err = strconv.ParseFloat(data[2], 64)
if err != nil {
return trans, recv, min, avg, max, stddev, err
}
stddev, err = strconv.ParseFloat(strings.Split(stats, "/")[3], 64)
if err != nil {
return trans, recv, min, avg, max, stddev, err
if len(data) == 4 {
stddev, err = strconv.ParseFloat(data[3], 64)
if err != nil {
return trans, recv, min, avg, max, stddev, err
}
}
}
}
Expand All @@ -268,7 +271,7 @@ func init() {
inputs.Add("ping", func() telegraf.Input {
return &Ping{
pingHost: hostPinger,
PingInterval: 1.0,
PingInterval: 0.0,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry about the delay merging, should/can we change this back to 1.0?

Count: 1,
Timeout: 1.0,
Deadline: 10,
Expand Down
28 changes: 25 additions & 3 deletions plugins/inputs/ping/ping_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,19 @@ PING www.google.com (216.58.218.164) 56(84) bytes of data.
rtt min/avg/max/mdev = 35.225/43.628/51.806/5.325 ms
`

// BusyBox v1.24.1 (2017-02-28 03:28:13 CET) multi-call binary
var busyBoxPingOutput = `
PING 8.8.8.8 (8.8.8.8): 56 data bytes
64 bytes from 8.8.8.8: seq=0 ttl=56 time=22.559 ms
64 bytes from 8.8.8.8: seq=1 ttl=56 time=15.810 ms
64 bytes from 8.8.8.8: seq=2 ttl=56 time=16.262 ms
64 bytes from 8.8.8.8: seq=3 ttl=56 time=15.815 ms

--- 8.8.8.8 ping statistics ---
4 packets transmitted, 4 packets received, 0% packet loss
round-trip min/avg/max = 15.810/17.611/22.559 ms
`

// Fatal ping output (invalid argument)
var fatalPingOutput = `
ping: -i interval too short: Operation not permitted
Expand All @@ -65,6 +78,15 @@ func TestProcessPingOutput(t *testing.T) {
assert.InDelta(t, 43.628, avg, 0.001)
assert.InDelta(t, 51.806, max, 0.001)
assert.InDelta(t, 5.325, stddev, 0.001)

trans, rec, min, avg, max, stddev, err = processPingOutput(busyBoxPingOutput)
assert.NoError(t, err)
assert.Equal(t, 4, trans, "4 packets were transmitted")
assert.Equal(t, 4, rec, "4 packets were transmitted")
assert.InDelta(t, 15.810, min, 0.001)
assert.InDelta(t, 17.611, avg, 0.001)
assert.InDelta(t, 22.559, max, 0.001)
assert.InDelta(t, -1.0, stddev, 0.001)
}

// Test that processPingOutput returns an error when 'ping' fails to run, such
Expand Down Expand Up @@ -106,7 +128,7 @@ func TestArgs(t *testing.T) {
"12000.0", "www.google.com"}
default:
expected = []string{"-c", "2", "-n", "-s", "16", "-I", "eth0", "-W",
"12.0", "www.google.com"}
"12", "www.google.com"}
}

p.Deadline = 24
Expand All @@ -117,7 +139,7 @@ func TestArgs(t *testing.T) {
"12000.0", "-t", "24", "www.google.com"}
default:
expected = []string{"-c", "2", "-n", "-s", "16", "-I", "eth0", "-W",
"12.0", "-w", "24", "www.google.com"}
"12", "-w", "24", "www.google.com"}
}

sort.Strings(actual)
Expand All @@ -133,7 +155,7 @@ func TestArgs(t *testing.T) {
"12000.0", "-t", "24", "-i", "1.2", "www.google.com"}
default:
expected = []string{"-c", "2", "-n", "-s", "16", "-I", "eth0", "-W",
"12.0", "-w", "24", "-i", "1.2", "www.google.com"}
"12", "-w", "24", "-i", "1.2", "www.google.com"}
}
sort.Strings(actual)
sort.Strings(expected)
Expand Down