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

fixing VPN server problem with multiple network interface #1156

Merged
merged 7 commits into from
Apr 25, 2022
Merged

fixing VPN server problem with multiple network interface #1156

merged 7 commits into from
Apr 25, 2022

Conversation

mrpalide
Copy link
Contributor

@mrpalide mrpalide commented Apr 19, 2022

Did you run make format && make check? Yes

Fixes #1145

Changes:

  • add new argument to vpn-server -netifc
  • add cli related command

How to test this PR:

  • check you have more than one default network interface by ip r command, if not make it.
  • make build
  • start vpn-server from UI and check logs, you should see an error like this:
    [2022-04-19T04:45:50+04:30] INFO (STDERR) [proc:vpn-server:8ba78bc35b91411081c546dbb5ca1edd]: time="2022-04-19T04:45:50+04:30" level=fatal msg="Error creating VPN server" error="multiple default network interface detected, please set once in setting or be single: [enp0s20u3 wlp7s0]"
  • set -netifc argument in vpn-server part of config
  • start vpn-server again, and would start normaly
  • actually, you can set it by skywire-cli config update vpns --netifc xxx [Not work now, get an error FATAL []: Failed to parse config. error="open : no such file or directory", need handle by @0pcom help ]
  • and, you can set it by hypervisor UI too [need Add a netifc field in the UI for configuring the VPN server #1158]

@mrpalide mrpalide changed the title [WIP] fixing VPN server problem with multiple network interface fixing VPN server problem with multiple network interface Apr 19, 2022
@0pcom 0pcom mentioned this pull request Apr 19, 2022
}
netIfcs, isMultiple := s.checkingNetworkInterface(defaultNetworkIfcs)
if isMultiple {
return nil, fmt.Errorf("multiple default network interface detected, please set once in setting or be single: %v", netIfcs)
Copy link
Member

Choose a reason for hiding this comment

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

"multiple default network interfaces detected...set a default one for VPN server or remove one: %v"

Copy link
Member

@jdknives jdknives Apr 20, 2022

Choose a reason for hiding this comment

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

Maybe

ifcs, hasMultiple := s.hasMutipleNetworkInterfaces(...)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

}
defaultNetworkIfc = defaultNetworkIfcs
} else {
defaultNetworkIfc = cfg.NetworkInteface
Copy link
Member

Choose a reason for hiding this comment

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

NetworkInterface

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@jdknives
Copy link
Member

Haven't tested it myself, but if it works, its almost good to go.

@ersonp
Copy link
Contributor

ersonp commented Apr 22, 2022

We need to add a check for the value of -netifc and it should be equal to the available interfaces. Currently we can pass anything it it for example

			{
				"name": "vpn-server",
				"args": [
					"-netifc",
					"asdasdasd"
				],
				"auto_start": false,
				"port": 44
			}

and it works.

[2022-04-22T13:52:20+05:30] INFO (STDERR) [proc:vpn-server:5937f58c91eb4e59a409c757314d6b30]: time="2022-04-22T13:52:20+05:30" level=info msg="Got default network interface: asdasdasd"
[2022-04-22T13:52:20+05:30] INFO (STDERR) [proc:vpn-server:5937f58c91eb4e59a409c757314d6b30]: time="2022-04-22T13:52:20+05:30" level=info msg="Got IPs of interface asdasdasd: []"

@ersonp
Copy link
Contributor

ersonp commented Apr 22, 2022

We should also add this error

[2022-04-22T14:08:04+05:30] INFO (STDERR) [proc:vpn-server:9a887a337a1148edb8292ef4081421f1]: time="2022-04-22T14:08:04+05:30" level=fatal msg="Error creating VPN server" error="multiple default network interfaces detected...set a default one for VPN server or remove one: [enp4s0 wlp5s0]

to proc errors just like the vpn-client adds it's errors.

@mrpalide
Copy link
Contributor Author

We should also add this error

[2022-04-22T14:08:04+05:30] INFO (STDERR) [proc:vpn-server:9a887a337a1148edb8292ef4081421f1]: time="2022-04-22T14:08:04+05:30" level=fatal msg="Error creating VPN server" error="multiple default network interfaces detected...set a default one for VPN server or remove one: [enp4s0 wlp5s0]

to proc errors just like the vpn-client adds it's errors.

Because mostly usage of vpn-server is on instances without GUI, I think this feature is not urgent for this release, so I make ticket for this [#1163].

@ersonp ersonp merged commit 61b62a8 into skycoin:develop Apr 25, 2022
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