muellmusik / Utopia

This is an attempt at a generic library of tools for making Network Music Apps in SuperCollider
43 stars 8 forks source link

addrBook.me is not kept in sync with the peer's own entry in the address book #3

Closed sidechained closed 6 years ago

sidechained commented 11 years ago

Hi Scott (and maybe Julian?),

I'm working with Jonas here in Guangzhou on a new project using two laptops and two BeagleBoards running Utopia for networking. Right now we're doing some basic ad hoc registration with the two laptops and then trying to share server addresses on top of that with an OSCObjectSpace, just to be able to play on each other's servers.

The OSCObjectSpace sync method doesn't work as expected. It tries to look for the first non-local player and make a copy of their object space, but this test fails because the peer contained in the address book has a remote IP, whereas the peer returned by 'addrBook.me' has a local IP. It seems like the latter should automatically be kept in sync with the former, is that right?

For now, to get around the problem we simply changed from this:

syncAddr = addr ?? { addrBook.peers.reject({|peer| peer == addrBook.me }).detect({|peer| peer.online }).addr };

To this:

syncAddr = addr ?? { addrBook.peers.reject({|peer| peer == addrBook[addrBook.me.name] }).detect({|peer| peer.online }).addr };

…so that we're not relying on the addrBook.me at all. But this is not a good long term solution. In general, we're confused as to why the local IP needs be added to the address book at all. Shouldn't the address book only ever contain remote address?

You can check the basic problem for yourself by running the code from Examples.scd:

(
~win = Window("AdHocSociety").front;
~win.layout = VLayout.new.add(~listView = ListView.new);

~addrBook = AddrBook.new;
// to get updates, just add a dependant
~addrBook.addDependant({|addrBook, what, who|
    {~listView.items = addrBook.peers.collectAs({|peer|
        peer.name ++ " | " ++ peer.addr.ip ++ " | " ++ if(peer.online, "online", "offline");
    }, Array)}.defer;
});

~addrBook.addMe; // will automatically add you using your user name

~hail = Hail(~addrBook);
)

Then executing these lines:

~addrBook.me // returns local address
~addrBook[~addrBook.me.name] // returns the remote address contained at the same name

The broader question is, why does addrBook.me have to be defined at all before Hailing. Why not only define it once the hail-reply has been received?

Cheers, Jonas and Graham

muellmusik commented 11 years ago

Hi,

Firstly, thanks very much for testing and using this! This is really great.

I've just finished up the QQuartzComposerView stuff, so to be honest my mind is pretty lost in that code, but I can possibly look at this tomorrow.

I had also noticed the issue with AddrBook:me. The problem is partly that NetAddr.localAddr always returns the loopback address, and NetAddr == does not test to see what this is equivalent to. Possibly that could be replaced (or augmented) by a primitive which returns the address of the device to which SC has bound. To be honest though, it is not entirely clear to me how SC determines which device to bind to, and whether this is consistent across platforms. With BEER we have sometimes found it necessary to turn off Wifi for instance when playing 'wired'.

I also feel that whether or not an AddrBook should contain 'me' is an open question. I think in some cases you may want to refer to a collection of addresses including the local machine, and in some cases not. You may be right that it is better just not to include the local machine. I need to go through and look at the places where AddrBook:me is called and have another think about this.

There may be a better solution. Please do have a go if you like. We can do pull requests as on the main SC project if people are in agreement.

Anyway, I'll try and look tomorrow.

S.

telephon commented 11 years ago

On 20.08.2013, at 20:35, muellmusik notifications@github.com wrote:

Hi,

Firstly, thanks very much for testing and using this! This is really great.

I've just finished up the QQuartzComposerView stuff, so to be honest my mind is pretty lost in that code, but I can possibly look at this tomorrow.

I had also noticed the issue with AddrBook:me. The problem is partly that NetAddr.localAddr always returns the loopback address, and NetAddr == does not test to see what this is equivalent to. Possibly that could be replaced (or augmented) by a primitive which returns the address of the device to which SC has bound. To be honest though, it is not entirely clear to me how SC determines which device to bind to, and whether this is consistent across platforms. With BEER we have sometimes found it necessary to turn off Wifi for instance when playing 'wired'.

I also feel that whether or not an AddrBook should contain 'me' is an open question. I think in some cases you may want to refer to a collection of addresses including the local machine, and in some cases not. You may be right that it is better just not to include the local machine. I need to go through and look at the places where AddrBook:me is called and have another think about this.

