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

Ability to use different ports in weave #531

Closed
abligh opened this issue Apr 9, 2015 · 7 comments
Closed

Ability to use different ports in weave #531

abligh opened this issue Apr 9, 2015 · 7 comments
Labels
Milestone

Comments

@abligh
Copy link
Contributor

abligh commented Apr 9, 2015

In order to run weave 'dockerless' (and possibly for other uses) it's useful to specify different ports for weave to run on. This allows you to run two instances of weave without container separation between them.

To do this I also had to fiddle around to allow the http interface to be disabled or bind to (e.g.) localhost only, which is useful in itself.

A lightly tested patch is here.
abligh@3ad0bb3

I wrote to rade as follows:

My only question is as follows. Let's say I launch two peers like this:

192.200.0.1$ weaver -iface=veth0 -port=1111
192.200.0.2$ weaver -iface=veth0 -port=2222 192.200.0.1:1111

That seems to work and 192.200.0.2 listens on port 2222 and connects to 192.200.0.1 successfully.

But does the first peer (192.200.0.1) ever attempt to connect back to
the second peer (192.200.0.2)? If it does, how does it know what
port to connect back on? I can't find where it would use the 'Port'
constant in the current code to do this, but it's possible an assumption
is being made it's the same as the local port.

If it does, how does it know what port to connect back on? I can't
find where it would use the 'Port' constant in the current code to do
this, but it's possible an assumption is being made it's the same as
the local port.

rade replied:

All peers in a weave network will attempt to connect to a) the IP & port of the remote end of any outbound outbound connection that any weave peer has, and b) the IP & standard port of the remote end of any inbound connection that any weave peer has.

See https://github.com/weaveworks/weave/blob/master/router/connection_maker.go#L148.
Looks like you missed feeding the port into that one.

I'm not entirely sure what to do about that. The code does this:

        address := conn.RemoteTCPAddr()
        // try both portnumber of connection and standard port.  Don't use remote side of inbound connection.
        if conn.Outbound() {
            addTarget(address)
        }
        if host, _, err := net.SplitHostPort(address); err == nil {
            addTarget(NormalisePeerAddr(host))
        }

This tries the remote port number of the connection (which if the connection is inbound will be arbitrary I presume), and the standard port, as NormalisePeerAddr adds the default port (6783) if no port is specified. Making NormalisePeerAddr look at router.ListenPort is not the right solution, as that would be the port this router is listening on, not the port the peer is listening on.

The right solution here would be to ensure conn.RemoteTCPAddr() returns the port the remote peer is listening on. If the remote peer instantiated the connection, or if it's not connected at all yet, presumably that needs passing in the weave protocol?

@rade
Copy link
Member

rade commented Apr 9, 2015

This tries the remote port number of the connection (which if the connection is inbound will be arbitrary I presume)

It only tries the remote port of the connection if the connection is outbound. So that will not be an arbitrary port.

Making NormalisePeerAddr look at router.ListenPort is not the right solution, as that would be the port this router is listening on, not the port the peer is listening on.

It's the rightest solution that is tangible. At the very least it will allow you to run disjoint weave networks by picking a different port for each.

And for the the most part joining up those networks will work too, because connecting to the remote ends of outbound addresses is often sufficient. The exception are situations like "A and B are connected. C can connect to A but not B. B can connect to C. Now C connects to A.". B learns about the inbound connection through gossip from A and attempts to connect to C on the standard port, but this will succeed only if it's the same "standard" port.

@abligh
Copy link
Contributor Author

abligh commented Apr 9, 2015

That's at least a simple solution. It obviously works just fine where every instance in the network is running on the same port. I was trying to 'do the right thing' here. I suspect the right thing is for whatever describes the IP address in the gossip exchange to also include the port (if it doesn't already) which would probably just work if it's a textual string as opposed to a packed quad in network order (for instance).

For illustration in the problem cases below, I'll use A:1001 as a notation to mean instance A running on port 1001.

Problem 1

This is simpler than the case you mentioned.

