Re: increase netcat's buffer...
On Mon, Oct 13, 2014 at 15:02, Arne Becker wrote: OK, no more fiddling with O_NONBLOCK. New diff below, tested with tcpbench and file transfers. I think this is good. Thanks, committed. We'll let it sit for a while and then see what if any changes need to take place on the buffer side. Maybe we can revisit the original impetus and try 64k again.
Re: increase netcat's buffer...
Ted Unangst wrote this message on Thu, Oct 30, 2014 at 12:09 -0400: On Mon, Oct 13, 2014 at 15:02, Arne Becker wrote: OK, no more fiddling with O_NONBLOCK. New diff below, tested with tcpbench and file transfers. I think this is good. Thanks, committed. We'll let it sit for a while and then see what if any changes need to take place on the buffer side. Maybe we can revisit the original impetus and try 64k again. I can't compile test this, but I think the vwrite cast could go away if you changed the atomicio's f type from ssize_t (*f) (int, void *, size_t) to ssize_t (*f) (int, const void *, size_t)... There isn't any reason why atomicio needs to have a non-const pointer to the buffer... -- John-Mark Gurney Voice: +1 415 225 5579 All that I will do, has been done, All that I have, has not.
Re: increase netcat's buffer...
Ping?
Re: increase netcat's buffer...
Hi again.
+/* make all fds non-blocking */
+for (n = 0; n 4; n++) {
+if (pfd[n].fd == -1)
+continue;
+flags = fcntl(pfd[n].fd, F_GETFL, 0);
+/*
+ * For sockets and pipes, we want non-block, but setting it
+ * might fail for files or devices, so we ignore the return
+ * code.
+ */
+fcntl(pfd[n].fd, F_SETFL, flags | O_NONBLOCK);
+}
Thanks. I think this is the trouble spot. Without this, we don't need
to fool around in atelnet either. And we probably don't really need
this. The point isn't really to create an nc that never blocks. In
particular, turning stdin and stdout non-blocking has weird effects
that has broken sh pipelines in the past.
Drop the above, the relevant chunk in atelnet, and I think it looks good.
OK, no more fiddling with O_NONBLOCK.
New diff below, tested with tcpbench and file transfers.
An interesting aside:
I have a test case like this:
host1$ ./nc -N host2 portX infile outfile
host2$ ./echoserver portX
With the 16k Buffer you get less syscalls obviously. Still this version
with the 16k buffer is slower for this test than the original nc with
the 2k buffer. Using the same 2k buffer makes them equal again. So far,
I'm not sure why.
For other tests, speed and CPU usage seem to be about equal.
Index: netcat.c
===
RCS file: /mount/cvsdev/cvs/openbsd/src/usr.bin/nc/netcat.c,v
retrieving revision 1.122
diff -u -p -r1.122 netcat.c
--- netcat.c20 Jul 2014 01:38:40 - 1.122
+++ netcat.c13 Oct 2014 12:48:09 -
@@ -65,6 +65,12 @@
#define PORT_MAX_LEN 6
#define UNIX_DG_TMP_SOCKET_SIZE19
+#define POLL_STDIN 0
+#define POLL_NETOUT 1
+#define POLL_NETIN 2
+#define POLL_STDOUT 3
+#define BUFSIZE 16384
+
/* Command Line Options */
intdflag; /* detached, no stdin */
intFflag; /* fdpass sock to stdout */
@@ -112,6 +118,8 @@ voidset_common_sockopts(int);
intmap_tos(char *, int *);
void report_connect(const struct sockaddr *, socklen_t);
void usage(int);
+ssize_t drainbuf(int, unsigned char *, size_t *);
+ssize_t fillbuf(int, unsigned char *, size_t *);
int
main(int argc, char *argv[])
@@ -391,7 +399,7 @@ main(int argc, char *argv[])
len);
if (connfd == -1) {
/* For now, all errnos are fatal */
- err(1, accept);
+ err(1, accept);
}
if (vflag)
report_connect((struct sockaddr
*)cliaddr, len);
@@ -730,66 +738,224 @@ local_listen(char *host, char *port, str
* Loop that polls on the network file descriptor and stdin.
*/
void
-readwrite(int nfd)
+readwrite(int net_fd)
{
- struct pollfd pfd[2];
- unsigned char buf[16 * 1024];
- int n, wfd = fileno(stdin);
- int lfd = fileno(stdout);
- int plen;
-
- plen = sizeof(buf);
-
- /* Setup Network FD */
- pfd[0].fd = nfd;
- pfd[0].events = POLLIN;
-
- /* Set up STDIN FD. */
- pfd[1].fd = wfd;
- pfd[1].events = POLLIN;
+ struct pollfd pfd[4];
+ int stdin_fd = STDIN_FILENO;
+ int stdout_fd = STDOUT_FILENO;
+ unsigned char netinbuf[BUFSIZE];
+ size_t netinbufpos = 0;
+ unsigned char stdinbuf[BUFSIZE];
+ size_t stdinbufpos = 0;
+ int n, num_fds, flags;
+ ssize_t ret;
+
+ /* don't read from stdin if requested */
+ if (dflag)
+ stdin_fd = -1;
+
+ /* stdin */
+ pfd[POLL_STDIN].fd = stdin_fd;
+ pfd[POLL_STDIN].events = POLLIN;
+
+ /* network out */
+ pfd[POLL_NETOUT].fd = net_fd;
+ pfd[POLL_NETOUT].events = 0;
+
+ /* network in */
+ pfd[POLL_NETIN].fd = net_fd;
+ pfd[POLL_NETIN].events = POLLIN;
+
+ /* stdout */
+ pfd[POLL_STDOUT].fd = stdout_fd;
+ pfd[POLL_STDOUT].events = 0;
+
+ while (1) {
+ /* both inputs are gone, buffers are empty, we are done */
+ if (pfd[POLL_STDIN].fd == -1 pfd[POLL_NETIN].fd == -1
+stdinbufpos == 0 netinbufpos == 0) {
+ close(net_fd);
+ return;
+ }
+ /* both outputs are gone, we can't continue */
+ if (pfd[POLL_NETOUT].fd == -1 pfd[POLL_STDOUT].fd == -1) {
+ close(net_fd);
+ return;
+ }
+ /* listen and net in gone, queues empty, done */
+ if (lflag pfd[POLL_NETIN].fd == -1
+stdinbufpos == 0 netinbufpos == 0) {
+ close(net_fd);
+
Re: increase netcat's buffer...
Hi.
-err(1, bind failed);
+errx(1, bind failed: %s, strerror(errno));
freeaddrinfo(ares);
This doesn't seem necessary, or correct.
Indeed. New patch below.
@@ -640,7 +648,7 @@ timeout_connect(int s, const struct sock
if (timeout != -1) {
flags = fcntl(s, F_GETFL, 0);
if (fcntl(s, F_SETFL, flags | O_NONBLOCK) == -1)
-err(1, set non-blocking mode);
+warn(unable to set non-blocking mode);
ok, maybe. i wonder what this will break...
Since this must be a socket, O_NONBLOCK really should not fail. Changed
back to err in patch below.
@@ -877,8 +1049,20 @@ atelnet(int nfd, unsigned char *buf, uns
p++;
obuf[2] = *p;
+
+if (!blocking) {
+flags = fcntl(nfd, F_GETFL, 0);
+if (fcntl(nfd, F_SETFL, flags ~O_NONBLOCK) == -1)
+warn(unable to set blocking mode);
+blocking = 1;
+}
if (atomicio(vwrite, nfd, obuf, 3) != 3)
warn(Write Error!);
+}
+if (blocking) {
+flags = fcntl(nfd, F_GETFL, 0);
+if (fcntl(nfd, F_SETFL, flags | O_NONBLOCK) == -1)
+warn(unable to set non-blocking mode);
}
}
I don't understand this part. What's the reasoning?
atelnet uses atomicio, which depends on blocking sockets. Since we call
atelnet from readwrite, the sockets are likely non-blocking.
If we enter the for()-loop in atelnet, we set the sockets to blocking
and remember that, so we do it only once.
If we made them blocking, we make them non-blocking again at the end.
We could also append the bytes that atelnet adds to the stdinbuf we have
in readwrite, but I wanted to keep it simple. Likely performance isn't
that much of an issue for telnet.
In any case, it is arguable if we should pass the bytes of the IAC
WILL/DO X on, since if netcat answers, the user likely isn't interested
in seeing them. Again, I kept it simple, it works like it did before.
Lightly tested against a telnet server.
- Arne
Index: netcat.c
===
RCS file: /mount/cvsdev/cvs/openbsd/src/usr.bin/nc/netcat.c,v
retrieving revision 1.121
diff -u -p -r1.121 netcat.c
--- netcat.c10 Jun 2014 16:35:42 - 1.121
+++ netcat.c9 Jul 2014 16:06:01 -
@@ -65,6 +65,12 @@
#define PORT_MAX_LEN 6
#define UNIX_DG_TMP_SOCKET_SIZE19
+#define POLL_STDIN 0
+#define POLL_NETOUT 1
+#define POLL_NETIN 2
+#define POLL_STDOUT 3
+#define BUFSIZE 16384
+
/* Command Line Options */
intdflag; /* detached, no stdin */
intFflag; /* fdpass sock to stdout */
@@ -112,6 +118,8 @@ voidset_common_sockopts(int);
intmap_tos(char *, int *);
void report_connect(const struct sockaddr *, socklen_t);
void usage(int);
+ssize_t drainbuf(int, unsigned char *, size_t *);
+ssize_t fillbuf(int, unsigned char *, size_t *);
int
main(int argc, char *argv[])
@@ -640,7 +648,7 @@ timeout_connect(int s, const struct sock
if (timeout != -1) {
flags = fcntl(s, F_GETFL, 0);
if (fcntl(s, F_SETFL, flags | O_NONBLOCK) == -1)
- err(1, set non-blocking mode);
+ err(1, unable to set non-blocking mode);
}
if ((ret = connect(s, name, namelen)) != 0 errno == EINPROGRESS) {
@@ -730,67 +738,229 @@ local_listen(char *host, char *port, str
* Loop that polls on the network file descriptor and stdin.
*/
void
-readwrite(int nfd)
+readwrite(int net_fd)
{
- struct pollfd pfd[2];
- unsigned char buf[16 * 1024];
- int n, wfd = fileno(stdin);
- int lfd = fileno(stdout);
- int plen;
-
- plen = sizeof(buf);
-
- /* Setup Network FD */
- pfd[0].fd = nfd;
- pfd[0].events = POLLIN;
+ struct pollfd pfd[4];
+ int stdin_fd = STDIN_FILENO;
+ int stdout_fd = STDOUT_FILENO;
+ unsigned char netinbuf[BUFSIZE];
+ size_t netinbufpos = 0;
+ unsigned char stdinbuf[BUFSIZE];
+ size_t stdinbufpos = 0;
+ int n, num_fds, flags;
+ ssize_t ret;
+
+ /* don't read from stdin if requested */
+ if (dflag)
+ stdin_fd = -1;
+
+ /* stdin */
+ pfd[POLL_STDIN].fd = stdin_fd;
+ pfd[POLL_STDIN].events = POLLIN;
+
+ /* network out */
+ pfd[POLL_NETOUT].fd = net_fd;
+ pfd[POLL_NETOUT].events = 0;
+
+ /* network in */
+ pfd[POLL_NETIN].fd = net_fd;
+ pfd[POLL_NETIN].events = POLLIN;
+
+ /* stdout */
+ pfd[POLL_STDOUT].fd = stdout_fd;
+ pfd[POLL_STDOUT].events = 0;
+
+
+ /* make all fds non-blocking */
+ for (n = 0; n 4; n++) {
+ if (pfd[n].fd == -1)
+ continue;
+ flags = fcntl(pfd[n].fd, F_GETFL, 0);
+ /*
+
Re: increase netcat's buffer...
On Wed, Jul 09, 2014 at 18:33, Arne Becker wrote:
atelnet uses atomicio, which depends on blocking sockets. Since we call
atelnet from readwrite, the sockets are likely non-blocking.
If we enter the for()-loop in atelnet, we set the sockets to blocking
and remember that, so we do it only once.
If we made them blocking, we make them non-blocking again at the end.
+ /* make all fds non-blocking */
+ for (n = 0; n 4; n++) {
+ if (pfd[n].fd == -1)
+ continue;
+ flags = fcntl(pfd[n].fd, F_GETFL, 0);
+ /*
+ * For sockets and pipes, we want non-block, but setting it
+ * might fail for files or devices, so we ignore the return
+ * code.
+ */
+ fcntl(pfd[n].fd, F_SETFL, flags | O_NONBLOCK);
+ }
Thanks. I think this is the trouble spot. Without this, we don't need
to fool around in atelnet either. And we probably don't really need
this. The point isn't really to create an nc that never blocks. In
particular, turning stdin and stdout non-blocking has weird effects
that has broken sh pipelines in the past.
Drop the above, the relevant chunk in atelnet, and I think it looks good.
Re: increase netcat's buffer...
On Thu, Jun 26, 2014 at 13:43, Arne Becker wrote:
Hi.
Now soliciting diffs to change readwrite to a loop with two buffers
that poll()s in all four directions. :)
Good thing you made me remember I wrote just this a while ago.
This is my first OpenBSD diff, so tell me if I missed anything obvious.
Tested quite extensively originally; for this diff I only checked a
simple nc to nc hello.
@@ -608,7 +616,7 @@ remote_connect(const char *host, const c
if (bind(s, (struct sockaddr *)ares-ai_addr,
ares-ai_addrlen) 0)
- err(1, bind failed);
+ errx(1, bind failed: %s, strerror(errno));
freeaddrinfo(ares);
This doesn't seem necessary, or correct.
@@ -640,7 +648,7 @@ timeout_connect(int s, const struct sock
if (timeout != -1) {
flags = fcntl(s, F_GETFL, 0);
if (fcntl(s, F_SETFL, flags | O_NONBLOCK) == -1)
- err(1, set non-blocking mode);
+ warn(unable to set non-blocking mode);
ok, maybe. i wonder what this will break...
-readwrite(int nfd)
+readwrite(int net_fd)
this part read ok, for the actual poll changes.
@@ -877,8 +1049,20 @@ atelnet(int nfd, unsigned char *buf, uns
p++;
obuf[2] = *p;
+
+ if (!blocking) {
+ flags = fcntl(nfd, F_GETFL, 0);
+ if (fcntl(nfd, F_SETFL, flags ~O_NONBLOCK) == -1)
+ warn(unable to set blocking mode);
+ blocking = 1;
+ }
if (atomicio(vwrite, nfd, obuf, 3) != 3)
warn(Write Error!);
+ }
+ if (blocking) {
+ flags = fcntl(nfd, F_GETFL, 0);
+ if (fcntl(nfd, F_SETFL, flags | O_NONBLOCK) == -1)
+ warn(unable to set non-blocking mode);
}
}
I don't understand this part. What's the reasoning?
Re: increase netcat's buffer...
Ted Unangst wrote:
On Mon, Jun 09, 2014 at 21:54, Theo de Raadt wrote:
A better patch is probably the following which also increases the size
of the buffer to at least 64k:
Agreed.
One thing to be aware of. That function is syncronous. It will read
as much as it can get, then it will do an atomic write operation to
flush the buffer out the other way.
If you have substantially different speeds, this can be a substantial
'buffer bloat'.
Since it is handling a session in both directions... expect to see
some substantial jaggies.
Having convinced me this a problem (it's already in the code, but 64k
buffers will make it worse), I scaled back to 16k.
Now soliciting diffs to change readwrite to a loop with two buffers
that poll()s in all four directions. :)
The attached code is based on a stripped down I/O buffer that I wrote as a
hobby project.
I'm not a big user of netcat so I'm not sure it'll work in all kinds of
situations. I don't want to spend a lot of time on it either but I hope the
attached code is useful as is, or may possibly inspire other people to come up
with something better. e.g., I tried adding some error handling but I'm not
sure I did it right. Another thing bothering me is the atelnet function. The
way I see it is that netcat sets up two communication channels, one for
incoming traffic on a socket to stdout, another one for traffic from stdin
going out of that socket. atelnet seems to short-circuit that which feels
wrong. (I suppose atelnet can't be removed as people probably rely on it)
First I'll shortly try to explain the idea behind the buffer (and a single
communication channel).
Assume data traveling from a file descriptor (fd1) to the buffer, and from the
buffer to another file descriptor (fd2).
initial buffer:
|--|
size
^
cur (current position in the buffer)
start with fd1 enabled and fd2 disabled.
1) a read from fd1 writes into the buffer:
|--|
size
---used---
^
cur (isn't used in this case)
switch off fd1 and enable fd2.
2) a read from buffer (cur position) writes to fd2:
(cur position gets updated by amount of data written to fd2)
|--|
size
-used-
^
cur
repeat this step until all data has been written to fd2:
|--|
size
^
cur
3) all data has been transferred, reset buffer, disable fd2 and enable fd1:
|--|
size
^
cur
4) repeat step 1, 2 and 3.
The patch itself:
Index: netcat.c
===
RCS file: /cvs/src/usr.bin/nc/netcat.c,v
retrieving revision 1.121
diff -u -p -u -r1.121 netcat.c
--- netcat.c10 Jun 2014 16:35:42 - 1.121
+++ netcat.c27 Jun 2014 12:42:18 -
@@ -732,60 +732,141 @@ local_listen(char *host, char *port, str
void
readwrite(int nfd)
{
- struct pollfd pfd[2];
- unsigned char buf[16 * 1024];
- int n, wfd = fileno(stdin);
- int lfd = fileno(stdout);
- int plen;
-
- plen = sizeof(buf);
+ static struct {
+ #define _BUFFER_SIZE (16*1024)
+ size_t size;
+ size_t used;
+ unsigned char *cur;
+ unsigned char data[_BUFFER_SIZE];
+ } buf[2] =
+ {
+ { _BUFFER_SIZE, 0, buf[0].data },
+ { _BUFFER_SIZE, 0, buf[1].data },
+ };
+ struct pollfd pfd[4];
+ int npfd;
+ ssize_t n;
/* Setup Network FD */
pfd[0].fd = nfd;
pfd[0].events = POLLIN;
+ pfd[1].fd = -1;
+ pfd[1].events = POLLOUT;
- /* Set up STDIN FD. */
- pfd[1].fd = wfd;
- pfd[1].events = POLLIN;
+ /* Set up STDIN/STDOUT FD. */
+ pfd[2].fd = -1;
+ pfd[2].events = POLLOUT;
+ pfd[3].fd = STDIN_FILENO;
+ pfd[3].events = POLLIN;
- while (pfd[0].fd != -1) {
+ while (pfd[0].fd != -1 || pfd[1].fd != -1 ||
+ pfd[2].fd != -1 || pfd[3].fd != -1) {
if (iflag)
sleep(iflag);
- if ((n = poll(pfd, 2 - dflag, timeout)) 0) {
+ npfd = poll(pfd, 4 - dflag, timeout);
+ if (npfd 0) {
close(nfd);
err(1, Polling Error);
}
- if (n == 0)
+ if (npfd == 0)
return;
if (pfd[0].revents POLLIN) {
- if ((n = read(nfd, buf, plen)) 0)
- return;
- else if (n == 0) {
+ n = read(pfd[0].fd, buf[0].data, buf[0].size);
+ if (n == -1) {
+ if (errno != EAGAIN errno != EINTR)
+
Re: increase netcat's buffer...
Hi.
Now soliciting diffs to change readwrite to a loop with two buffers
that poll()s in all four directions. :)
Good thing you made me remember I wrote just this a while ago.
This is my first OpenBSD diff, so tell me if I missed anything obvious.
Tested quite extensively originally; for this diff I only checked a
simple nc to nc hello.
Index: netcat.c
===
RCS file: /mount/cvsdev/cvs/openbsd/src/usr.bin/nc/netcat.c,v
retrieving revision 1.121
diff -u -p -r1.121 netcat.c
--- netcat.c10 Jun 2014 16:35:42 - 1.121
+++ netcat.c26 Jun 2014 11:29:45 -
@@ -65,6 +65,12 @@
#define PORT_MAX_LEN 6
#define UNIX_DG_TMP_SOCKET_SIZE19
+#define POLL_STDIN 0
+#define POLL_NETOUT 1
+#define POLL_NETIN 2
+#define POLL_STDOUT 3
+#define BUFSIZE 16384
+
/* Command Line Options */
intdflag; /* detached, no stdin */
intFflag; /* fdpass sock to stdout */
@@ -112,6 +118,8 @@ voidset_common_sockopts(int);
intmap_tos(char *, int *);
void report_connect(const struct sockaddr *, socklen_t);
void usage(int);
+ssize_t drainbuf(int, unsigned char *, size_t *);
+ssize_t fillbuf(int, unsigned char *, size_t *);
int
main(int argc, char *argv[])
@@ -608,7 +616,7 @@ remote_connect(const char *host, const c
if (bind(s, (struct sockaddr *)ares-ai_addr,
ares-ai_addrlen) 0)
- err(1, bind failed);
+ errx(1, bind failed: %s, strerror(errno));
freeaddrinfo(ares);
}
@@ -640,7 +648,7 @@ timeout_connect(int s, const struct sock
if (timeout != -1) {
flags = fcntl(s, F_GETFL, 0);
if (fcntl(s, F_SETFL, flags | O_NONBLOCK) == -1)
- err(1, set non-blocking mode);
+ warn(unable to set non-blocking mode);
}
if ((ret = connect(s, name, namelen)) != 0 errno == EINPROGRESS) {
@@ -730,67 +738,229 @@ local_listen(char *host, char *port, str
* Loop that polls on the network file descriptor and stdin.
*/
void
-readwrite(int nfd)
+readwrite(int net_fd)
{
- struct pollfd pfd[2];
- unsigned char buf[16 * 1024];
- int n, wfd = fileno(stdin);
- int lfd = fileno(stdout);
- int plen;
-
- plen = sizeof(buf);
-
- /* Setup Network FD */
- pfd[0].fd = nfd;
- pfd[0].events = POLLIN;
+ struct pollfd pfd[4];
+ int stdin_fd = STDIN_FILENO;
+ int stdout_fd = STDOUT_FILENO;
+ unsigned char netinbuf[BUFSIZE];
+ size_t netinbufpos = 0;
+ unsigned char stdinbuf[BUFSIZE];
+ size_t stdinbufpos = 0;
+ int n, num_fds, flags;
+ ssize_t ret;
+
+ /* don't read from stdin if requested */
+ if (dflag)
+ stdin_fd = -1;
+
+ /* stdin */
+ pfd[POLL_STDIN].fd = stdin_fd;
+ pfd[POLL_STDIN].events = POLLIN;
+
+ /* network out */
+ pfd[POLL_NETOUT].fd = net_fd;
+ pfd[POLL_NETOUT].events = 0;
+
+ /* network in */
+ pfd[POLL_NETIN].fd = net_fd;
+ pfd[POLL_NETIN].events = POLLIN;
+
+ /* stdout */
+ pfd[POLL_STDOUT].fd = stdout_fd;
+ pfd[POLL_STDOUT].events = 0;
+
+
+ /* make all fds non-blocking */
+ for (n = 0; n 4; n++) {
+ if (pfd[n].fd == -1)
+ continue;
+ flags = fcntl(pfd[n].fd, F_GETFL, 0);
+ /*
+* For sockets and pipes, we want non-block, but setting it
+* might fail for files or devices, so we ignore the return
+* code.
+*/
+ fcntl(pfd[n].fd, F_SETFL, flags | O_NONBLOCK);
+ }
- /* Set up STDIN FD. */
- pfd[1].fd = wfd;
- pfd[1].events = POLLIN;
+ while (1) {
+ /* both inputs are gone, buffers are empty, we are done */
+ if (pfd[POLL_STDIN].fd == -1 pfd[POLL_NETIN].fd == -1
+stdinbufpos == 0 netinbufpos == 0) {
+ close(net_fd);
+ return;
+ }
+ /* both outputs are gone, we can't continue */
+ if (pfd[POLL_NETOUT].fd == -1 pfd[POLL_STDOUT].fd == -1) {
+ close(net_fd);
+ return;
+ }
+ /* listen and net in gone, queues empty, done */
+ if (lflag pfd[POLL_NETIN].fd == -1
+stdinbufpos == 0 netinbufpos == 0) {
+ close(net_fd);
+ return;
+ }
- while (pfd[0].fd != -1) {
+ /* help says -i is for wait between lines sent. We read and
+* write arbitray amounts of data, and we don't want to start
+* scanning for newlines,
Re: increase netcat's buffer...
On Thu, Jun 26, 2014 at 7:43 AM, Arne Becker arne_bec...@genua.de wrote:
Hi.
Now soliciting diffs to change readwrite to a loop with two buffers
that poll()s in all four directions. :)
Good thing you made me remember I wrote just this a while ago.
This is my first OpenBSD diff, so tell me if I missed anything obvious.
Tested quite extensively originally; for this diff I only checked a
simple nc to nc hello.
Index: netcat.c
===
RCS file: /mount/cvsdev/cvs/openbsd/src/usr.bin/nc/netcat.c,v
retrieving revision 1.121
diff -u -p -r1.121 netcat.c
--- netcat.c10 Jun 2014 16:35:42 - 1.121
+++ netcat.c26 Jun 2014 11:29:45 -
@@ -65,6 +65,12 @@
#define PORT_MAX_LEN 6
#define UNIX_DG_TMP_SOCKET_SIZE19
+#define POLL_STDIN 0
+#define POLL_NETOUT 1
+#define POLL_NETIN 2
+#define POLL_STDOUT 3
+#define BUFSIZE 16384
+
/* Command Line Options */
intdflag; /* detached, no stdin */
intFflag; /* fdpass sock to stdout */
@@ -112,6 +118,8 @@ voidset_common_sockopts(int);
intmap_tos(char *, int *);
void report_connect(const struct sockaddr *, socklen_t);
void usage(int);
+ssize_t drainbuf(int, unsigned char *, size_t *);
+ssize_t fillbuf(int, unsigned char *, size_t *);
int
main(int argc, char *argv[])
@@ -608,7 +616,7 @@ remote_connect(const char *host, const c
if (bind(s, (struct sockaddr *)ares-ai_addr,
ares-ai_addrlen) 0)
- err(1, bind failed);
+ errx(1, bind failed: %s, strerror(errno));
freeaddrinfo(ares);
}
@@ -640,7 +648,7 @@ timeout_connect(int s, const struct sock
if (timeout != -1) {
flags = fcntl(s, F_GETFL, 0);
if (fcntl(s, F_SETFL, flags | O_NONBLOCK) == -1)
- err(1, set non-blocking mode);
+ warn(unable to set non-blocking mode);
}
if ((ret = connect(s, name, namelen)) != 0 errno == EINPROGRESS) {
@@ -730,67 +738,229 @@ local_listen(char *host, char *port, str
* Loop that polls on the network file descriptor and stdin.
*/
void
-readwrite(int nfd)
+readwrite(int net_fd)
{
- struct pollfd pfd[2];
- unsigned char buf[16 * 1024];
- int n, wfd = fileno(stdin);
- int lfd = fileno(stdout);
- int plen;
-
- plen = sizeof(buf);
-
- /* Setup Network FD */
- pfd[0].fd = nfd;
- pfd[0].events = POLLIN;
+ struct pollfd pfd[4];
+ int stdin_fd = STDIN_FILENO;
+ int stdout_fd = STDOUT_FILENO;
+ unsigned char netinbuf[BUFSIZE];
+ size_t netinbufpos = 0;
+ unsigned char stdinbuf[BUFSIZE];
+ size_t stdinbufpos = 0;
+ int n, num_fds, flags;
+ ssize_t ret;
+
+ /* don't read from stdin if requested */
+ if (dflag)
+ stdin_fd = -1;
+
+ /* stdin */
+ pfd[POLL_STDIN].fd = stdin_fd;
+ pfd[POLL_STDIN].events = POLLIN;
+
+ /* network out */
+ pfd[POLL_NETOUT].fd = net_fd;
+ pfd[POLL_NETOUT].events = 0;
+
+ /* network in */
+ pfd[POLL_NETIN].fd = net_fd;
+ pfd[POLL_NETIN].events = POLLIN;
+
+ /* stdout */
+ pfd[POLL_STDOUT].fd = stdout_fd;
+ pfd[POLL_STDOUT].events = 0;
+
+
+ /* make all fds non-blocking */
+ for (n = 0; n 4; n++) {
+ if (pfd[n].fd == -1)
+ continue;
+ flags = fcntl(pfd[n].fd, F_GETFL, 0);
+ /*
+* For sockets and pipes, we want non-block, but setting it
+* might fail for files or devices, so we ignore the return
+* code.
+*/
+ fcntl(pfd[n].fd, F_SETFL, flags | O_NONBLOCK);
+ }
- /* Set up STDIN FD. */
- pfd[1].fd = wfd;
- pfd[1].events = POLLIN;
+ while (1) {
+ /* both inputs are gone, buffers are empty, we are done */
+ if (pfd[POLL_STDIN].fd == -1 pfd[POLL_NETIN].fd == -1
+stdinbufpos == 0 netinbufpos == 0) {
+ close(net_fd);
+ return;
+ }
+ /* both outputs are gone, we can't continue */
+ if (pfd[POLL_NETOUT].fd == -1 pfd[POLL_STDOUT].fd == -1) {
+ close(net_fd);
+ return;
+ }
+ /* listen and net in gone, queues empty, done */
+ if (lflag pfd[POLL_NETIN].fd == -1
lflag ???
warning only one ref in the diff
+stdinbufpos == 0 netinbufpos == 0) {
+ close(net_fd);
+ return;
+
Re: increase netcat's buffer...
On 2014/06/26 08:13, sven falempin wrote: trim lots of pointless lines + close(net_fd); + return; + } + /* listen and net in gone, queues empty, done */ + if (lflag pfd[POLL_NETIN].fd == -1 lflag ??? warning only one ref in the diff lflag is not new.
Re: increase netcat's buffer...
Hi. + /* listen and net in gone, queues empty, done */ + if (lflag pfd[POLL_NETIN].fd == -1 lflag ??? warning only one ref in the diff lflag is a global, the listen flag, as in: nc -l 127.0.0.1 80 I believe this is correct. Only when we listen do we want to close when the network input is gone. - Arne
Re: increase netcat's buffer...
On Thu, Jun 26, 2014 at 8:21 AM, Arne Becker arne_bec...@genua.de wrote: Hi. + /* listen and net in gone, queues empty, done */ + if (lflag pfd[POLL_NETIN].fd == -1 lflag ??? warning only one ref in the diff lflag is a global, the listen flag, as in: nc -l 127.0.0.1 80 I believe this is correct. Only when we listen do we want to close when the network input is gone. - Arne i have Zero idea if it is right or wrong, just warn because the symbol was lonely. For testing i would do something like nc - l | nc and run something like iperf3 on both end iperf client -tcp/udp nc -l | nc client -tcp/udp iperf server For a review i dislike + unsigned char stdinbuf[BUFSIZE]; and the memmove on it: Dear tech savvy, isn it better to malloc a buffer like this instead of alloca it ? or just a static buffer would be better so it is in the program mem space ? If the buffer is fixed, dont bother memmove it, just remember the begining and the end: http://en.wikipedia.org/wiki/Circular_buffer Is there a bunch of macro like TAIL_ ??? available for this ? -- - () ascii ribbon campaign - against html e-mail /\
Re: increase netcat's buffer...
On 2014/06/26 08:33, sven falempin wrote: i have Zero idea if it is right or wrong, just warn because the symbol was lonely. A diff only tells part of the story, it is also necessary to look at the surrounding code.
Re: increase netcat's buffer...
Hi. If the buffer is fixed, dont bother memmove it, just remember the begining and the end: http://en.wikipedia.org/wiki/Circular_buffer There's a tradeoff - lots of memmove vs. lots of very small reads/writes if you get near the end of the buffer. My gut feeling told me that local things, even if they require a syscall, will be a lot faster than things over the network, so I chose the version with larger reads and writes. A BIP buffer [1] would be the best solution to this problem. If such a thing already exists in the codebase, please point me to it. For this diff, I benchmarked the binary, and found no significant change in speed versus the previous implementation. I haven't really paid attention to resource usage. [1] http://www.codeproject.com/Articles/3479/The-Bip-Buffer-The-Circular-Buffer-with-a-Twist
Re: increase netcat's buffer...
On Thu, Jun 26, 2014 at 9:37 AM, Arne Becker arne_bec...@genua.de wrote: Hi. If the buffer is fixed, dont bother memmove it, just remember the begining and the end: http://en.wikipedia.org/wiki/Circular_buffer There's a tradeoff - lots of memmove vs. lots of very small reads/writes if you get near the end of the buffer. My gut feeling told me that local things, even if they require a syscall, will be a lot faster than things over the network, so I chose the version with larger reads and writes. A BIP buffer [1] would be the best solution to this problem. If such a thing already exists in the codebase, please point me to it. For this diff, I benchmarked the binary, and found no significant change in speed versus the previous implementation. I haven't really paid attention to resource usage. [1] http://www.codeproject.com/Articles/3479/The-Bip-Buffer-The-Circular-Buffer-with-a-Twist My circular buffer never made more than 2 reads or writes per read or write call, and very often only one. this BIP is just a proper implementation as far as i am concerned. does'nt change you are on the stack with the buffer, and this is not good (right?) Only system devs may explain this accurately and i do not want to make wrong statement anyway. Moreover there 's architecture i have never used that may behave differently. -- - () ascii ribbon campaign - against html e-mail /\
Re: increase netcat's buffer...
On Thu, Jun 26, 2014 at 08:33, sven falempin wrote: For a review i dislike + unsigned char stdinbuf[BUFSIZE]; and the memmove on it: Dear tech savvy, isn it better to malloc a buffer like this instead of alloca it ? or just a static buffer would be better so it is in the program mem space ? No, there's nothing wrong with a stack buffer of moderate size in a program like this. We don't use alloca() much though.
Re: increase netcat's buffer...
On Thu, Jun 26, 2014 at 10:09 AM, Ted Unangst t...@tedunangst.com wrote: On Thu, Jun 26, 2014 at 08:33, sven falempin wrote: For a review i dislike + unsigned char stdinbuf[BUFSIZE]; and the memmove on it: Dear tech savvy, isn it better to malloc a buffer like this instead of alloca it ? or just a static buffer would be better so it is in the program mem space ? No, there's nothing wrong with a stack buffer of moderate size in a program like this. We don't use alloca() much though. Ted, What is a 'good maximum' size for a buffer like this in amd64 world and arm ? From which amount should we consider to not use the stack. (I know it depends the type of function, but lets focus on this case ) Cheers. -- - () ascii ribbon campaign - against html e-mail /\
Re: increase netcat's buffer...
On Thu, Jun 26, 2014 at 11:12, sven falempin wrote: What is a 'good maximum' size for a buffer like this in amd64 world and arm ? From which amount should we consider to not use the stack. (I know it depends the type of function, but lets focus on this case ) Stack rlimit is 4096K, so there shouldn't be anything wrong with using some number up to a few hundred K. After about a megabyte I'd probably start asking questions, but only because a fixed size buffer that large is unusual, regardless of where it's been allocated.
Re: increase netcat's buffer...
sven falempin wrote this message on Tue, Jun 17, 2014 at 07:42 -0400:
On Mon, Jun 16, 2014 at 10:57 PM, Ted Unangst t...@tedunangst.com wrote:
On Sat, Jun 14, 2014 at 10:55, Ted Unangst wrote:
On Fri, Jun 13, 2014 at 10:40, sven falempin wrote:
Now soliciting diffs to change readwrite to a loop with two buffers
that poll()s in all four directions. :)
Is using kqueue ok ?
like : http://pastebin.com/F1c5Hswi
uh, maybe. the kqueue changes make it harder to review at first, but
I'll look closer soonish.
Turns out pastebin isn't a very good place for storing diffs. It's
gone now, making it very hard to review. Can you send the diff to the
list? Thanks.
Index: netcat.c
===
RCS file: /cvs/src/usr.bin/nc/netcat.c,v
retrieving revision 1.121
diff -u -p -r1.121 netcat.c
--- netcat.c10 Jun 2014 16:35:42 - 1.121
+++ netcat.c14 Jun 2014 17:46:31 -
@@ -36,6 +36,7 @@
#include sys/time.h
#include sys/uio.h
#include sys/un.h
+#include sys/event.h
#include netinet/in.h
#include netinet/in_systm.h
@@ -734,58 +735,124 @@ readwrite(int nfd)
{
struct pollfd pfd[2];
pfd can be removed, I don't think you're using it...
unsigned char buf[16 * 1024];
+ struct kevent keventlist[4];
+ quad_t write_space_network;
+ quad_t write_space_stdout;
+ quad_t available;
int n, wfd = fileno(stdin);
int lfd = fileno(stdout);
int plen;
+ int kq;
+ int evcount;
+ int evid;
plen = sizeof(buf);
- /* Setup Network FD */
- pfd[0].fd = nfd;
- pfd[0].events = POLLIN;
+ /* Set up kqueue*/
+ kq = kqueue();
+ if (kq == -1) {
+ close(nfd);
+ err(1, Kqueue Error);
+ }
+
+ /* Set up STDOUT FD. */
+ EV_SET(keventlist[0], nfd, EVFILT_WRITE, EV_ADD, 0, 0, NULL);
+
+ /* Set up Network FD */
+ EV_SET(keventlist[1], lfd, EVFILT_WRITE, EV_ADD, 0, 0, NULL);
+ EV_SET(keventlist[2], nfd, EVFILT_READ, EV_ADD, 0, 0, NULL);
/* Set up STDIN FD. */
- pfd[1].fd = wfd;
- pfd[1].events = POLLIN;
+ EV_SET(keventlist[3], wfd, EVFILT_READ, EV_ADD, 0, 0, NULL);
- while (pfd[0].fd != -1) {
+ /* Registering events */
+ evcount = kevent(kq, keventlist, 4, NULL, 0, NULL);
You should pass the keventlist in to see if any erros happened while
registering.. it'd be bad if there was, but we did tell the user that
IO isn't going to happen...
+ if (evcount == -1) {
+ close(nfd);
We should probably close kq here... All the places later in the
function that close nfd, should close kq...
+ err(1, Kevent Error);
+ }
+
+ while (evcount != -1) {
if (iflag)
sleep(iflag);
-
- if ((n = poll(pfd, 2 - dflag, timeout)) 0) {
+ evcount = kevent(kq, NULL, 0, keventlist, 4, NULL);
+ if (evcount == -1) {
close(nfd);
- err(1, Polling Error);
+ err(1, Kevent Error);
}
+ for (evid = 0; evid evcount; evid++) {
+ struct kevent event = keventlist[evid];
+ if (event.flags EV_ERROR) {
+ close(nfd);
+ err(1, Kevent Error);
+ }
- if (n == 0)
- return;
-
- if (pfd[0].revents POLLIN) {
- if ((n = read(nfd, buf, plen)) 0)
- return;
- else if (n == 0) {
- shutdown(nfd, SHUT_RD);
- pfd[0].fd = -1;
- pfd[0].events = 0;
- } else {
- if (tflag)
- atelnet(nfd, buf, n);
- if (atomicio(vwrite, lfd, buf, n) != n)
+ if (event.ident == nfd) {
+ if (event.flags EV_EOF) {
You (at least on FreeBSD) can have data along w/ the EOF flag:
It is
possible for EOF to be returned (indicating the con-
nection is gone) while there is still data pending
in the socket buffer.
So, you should check for data first..
Also, you don't check which direction got the EOF, so you might be
shutting down the wrong direction..
+ shutdown(nfd, SHUT_RD);
return;
Are you sure you want to return here? What if there is more data in
the other direction?
+ } else {
+ if (event.filter == EVFILT_WRITE) {
Re: increase netcat's buffer...
On Sun, Jun 22, 2014 at 9:20 AM, John-Mark Gurney j...@funkthat.com wrote:
sven falempin wrote this message on Tue, Jun 17, 2014 at 07:42 -0400:
On Mon, Jun 16, 2014 at 10:57 PM, Ted Unangst t...@tedunangst.com
wrote:
On Sat, Jun 14, 2014 at 10:55, Ted Unangst wrote:
On Fri, Jun 13, 2014 at 10:40, sven falempin wrote:
Now soliciting diffs to change readwrite to a loop with two
buffers
that poll()s in all four directions. :)
Is using kqueue ok ?
like : http://pastebin.com/F1c5Hswi
uh, maybe. the kqueue changes make it harder to review at first, but
I'll look closer soonish.
Turns out pastebin isn't a very good place for storing diffs. It's
gone now, making it very hard to review. Can you send the diff to the
list? Thanks.
Index: netcat.c
===
RCS file: /cvs/src/usr.bin/nc/netcat.c,v
retrieving revision 1.121
diff -u -p -r1.121 netcat.c
--- netcat.c10 Jun 2014 16:35:42 - 1.121
+++ netcat.c14 Jun 2014 17:46:31 -
@@ -36,6 +36,7 @@
#include sys/time.h
#include sys/uio.h
#include sys/un.h
+#include sys/event.h
#include netinet/in.h
#include netinet/in_systm.h
@@ -734,58 +735,124 @@ readwrite(int nfd)
{
struct pollfd pfd[2];
pfd can be removed, I don't think you're using it...
unsigned char buf[16 * 1024];
+ struct kevent keventlist[4];
+ quad_t write_space_network;
+ quad_t write_space_stdout;
+ quad_t available;
int n, wfd = fileno(stdin);
int lfd = fileno(stdout);
int plen;
+ int kq;
+ int evcount;
+ int evid;
plen = sizeof(buf);
- /* Setup Network FD */
- pfd[0].fd = nfd;
- pfd[0].events = POLLIN;
+ /* Set up kqueue*/
+ kq = kqueue();
+ if (kq == -1) {
+ close(nfd);
+ err(1, Kqueue Error);
+ }
+
+ /* Set up STDOUT FD. */
+ EV_SET(keventlist[0], nfd, EVFILT_WRITE, EV_ADD, 0, 0, NULL);
+
+ /* Set up Network FD */
+ EV_SET(keventlist[1], lfd, EVFILT_WRITE, EV_ADD, 0, 0, NULL);
+ EV_SET(keventlist[2], nfd, EVFILT_READ, EV_ADD, 0, 0, NULL);
/* Set up STDIN FD. */
- pfd[1].fd = wfd;
- pfd[1].events = POLLIN;
+ EV_SET(keventlist[3], wfd, EVFILT_READ, EV_ADD, 0, 0, NULL);
- while (pfd[0].fd != -1) {
+ /* Registering events */
+ evcount = kevent(kq, keventlist, 4, NULL, 0, NULL);
You should pass the keventlist in to see if any erros happened while
registering.. it'd be bad if there was, but we did tell the user that
IO isn't going to happen...
+ if (evcount == -1) {
+ close(nfd);
We should probably close kq here... All the places later in the
function that close nfd, should close kq...
+ err(1, Kevent Error);
+ }
+
+ while (evcount != -1) {
if (iflag)
sleep(iflag);
-
- if ((n = poll(pfd, 2 - dflag, timeout)) 0) {
+ evcount = kevent(kq, NULL, 0, keventlist, 4, NULL);
+ if (evcount == -1) {
close(nfd);
- err(1, Polling Error);
+ err(1, Kevent Error);
}
+ for (evid = 0; evid evcount; evid++) {
+ struct kevent event = keventlist[evid];
+ if (event.flags EV_ERROR) {
+ close(nfd);
+ err(1, Kevent Error);
+ }
- if (n == 0)
- return;
-
- if (pfd[0].revents POLLIN) {
- if ((n = read(nfd, buf, plen)) 0)
- return;
- else if (n == 0) {
- shutdown(nfd, SHUT_RD);
- pfd[0].fd = -1;
- pfd[0].events = 0;
- } else {
- if (tflag)
- atelnet(nfd, buf, n);
- if (atomicio(vwrite, lfd, buf, n) != n)
+ if (event.ident == nfd) {
+ if (event.flags EV_EOF) {
You (at least on FreeBSD) can have data along w/ the EOF flag:
It is
possible for EOF to be returned (indicating the con-
nection is gone) while there is still data pending
in the socket buffer.
So, you should check for data first..
Also, you don't check which direction got the EOF, so you might be
shutting down the wrong direction..
+ shutdown(nfd, SHUT_RD);
return;
Are
Re: increase netcat's buffer...
On Fri, Jun 13, 2014 at 07:50, sven falempin wrote: Now soliciting diffs to change readwrite to a loop with two buffers that poll()s in all four directions. :) Is using kqueue ok ? Sorry. Returning to this point in the thread, I agree with Theo the answer is no. People are running this code on linux, and using kqueue just makes life hard for them. There's no benefit in a program like this. Same for libevent. Unnecessary complication.
Re: increase netcat's buffer...
On Mon, Jun 16, 2014 at 10:57 PM, Ted Unangst t...@tedunangst.com wrote: On Sat, Jun 14, 2014 at 10:55, Ted Unangst wrote: On Fri, Jun 13, 2014 at 10:40, sven falempin wrote: Now soliciting diffs to change readwrite to a loop with two buffers that poll()s in all four directions. :) Is using kqueue ok ? like : http://pastebin.com/F1c5Hswi uh, maybe. the kqueue changes make it harder to review at first, but I'll look closer soonish. Turns out pastebin isn't a very good place for storing diffs. It's gone now, making it very hard to review. Can you send the diff to the list? Thanks. Did you get the other mail ? -- - () ascii ribbon campaign - against html e-mail /\
Re: increase netcat's buffer...
On Mon, Jun 16, 2014 at 10:57 PM, Ted Unangst t...@tedunangst.com wrote:
On Sat, Jun 14, 2014 at 10:55, Ted Unangst wrote:
On Fri, Jun 13, 2014 at 10:40, sven falempin wrote:
Now soliciting diffs to change readwrite to a loop with two buffers
that poll()s in all four directions. :)
Is using kqueue ok ?
like : http://pastebin.com/F1c5Hswi
uh, maybe. the kqueue changes make it harder to review at first, but
I'll look closer soonish.
Turns out pastebin isn't a very good place for storing diffs. It's
gone now, making it very hard to review. Can you send the diff to the
list? Thanks.
Index: netcat.c
===
RCS file: /cvs/src/usr.bin/nc/netcat.c,v
retrieving revision 1.121
diff -u -p -r1.121 netcat.c
--- netcat.c10 Jun 2014 16:35:42 - 1.121
+++ netcat.c14 Jun 2014 17:46:31 -
@@ -36,6 +36,7 @@
#include sys/time.h
#include sys/uio.h
#include sys/un.h
+#include sys/event.h
#include netinet/in.h
#include netinet/in_systm.h
@@ -734,58 +735,124 @@ readwrite(int nfd)
{
struct pollfd pfd[2];
unsigned char buf[16 * 1024];
+ struct kevent keventlist[4];
+ quad_t write_space_network;
+ quad_t write_space_stdout;
+ quad_t available;
int n, wfd = fileno(stdin);
int lfd = fileno(stdout);
int plen;
+ int kq;
+ int evcount;
+ int evid;
plen = sizeof(buf);
- /* Setup Network FD */
- pfd[0].fd = nfd;
- pfd[0].events = POLLIN;
+ /* Set up kqueue*/
+ kq = kqueue();
+ if (kq == -1) {
+ close(nfd);
+ err(1, Kqueue Error);
+ }
+
+ /* Set up STDOUT FD. */
+ EV_SET(keventlist[0], nfd, EVFILT_WRITE, EV_ADD, 0, 0, NULL);
+
+ /* Set up Network FD */
+ EV_SET(keventlist[1], lfd, EVFILT_WRITE, EV_ADD, 0, 0, NULL);
+ EV_SET(keventlist[2], nfd, EVFILT_READ, EV_ADD, 0, 0, NULL);
/* Set up STDIN FD. */
- pfd[1].fd = wfd;
- pfd[1].events = POLLIN;
+ EV_SET(keventlist[3], wfd, EVFILT_READ, EV_ADD, 0, 0, NULL);
- while (pfd[0].fd != -1) {
+ /* Registering events */
+ evcount = kevent(kq, keventlist, 4, NULL, 0, NULL);
+ if (evcount == -1) {
+ close(nfd);
+ err(1, Kevent Error);
+ }
+
+ while (evcount != -1) {
if (iflag)
sleep(iflag);
-
- if ((n = poll(pfd, 2 - dflag, timeout)) 0) {
+ evcount = kevent(kq, NULL, 0, keventlist, 4, NULL);
+ if (evcount == -1) {
close(nfd);
- err(1, Polling Error);
+ err(1, Kevent Error);
}
+ for (evid = 0; evid evcount; evid++) {
+ struct kevent event = keventlist[evid];
+ if (event.flags EV_ERROR) {
+ close(nfd);
+ err(1, Kevent Error);
+ }
- if (n == 0)
- return;
-
- if (pfd[0].revents POLLIN) {
- if ((n = read(nfd, buf, plen)) 0)
- return;
- else if (n == 0) {
- shutdown(nfd, SHUT_RD);
- pfd[0].fd = -1;
- pfd[0].events = 0;
- } else {
- if (tflag)
- atelnet(nfd, buf, n);
- if (atomicio(vwrite, lfd, buf, n) != n)
+ if (event.ident == nfd) {
+ if (event.flags EV_EOF) {
+ shutdown(nfd, SHUT_RD);
return;
+ } else {
+ if (event.filter == EVFILT_WRITE) {
+ write_space_network =
event.data;
+ }
+ else if (event.filter ==
EVFILT_READ) {
+ available = MIN(event.data,
write_space_stdout);
+ available = MIN(available,
plen);
+ if (available 0) {
+ n = read(nfd, buf,
available);
+ if (n != available)
{
+ return;
+ } else {
+ n =
write(lfd, buf, available);
+ if (n !=
available) {
+
return;
+
Re: increase netcat's buffer...
It should not use kevent. That makes the code non-portable. Use poll().
Re: increase netcat's buffer...
On Fri, Jun 13, 2014 at 10:40, sven falempin wrote: Now soliciting diffs to change readwrite to a loop with two buffers that poll()s in all four directions. :) Is using kqueue ok ? like : http://pastebin.com/F1c5Hswi uh, maybe. the kqueue changes make it harder to review at first, but I'll look closer soonish.
Re: increase netcat's buffer...
On Fri, Jun 13, 2014 at 7:50 AM, sven falempin sven.falem...@gmail.com wrote: On Tue, Jun 10, 2014 at 12:45 PM, Ted Unangst t...@tedunangst.com wrote: On Mon, Jun 09, 2014 at 21:54, Theo de Raadt wrote: A better patch is probably the following which also increases the size of the buffer to at least 64k: Agreed. One thing to be aware of. That function is syncronous. It will read as much as it can get, then it will do an atomic write operation to flush the buffer out the other way. If you have substantially different speeds, this can be a substantial 'buffer bloat'. Since it is handling a session in both directions... expect to see some substantial jaggies. Having convinced me this a problem (it's already in the code, but 64k buffers will make it worse), I scaled back to 16k. Now soliciting diffs to change readwrite to a loop with two buffers that poll()s in all four directions. :) Is using kqueue ok ? like : http://pastebin.com/F1c5Hswi
Re: increase netcat's buffer...
On Mon, Jun 09, 2014 at 21:54, Theo de Raadt wrote: A better patch is probably the following which also increases the size of the buffer to at least 64k: Agreed. One thing to be aware of. That function is syncronous. It will read as much as it can get, then it will do an atomic write operation to flush the buffer out the other way. If you have substantially different speeds, this can be a substantial 'buffer bloat'. Since it is handling a session in both directions... expect to see some substantial jaggies. Having convinced me this a problem (it's already in the code, but 64k buffers will make it worse), I scaled back to 16k. Now soliciting diffs to change readwrite to a loop with two buffers that poll()s in all four directions. :)
Re: increase netcat's buffer...
On 10/06/14 17:45, Ted Unangst wrote: Now soliciting diffs to change readwrite to a loop with two buffers that poll()s in all four directions. :) Would libevent also be an option? bernd
Re: increase netcat's buffer...
On 10/06/14 19:57, Bernte wrote: On 10/06/14 17:45, Ted Unangst wrote: Now soliciting diffs to change readwrite to a loop with two buffers that poll()s in all four directions. :) Would libevent also be an option? bernd Ignore that, it is already using poll, so need to change that. Stupid idea. bernd
Re: increase netcat's buffer...
On Mon, Jun 09, 2014 at 20:12, John-Mark Gurney wrote: A better patch is probably the following which also increases the size of the buffer to at least 64k: Agreed.
Re: increase netcat's buffer...
A better patch is probably the following which also increases the size of the buffer to at least 64k: Agreed. One thing to be aware of. That function is syncronous. It will read as much as it can get, then it will do an atomic write operation to flush the buffer out the other way. If you have substantially different speeds, this can be a substantial 'buffer bloat'. Since it is handling a session in both directions... expect to see some substantial jaggies.