There may be a better solution. Please do have a go if you like. We can do pull requests as on the main SC project if people are in agreement.

Anyway, I'll try and look tomorrow.

Hi,

iirc, in Republic we keep all addresses (including my own mirrored through the net) as well as one's own nickname. So that we can implement send the following way:

prSendWithDict { |dict, names, messages, latency| // var isServer = dict.choose.isKindOf(Server); // if (isServer) { "prSendWithDict, server - incoming names: ".repost; names.repostcs; }; names = names ? nickname; // send to myself if none is given

//  if (isServer) {"replaced names: ".repost; names.repostcs;};

    if(verbose) { "sent messages to: %.\nmessages: %\nlatency: %\n"
            .repostf(names, messages, latency)};
    if(names == \all) {
//  if (isServer) {"sending to all.".repostln;};

//      dict.repostcs;
        dict.do { |recv| recv.sendBundle(latency, *messages) }
    } {
//  if (isServer) { "names.asArray: ".repost; };

        names = this.replaceGroupNames(names.asArray);

        names.do { |name|
            var recv;
            if(name.isInteger) {
                name = nameList.wrapAt(name + myNameIndex);
            };
            recv = dict.at(name);
            if(recv.isNil) {
                "% is currently absent.\n".repostf(name)
            } {
                recv.sendBundle(latency, *messages)
            };
        }
    }
}

replaceGroupNames { |names| ^names.collect { |name| if (groups[name].notNil) { groups[name] } { name } }.flat; }

This is really a good scheme because you can have: messages that go to

muellmusik commented 10 years ago

Here is a possible hack to deal with this issue:

LocalNetAddr : NetAddr { var <alternateNames;

*new { arg hostname = "127.0.0.1", port=0;
    ^super.new(hostname, port).init;
}

init {
    alternateNames = Set.new;
    if(thisProcess.platform.name == \osx, {
        ["en0", "en1"].do({|interface|
            var hostname = "ipconfig getifaddr %".format(interface).unixCmdGetStdOut;
            if(hostname.size > 0, {alternateNames.add(hostname)});
        });
    });
}

addName {|name| alternateNames.add(name); }

== {|that| ^(super == that) || (alternateNames.includesEqual(that.hostName) && (that.port == this.port)) }

}

Hail or Registrar could be modified to capture alternate hostnames for the local address from other peers as well. But probably the best solution is to add a primitive to net address that checks if a given address is equivalent to local on any interface.

Thoughts?

S.

On 20 Aug 2013, at 19:26, Scott Wilson i@scottwilson.ca wrote:

Hi,

Firstly, thanks very much for testing and using this! This is really great.

I've just finished up the QQuartzComposerView stuff, so to be honest my mind is pretty lost in that code, but I can possibly look at this tomorrow.

I had also noticed the issue with AddrBook:me. The problem is partly that NetAddr.localAddr always returns the loopback address, and NetAddr == does not test to see what this is equivalent to. Possibly that could be replaced (or augmented) by a primitive which returns the address of the device to which SC has bound. To be honest though, it is not entirely clear to me how SC determines which device to bind to, and whether this is consistent across platforms. With BEER we have sometimes found it necessary to turn off Wifi for instance when playing 'wired'.

I also feel that whether or not an AddrBook should contain 'me' is an open question. I think in some cases you may want to refer to a collection of addresses including the local machine, and in some cases not. You may be right that it is better just not to include the local machine. I need to go through and look at the places where AddrBook:me is called and have another think about this.

There may be a better solution. Please do have a go if you like. We can do pull requests as on the main SC project if people are in agreement.

Anyway, I'll try and look tomorrow.

S.

telephon commented 10 years ago

On 20.08.2013, at 13:15, sidechained notifications@github.com wrote:

The OSCObjectSpace sync method doesn't work as expected. It tries to look for the first non-local player and make a copy of their object space, but this test fails because the peer contained in the address book has a remote IP, whereas the peer returned by 'addrBook.me' has a local IP. It seems like the latter should automatically be kept in sync with the former, is that right?

The way to get the real external address should be via the network, sending out to a broadcast (or if some non-broadcast server based scheme is used), and the receiving the own addr externally.

Hail or Registrar could be modified to capture alternate hostnames for the local address from other peers as well. But probably the best solution is to add a primitive to net address that checks if a given address is equivalent to local on any interface.

Thoughts?

