Get rid of /proc/sys/net/unix/max_dgram_qlen

2006-08-22 Thread Indan Zupancic
Hello,

Here's a patch to get rid of max_dgram_qlen proc option. All it does is
slow down unix datagram packet sending, without giving the program any
control over it.

Applying it decreases code size, simplifies the code and makes poll
behaviour more logical for connected datagram sockets in regard to
POLLOUT. With the current code poll can say the socket is writable,
because it only checks the buffers, while sendmsg will fail because too
many packets are queued up already. In practice this means that a slow
reader will cause a non-blocking sender to hog the CPU.

Either this, or it should be implemented correctly, which means poll needs
to be fixed to also check for max_dgram_qlen, and maybe an ioctl should be
added to change the variable per socket.

Greetings,

Indan


diff --git a/net/unix/Makefile b/net/unix/Makefile
--- a/net/unix/Makefile Sun Aug 06 19:00:05 2006 +
+++ b/net/unix/Makefile Tue Aug 22 21:06:09 2006 +0200
@@ -5,4 +5,3 @@ obj-$(CONFIG_UNIX)  += unix.o
 obj-$(CONFIG_UNIX) += unix.o

 unix-y := af_unix.o garbage.o
-unix-$(CONFIG_SYSCTL)  += sysctl_net_unix.o
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
--- a/net/unix/af_unix.cSun Aug 06 19:00:05 2006 +
+++ b/net/unix/af_unix.cTue Aug 22 21:05:39 2006 +0200
@@ -117,8 +117,6 @@
 #include net/checksum.h
 #include linux/security.h

-int sysctl_unix_max_dgram_qlen = 10;
-
 struct hlist_head unix_socket_table[UNIX_HASH_SIZE + 1];
 DEFINE_SPINLOCK(unix_table_lock);
 static atomic_t unix_nr_socks = ATOMIC_INIT(0);
@@ -586,7 +584,6 @@ static struct sock * unix_create1(struct
af_unix_sk_receive_queue_lock_key);

sk-sk_write_space  = unix_write_space;
-   sk-sk_max_ack_backlog  = sysctl_unix_max_dgram_qlen;
sk-sk_destruct = unix_sock_destructor;
u = unix_sk(sk);
u-dentry = NULL;
@@ -1379,23 +1376,6 @@ restart:
goto out_unlock;
}

-   if (unix_peer(other) != sk 
-   (skb_queue_len(other-sk_receive_queue) 
-other-sk_max_ack_backlog)) {
-   if (!timeo) {
-   err = -EAGAIN;
-   goto out_unlock;
-   }
-
-   timeo = unix_wait_for_peer(other, timeo);
-
-   err = sock_intr_errno(timeo);
-   if (signal_pending(current))
-   goto out_free;
-
-   goto restart;
-   }
-
skb_queue_tail(other-sk_receive_queue, skb);
unix_state_runlock(other);
other-sk_data_ready(other, len);
@@ -2076,7 +2056,6 @@ static int __init af_unix_init(void)
 #ifdef CONFIG_PROC_FS
proc_net_fops_create(unix, 0, unix_seq_fops);
 #endif
-   unix_sysctl_register();
 out:
return rc;
 }
@@ -2084,7 +2063,6 @@ static void __exit af_unix_exit(void)
 static void __exit af_unix_exit(void)
 {
sock_unregister(PF_UNIX);
-   unix_sysctl_unregister();
proc_net_remove(unix);
proto_unregister(unix_proto);
 }
diff --git a/net/unix/sysctl_net_unix.c b/net/unix/sysctl_net_unix.c deleted 
file mode 100644
--- a/net/unix/sysctl_net_unix.cSun Aug 06 19:00:05 2006 +
+++ /dev/null   Thu Jan 01 00:00:00 1970 +
@@ -1,60 +0,0 @@
-/*
- * NET4:   Sysctl interface to net af_unix subsystem.
- *
- * Authors:Mike Shaver.
- *
- * This program is free software; you can redistribute it and/or - 
*   modify it under the terms
of the GNU General Public License
- * as published by the Free Software Foundation; either version
- * 2 of the License, or (at your option) any later version.
- */
-
-#include linux/mm.h
-#include linux/sysctl.h
-
-#include net/af_unix.h
-
-static ctl_table unix_table[] = {
-   {
-   .ctl_name   = NET_UNIX_MAX_DGRAM_QLEN,
-   .procname   = max_dgram_qlen,
-   .data   = sysctl_unix_max_dgram_qlen,
-   .maxlen = sizeof(int),
-   .mode   = 0644,
-   .proc_handler   = proc_dointvec
-   },
-   { .ctl_name = 0 }
-};
-
-static ctl_table unix_net_table[] = {
-   {
-   .ctl_name   = NET_UNIX,
-   .procname   = unix,
-   .mode   = 0555,
-   .child  = unix_table
-   },
-   { .ctl_name = 0 }
-};
-
-static ctl_table unix_root_table[] = {
-   {
-   .ctl_name   = CTL_NET,
-   .procname   = net,
-   .mode   = 0555,
-   .child  = unix_net_table
-   },
-   { .ctl_name = 0 }
-};
-
-static struct ctl_table_header * unix_sysctl_header;
-
-void unix_sysctl_register(void)
-{
-   unix_sysctl_header = register_sysctl_table(unix_root_table, 0);
-}
-
-void unix_sysctl_unregister(void)
-{
-   unregister_sysctl_table(unix_sysctl_header);
-}
-