Let's suppose we have A:1001 configured to connect to B:1002. A therefore initiates a TCP connection to B:1002. B is initiated without prior knowledge of A. Are there circumstances (for instance if the connection becomes interrupted) where B will attempt to connect back to A - here it's making an outbound connection, but there currently is no outbound connection? If so, does it have any way of knowing to connect back on port 1001. For instance in the gossip protocol handshake does A describe itself as A or does it - or could it - describe itself as A:1001? If not, then with the above change, it will try port 1002 rather than port 6783 - but neither of these will work.

Connecting to A:1002 could theoretically connect to an entirely different weave instance, and bridge to an entirely different weave network. I am aware of the use of encryption to avoid this, but it would be best not having the logs filled with spurious authentication attempts.

Problem 2

Along the lines you mentioned, but simpler.

A:1001 and B:1001 are connected, but are both behind the same NAT. Nothing outside can connect in.C:2222 is out on the internet somewhere. A:1001 is configured to connect to C:2222 which succeeds. B cannot connect to C because it has no way of knowing it's on port 2222. C cannot connect to B because of the NAT. This would have worked if C was on the default port. It would also work if B learnt from A that C was C:2222 not just C.

@rade
Copy link
Member

rade commented Apr 9, 2015

I suspect the right thing is for whatever describes the IP address in the gossip exchange to also include the port (if it doesn't already) which would probably just work if it's a textual string as opposed to a packed quad in network order (for instance).

That's exactly what happens. When weave gossips information about connections, the remote endpoint of the connection is an IP:port combo in text form.

Are there circumstances (for instance if the connection becomes interrupted) where B will attempt to connect back to A - here it's making an outbound connection, but there currently is no outbound connection?

That is the wrong question :) Peers do not make connections to other peers; they make connections to IP addresses and ports, where they (hope to) find other peers.

The IP:port combinations a peer will attempt to connect to come from three sources:

  1. IP:port specified when that peer was weave launched or weave connected
  2. IP:port of the remote end of an outbound connection another peer has
  3. IP:standard-port of the remote end of an inbound connection that another peer has

Your concern about attempting to connect the wrong port will generally only manifest due to 3. However it can also arise anyway, because the same IP:port can represent completely different endpoints on different hosts.

It would also work if B learnt from A that C was C:2222 not just C.

That is (almost) exactly what happens. B learns from A that A has a connection to SomeIP:SomePort, where it found C. B will attempt to connect to that SomeIP:SomePort; it doesn't care what peer it finds there.

@abligh
Copy link
Contributor Author

abligh commented Apr 9, 2015

OK great.

Would you prefer I fixed NormalisePeerAddr to look at the command line option then? I think it makes no difference to actual connectivity, as if the port is already sent, then (3) is never going to happen above, and if you are using different ports at all (3) is going to be at best unreliable.

Advantage of fixing it: you can change the port with -port on the command line and not have to specify peer ports if all the port settings are the same across a whole weave network.

Disadvantage of fixing it: currently omitting the port number means port 6783. If an IP address without a port were ever sent on the wire, the meaning of that would be different for sender and receiver if they had different -port settings.

@rade
Copy link
Member

rade commented Apr 9, 2015

Would you prefer I fixed NormalisePeerAddr to look at the command line option then?

Yes. Because we want a weave network where all peers are configured to use port 2345 to behave the same as one where all peers use the standard port. There are scenarios where (3) does provide extra connectivity that isn't accomplished with 1+2 alone. It won't help you in connecting peers that have been configured with different standard ports, but will help in connecting peers that have the same non-standard port.

If an IP address without a port were ever sent on the wire.

That would be a violation of the weave protocol.

@abligh
Copy link
Contributor Author

abligh commented Apr 9, 2015

Thanks. I've created PR #534

though it fails to pass Travis CI as it changes the API, hence this issue appears: #534 (comment)

@rade
Copy link
Member

rade commented Apr 10, 2015

resolved by #534.

@rade rade closed this as completed Apr 10, 2015
@rade rade added the feature label Apr 18, 2015
@rade rade added this to the 0.10.0 milestone Apr 18, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants