Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

Can't get datagram fd without referencing internal API #8057

Closed
benjamincburns opened this issue Aug 2, 2014 · 8 comments
Closed

Can't get datagram fd without referencing internal API #8057

benjamincburns opened this issue Aug 2, 2014 · 8 comments

Comments

@benjamincburns
Copy link

When I open a datagram socket using dgram.createSocket('udp4'), I'd expect the fd member of the socket to be set to the socket's fd once it's been properly opened. However, it's never set there, and in order to get the socket's fd, I have to snoop on the private member socket._handle.fd.

Why is this important to me?

I'm working on a module to implement support for TUNTAP network interfaces as an enabler of node-based virtual networks (think VPNs and VM networks). As a personal goal I'm attempting to write this module with no native code, and it's almost doable by using the ref and ioctl modules.

I say almost because while I'm able to create the TUN/TAP device, I'm not able to get/set options on it (address, broadcast, netmask, mtu) without referencing the private datagram socket API (_handle).

Example code:

TunTap.prototype.setAddress = function(addr) {
    var ifr = new ifreq();
    ifr.ref().fill(0);

    // the structure of ifreq is really ugly without the C preprocessor :-(
    ifr.ifrn_name.buffer.write(this.name);
    ifr.ifr_ifru.ifru_addr.sockaddr_in.sin_family = AF_INET;
    ifr.ifr_ifru.ifru_addr.sockaddr_in.sin_addr = inet_aton(addr);

    var socket = dgram.createSocket('udp4');

    // socket isn't open yet, so we bind it to let it open
    socket.bind();
    socket.on('listening', function() {
        try {
            //////// vvv PROBLEM IS HERE vvv ////////
            var fd = socket.fd;
            if (fd == null || fd < 0 ) { // fd is -42 here for some reason...
                fd = socket._handle.fd;
            }
            //////// ^^^ PROBLEM IS HERE ^^^ ////////
            ioctl(fd, SIOCSIFADDR, ifr.ref());
            this.emit('addr', addr);
        } catch (err) {
            this.emit('error', 'error setting address on device ' + this.name +
                    ' to ' + addr + ' due to error: ' + err);
        }
        socket.close();
    }.bind(this));
}
@benjamincburns
Copy link
Author

I'd be very happy to submit a PR which updates dgram.Socket.fd to dgram.Socket._handle.fd when the latter is numeric and greater than or equal to 0. However, the comments in the code here and here and here lead me to believe that this will cause problems. Would be happy to "unhack" this if necessary, but I think I'd need some direction as to what's going on here.

Perhaps since dgram is considered stable we could expose the internal _handle member as handle? I'd also be happy to submit a PR for this, along with some supporting documentation for this member.

@trevnorris
Copy link

I think it would be better to make dgram.Socket#fd a getter. That way in the future if we change the underlying _handle the getter can also be updated.

@indutny What do you think?

BTW, "stable" simply means the user API. Not the internal implementation.

@benjamincburns
Copy link
Author

If this issue is addressed, I'd be quite pleased if the resultant API was consistent across all of the various stream-like classes in node's js runtime. Ideally I don't think it's too unreasonable to say that if a class makes use of an fd, is stream-like, and is part of the node API, I shouldn't need any instanceof or typeof or hasOwnProperty checks to interact with it (including when I need to get the underlying fd).

It strikes me that classes which fit those criteria would all inherit from some common subclass, anyway - perhaps fs.XXXStream? Has any effort or consideration toward this kind of generalization been made?

@saghul
Copy link
Member

saghul commented Aug 10, 2014

I think it would be better to make dgram.Socket#fd a getter. That way in the future if we change the underlying _handle the getter can also be updated.

FWIW, the function that currently gets the fd (https://github.com/joyent/node/blob/master/src/stream_wrap.cc#L70) replies on non-public libuv implementation details.

@benjamincburns
Copy link
Author

@saghul - sounds like more fodder for the discussion over at #8002. Is there a similar issue over in libuv-land that I can follow? If not, since I think you've got some goals in mind, would you mind raising one? Will be very happy to contribute in any way that I can.

@saghul
Copy link
Member

saghul commented Aug 11, 2014

@benjamincburns yeah, it's indeed related. Currently there is no libuv issue opened for this, and we have resisted those which wanted to expose the fd, because it's treated as an internal implementation detail. There are ways around it, but not for all cases, so we'll get to come up with something.

I don't have any concrete suggestions as of now (well, the obvious uv_get_fd, I guess), but please do open an issue on libuv so we can discuss this further.

@trevnorris
Copy link

@benjamincburns If you do, please link back to this issue for dependency tracking.

@saghul
Copy link
Member

saghul commented Aug 23, 2014

FYI: joyent/libuv#1435

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

No branches or pull requests

5 participants