-
Notifications
You must be signed in to change notification settings - Fork 44
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.
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
Added find max bandwidth feature. #96
base: master
Are you sure you want to change the base?
Added find max bandwidth feature. #96
Conversation
the flag to use it is -findMax and specify a server address, it will run multiple 3 seconds tests to find the maximum bandwidth.
…xBandwidth` function
…com:kaldughayem/scion-apps into kaldughayem/bwtestclient-find-max-bandwidth
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 1 files reviewed, 7 unresolved discussions (waiting on @kaldughayem)
bwtester/bwtestclient/bwtestclient.go, line 599 at r1 (raw file):
) clientBw := int64(512000)
initialTargetBw := 512e3 // 512 kbps
bwtester/bwtestclient/bwtestclient.go, line 606 at r1 (raw file):
// Setup the paths
l. 607-628 duplicates code from 342-363, refactor into a function.
bwtester/bwtestclient/bwtestclient.go, line 630 at r1 (raw file):
CCConn
Client control channel setup (630-652) is also duplicates code from l. 369-390 => refactor into a function.
bwtester/bwtestclient/bwtestclient.go, line 660 at r1 (raw file):
//Data channel connection
This section duplicates code from line 392 onwards. Refactor it so that the target bandwidth is a parameter,
bwtester/bwtestclient/bwtestclient.go, line 672 at r1 (raw file):
Do the test for 10 seconds
The comment does not match the value used.
bwtester/bwtestclient/bwtestclient.go, line 674 at r1 (raw file):
clientBwpStr := fmt.Sprintf("3,?,?,%dbps", clientBw)
Don't build the string to then again parse it, set the BwtestParameters directly.
bwtester/bwtestclient/bwtestclient.go, line 1000 at r1 (raw file):
var newBandwidth, newThreshold int64
Use named return values
`bwtestclient`: - `receiveDone` is a channel now - Refactored code to reduce duplication and make it easier to read - Changed some error logging messages `bwtestlib`: - `HandleDCConnReceive` takes a channel as a parameter instead of Lock, and signals it when it is done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 2 files reviewed, 7 unresolved discussions (waiting on @FR4NK-W)
bwtester/bwtestclient/bwtestclient.go, line 599 at r1 (raw file):
Previously, FR4NK-W wrote…
initialTargetBw := 512e3 // 512 kbps
Done.
bwtester/bwtestclient/bwtestclient.go, line 606 at r1 (raw file):
Previously, FR4NK-W wrote…
// Setup the paths
l. 607-628 duplicates code from 342-363, refactor into a function.
Done.
bwtester/bwtestclient/bwtestclient.go, line 630 at r1 (raw file):
Previously, FR4NK-W wrote…
CCConn
Client control channel setup (630-652) is also duplicates code from l. 369-390 => refactor into a function.
Done.
bwtester/bwtestclient/bwtestclient.go, line 660 at r1 (raw file):
Previously, FR4NK-W wrote…
//Data channel connection
This section duplicates code from line 392 onwards. Refactor it so that the target bandwidth is a parameter,
Done.
bwtester/bwtestclient/bwtestclient.go, line 672 at r1 (raw file):
Previously, FR4NK-W wrote…
Do the test for 10 seconds
The comment does not match the value used.
Forgot to change the value after testing.
Done
bwtester/bwtestclient/bwtestclient.go, line 674 at r1 (raw file):
Previously, FR4NK-W wrote…
clientBwpStr := fmt.Sprintf("3,?,?,%dbps", clientBw)
Don't build the string to then again parse it, set the BwtestParameters directly.
Done.
bwtester/bwtestclient/bwtestclient.go, line 1000 at r1 (raw file):
Previously, FR4NK-W wrote…
var newBandwidth, newThreshold int64
Use named return values
Done.
`bwtestclient`: - `receiveDone` is a channel now - Refactored code to reduce duplication and make it easier to read - Changed some error logging messages `bwtestlib`: - `HandleDCConnReceive` takes a channel as a parameter instead of mutex lock, and signals it when it is done
…com:kaldughayem/scion-apps into kaldughayem/bwtestclient-find-max-bandwidth
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 2 files reviewed, 9 unresolved discussions (waiting on @FR4NK-W and @kaldughayem)
bwtester/bwtestclient/bwtestclient.go, line 77 at r2 (raw file):
maxBandwidth bool pathEntry *sciond.PathReplyEntry
Just making everything global variables is really ugly. Most of these can and should be converted to appropriately scoped local variable without much effort.
bwtester/bwtestclient/bwtestclient.go, line 557 at r2 (raw file):
// connection, sets up the Data connection addresses, starts the Data Channel // connection, then it updates packet size. func initConns() {
To avoid the global variables: this can take client and server addresses as parameters and return the two connection objects (and an error code).
…lags with find max bandwidth option - Reduced global variables to 2 variables, all others are scoped local variables. - initConns function takes client and server addresses as input (along with pathAlgo and interactive for choosing the paths while setting up the addresses), and returns the connections and an error. - find max bandwidth option can be used with the 'cs' and/or the 'sc' flags to set the duration, packet sizes, and starting bandwidth for the test. If the flags are not set, default values are used
a1bf40c
to
8f025a8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 2 files reviewed, 9 unresolved discussions (waiting on @FR4NK-W and @matzf)
bwtester/bwtestclient/bwtestclient.go, line 77 at r2 (raw file):
Previously, matzf (Matthias Frei) wrote…
Just making everything global variables is really ugly. Most of these can and should be converted to appropriately scoped local variable without much effort.
Done.
Only two global variables are there now.
bwtester/bwtestclient/bwtestclient.go, line 557 at r2 (raw file):
Previously, matzf (Matthias Frei) wrote…
To avoid the global variables: this can take client and server addresses as parameters and return the two connection objects (and an error code).
Done.
Usage: add the flag
-findMax
and specify a server address with-s
, it will run multiple 3 seconds bandwidth tests until it finds the maximum bandwidth.Testing:
Using another AS inside a docker container running the server, and limit the bandwidth on that AS's interface using a tool. I use tcconfig for that, tc the traffic control.
Measure the bandwidth to a remote AS, the attachment points for example, and the returned bandwidth should roughly correspond to the ratio of upload:download speed available for the machine you're running the tests from.
This change is