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: ignore client's port offer if outside of server testports range #29

Merged
merged 1 commit into from Feb 19, 2018

Conversation

Projects
None yet
2 participants
@vvidic
Contributor

vvidic commented Feb 15, 2018

If the twamp-server always accepts the client suggested port number
it might end up binding to a port outside of testport range as defined
in the config file. Ports outside of this range might be blocked by
an external firewall and cause twamp test packets to be blocked.

twamp: ignore client's port offer if outside of server testports range
If the twamp-server always accepts the client suggested port number
it might end up binding to a port outside of testport range as defined
in the config file.  Ports outside of this range might be blocked by
an external firewall and cause twamp test packets to be blocked.
@vvidic

This comment has been minimized.

Contributor

vvidic commented Feb 15, 2018

With this patch twping port on the server side starts to use testports 18760-19960 range:

--- twping statistics from [localhost]:9085 to [localhost]:18919 ---
SID:    7f000001de301d81c952ea772d1b3e84
first:  2018-02-15T15:53:55.076
last:   2018-02-15T15:54:05.793
100 sent, 0 lost (0.000%), 0 send duplicates, 0 reflect duplicates

@vvidic vvidic requested review from mfeit-internet2, erik-geant and tonin Feb 17, 2018

@vvidic

This comment has been minimized.

Contributor

vvidic commented Feb 17, 2018

Please review this one too.

@tonin

tonin approved these changes Feb 19, 2018

As this is only run when twoway request is coming in, it looks safe to accept it.

@tonin tonin merged commit c0fe065 into master Feb 19, 2018

@tonin

This comment has been minimized.

Member

tonin commented Feb 19, 2018

Merging so that Victor can test tomorrow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment