| Message ID | 13e8aad86903481261581de7c29444e3@mindstab.net |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On Wed, Jan 22, 2014 at 07:11:20AM -0800, Dan Ballard wrote: > diff --git a/net/core/sock.c b/net/core/sock.c > index 5393b4b..1ff69d1 100644 > --- a/net/core/sock.c > +++ b/net/core/sock.c > @@ -915,6 +915,10 @@ set_rcvbuf: > sk->sk_max_pacing_rate); > break; > > + case SO_MAX_DGRAM_QLEN: > + sk->sk_max_ack_backlog = val; > + break; > + Shouldn't the backlog be capped for unprivileged users to some configurable value? I even think that max_dgram_qlen should be the upper bound. I guess it is not that serious as socket read accounting does account all packets which sit in the backlog queue. Greetings, Hannes -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2014-01-22 at 07:11 -0800, Dan Ballard wrote: > Provides a new option for setsockopt SO_MAX_DGRAM_QLEN that sets and > gets a socket specific max datagram queue length. Currently each socket > has one but it's only ever initialized from > /proc/sys/net/unix/max_dgram_qlen and then never adjustable later. Now > each socket can have it individually tweaked during it's life. > Your patch suffers from many problems. It breaks listen() It doesn't compile on a lot of arches. sk_max_ack_backlog is an unsigned short. max_dgram_qlen is used, but only for Unix sockets at this time Try : "git grep -n max_dgram_qlen" for details > Signed-off-by: Dan Ballard <dan@mindstab.net> > --- > include/uapi/asm-generic/socket.h | 2 ++ > net/core/sock.c | 7 +++++++ > 2 files changed, 9 insertions(+) > > diff --git a/include/uapi/asm-generic/socket.h > b/include/uapi/asm-generic/socket.h > index 38f14d0..f8c3e6b 100644 > --- a/include/uapi/asm-generic/socket.h > +++ b/include/uapi/asm-generic/socket.h > @@ -80,4 +80,6 @@ > > #define SO_MAX_PACING_RATE 47 > > +#define SO_MAX_DGRAM_QLEN 48 > + > #endif /* __ASM_GENERIC_SOCKET_H */ > diff --git a/net/core/sock.c b/net/core/sock.c > index 5393b4b..1ff69d1 100644 > --- a/net/core/sock.c > +++ b/net/core/sock.c > @@ -915,6 +915,10 @@ set_rcvbuf: > sk->sk_max_pacing_rate); > break; > > + case SO_MAX_DGRAM_QLEN: > + sk->sk_max_ack_backlog = val; > + break; > + > default: > ret = -ENOPROTOOPT; > break; > @@ -1182,6 +1186,9 @@ int sock_getsockopt(struct socket *sock, int > level, int optname, > v.val = sk->sk_max_pacing_rate; > break; > > + case SO_MAX_DGRAM_QLEN: > + v.val = sk->sk_max_ack_backlog; > + break; > default: > return -ENOPROTOOPT; > } -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 01/22/2014 04:11 PM, Dan Ballard wrote: > Provides a new option for setsockopt SO_MAX_DGRAM_QLEN that sets and > gets a socket specific max datagram queue length. Currently each socket > has one but it's only ever initialized from > /proc/sys/net/unix/max_dgram_qlen and then never adjustable later. Now > each socket can have it individually tweaked during it's life. > > Signed-off-by: Dan Ballard <dan@mindstab.net> > --- > include/uapi/asm-generic/socket.h | 2 ++ > net/core/sock.c | 7 +++++++ > 2 files changed, 9 insertions(+) > > diff --git a/include/uapi/asm-generic/socket.h b/include/uapi/asm-generic/socket.h > index 38f14d0..f8c3e6b 100644 > --- a/include/uapi/asm-generic/socket.h > +++ b/include/uapi/asm-generic/socket.h > @@ -80,4 +80,6 @@ > > #define SO_MAX_PACING_RATE 47 > > +#define SO_MAX_DGRAM_QLEN 48 > + This needs to be added in more than just asm-generic, e.g. have a look at SO_MAX_PACING_RATE or SO_BPF_EXTENSIONS. Also you might need to rebase to current net-next head and maybe describe use cases more in-depth; next to what Hannes just commented. > #endif /* __ASM_GENERIC_SOCKET_H */ > diff --git a/net/core/sock.c b/net/core/sock.c > index 5393b4b..1ff69d1 100644 > --- a/net/core/sock.c > +++ b/net/core/sock.c > @@ -915,6 +915,10 @@ set_rcvbuf: > sk->sk_max_pacing_rate); > break; > > + case SO_MAX_DGRAM_QLEN: > + sk->sk_max_ack_backlog = val; > + break; > + > default: > ret = -ENOPROTOOPT; > break; > @@ -1182,6 +1186,9 @@ int sock_getsockopt(struct socket *sock, int level, int optname, > v.val = sk->sk_max_pacing_rate; > break; > > + case SO_MAX_DGRAM_QLEN: > + v.val = sk->sk_max_ack_backlog; > + break; > default: > return -ENOPROTOOPT; > } -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jan 22, 2014 at 04:20:36PM +0100, Hannes Frederic Sowa wrote: > On Wed, Jan 22, 2014 at 07:11:20AM -0800, Dan Ballard wrote: > > diff --git a/net/core/sock.c b/net/core/sock.c > > index 5393b4b..1ff69d1 100644 > > --- a/net/core/sock.c > > +++ b/net/core/sock.c > > @@ -915,6 +915,10 @@ set_rcvbuf: > > sk->sk_max_pacing_rate); > > break; > > > > + case SO_MAX_DGRAM_QLEN: > > + sk->sk_max_ack_backlog = val; > > + break; > > + > > Shouldn't the backlog be capped for unprivileged users to some configurable > value? I even think that max_dgram_qlen should be the upper bound. > > I guess it is not that serious as socket read accounting does account all > packets which sit in the backlog queue. Just a follow-up: sk_max_ack_backlog is also responsible for limiting the af_unix dgram queues. Currently there is no socket accounting for the read side of those unix dgram sockets. I tried to fix this once here, http://patchwork.ozlabs.org/patch/231032/, but until that is done we depend on max_dgram_qlen to limit those queues at all. I hope I can get back to this patch anytime soon, as it solves the problem that a bidirectional protocol ping-ponging with a dgram server socket and not fetching its messages from the backlog queue can bring a server to halt because it doesn't have any send space on the socket anymore. Greetings, Hannes -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/uapi/asm-generic/socket.h b/include/uapi/asm-generic/socket.h index 38f14d0..f8c3e6b 100644 --- a/include/uapi/asm-generic/socket.h +++ b/include/uapi/asm-generic/socket.h @@ -80,4 +80,6 @@ #define SO_MAX_PACING_RATE 47 +#define SO_MAX_DGRAM_QLEN 48 + #endif /* __ASM_GENERIC_SOCKET_H */ diff --git a/net/core/sock.c b/net/core/sock.c index 5393b4b..1ff69d1 100644 --- a/net/core/sock.c +++ b/net/core/sock.c @@ -915,6 +915,10 @@ set_rcvbuf: sk->sk_max_pacing_rate); break; + case SO_MAX_DGRAM_QLEN: + sk->sk_max_ack_backlog = val; + break; + default: ret = -ENOPROTOOPT; break; @@ -1182,6 +1186,9 @@ int sock_getsockopt(struct socket *sock, int level, int optname, v.val = sk->sk_max_pacing_rate; break; + case SO_MAX_DGRAM_QLEN: + v.val = sk->sk_max_ack_backlog; + break; default: return -ENOPROTOOPT;
Provides a new option for setsockopt SO_MAX_DGRAM_QLEN that sets and gets a socket specific max datagram queue length. Currently each socket has one but it's only ever initialized from /proc/sys/net/unix/max_dgram_qlen and then never adjustable later. Now each socket can have it individually tweaked during it's life. Signed-off-by: Dan Ballard <dan@mindstab.net> --- include/uapi/asm-generic/socket.h | 2 ++ net/core/sock.c | 7 +++++++ 2 files changed, 9 insertions(+) }