frp icon indicating copy to clipboard operation
frp copied to clipboard

nathole need a ip filter for zerotiler-like virtual network[Feature Request]

Open yangyaofei opened this issue 1 year ago β€’ 9 comments

Describe the feature request

Like this issue: https://github.com/fatedier/frp/issues/3457, the zerotier ip has a low TTL. So , the nathole will use zerotier for transmission. But the zerotier can be very slow.

I think add a config nat_address_filter like 192.168.0.0/24 which will remove these ips from nathole.

To test that thought, I add several code in https://github.com/fatedier/frp/blob/dev/client/visitor/xtcp.go#L278:


func (sv *XTCPVisitor) makeNatHole() {
	xl := xlog.FromContextSafe(sv.ctx)
	xl.Trace("makeNatHole start")
	if err := nathole.PreCheck(sv.ctx, sv.helper.MsgTransporter(), sv.cfg.ServerName, 5*time.Second); err != nil {
		xl.Warn("nathole precheck error: %v", err)
		return
	}

	xl.Trace("nathole prepare start")
	prepareResult, err := nathole.Prepare([]string{sv.clientCfg.NatHoleSTUNServer})
	if err != nil {
		xl.Warn("nathole prepare error: %v", err)
		return
	}
        // Simple Filter one IP
	var ipAddress = "172.16.1.70"
	var filteredAddrs []string
	for _, addr := range prepareResult.AssistedAddrs {
		addrParts := strings.Split(addr, ":")
		if addrParts[0] != ipAddress {
			filteredAddrs = append(filteredAddrs, addr)
		}
	}
	prepareResult.AssistedAddrs = filteredAddrs    

I test the code above, it will make frp go direct not through zerotier.

Describe alternatives you've considered

The things I don't know is that if this is a good implement or not. Totally newbie for go.

So if this is good and this config make sense, I will make a PR for this.

Affected area

  • [ ] Docs
  • [ ] Installation
  • [X] Performance and Scalability
  • [ ] Security
  • [ ] User Experience
  • [ ] Test and Release
  • [ ] Developer Infrastructure
  • [ ] Client Plugin
  • [ ] Server Plugin
  • [ ] Extensions
  • [ ] Others

yangyaofei avatar Oct 23 '23 08:10 yangyaofei

I don't understand why you would want to use both zerotier and frp at the same time. Zerotier already supports peer-to-peer (p2p) connections, so I don't see the need to introduce more configuration complexity by using frp.

fatedier avatar Oct 23 '23 09:10 fatedier

@fatedier zerotier rubost and easy to use for a group of people to connect each others. I use it for ssh.

but it is not stable, and slow due to the reason that we all know. And it cannot make nat hole in my network(but frp can). So,when we need to debug or transfer files or deploy service from Lan to web site, we will use frp.

I need them allπŸ˜‚πŸ˜‚πŸ˜‚

yangyaofei avatar Oct 23 '23 09:10 yangyaofei

I tend to put it aside first and wait for more feedback on this scenario.

fatedier avatar Oct 23 '23 09:10 fatedier

Can I make PR for this?

yangyaofei avatar Oct 23 '23 09:10 yangyaofei

It will not accepted until we have clear and meaningful problem scenarios that are helpful to most people, especially when new configuration items need to be added.

The scenario you described, from my personal perspective, does not seem worth adding new configuration options to solve the problem. However, I am unsure if this is a common requirement for disabling internal addresses in AssistedAddrs(something like DisableInnerAssistedAddrs) , so I will temporarily leave it here and wait for more feedback from users.

fatedier avatar Oct 23 '23 09:10 fatedier

ok, I think most users can't notice this. This (wrong nathole) will disappear in lots of circumstances. I think it more like a performance bug (but it's a feature).

I will fork this for my own usage, pin me if you need a PR.

yangyaofei avatar Oct 23 '23 09:10 yangyaofei

@yangyaofei BTW, I think you can consider whether there is a possibility of optimization without introducing new configuration items.

I prefer not to expose complexity to users as it may increase their understanding cost. Many users may also misuse it, resulting in various issues being created, which will ultimately lead to high maintenance costs for the project. Not just this one feature, I mean various kinds of requirements. Some features may only be used by a few people in the end, but in order to ensure the correctness of each refactoring/iteration, it is necessary for developers to invest in long-term maintenance costs.

fatedier avatar Oct 23 '23 10:10 fatedier

I understand this philosophy but not fully agree. I think smart optimization may lead some unknown issues and hard to find and resolve.

Maybe add a config like enableSmartNatHole to enable is a good way.

yangyaofei avatar Oct 23 '23 10:10 yangyaofei