NDN-CCL

I tested with NFD v0.3 and the /localhop/nfd/rib/register works as expected (the prerequisites are setting command signing info on the face and configuring the NFD with localhop_security enabled, I used any trust anchor). Jeff mentioned that http://redmine.named-data.net/projects/nfd/wiki/ScopeControl says:
Generally, the following faces are local:

• the InternalFace (in NFD management module)
• UNIX-domain socket face
• TCP face whose remote address is a loopback address
• WebSocket face whose remote address is a loopback address

This means we could add an isLocal() method to Transport and check this to determine whether to use /localhost... or /localhop... for prefix registration. Thoughts?

For me, this use of remote registration is non-standard and I wouldn't add mechanics to automatically handle it. My preference is to add something explicit in Face to request localhop, not localhost registration.

What do you mean by non-standard? If the NFD provides a feature that allows remote registration, why shouldn't my application take advantage of it? If not, I'm limited to building applications only on Ubuntu (and a few others).

I'm not saying that the remote registration function is not useful. I was just thinking that switch between remote and local registration can be explicit, instead of implicit.

I'm not really objecting to do it implicitly. It is, technically, an internal detail for the face abstraction and make sense to have and I see no real negative implications.

Alex and Jeff, take a look at this revision and associated integration test. In the integration-tests directory, run

mvn -q test -DclassName=TestRemotePrefixRegistration -Dip=[IP address to remote NFD]

I like the idea of a separate Face method to do remote prefix registration because it is not just a matter of changing "localhost" to "localhop" since there could be different certificates needed by the forwarder to accept the remote registration. In a test environment where all the forwarders accept everything, this isn't a problem. But for productions systems, I think the application may have to be aware of when a remote prefix registration is being performed so that it can potentially use a different signing identity for the registration request.

To help the application, we could add Face.isLocal() which checks its internal transport. A question: Is it possible that isLocal() needs to do a network lookup (a potentially asynchronous operation)?

I'm convinced. I initially wanted to hide the localhost/localhop complexity but not being able to explicitly choose could become a problem. I will make a new branch with the explicit call.