It would be OK if me has the loopback IP first (before the ping over the network) and then somehow correct it, once it has been received.

It makes sense to have "me" in the address book if you want to keep (certain) lookups uniform. But indeed this is a very fundamental design decision. I think in order to support both perspectives, "me" might be needed as a functional option.

A good example is sending to servers: as all of them are "on the net" relative to sclang, they are on the same level, so you want to treat them equally (at least this is how we always did it in powerbooks unplugged).

muellmusik commented 10 years ago

On 12 Oct 2013, at 20:24, Julian Rohrhuber notifications@github.com wrote:

On 20.08.2013, at 13:15, sidechained notifications@github.com wrote:

The OSCObjectSpace sync method doesn't work as expected. It tries to look for the first non-local player and make a copy of their object space, but this test fails because the peer contained in the address book has a remote IP, whereas the peer returned by 'addrBook.me' has a local IP. It seems like the latter should automatically be kept in sync with the former, is that right?

The way to get the real external address should be via the network, sending out to a broadcast (or if some non-broadcast server based scheme is used), and the receiving the own addr externally.

Hail or Registrar could be modified to capture alternate hostnames for the local address from other peers as well. But probably the best solution is to add a primitive to net address that checks if a given address is equivalent to local on any interface.

Thoughts?

It would be OK if me has the loopback IP first (before the ping over the network) and then somehow correct it, once it has been received.

Well, in trying to think through possible use cases, I can imagine a situation in which a client is connected to two network interfaces. I'm fairly sure that's not possible now, but I can imagine it could be.

Discovering the external addresses by reply could still work in that case, but I'm not sure that there aren't any edge cases that wouldn't need you to know it immediately. That's why I suggest a primitive as the best solution. LocalNetAddr is just intended a hack for the moment.

Also, I don't know if there are any latency (I know very small) differences between sending to loopback and sending to real address.

It makes sense to have "me" in the address book if you want to keep (certain) lookups uniform. But indeed this is a very fundamental design decision. I think in order to support both perspectives, "me" might be needed as a functional option.

Yes, I'm still not sure of how best to deal with this. You want to have:

In my experimental branch I have something called a PeerGroup, which is actually a sort of group proxy/promise. So one way to do it is that you get a PeerGroup which excludes you, and that is lazily evaluated every time it's needed. I'll make this public soon.

A good example is sending to servers: as all of them are "on the net" relative to sclang, they are on the same level, so you want to treat them equally (at least this is how we always did it in powerbooks unplugged).

Yes, I struggle with this. They are equal, but not the same of course, and don't map only onto sets of clients. You want to have multiple servers per node, nodes with a server but not a client, etc. But I have some ideas about this.

S.

telephon commented 10 years ago

On 13.10.2013, at 12:07, muellmusik notifications@github.com wrote:

It would be OK if me has the loopback IP first (before the ping over the network) and then somehow correct it, once it has been received.

Well, in trying to think through possible use cases, I can imagine a situation in which a client is connected to two network interfaces. I'm fairly sure that's not possible now, but I can imagine it could be.

Discovering the external addresses by reply could still work in that case, but I'm not sure that there aren't any edge cases that wouldn't need you to know it immediately. That's why I suggest a primitive as the best solution. LocalNetAddr is just intended a hack for the moment.

Are you sure that the primitive could detect the right interface in that case? This was what made it difficult using a unixCmd.

Also, I don't know if there are any latency (I know very small) differences between sending to loopback and sending to real address.

OK, this is a point. Surprisingly intricate, the whole "me" issue!

A good example is sending to servers: as all of them are "on the net" relative to sclang, they are on the same level, so you want to treat them equally (at least this is how we always did it in powerbooks unplugged).

Yes, I struggle with this. They are equal, but not the same of course, and don't map only onto sets of clients. You want to have multiple servers per node, nodes with a server but not a client, etc. But I have some ideas about this.

Looking forward to this!

Working with servers, there are a few complexities which Republic solves partly:

muellmusik commented 10 years ago

On 14 Oct 2013, at 11:38, Julian Rohrhuber notifications@github.com wrote:

On 13.10.2013, at 12:07, muellmusik notifications@github.com wrote:

It would be OK if me has the loopback IP first (before the ping over the network) and then somehow correct it, once it has been received.

Well, in trying to think through possible use cases, I can imagine a situation in which a client is connected to two network interfaces. I'm fairly sure that's not possible now, but I can imagine it could be.

