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

Add NetworkProbe to switch case expression. #597

Merged

Conversation

taras-skycoin
Copy link
Member

@taras-skycoin taras-skycoin commented Nov 6, 2020

Did you run make format && make check?
yes
Fixes https://github.com/SkycoinPro/skywire-services/issues/316

Changes:

  • Allow to route NetworkProbePackat

How to test this PR:
You need to set up a test environment.

Copy link
Member

@jdknives jdknives left a comment

Choose a reason for hiding this comment

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

This change alone won't do. What happens is that Network Probe packets get forwarded but the remote visor identifies them as data packets and therefore forwards them to the application using the route. This causes problems.

Bildschirmfoto 2020-11-07 um 14 53 08

Copy link
Member

@jdknives jdknives left a comment

Choose a reason for hiding this comment

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

Good job. This fixed the issues with NetworkProbe packets being forwarded to applications. Before we merge, if this PR is related to this issue

Bildschirmfoto 2020-11-10 um 13 59 44

Please try to reproduce this warning on Visor B in integration env. @taras-skycoin

Copy link
Member

@jdknives jdknives left a comment

Choose a reason for hiding this comment

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

Just checked and the error is not related to your PR. Can be merged after one more review.

@jdknives jdknives merged commit 4b2708b into skycoin:develop Nov 12, 2020
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.

3 participants