With regards to the method of determining if the address is local or not, Java will only make a DNS request to resolve the string if it is a host name; for IP addresses, it will parse the string and check that the first octet is 127 (in IPv4's case). Either way, the operation is synchronous because it blocks until complete.

You mentioned in a separate conversation that we may want to make the connection flow fully asynchronous (i.e. with new callback types, etc.). If so, I don't recommend we do it in this feature: if we set a 'local' flag in connect() and make isLocal() call the connect() method if the flag is null then any upgrades to a fully asynchronous pattern should work. The disadvantage of this is the connection is opened when the user calls isLocal(), which may be unexpected behavior; the advantage is the potential DNS lookup is only done once. Thoughts?

I have no other place to comment on the code. Udp face is never considered local, so udp transport's isLocal() can/should always return false.

Updated UDP transport accordingly, see PR at https://github.com/named-data/jndn/pull/6. Like we talked about on Skype, we can discuss explicit vs implicit remote registration during the dev meeting today.

Hi Andrew. If TcpTransport.isLocal can do a costly DNS lookup, should it cache the result for the next call to registerPrefix?

Yes, I'm having trouble figuring out the best way to do that, though. Because Node is the only one maintaining a reference to the ConnectionInfo I feel like Transport.isLocal(ConnectionInfo ...) should not cache the call; is this correct and should I add an transportIsLocal_ field to Node? Also, can I assume ConnectionInfo will never change?

Yes, we can assume that ConnectionInfo will not change because it is the one defined by TcpTransport which doesn't change.

I meant that TcpTransport.isLocal should cache the result. We don't need transportIsLocal_ in Node. TcpTransport.isLocal can just remember the ConnectionInfo from the last call. (It should not do anything fancy like keep a dictionary of results for different ConnectionInfos.) If the ConnectionInfo.getHost() is the same as from the last call, then use the cached value for isLocal.

Ok, take a look at the current state:

• isLocal() exposed on Face, https://github.com/named-data/jndn/pull/6/files#diff-ce7dca71ec9883faf552d39c5706d149R627
• UDPTransport is always non-local, https://github.com/named-data/jndn/pull/6/files#diff-e56cdda894607d1ab3960e8fe1475844R76
• TCPTransport will cache potential DNS lookup, https://github.com/named-data/jndn/pull/6/files#diff-16b55b0173b26889a088013206a2911aR78
• Node determines localhost/localhop according to the result of isLocal(), fails if it can't figure it out, https://github.com/named-data/jndn/pull/6/files#diff-10cf1796124a824f7bbe49532293ed58L1002

The use prefix registration is only possible after Face is actually connected to the remote end = DNS resolution is already performed. Andrew, is this is what you're referring by "caching of potential DNS lookup"?

No, Jeff and I had talked about actually connecting the transport when isLocal() is called but I don't think that is necessary. The current caching is only in TcpTransport and is of a DNS lookup that the Java class InetAddress may make if it receives a host name instead of an IP address; in that case, the library will block until the host name is resolved. With this model, the user can call face.isLocal() before the connection is ever made.

In TcpTransport.isLocal, you check "connectionInfo != connectionInfo_":

https://github.com/named-data/jndn/blob/0c716da19d46ec53721992103c59e5afea65e876/src/net/named_data/jndn/transport/TcpTransport.java#L94

I would suggest "connectionInfo.getHost() != connectionInfo_.getHost()" for two reasons. For local/remote you don't care about the port, and TcpTransport.ConnectionInfo doesn't have an equals() method.

Sounds good to me. That check is comparing whether the references are the same, which they should be if ConnectionInfo never changes; however, getHost() is a lot clearer.

I would thrown an exception in isLocal when transport is not connected.

Actually... is it clearer?

    if(connectionInfo_ == null || !((ConnectionInfo) connectionInfo).getHost()
      .equals(connectionInfo_.getHost()))
    {
      connectionInfo_ = (ConnectionInfo) connectionInfo;
      InetAddress address = InetAddress.getByName(connectionInfo_.getHost());
      isLocal_ = address.isLoopbackAddress();
    }

Alex, but the Transport can still know if its local or not without being connected, right?

I'm thinking primarily about the use case. If the only use case (and should be in my opinion) to check isLocal after transport is connected, then there is no much value in adding special logic to fix the issue, if this path will never be used.

Actually... is it clearer?

If you don't like how that looks, then I'd be happy if you added an equals() method to TcpTransport.ConnectionInfo and do "!connectionInfo.equals(connectionInfo_)" . It just looks weird to me to do object equality when that's not what we really mean.

Think of this use case: one module in your application creates a face and sets it up with security, etc. It then passes it to another module; the other module wants to know if it is local/non-local. It doesn't want to connect yet, express interests, etc.; it just wants to know local/non-local. Why connect the face when you don't have to?

(I have this very scenario in my code and I have a bunch of extra mechanisms to remember if it's local which, by the way, I check by sending an Interest to /localhost/nfd; any implementation is better than that...).

Jeff Thompson wrote:

Actually... is it clearer?

If you don't like how that looks, then I'd be happy if you added an equals() method to TcpTransport.ConnectionInfo and do "!connectionInfo.equals(connectionInfo_)" . It just looks weird to me to do object equality when that's not what we really mean.

It must be late in the day... not feeling picky: https://github.com/named-data/jndn/commit/7ce429c72a4c9ced1e081ec6cccc29fe88016c59

Alex Afanasyev wrote:

I'm thinking primarily about the use case. If the only use case (and should be in my opinion) to check isLocal after transport is connected, then there is no much value in adding special logic to fix the issue, if this path will never be used.

Are you still concerned that no one will use face.isLocal() before the transport is connected (because I would like to...)?

I wonder for the reason you would like to do it?

See #23... multiple modules accessing the same face and they want to know if it is local or not (they don't need/want to connect yet).

I would still argue that these are unnecessary details that need to be handled by face itself.

I can only be saying from the perspective of ndn-cxx implementation. This implementation specifically does nothing on construction and does not connect up until there is an operation, such as expressInterest, registerPrefix (some setInterestFilter overloads), or put, scheduled on the face.

My argument here is that face is a barrier between application and whatever forwarder. Application should not deal with any of the logic that happens beyond the public interface, which would ensure that such logic can be altered at any point of time without affecting the application.

I agree with all of the points about abstraction. What I need is some way to know if I am connected to a local NFD or not and it would seem the clearest way is to expose isLocal() on Face. My application needs this piece of information because if it does have a local NFD, it should behave differently (e.g. listen for requests to route back like we talked about with LocalControlHeaders).

Here are the options I see:

1. No Face.isLocal(): then I need some other non-hackish way to determine if the NFD is local
2. Face.isLocal() throws exception if not yet connected: in this case, the application has to understand even more of the inner workings of Face, i.e. that it will not know if it is local/non-local until connected; additionally, this doesn't make sense for a whole class of transports because we always know if they are local/non-local (e.g. UdpTransport).
3. Face.isLocal() that simply works: this is what I would like because it is clear and usable (e.g. I don't have to expressInterest... for no reason as in #2); I agree that the potential DNS lookup described in #15 is not optimal but the documentation should clearly reflect this potential block (and this scenario would go away if NDN ever fully replaced IP).

Are there other options... or better ways to do any of the three?

What about a slightly different way. I'm kind of considering what you want to do as a special case (therefore my suggestion is about that).

Instead of trying to mingle with the Face interface, your application can simply explicitly create a transport instance and pass it to the face. This doesn't imply it be connected at the time of creation, face can request connection at a later point of time. This way, you can observe details of what face is doing with the transport and get details about the transport (we already have isLocal() there).

I like it. No Face.isLocal(), just Transport.isLocal().

The problem now lies in the implementation. The only object with a reference to ConnectionInfo is Node so the Transport itself doesn't know what it's connection parameters are unless Node tells it what they are. We could refactor and save a reference to ConnectionInfo in the transport itself, but I don't know the implications of that. Jeff, what do you think?

My opinion about ConnectionInfo concept is recorded as part of issue #2571

Actually, the implementation of this doesn't really depend on ConnectionInfo. Transport.isLocal can throw when transport is not connected and when transport is connected, it is always possible to get IP address for the remote end from the underlying socket. So, I don't see the problem.

Are you suggesting #2 from note #29? Then I have to be connected to check if the transport is local and those two things shouldn't be dependent. I have to express an Interest for no reason except to get connected so that I can check if the transport is local?

No. I'm not suggesting adding anything to Face interface.

Without initiating connection (=not knowing IP), you cannot reliably known whether you're local or not. I'm still confused about overall use case :), as it looks a little strange (I think of NDN as a way to remove the need of connections and knowing anything about connections; knowing when something is connected/where connected is kind of irrelevant for me)...

In your case. Why your application wouldn't know a priori that it supposed to connect to local or to remote, especially considering very different way application need to behave in these cases.

The application could know a priori: it COULD duplicate the logic in the Face constructor to know what type of Transport it is connecting to and it COULD duplicate a reference to the connection info to inspect whether the given IP is localhost or not and it COULD maintain a state variable that all different sub-modules/threads could inspect to determine whether the NFD is local or not. But... why?

Why not just maintain that state where it makes more sense: in the Transport? And why should Transport.isLocal() throw an error when it is not connected? You don't need to connect to parse an IP...

Throwing the error was just my suggestion, not requirement.

I'm +1 with having only Transport.isLocal(), which optionally does synchronous DNS resolution (when not connected and hostname was supplied). We just need to have a disclaimer in the method documentation that improper use may result in undefined results (e.g., when the supplied hostname can resolve into different things).

Hi Andrew. Your existing pull request https://github.com/named-data/jndn/pull/6 adds a Face method isLocal() which simply calls the internal Transport.isLocal(connectionInfo). This is a good first pass. I will amend this to mark it as an experimental API method and that it may block to perform a DNS lookup. The documentation for Face.isLocal won't mention the Transport since we want to minimize the exposure of the Transport class to the application.

Your pull request also has updates to integration-tests/pom.xml. Thanks for the updates, but I am having trouble running it. I cd to intregraion-test and run "mvn test". Is is possible to run "mvn test" in this subfolder, or "mvn integration-test" in the top folder?

Jeff, the syntax for running the integration test should be mvn test -DclassName=TestRemotePrefixRegistration -Dip=[IP address to remote NFD]; that is the same syntax that we were using in examples. Also, note from that the remote NFD has to have localhop configuration enabled for any certificate (or at least the certificate you're using to register the prefix). I documented all of this in the Java file but we may want to add a README in integration-tests (and maybe examples) to simplify. Does this solve the errors you are seeing?

Ah, now I see the documentation in the Java file. It is clear enough. I was able to run it. It registers a prefix and waits. If I send an interest from another program, it responds with a data packet. Then it seems to keep waiting. Is this the intended behavior? Shouldn't an integration test do its thing and then quit with success or failure?

Good point, it's more an example than a test. Because it has some setup involved (potentially modifying the NFD configuration, setting up an external NFD) it seems like it would be hard to run it as a test. Should we move it to examples?

To resolve AlexA's points earlier -

Agreed that the logic and state for isLocal() goes in Transport().

Each Face has only one Transport. So, if the Transport is local, then so is the Face.

So, Face.isLocal() is just a helper to call Transport.isLocal() succinctly. Is there an objection to this?

As suggested, I moved TestRemotePrefixRegistration from integration-tests to examples.

The Node.js DNS lookup is async-only, so in NDN-JS I had to make Face.isLocal async: isLocal(onResult, onError)

https://github.com/named-data/ndn-js/blob/0fee2d8523f0ca40a5276dbc320f7f7e7b89ced4/js/face.js#L1253