-
To unsubscribe from this 

Re: Get rid of /proc/sys/net/unix/max_dgram_qlen

2006-08-22 Thread Alexey Kuznetsov
Hello!

 Either this, or it should be implemented correctly, which means poll needs
 to be fixed to also check for max_dgram_qlen,

Feel free to do this correctly. :-)
Deleting wrong code rarely helps.

It is the only protection of commiting infinite amount of memory to a socket.

Alexey
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Get rid of /proc/sys/net/unix/max_dgram_qlen

2006-08-22 Thread Indan Zupancic
On Tue, August 22, 2006 22:39, Alexey Kuznetsov said:
 Feel free to do this correctly. :-)
 Deleting wrong code rarely helps.

 It is the only protection of commiting infinite amount of memory to a socket.

Doesn't the if (atomic_read(sk-sk_wmem_alloc)  sk-sk_sndbuf) check in 
sock_alloc_send_pskb()
limit things already? Or don't unix datagram sockets have a limited sendbuffer? 
Ouch, that
complicates things.

But wait a moment, I'm running a kernel now with the patch applied and some 
testcode shows that
everything seems to behave as expected, with send() returning EAGAIN after a 
while, and poll
giving POLLOUT when more data can be send.

What am I missing?

Indan


-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Get rid of /proc/sys/net/unix/max_dgram_qlen

2006-08-22 Thread Alexey Kuznetsov
Hello!

  It is the only protection of commiting infinite amount of memory to a 
  socket.
 
 Doesn't the if (atomic_read(sk-sk_wmem_alloc)  sk-sk_sndbuf) check in 
 sock_alloc_send_pskb()
 limit things already?

Unfortunately, it does not. You can open a socket, send
something to a selected victim, close it, and repeat this
until receiver accumulates enough of skbs to kill the system.

Alexey
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Get rid of /proc/sys/net/unix/max_dgram_qlen

2006-08-22 Thread Indan Zupancic
On Wed, August 23, 2006 0:34, Alexey Kuznetsov said:
  It is the only protection of commiting infinite amount of memory to a 
  socket.

 Doesn't the if (atomic_read(sk-sk_wmem_alloc)  sk-sk_sndbuf) check in
 sock_alloc_send_pskb()
 limit things already?

 Unfortunately, it does not. You can open a socket, send
 something to a selected victim, close it, and repeat this
 until receiver accumulates enough of skbs to kill the system.

Well, it seems the devil is in the details, as usual.

Isn't a socket freed until all skb are handled? In which case the limit on the 
number of open
files limits the total memory usage? (Same as with streaming sockets?)

Greetings,

Indan


-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Get rid of /proc/sys/net/unix/max_dgram_qlen

2006-08-22 Thread Alexey Kuznetsov
Hello!

 Isn't a socket freed until all skb are handled? In which case the limit on 
 the number of open
 files limits the total memory usage? (Same as with streaming sockets?)

Alas. Number of closed sockets is not limited. Actually, it is limited
by sk_max_ack_backlog*max_files, which is a lot.

The problem is specific for unconnected datagram sockets
(predicate unix_peer(other) != sk)
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Get rid of /proc/sys/net/unix/max_dgram_qlen

2006-08-22 Thread Indan Zupancic
On Wed, August 23, 2006 1:32, Alexey Kuznetsov said:

 Isn't a socket freed until all skb are handled? In which case the limit on 
 the number of open
 files limits the total memory usage? (Same as with streaming sockets?)

 Alas. Number of closed sockets is not limited. Actually, it is limited
 by sk_max_ack_backlog*max_files, which is a lot.

Hmm... So setting sk_max_ack_backlog to 1 makes it limited by max_files,
which should make the worst case the same as for streaming sockets, right?

 The problem is specific for unconnected datagram sockets
 (predicate unix_peer(other) != sk)

Doesn't that mean that both sockets are connected to eachother? I mean,
if only this socket connects to the other the above check isn't true.

Greetings,

Indan


-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html