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.

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

twamp toolchain & tests #16

Merged
merged 44 commits into from
Dec 7, 2017
Merged

twamp toolchain & tests #16

merged 44 commits into from
Dec 7, 2017

Conversation

erik-geant
Copy link
Contributor

The patch submitted some time ago by Robert Shearman and Duncan Eastoe was essentially working and stable. This patch includes those changes and unit tests for both owamp and twamp.

rshearman and others added 30 commits February 23, 2016 16:29
Run the command:
  sed -i 's/^\."/\.\\"/' *.man *.ms
over the man pages to eliminate warnings about macro " being
undefined. The macro for a comment is "/ instead.
Initial changes to support RFC5357 TWAMP protocol which is based on
and very similar to the OWAMP protocol.
Make twamp create a socket listening on the TWAMP TCP port, and allow
it to accept connections.

Create control state for the connection which is flagged as for a two
way connection.
Allow twping to open a socket connecting to a TWAMP server, using the
well-known TWAMP TCP port by default.

Flag the created control state as for a two way connection.
Handle the new TWAMP TestRequestTW message, which is similar to the
TestRequest message but without the Schedule Slot Descriptions and
only one HMAC.

Send negative accept messages for unexpected messages in the TWAMP server.

Don't handle FetchSession message in the TWAMP server, and don't send
StopSessions messages. The StopSessions message for twoway connections
doesn't have any session description records.
Adapt twping so that it requests just one two-way test session, rather
than a sender and a receiver session that owping does. Remove command
line options that don't make sense as a result.

Don't send a SID in the session request message for TWAMP. Ensure it's
filled in based on the value in the response.

Disallow use of OWPFetchSession for TWAMP control sessions.
Initialise endpoint for twoway sessions, using twoway packet size to
allocate payload buffer and set expected payload length. Re-use
existing code for creating datafile for two-way client and setting
socket options for receiving TTL and ensuring send/receiver buffers
are large enough.

Setup a second AES key for the two-way test sessions so that they can
both encrypt and decrypt packets.
Implement reflector for TWAMP server and TWAMP client sender/receiver.
Create a new format for two-way records, consisting simply of two
one-way records. Use the top bit of the version field to distinguish
between the two formats without having to define a new header format.

Add two-way stats records from the two-way client. Upon summarising,
display two-way packet delay variance calculated from round trip time
(client received - client sent) minus the packet processing delay
(reflector sent - reflector received), with the error being
accumulated from these. Display the min/max reflector processing time.

Reorder buffers aren't needed for TWAMP as twping waits for a sent
packet to be received before sending the next packet, so disable the
use and reporting of them for TWAMP.
Don't use owampd config, policy and state files for twampd. Make it
use its own ones.

Fix memory leaks in OWPDPolicyInstall.
Add documentation for twampd, extracting out command line options
common to both that are more likely than other details to change in
the future.

Add documentation for twping based on owping.
Add support for configurable REFWAIT timeout (idle test session
timeout) as per TWAMP RFC. Add a new global config option, testtimeout
to change this from the default.
The min, median and max delay values for the forward and backward
directions are now displayed after a twping session.

PDV for the forward and backward directions is also now shown.

Min/max processing delay is now shown in the machine readable output.

Median values for all directions are now displayed in the machine
readable output.

PDV is now shown in the machine readable output.
This prevents a "TTL not reported" message when there are no hops.

This also ensures that the min and max TTL and buckets are displayed
in the machine readable output.

MAXTTL in the machine readable output now displays the maximum TTL,
not the minimum TTL.
Previously, a maximum of 255 counts could be represented for each
TTL value. This commit ensures that a count can be represented for
each test packet for sessions with up to the maximum allowed number
of test packets.
Both the forward and backward hop count is now displayed after a
twping session.

In the machine readable output, the min and max TTL is shown for both
directions. The bucket output now also shows counts for both
directions.
Duplicate packet statistics are now calculated and displayed for
both the forward and backward directions.
@vvidic
Copy link
Contributor

vvidic commented Aug 21, 2017

Tried to test with twamp-gui but it does not work in either direction: 100% packet loss. The TCP control connection setup seems to work, but there is something broken with the UDP test packets.

@vvidic
Copy link
Contributor

vvidic commented Aug 21, 2017

I found twampd and twping binaries, but do we also need something like powstream for background measurements?

@vvidic
Copy link
Contributor

vvidic commented Aug 21, 2017

Communicating with Cisco CSR1000v virtual router also shows some protocol problems:

