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

command option to specify different ports for discovery (UDP) and listening (TCP) #22040

Closed
salanfe opened this issue Dec 20, 2020 · 8 comments
Closed

Comments

@salanfe
Copy link
Contributor

salanfe commented Dec 20, 2020

Rationale

currently the flag --port value of the networking options sets the same port for both UDP and TCP connections.

The issue is that Kubernetes does not support Service of type LoadBalancer with mix protocol on the same port. Creating the following Service:

apiVersion: v1
kind: Service
metadata:
  name: geth-p2p
  labels:
    app: geth-p2p
spec:
  type: LoadBalancer
  selector:
    app: geth
  ports:
    - name: listening
      port: 30303
      targetPort: 30303
      protocol: TCP
    - name: discovery
      port: 30303
      targetPort: 30303
      protocol: UDP

fails with the error:

the Service "geth-p2p" is invalid: spec.ports: Invalid value [...] cannot create an external load balancer with mix protocols

I'm currently deploying a eth2.0 staking system with multiple validators and kubernetes is for me the platform of choice for managing multiple workloads.

There are some workarounds like this one https://medium.com/asl19-developers/build-your-own-cloud-agnostic-tcp-udp-loadbalancer-for-your-kubernetes-apps-3959335f4ec3 but I would hope it's not necessary to rely on such hacks.

However TCP only is working fine, my node can sync. Nonetheless, the discovery through UDP is obviously not working for workloads in kubernetes that share the same port for TCP or UDP.

Implementation

I haven’t looked in details in the code-base what would be the effort to decouple the 2 protocols. I believe I would start from there

udpPortFlag = cli.IntFlag{

@ligi
Copy link
Member

ligi commented Dec 20, 2020

I can understand a load balancer for the RPC port - but I fail to understand a load balancer for the P2P port. Guess this might even fail to work as the nodes will have different node-keys (hope @fjl can chime in here). Can you elaborate a bit why you want a load balancer in front of the P2P port?

@salanfe
Copy link
Contributor Author

salanfe commented Dec 20, 2020

Thanks @ligi for the comment. Here the LoadBalancer has a 1 to 1 relationship with the stateful upstream Geth node. The LoadBalancer service is just a way to expose Geth to the outside world so that ingress TCP (and UDP) traffic can reach the node

@salanfe
Copy link
Contributor Author

salanfe commented Dec 22, 2020

@ligi in the doc, and from the logs of my node, I see that there is a discport. Example

enode://6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@10.3.58.6:30303?discport=30301

However I fail to understand how to set this discport. Do you have a hint ?

@fjl
Copy link
Contributor

fjl commented Dec 23, 2020

This is totally possible to implement, we just need to do it. The best place for this would be a new field in p2p.Config, then pass that through from cmd/utils/flags.go initialization code.

@ioitiki
Copy link

ioitiki commented May 25, 2021

+1 This would be very useful. Is there any progress on this?

@Nayshins
Copy link

Here is a workaround for the time being: https://people.lsdopen.io/gke-mixed-udp-tcp-loadbalancer/

@zeim839
Copy link
Contributor

zeim839 commented May 26, 2022

Hey all, here's a proposed solution:

Create a new discport flag in networking settings:

// ...
DiscPortFlag = cli.IntFlag {
    Name:  "discport",
    Usage: "Use a custom UDP port for P2P discovery",
    Value: 30303,
}

Add the flag to geth's node options:

utils.DiscPortFlag

Create a DiscAddr config parameter in P2P's server:

DiscAddr string

Modify the flag parser's setListenAddress to interpret the new flag and P2P server config parameter:

func setListenAddress(ctx *cli.Context, cfg *p2p.Config) {
	if ctx.GlobalIsSet(ListenPortFlag.Name) {
		cfg.ListenAddr = fmt.Sprintf(":%d", ctx.GlobalInt(ListenPortFlag.Name))
	}
	if ctx.GlobalIsSet(DiscPortFlag.Name) {
		cfg.DiscAddr = fmt.Sprintf(":%d", ctx.GlobalInt(DiscPortFlag.Name))
	}
}

Modify the P2P server's setupDiscovery to initialize with the custom port:

func (srv *Server) setupDiscovery() error {
        // ...

        if srv.NoDiscovery && !srv.DiscoveryV5 {
		return nil
	}

        // MODIFICATION:
	listenAddr := srv.ListenAddr
	if (srv.DiscAddr != "") {
		listenAddr = srv.Config.DiscAddr
	}

	addr, err := net.ResolveUDPAddr("udp", listenAddr)
	if err != nil {
		return err
	}

        // ...
}

Now, running geth with the flag:

geth --discport=1234

Yields an enode address with a custom UDP discport:

INFO [05-26|18:39:44.962] Started P2P networking                   self="enode://<id>@127.0.0.1:30303?discport=1234"

Let me know what you all think. Note that If this is implemented then the command will also have to be included in --help.

@fjl
Copy link
Contributor

fjl commented Jun 28, 2022

Flag --discovery.port added in #24979

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

No branches or pull requests

6 participants