Skip to content

Commit d4a3805

Browse files
authored
bwtester: tests and fixes for parameter parsing (#205)
Add tests for parseBwtestParameters, with small changes to function interface to allow calling this from the tests. Fix discrepancy in computation of bandwidth from other parameters which had previously led to valid command line parameters to be rejected (for example `1,?,?,1M`).
1 parent 19c614d commit d4a3805

File tree

3 files changed

+169
-22
lines changed

3 files changed

+169
-22
lines changed

bwtester/bwtestclient/bwtestclient.go

Lines changed: 24 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,6 @@ const (
4747
WildcardChar = "?"
4848
)
4949

50-
var (
51-
InferedPktSize int64
52-
)
53-
5450
func prepareAESKey() []byte {
5551
key := make([]byte, 16)
5652
n, err := rand.Read(key)
@@ -81,7 +77,7 @@ func printUsage() {
8177
// Input format (time duration,packet size,number of packets,target bandwidth), no spaces, question mark ? is wildcard
8278
// The value of the wildcard is computed from the other values, if more than one wildcard is used,
8379
// all but the last one are set to the defaults values
84-
func parseBwtestParameters(s string) BwtestParameters {
80+
func parseBwtestParameters(s string, defaultPktSize int64) (BwtestParameters, error) {
8581
if !strings.Contains(s, ",") {
8682
// Using simple bandwidth setting with all defaults except bandwidth
8783
s = "?,?,?," + s
@@ -102,7 +98,7 @@ func parseBwtestParameters(s string) BwtestParameters {
10298
if a[0] == WildcardChar {
10399
wildcards -= 1
104100
if wildcards == 0 {
105-
a2 = getPacketSize(a[1])
101+
a2 = getPacketSize(a[1], defaultPktSize)
106102
a3 = getPacketCount(a[2])
107103
a4 = parseBandwidth(a[3])
108104
a1 = (a2 * 8 * a3) / a4
@@ -131,10 +127,10 @@ func parseBwtestParameters(s string) BwtestParameters {
131127
a4 = parseBandwidth(a[3])
132128
a2 = (a4 * a1) / (a3 * 8)
133129
} else {
134-
a2 = InferedPktSize
130+
a2 = int64(defaultPktSize)
135131
}
136132
} else {
137-
a2 = getPacketSize(a[1])
133+
a2 = getPacketSize(a[1], defaultPktSize)
138134
}
139135
if a[2] == WildcardChar {
140136
wildcards -= 1
@@ -155,10 +151,15 @@ func parseBwtestParameters(s string) BwtestParameters {
155151
} else {
156152
a4 = parseBandwidth(a[3])
157153
// allow a deviation of up to one packet per 1 second interval, since we do not send half-packets
158-
if a2*a3*8/a1 > a4+a2*a1 || a2*a3*8/a1 < a4-a2*a1 {
159-
Check(fmt.Errorf("Computed target bandwidth does not match parameters, "+
160-
"use wildcard or specify correct bandwidth, expected %d, provided %d",
161-
a2*a3*8/a1, a4))
154+
expected := a2 * a3 * 8 / a1
155+
leeway := 8 * a2
156+
lo := expected - leeway
157+
hi := expected + leeway
158+
if a4 < lo || a4 > hi {
159+
return BwtestParameters{},
160+
fmt.Errorf("Computed target bandwidth does not match parameters, "+
161+
"use wildcard or specify correct bandwidth, expected %d-%d, provided %d",
162+
lo, hi, a4)
162163
}
163164
}
164165
key := prepareAESKey()
@@ -168,7 +169,7 @@ func parseBwtestParameters(s string) BwtestParameters {
168169
NumPackets: a3,
169170
PrgKey: key,
170171
Port: 0,
171-
}
172+
}, nil
172173
}
173174

174175
func parseBandwidth(bw string) int64 {
@@ -223,11 +224,11 @@ func getDuration(duration string) int64 {
223224
return a1
224225
}
225226

226-
func getPacketSize(size string) int64 {
227+
func getPacketSize(size string, defaultPktSize int64) int64 {
227228
a2, err := strconv.ParseInt(size, 10, 64)
228229
if err != nil {
229-
fmt.Printf("Invalid packet size %v provided, using default value %d\n", a2, InferedPktSize)
230-
a2 = InferedPktSize
230+
fmt.Printf("Invalid packet size %v provided, using default value %d\n", a2, defaultPktSize)
231+
a2 = defaultPktSize
231232
}
232233

233234
if a2 < MinPacketSize {
@@ -330,24 +331,25 @@ func main() {
330331
context.TODO(), "udp", clientDCAddr, serverDCAddr, addr.SvcNone)
331332
Check(err)
332333

334+
// use default packet size when within same AS and pathEntry is not set
335+
inferedPktSize := int64(DefaultPktSize)
333336
// update default packet size to max MTU on the selected path
334337
if path != nil {
335-
InferedPktSize = int64(path.Metadata().MTU)
336-
} else {
337-
// use default packet size when within same AS and pathEntry is not set
338-
InferedPktSize = DefaultPktSize
338+
inferedPktSize = int64(path.Metadata().MTU)
339339
}
340340
if !flagset["cs"] && flagset["sc"] { // Only one direction set, used same for reverse
341341
clientBwpStr = serverBwpStr
342342
fmt.Println("Only sc parameter set, using same values for cs")
343343
}
344-
clientBwp = parseBwtestParameters(clientBwpStr)
344+
clientBwp, err = parseBwtestParameters(clientBwpStr, inferedPktSize)
345+
Check(err)
345346
clientBwp.Port = uint16(clientDCAddr.Port)
346347
if !flagset["sc"] && flagset["cs"] { // Only one direction set, used same for reverse
347348
serverBwpStr = clientBwpStr
348349
fmt.Println("Only cs parameter set, using same values for sc")
349350
}
350-
serverBwp = parseBwtestParameters(serverBwpStr)
351+
serverBwp, err = parseBwtestParameters(serverBwpStr, inferedPktSize)
352+
Check(err)
351353
serverBwp.Port = uint16(serverDCAddr.Host.Port)
352354
fmt.Println("\nTest parameters:")
353355
fmt.Println("clientDCAddr -> serverDCAddr", clientDCAddr, "->", serverDCAddr)
Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
// Copyright 2021 ETH Zurich
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package main
16+
17+
import (
18+
"testing"
19+
"time"
20+
21+
"github.com/stretchr/testify/assert"
22+
"github.com/stretchr/testify/require"
23+
)
24+
25+
func TestParseParameters(t *testing.T) {
26+
defaultDuration := time.Second * DefaultDuration // XXX should be defined as Duration
27+
28+
cases := []struct {
29+
name string
30+
input string
31+
inferedPktSize int // input, value inferred from path MTU
32+
expectedDuration time.Duration
33+
expectedPacketSize int
34+
expectedNumPackets int
35+
expectErr bool
36+
}{
37+
{
38+
name: "bw 1Mbps",
39+
input: "1Mbps",
40+
inferedPktSize: 1400,
41+
expectedDuration: defaultDuration,
42+
expectedPacketSize: 1400,
43+
expectedNumPackets: 267,
44+
},
45+
{
46+
name: "bw 10Mbps",
47+
input: "10Mbps",
48+
inferedPktSize: 1400,
49+
expectedDuration: defaultDuration,
50+
expectedPacketSize: 1400,
51+
expectedNumPackets: 2678,
52+
},
53+
{
54+
name: "1s, 1Mbps",
55+
input: "1,?,?,1Mbps",
56+
inferedPktSize: 1400,
57+
expectedDuration: time.Second,
58+
expectedPacketSize: 1400,
59+
expectedNumPackets: 89,
60+
},
61+
{
62+
name: "1s, 1Mbps",
63+
input: "1,?,?,1Mbps",
64+
inferedPktSize: 1400,
65+
expectedDuration: time.Second,
66+
expectedPacketSize: 1400,
67+
expectedNumPackets: 89,
68+
},
69+
{
70+
name: "computed bw",
71+
input: "1,1000,1000,?",
72+
expectedDuration: time.Second,
73+
expectedPacketSize: 1000,
74+
expectedNumPackets: 1000,
75+
},
76+
{
77+
name: "computed bw, default pkt size",
78+
input: "1,?,1000,?",
79+
inferedPktSize: 1000,
80+
expectedDuration: time.Second,
81+
expectedPacketSize: 1000,
82+
expectedNumPackets: 1000,
83+
},
84+
{
85+
name: "redundant bw",
86+
input: "1,1000,1000,8Mbps",
87+
inferedPktSize: 1000,
88+
expectedDuration: time.Second,
89+
expectedPacketSize: 1000,
90+
expectedNumPackets: 1000,
91+
},
92+
{
93+
name: "redundant bw, longer duration",
94+
input: "10,125,10,1000bps",
95+
inferedPktSize: 125,
96+
expectedDuration: 10 * time.Second,
97+
expectedPacketSize: 125,
98+
expectedNumPackets: 10,
99+
},
100+
{
101+
name: "redundant bw, plus one packet per second",
102+
input: "10,125,10,2000bps", // should accept +1 packet/s, so +8*125B == +1000b
103+
inferedPktSize: 125,
104+
expectedDuration: 10 * time.Second,
105+
expectedPacketSize: 125,
106+
expectedNumPackets: 10,
107+
},
108+
{
109+
name: "redundant bw, minus one packet per second",
110+
input: "10,125,10,1bps", // should accept -1 packet/s, so -8*125B == -1000b
111+
inferedPktSize: 125,
112+
expectedDuration: 10 * time.Second,
113+
expectedPacketSize: 125,
114+
expectedNumPackets: 10,
115+
},
116+
{
117+
name: "conflicting bw",
118+
input: "1,1000,1000,1Mbps", // actual: 8Mbps
119+
expectErr: true,
120+
},
121+
{
122+
name: "computed packet size",
123+
input: "3,?,3000,8Mbps",
124+
inferedPktSize: 100,
125+
expectedDuration: 3 * time.Second,
126+
expectedPacketSize: 1000,
127+
expectedNumPackets: 3000,
128+
},
129+
}
130+
131+
for _, c := range cases {
132+
t.Run(c.name, func(t *testing.T) {
133+
ret, err := parseBwtestParameters(c.input, int64(c.inferedPktSize))
134+
if c.expectErr {
135+
assert.Error(t, err)
136+
} else {
137+
require.NoError(t, err)
138+
assert.Equal(t, c.expectedDuration, ret.BwtestDuration, "duration")
139+
assert.Equal(t, int64(c.expectedPacketSize), ret.PacketSize, "packet size")
140+
assert.Equal(t, int64(c.expectedNumPackets), ret.NumPackets, "num packets")
141+
}
142+
})
143+
}
144+
}

go.mod

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ require (
1515
github.com/pelletier/go-toml v1.9.3
1616
github.com/scionproto/scion v0.6.1-0.20210929154253-764d6e2afe47
1717
github.com/smartystreets/goconvey v1.6.4
18+
github.com/stretchr/testify v1.7.0
1819
golang.org/x/crypto v0.0.0-20210322153248-0c34fe9e7dc2
1920
golang.org/x/image v0.0.0-20191009234506-e7c1f5e7dbb8
2021
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1

0 commit comments

Comments
 (0)