$ ./owping/twping 192.168.122.104
Approximately 13.0 seconds until results available
twping: FILE=endpoint.c, LINE=3412, Unable to send([Router]:0:(#0): Invalid argument
twping: FILE=endpoint.c, LINE=3412, Unable to send([Router]:0:(#1): Invalid argument
twping: FILE=endpoint.c, LINE=3412, Unable to send([Router]:0:(#24): Invalid argument
twping: FILE=endpoint.c, LINE=3412, Unable to send([Router]:0:(#38): Invalid argument
twping: FILE=endpoint.c, LINE=3412, Unable to send([Router]:0:(#54): Invalid argument
twping: FILE=endpoint.c, LINE=3412, Unable to send([Router]:0:(#74): Invalid argument
twping: FILE=endpoint.c, LINE=3412, Unable to send([Router]:0:(#94): Invalid argument

--- twping statistics from [192.168.122.104]:9488 to [jessie-amd64]:0 ---
SID:    c0a87a68dd454001916872b03d398cbb
first:  2017-08-21T13:18:27.387
last:   2017-08-21T13:18:38.192
7 sent, 7 lost (100.000%), 0 send duplicates, 0 reflect duplicates
round-trip time min/median/max = nan/nan/nan ms, (err=3.68 ms)
send time min/median/max = nan/nan/nan ms, (err=3.68 ms)
reflect time min/median/max = nan/nan/nan ms, (err=3.68 ms)
reflector processing time min/max = nan/nan ms
two-way jitter = nan ms (P95-P50)
send jitter = nan ms (P95-P50)
reflect jitter = nan ms (P95-P50)
send TTL not reported
reflect TTL not reported

Testing the same router with twamp-gui works without errors.

@deastoe
Copy link
Contributor

deastoe commented Sep 6, 2017

Hello,

I'm the original author of some of these patches. Thanks for picking these up and working at integrating them.

I've pushed some additional patches here: https://github.com/deastoe/owamp/commits/twamp-fixes. These should fix the inter-op issues with the Cisco CSR1000v and with the twamp-gui sender.

A quick test using twping towards the twamp-gui reflector points to an issue in twamp-gui. The reflected test packets had the error estimate fields set to all zeros, which twping detected as invalid and aborted the session.

@vvidic
Copy link
Contributor

vvidic commented Sep 11, 2017

After adding twamp-fixes Cisco CSR1000v looks better:

$ ./owping/twping 192.168.122.104
Approximately 13.0 seconds until results available

Directional delays may be inaccurate due to out of sync clocks!

--- twping statistics from [192.168.122.104]:9752 to [jessie-amd64]:9752 ---
SID:    c0a87a68dd60d2dc38d4fdf3ad0f8b0a
first:  2017-09-11T11:16:13.392
last:   2017-09-11T11:16:24.512
100 sent, 0 lost (0.000%), 0 send duplicates, 0 reflect duplicates
round-trip time min/median/max = 1.25/1.9/4.21 ms, (err=4.29e+12 ms)
send time min/median/max = -0.424/0.2/0.671 ms, (err=2.15e+12 ms)
reflect time min/median/max = 1.56/1.7/4.02 ms, (err=2.15e+12 ms)
reflector processing time min/max = 0/0 ms
two-way jitter = 0.5 ms (P95-P50)
send jitter = 0.4 ms (P95-P50)
reflect jitter = 0.2 ms (P95-P50)
send hops = 0 (consistently)
reflect hops = 0 (consistently)

@vvidic
Copy link
Contributor

vvidic commented Sep 11, 2017

Some tests are still failing when building as normal user, Erik can you check that?

FAIL: owping_enc
================

waiting for server: No such file or directory

FAIL: owping_clear
==================

waiting for server: No such file or directory

FAIL: twping_enc
================

waiting for server: No such file or directory

FAIL: twping_clear
==================

waiting for server: No such file or directory

@erik-geant
Copy link
Contributor Author

Do these tests fail every time you run make check? So far I'm not able to reproduce this ... under which environment do you run the tests?

Incidentally, the "waiting for server: No such file or directory" messages are expected and are not an error. The unit test just tries a few times waiting for the simulated server to startup before starting the ping client, and for simplicity prints errstring until it connects. Those messages don't appear in the production code.

@vvidic
Copy link
Contributor

vvidic commented Sep 11, 2017 via email

@erik-geant
Copy link
Contributor Author

I added a few progress messages in a temporary branch called twamp_test_debugging. Can you try this on your system?

I also noticed one branch where it could have been returning with an error and a proper message wasn't printed, so this might be where it's failing in your environment.

@vvidic
Copy link
Contributor

vvidic commented Sep 11, 2017

Here is the output now:

FAIL: owping_enc
================

waiting for server: No such file or directory
connected to server ...
calling XWPControlOpen...
XWPControlOpen returned
initialized tspec struct, calling OWPSessionRequest
cleaning up, result: 0

FAIL: owping_clear
==================

waiting for server: No such file or directory
connected to server ...
calling XWPControlOpen...
XWPControlOpen returned
initialized tspec struct, calling OWPSessionRequest
cleaning up, result: 0

FAIL: twping_enc
================

waiting for server: No such file or directory
connected to server ...
calling XWPControlOpen...
XWPControlOpen returned
initialized tspec struct, calling OWPSessionRequest
cleaning up, result: 0

FAIL: twping_clear
==================

waiting for server: No such file or directory
connected to server ...
calling XWPControlOpen...
XWPControlOpen returned
initialized tspec struct, calling OWPSessionRequest
cleaning up, result: 0

@erik-geant
Copy link
Contributor Author

maybe a timing problem ...?

I added a few more debug messages on that branch. Can you try again on your system?

@vvidic
Copy link
Contributor

vvidic commented Sep 11, 2017

The tests are passing with this patches now, can you make the threads behave so we don't have this timing problems?

@erik-geant
Copy link
Contributor Author

I took out most of those debugging messages to ensure I found the race condition and the additional printing isn't affecting the timing.

I put the change on the new branch: twamp_test_bugfix

Can you confirm the test pass on your system using this branch?

@vvidic
Copy link
Contributor

vvidic commented Sep 12, 2017 via email

@erik-geant
Copy link
Contributor Author

merged that change onto twamp branch

the build/test environment i'm working on right now is ubuntu xenial64, gcc 5.4.0, automake 1.15

... I also see those warnings and will fix them today

@erik-geant
Copy link
Contributor Author

I pushed some changes that fix a few of those warnings.

Which gcc -std option were used to generate those errors about loop variables? The gcc options pasted into the sample don't generate those in my current environment (which uses default -std value of gnu11). Requiring a very old standard like c90 then I can see those errors, as expected, but then the rest of the project can't be compiled.

@vvidic
Copy link
Contributor

vvidic commented Sep 12, 2017 via email

@erik-geant
Copy link
Contributor Author

Ok, that makes sense now. The gcc default language standard was changed from gnu90 to gnu11 in gcc 5.1.0.

@vvidic
Copy link
Contributor

vvidic commented Sep 12, 2017 via email

@vvidic
Copy link
Contributor

vvidic commented Sep 12, 2017 via email

@deastoe
Copy link
Contributor

deastoe commented Sep 13, 2017

I sent some patches to twamp-gui to fill the error estimate fields, so the direction twping -> twamp-responder should work now, please test.

Thanks - tests now appear to complete:


Directional delays may be inaccurate due to out of sync clocks!

--- twping statistics from [10.10.1.2]: to [10.10.1.1]: ---
SID:    00000000000000000000000000000000
first:  2017-09-13T11:46:28.306
last:   2017-09-13T11:46:37.988
100 sent, 0 lost (0.000%), 0 send duplicates, 0 reflect duplicates
round-trip time min/median/max = 0.389/1.2/19.8 ms, (err=0.0322 ms)
send time min/median/max = 1.86e+03/1.86e+03/1.88e+03 ms, (err=0.0161 ms)
reflect time min/median/max = -1.86e+03/-1.86e+03/-1.86e+03 ms, (err=0.0161 ms)
reflector processing time min/max = 0/1 ms
two-way jitter = 4.5 ms (P95-P50)
send jitter = 3.1 ms (P95-P50)
reflect jitter = 3.5 ms (P95-P50)
send hops = 255 (consistently)
reflect hops = 191 (consistently)

Looks like twamp-responder is not generating/returning a session ID, and also not setting the reflected test packet TTL values as per the RFC:

  • Extract the Sender TTL value from the TTL/Hop Limit value of
    received packets. Session-Reflector implementations SHOULD fetch
    the TTL/Hop Limit value from the IP header of the packet,
    replacing the value of 255 set by the Session-Sender. If an
    implementation does not fetch the actual TTL value (the only good
    reason not to do so is an inability to access the TTL field of
    arriving packets), it MUST set the Sender TTL value as 255.

So we need to make a patch that would take the addresses from the TCP control connection and use them for the UDP test packets if the 0-address was used during test setup.

I have put out a fix for this in PR #21.

@vvidic
Copy link
Contributor

vvidic commented Sep 13, 2017 via email

@vvidic
Copy link
Contributor

vvidic commented Sep 13, 2017

Do we want to use a new port range for TWAMP UDP tests or reuse the range from OWAMP?

@deastoe
Copy link
Contributor

deastoe commented Sep 13, 2017

Sent them a patch to return TTL of 255 since the real TTL value is not available from the QUdpSocket class.

Thank you.

Is there something wrong with the timestamps too since this message appears:

This message appears when any calculated directional delay is negative. This can occur for a number of reasons such as unsynced clocks, or when the latency between the sender and reflector is so small that the difference between the clocks of the sender and reflector becomes significant.

Therefore you can see this message when using twampd as the reflector and is not necessarily indicative of a problem with twamp-gui.

@vvidic
Copy link
Contributor

vvidic commented Sep 18, 2017 via email

@vvidic
Copy link
Contributor

vvidic commented Dec 7, 2017

If the test mesh is in a good state we could merge this to see if any problems with owamp appear?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants