spipe optimization: avoid socketpair(2) when unnecessary

[Bill Trost]

Hi, all,

Below is a quick hack to eliminate the socket pair used to
pushbits in the case that file descriptor 0 really is a
socket. I'm not convinced that this is the "best" way to
detect the socketness of fd 0, but at least I was able to
use the getsockopt result in a vaguely constructive way.

I built this on Linux, but see no reason for it not to
compile under FreeBSD.

Enjoy,
Bill


--- a/spipe/main.c
+++ b/spipe/main.c
@@ -56,6 +56,9 @@ main(int argc, char * argv[])
struct proto_secret * K;
int ch;
int s[2];
+ int socket0 = 0;
+ int acceptcon;
+ int optsize;

WARNP_INIT;

@@ -123,29 +126,40 @@ main(int argc, char * argv[])
}

/*
- * Create a socket pair to push bits through. The spiped protocol
- * code expects to be handed a socket to read/write bits to, and our
- * stdin/stdout might not be sockets (in fact, almost certainly aren't
- * sockets); so we'll hand one end of the socket pair to the spiped
- * protocol code and shuttle bits between stdin/stdout and the other
- * end of the socket pair ourselves.
+ * If stdin is a socket, then we can pass
+ * it to the spiped protocol directly.
*/
- if (socketpair(AF_UNIX, SOCK_STREAM, 0, s)) {
+ optsize = sizeof(acceptcon);
+ if (getsockopt(0, SOL_SOCKET, SO_DEBUG, &acceptcon, &optsize) == 0 && !acceptcon) {
+ socket0 = 1;
+ }
+ else if (socketpair(AF_UNIX, SOCK_STREAM, 0, s)) {
+ /*
+ * Create a socket pair to push bits through. The
+ * spiped protocol code expects to be handed a socket
+ * to read/write bits to, and our stdin/stdout aren't
+ * sockets (as is typical); so we'll hand one end of the
+ * socket pair to the spiped protocol code and shuttle
+ * bits between stdin/stdout and the other end of the
+ * socket pair ourselves.
+ */
warnp("socketpair");
exit(1);
}

/* Set up a connection. */
- if (proto_conn_create(s[1], sas_t, 0, opt_f, K, opt_o,
+ if (proto_conn_create(socket0 ? 0 : s[1], sas_t, 0, opt_f, K, opt_o,
callback_conndied, NULL)) {
warnp("Could not set up connection");
exit(1);
}

- /* Push bits from stdin into the socket. */
- if (pushbits(STDIN_FILENO, s[0]) || pushbits(s[0], STDOUT_FILENO)) {
- warnp("Could not push bits");
- exit(1);
+ if (!socket0) {
+ /* Push bits from stdin into the socket. */
+ if (pushbits(STDIN_FILENO, s[0]) || pushbits(s[0], STDOUT_FILENO)) {
+ warnp("Could not push bits");
+ exit(1);
+ }
}

/* Loop until we die. *

[Colin Percival]

Post by Bill Trost
Below is a quick hack to eliminate the socket pair used to
pushbits in the case that file descriptor 0 really is a
socket.

Interesting -- do you have a use case where this optimization is
likely to matter? It seems to me that this is only relevant if
you are pushing a very large number of bits through; and even then,
I'd expect the cryptographic work to overwhelm the data-copying and
keep the speedup from this to a minimum.

Post by Bill Trost
I'm not convinced that this is the "best" way to
detect the socketness of fd 0, but at least I was able to
use the getsockopt result in a vaguely constructive way.

I believe the standards-compliant way to detect if a descriptor
is a socket is to fstat it and then use S_ISSOCK on the results.

Post by Bill Trost
[...]
/* Set up a connection. */
- if (proto_conn_create(s[1], sas_t, 0, opt_f, K, opt_o,
+ if (proto_conn_create(socket0 ? 0 : s[1], sas_t, 0, opt_f, K, opt_o,

This is a bug since proto_conn_create writes incoming data back to
the same socket as it's reading from -- you'll need to change it to
take separate "in" and "out" sockets and then pass 0 and 1 (if they
are sockets). In some cases spipe will be called with descriptors 0
and 1 being the same socket, but not always...

[Bill Trost]

Post by Bill Trost
Below is a quick hack to eliminate the socket pair used to
pushbits in the case that file descriptor 0 really is a socket.

Interesting -- do you have a use case where this optimization
is likely to matter? It seems to me that this is only relevant
if you are pushing a very large number of bits through; and
even then, I'd expect the cryptographic work to overwhelm the
data-copying and keep the speedup from this to a minimum.

I agree. My plan was to use this between thunderbird and an
spipe connection to my IMAP client, using a little wrapper
around spipe to verify that the connecting client has the
same user ID as the wrapper. It seemed easier than trying to
convince thunderbird to connect to a unix-domain socket....

Post by Bill Trost
[...]
/* Set up a connection. */
- if (proto_conn_create(s[1], sas_t, 0, opt_f, K, opt_o,
+ if (proto_conn_create(socket0 ? 0 : s[1], sas_t, 0, opt_f, K, opt_o,

This is a bug.... In some cases spipe will be called with
descriptors 0 and 1 being the same socket, but not always...

Wow, talk about obscure use cases! :-)

Yeah, I can see where someone might want to do that. OK, never
mind, I never claimed it was a particularly good idea....

Thanks,
Bill

[Colin Percival]

Post by Colin Percival

Post by Bill Trost
Below is a quick hack to eliminate the socket pair used to
pushbits in the case that file descriptor 0 really is a socket.

Interesting -- do you have a use case where this optimization
is likely to matter? It seems to me that this is only relevant
if you are pushing a very large number of bits through; and
even then, I'd expect the cryptographic work to overwhelm the
data-copying and keep the speedup from this to a minimum.
I agree. My plan was to use this between thunderbird and an
spipe connection to my IMAP client, using a little wrapper
around spipe to verify that the connecting client has the
same user ID as the wrapper. It seemed easier than trying to
convince thunderbird to connect to a unix-domain socket....

I take it you're on a multi-user system? I use spiped to protect
connections from my laptop to my POP3 server, but I'm the only user
on my laptop (by which I mean "human", not uid), and my POP3 account
has a password on it anyway, so I'm not too concerned about checking
which uid is connecting to 127.0.0.1:110.

Post by Colin Percival

Post by Bill Trost
[...]
/* Set up a connection. */
- if (proto_conn_create(s[1], sas_t, 0, opt_f, K, opt_o,
+ if (proto_conn_create(socket0 ? 0 : s[1], sas_t, 0, opt_f, K, opt_o,

This is a bug.... In some cases spipe will be called with
descriptors 0 and 1 being the same socket, but not always...
Wow, talk about obscure use cases! :-)
Yeah, I can see where someone might want to do that. OK, never
mind, I never claimed it was a particularly good idea....

Actually, I like this idea -- it just needs a bit more work. :-)