Discovering the external addresses by reply could still work in that case, but I'm not sure that there aren't any edge cases that wouldn't need you to know it immediately. That's why I suggest a primitive as the best solution. LocalNetAddr is just intended a hack for the moment.

Are you sure that the primitive could detect the right interface in that case? This was what made it difficult using a unixCmd.

Should be. But I am not sure how SC decides which interface. It seems it only takes one, at least based on the fact that when running wired we've often had to turn off Wifi. But if that's true, once the socket is live though surely you should be able to query the address? Or, you need to store all viable ones?

Anyway, Tim seems to have reimplemented all the socket stuff using boost:asio (is it just me or is a lot of the 'maintenance' stuff he was complaining about having to do just reimplementing things using libraries most devs didn't know?). I can't quite see how to do it but this might provide a hint:

http://stackoverflow.com/questions/2674314/get-local-ip-address-using-boost-asio

Also, I don't know if there are any latency (I know very small) differences between sending to loopback and sending to real address.

OK, this is a point. Surprisingly intricate, the whole "me" issue!

Yes! It has the feeling of 'which slightly awkward compromise do you prefer?'

A good example is sending to servers: as all of them are "on the net" relative to sclang, they are on the same level, so you want to treat them equally (at least this is how we always did it in powerbooks unplugged).

Yes, I struggle with this. They are equal, but not the same of course, and don't map only onto sets of clients. You want to have multiple servers per node, nodes with a server but not a client, etc. But I have some ideas about this.

Looking forward to this!

Working with servers, there are a few complexities which Republic solves partly:

  • One needs to allow the server to send their capabilities to everyone so that one can know e.g. the channel distribution.

Yes, I was thinking they should send their options as well, but I've not implemented that yet.

  • One needs to reserve not only nodeIDs (using clientID) but also bus index ranges to clients, so that they don't fall over each others' cables.

Does clientID not reserve private busses? That should be in Common.

S.

muellmusik commented 10 years ago

On 14 Oct 2013, at 12:40, Scott Wilson i@scottwilson.ca wrote:

I can't quite see how to do it but this might provide a hint:

Probably here:

http://www.boost.org/doc/libs/1_40_0/doc/html/boost_asio/reference/ip__basic_endpoint/address.html

S.

muellmusik commented 9 years ago

Okay, sorry so long to get back to this.

The problem is slightly different since I added the isLocal and *matchLangIP methods to NetAddr. These mean that NetAddr(someExternalIP).matches(NetAddr.localAddr) returns true if both refer to the local machine. Similarly Peer:== uses matches to compare NetAddrs. In that sense it doesn't matter what the IP address is for me as long as it matches local.

But I think the remaining issue is that ~addrBook.me is not the same Peer object as ~addrBook[~addrBook.me.name]. Normally this is not a problem, but if somebody tested for identity rather than equivalence, it would fail.

My instinct here is simply to store the name rather than the Peer object, and have the me method look it up when requested. In some ways I don't think it particularly matters if the IP stored in the me peer is loopback or some external one (it will be the one on the network connected whichever interface you are broadcasting on), but I suspect that similar issues with identity tests could occur if a Peer is replaced by one with a different address. (Issue #9 is a related problem.) So I think what should probably happen, is that if a Hail gets a reply from a Peer with the same name as one already in the book, it should re-authenticate to prevent spoofing, and on success update the changed IP and/or port in the existing Peer object. That way any held references to that Peer will be up-to-date.

Thoughts?

muellmusik commented 9 years ago

But I think the remaining issue is that ~addrBook.me is not the same Peer object as ~addrBook[~addrBook.me.name]. Normally this is not a problem, but if somebody tested for identity rather than equivalence, it would fail.

Okay, I fixed this with PR #10 and b681ebc.

One remaining query. I am tempted to add an additional OSCFunc in Hail (and similar in registrant) like so:

OSCFunc({|msg, time, addr|
    addrBook.me.addr = addr; 
    addrBook.me.changed(\addr);
}, oscPath, recvPort: addrBook.me.addr.port, argTemplate: [nil, addrBook.me.name]).oneShot;

This would update the internal address to the match the externally visible one. Any reason not to do this? Is there any performance difference in sending to localhost rather than an equivalent address?

muellmusik commented 9 years ago

This would update the internal address to the match the externally visible one.

Sorry, which can be useful for display purposes for instance.

muellmusik commented 6 years ago

I think this is actually all resolved now. Any reason not to close?

telephon commented 6 years ago

thanks!!