| Message ID | 201107230012.HED65612.JFVSFOOOMHtFLQ@I-love.SAKURA.ne.jp |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Date: Sat, 23 Jul 2011 00:12:53 +0900 > I think the regression for SMACK can be fixed with below patch. > > Should I pass nosec flags down to "struct security_operations"->sendmsg() > so that SELinux checks sock_has_perm() for only once when multiple different > destination's addresses are passed to sendmmsg()? > > static int selinux_socket_sendmsg(struct socket *sock, struct msghdr *msg, > int size, int nosec) > { > return nosec ? 0 : sock_has_perm(current, sock->sk, SOCKET__WRITE); > } Ugh, this takes away a non-trivial part of the performance gain of sendmmsg(). I would instead rather that you check ahead of time whether this actually is a send to different addresses. If they are all the same, keep the nosec code path. -- 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
22.07.2011 19:22, David Miller wrote: > From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > Date: Sat, 23 Jul 2011 00:12:53 +0900 > >> I think the regression for SMACK can be fixed with below patch. >> >> Should I pass nosec flags down to "struct security_operations"->sendmsg() >> so that SELinux checks sock_has_perm() for only once when multiple different >> destination's addresses are passed to sendmmsg()? >> >> static int selinux_socket_sendmsg(struct socket *sock, struct msghdr *msg, >> int size, int nosec) >> { >> return nosec ? 0 : sock_has_perm(current, sock->sk, SOCKET__WRITE); >> } > > Ugh, this takes away a non-trivial part of the performance gain of > sendmmsg(). > > I would instead rather that you check ahead of time whether this > actually is a send to different addresses. If they are all the > same, keep the nosec code path. Why to optimize for this case when destination addresses are the same? How common this usage case is, or even where it _can_ happen alot (I noticed samba.org address in the Cc list). When I saw recvmmsg()/sendmmsg() here, my first thought was an authoritative DNS server which can read several requests at a time and answer them all at once too - this way it all will go to different addresses. I understand the initial change takes away good portion of performance improvement, but I think the optimisation should be performed in a different place than for a not-so-common cenario. Thanks, /mjt -- 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
Michael Tokarev wrote: > (I noticed samba.org address in the Cc list). That's because Anton Blanchard is author of sendmmsg() system call. > When I saw recvmmsg()/sendmmsg() here, my first thought was an > authoritative DNS server which can read several requests at a > time and answer them all at once too - this way it all will go > to different addresses. I don't know what application wants sendmmsg(). Since users can send up to UIO_MAXIOV (= 1024) "struct iovec" blocks using sendmsg(), they will use sendmsg() rather than sendmmsg() if the destination address are the same. Therefore, I guess users will use sendmmsg() for sending to multiple different destination addresses. If so, optimization based on destination address will do more harm than benefit; simply passing nosec flag down to LSM modules (so that SELinux will skip sock_has_perm() call and SMACK will not skip smack_netlabel_send() call) will be sufficient for 3.0.x stable release. Anton, how do you want to use sendmmsg()? -- 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
Hi, > > (I noticed samba.org address in the Cc list). > > That's because Anton Blanchard is author of sendmmsg() system call. Ignore the From address - I wasn't adding sendmmsg with samba in mind. > > When I saw recvmmsg()/sendmmsg() here, my first thought was an > > authoritative DNS server which can read several requests at a > > time and answer them all at once too - this way it all will go > > to different addresses. > > I don't know what application wants sendmmsg(). Since users can send > up to UIO_MAXIOV (= 1024) "struct iovec" blocks using sendmsg(), they > will use sendmsg() rather than sendmmsg() if the destination address > are the same. But if an application needs to maintain packet boundaries, then sendmsg isn't going to help is it? > Therefore, I guess users will use sendmmsg() for sending to multiple > different destination addresses. If so, optimization based on > destination address will do more harm than benefit; simply passing > nosec flag down to LSM modules (so that SELinux will skip > sock_has_perm() call and SMACK will not skip smack_netlabel_send() > call) will be sufficient for 3.0.x stable release. > > Anton, how do you want to use sendmmsg()? I was using it for packet generation, using raw sockets. Anton -- 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
Anton Blanchard wrote: > > > When I saw recvmmsg()/sendmmsg() here, my first thought was an > > > authoritative DNS server which can read several requests at a > > > time and answer them all at once too - this way it all will go > > > to different addresses. > > > > I don't know what application wants sendmmsg(). Since users can send > > up to UIO_MAXIOV (= 1024) "struct iovec" blocks using sendmsg(), they > > will use sendmsg() rather than sendmmsg() if the destination address > > are the same. > > But if an application needs to maintain packet boundaries, then sendmsg > isn't going to help is it? Well, such application might want to use RDM or SeqPacket... but your point is to maintain packet boundaries. You are assuming that sendmmsg() will be used for sending as much data as possible while preserving packet boundaries. OK. Then, the question is how to reduce performance loss by redundant security_socket_sendmsg() calls. If sendmmsg() likely contains single (or few) destination(s), trying to optimize security_socket_sendmsg() calls by comparing destination address (as proposed at http://www.spinics.net/linux/fedora/linux-security-module/msg11510.html ) would help. Otherwise, no optimization (as proposed at http://www.spinics.net/linux/fedora/linux-security-module/msg11504.html ) would be better. Which approach do you like? -- 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 7/25/2011 6:15 AM, Tetsuo Handa wrote: > Anton Blanchard wrote: >>>> When I saw recvmmsg()/sendmmsg() here, my first thought was an >>>> authoritative DNS server which can read several requests at a >>>> time and answer them all at once too - this way it all will go >>>> to different addresses. >>> I don't know what application wants sendmmsg(). Since users can send >>> up to UIO_MAXIOV (= 1024) "struct iovec" blocks using sendmsg(), they >>> will use sendmsg() rather than sendmmsg() if the destination address >>> are the same. >> But if an application needs to maintain packet boundaries, then sendmsg >> isn't going to help is it? > Well, such application might want to use RDM or SeqPacket... but your point is > to maintain packet boundaries. You are assuming that sendmmsg() will be used > for sending as much data as possible while preserving packet boundaries. > > OK. Then, the question is how to reduce performance loss by redundant > security_socket_sendmsg() calls. Not to be splitting hairs, but if the packets are headed to different destinations the calls to security_socket_sendmsg() are not redundant, they are necessary and appropriate. What you have with sendmmsg() is an optimization that sacrifices correctness for performance. > If sendmmsg() likely contains single (or few) > destination(s), trying to optimize security_socket_sendmsg() calls by comparing > destination address (as proposed at > http://www.spinics.net/linux/fedora/linux-security-module/msg11510.html > ) would help. Otherwise, no optimization (as proposed at > http://www.spinics.net/linux/fedora/linux-security-module/msg11504.html > ) would be better. Which approach do you like? I fear that you are going to find that the work you have to do to reduce the number of calls is going to outweigh the benefits of your optimization, as has been pointed out earlier. My recommendation is that the sendmmsg() interface is ill conceived and that you should look for alternative ways to improve the performance of the use case. -- 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
Casey Schaufler wrote: > > OK. Then, the question is how to reduce performance loss by redundant > > security_socket_sendmsg() calls. > > Not to be splitting hairs, but if the packets are headed to > different destinations the calls to security_socket_sendmsg() > are not redundant, they are necessary and appropriate. What > you have with sendmmsg() is an optimization that sacrifices > correctness for performance. Excuse me, but this thread is not trying to remove necessary and appropriate security_socket_sendmsg() calls. Linux 3.0 was released without necessary and appropriate security_socket_sendmsg() calls, and I'm trying to correct it (via msg11504.html or msg11510.html) for Linux 3.0.x stable release. > I fear that you are going to find that the work you have > to do to reduce the number of calls is going to outweigh > the benefits of your optimization, as has been pointed out > earlier. I fear it too. Unless many dozens (maybe some hundreds) of packets are sent by sendmmsg(), msg11504.html might show better performance than msg11510.html . But I don't have a machine to benchmark. -- 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 7/25/2011 9:43 AM, Tetsuo Handa wrote: > Casey Schaufler wrote: >>> OK. Then, the question is how to reduce performance loss by redundant >>> security_socket_sendmsg() calls. >> Not to be splitting hairs, but if the packets are headed to >> different destinations the calls to security_socket_sendmsg() >> are not redundant, they are necessary and appropriate. What >> you have with sendmmsg() is an optimization that sacrifices >> correctness for performance. > Excuse me, but this thread is not trying to remove necessary and appropriate > security_socket_sendmsg() calls. Linux 3.0 was released without necessary and > appropriate security_socket_sendmsg() calls, and I'm trying to correct it (via > msg11504.html or msg11510.html) for Linux 3.0.x stable release. I understand. Sorry if I did a poor job of jumping into the thread. >> I fear that you are going to find that the work you have >> to do to reduce the number of calls is going to outweigh >> the benefits of your optimization, as has been pointed out >> earlier. > I fear it too. Unless many dozens (maybe some hundreds) of packets are sent by > sendmmsg(), msg11504.html might show better performance than msg11510.html . > But I don't have a machine to benchmark. Is there some chance that the original authors could step up to help with the benchmarking effort on this repair? Having been on the end where I introduced problems more than once, I have a good understanding of the principle "you broke it, you bought it". -- 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
Hi, > I fear it too. Unless many dozens (maybe some hundreds) of packets > are sent by sendmmsg(), msg11504.html might show better performance > than msg11510.html . But I don't have a machine to benchmark. Not sure what happened to your email but the gains are evident at just 2 packets. I can help with testing - the commit included a microbenchmark for the purposes of analysing its performance. Anton -- net: Add sendmmsg socket system call This patch adds a multiple message send syscall and is the send version of the existing recvmmsg syscall. This is heavily based on the patch by Arnaldo that added recvmmsg. I wrote a microbenchmark to test the performance gains of using this new syscall: http://ozlabs.org/~anton/junkcode/sendmmsg_test.c The test was run on a ppc64 box with a 10 Gbit network card. The benchmark can send both UDP and RAW ethernet packets. 64B UDP batch pkts/sec 1 804570 2 872800 (+ 8 %) 4 916556 (+14 %) 8 939712 (+17 %) 16 952688 (+18 %) 32 956448 (+19 %) 64 964800 (+20 %) 64B raw socket batch pkts/sec 1 1201449 2 1350028 (+12 %) 4 1461416 (+22 %) 8 1513080 (+26 %) 16 1541216 (+28 %) 32 1553440 (+29 %) 64 1557888 (+30 %) We see a 20% improvement in throughput on UDP send and 30% on raw socket send. [ Add sparc syscall entries. -DaveM ] -- 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
Anton Blanchard wrote: > Not sure what happened to your email but the gains are evident at > just 2 packets. I know your benchmark result, but that result is based on calling security_socket_sendmsg() only once. What we worry is that overhead by security_socket_sendmsg() kills the performance gain for batched case. > I can help with testing - the commit included a microbenchmark for > the purposes of analysing its performance. Yes please. The patch in msg11510.html would want some more discussion, but the patch in msg11504.html is ready to benchmark on your environment. Does SELinux want to receive nosec flag at selinux_socket_sendmsg() because calling security_socket_sendmsg() more than once is meaningless for SELinux? -- 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 Tue, Jul 26, 2011 at 7:21 AM, Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> wrote: > Does SELinux want to receive nosec flag at selinux_socket_sendmsg() because > calling security_socket_sendmsg() more than once is meaningless for SELinux? SELinux would not mind having a flag such that we could expedite the call on the 2nd or later. Did we finally find the first place where SELinux is going to be the faster LSM! Never thought I'd see the day! -Eric -- 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
--- linux-3.0.orig/net/socket.c +++ linux-3.0/net/socket.c @@ -580,20 +580,6 @@ int sock_sendmsg(struct socket *sock, st } EXPORT_SYMBOL(sock_sendmsg); -int sock_sendmsg_nosec(struct socket *sock, struct msghdr *msg, size_t size) -{ - struct kiocb iocb; - struct sock_iocb siocb; - int ret; - - init_sync_kiocb(&iocb, NULL); - iocb.private = &siocb; - ret = __sock_sendmsg_nosec(&iocb, sock, msg, size); - if (-EIOCBQUEUED == ret) - ret = wait_on_sync_kiocb(&iocb); - return ret; -} - int kernel_sendmsg(struct socket *sock, struct msghdr *msg, struct kvec *vec, size_t num, size_t size) { @@ -1872,7 +1858,7 @@ SYSCALL_DEFINE2(shutdown, int, fd, int, #define COMPAT_FLAGS(msg) COMPAT_MSG(msg, msg_flags) static int __sys_sendmsg(struct socket *sock, struct msghdr __user *msg, - struct msghdr *msg_sys, unsigned flags, int nosec) + struct msghdr *msg_sys, unsigned flags) { struct compat_msghdr __user *msg_compat = (struct compat_msghdr __user *)msg; @@ -1953,8 +1939,7 @@ static int __sys_sendmsg(struct socket * if (sock->file->f_flags & O_NONBLOCK) msg_sys->msg_flags |= MSG_DONTWAIT; - err = (nosec ? sock_sendmsg_nosec : sock_sendmsg)(sock, msg_sys, - total_len); + err = sock_sendmsg(sock, msg_sys, total_len); out_freectl: if (ctl_buf != ctl) @@ -1979,7 +1964,7 @@ SYSCALL_DEFINE3(sendmsg, int, fd, struct if (!sock) goto out; - err = __sys_sendmsg(sock, msg, &msg_sys, flags, 0); + err = __sys_sendmsg(sock, msg, &msg_sys, flags); fput_light(sock->file, fput_needed); out: @@ -2014,18 +1999,19 @@ int __sys_sendmmsg(int fd, struct mmsghd while (datagrams < vlen) { /* - * No need to ask LSM for more than the first datagram. + * Need to ask LSM every time in case LSM might check + * destination's address. */ if (MSG_CMSG_COMPAT & flags) { err = __sys_sendmsg(sock, (struct msghdr __user *)compat_entry, - &msg_sys, flags, datagrams); + &msg_sys, flags); if (err < 0) break; err = __put_user(err, &compat_entry->msg_len); ++compat_entry; } else { err = __sys_sendmsg(sock, (struct msghdr __user *)entry, - &msg_sys, flags, datagrams); + &msg_sys, flags); if (err < 0) break; err = put_user(err, &entry->msg_len);
I think the regression for SMACK can be fixed with below patch. Should I pass nosec flags down to "struct security_operations"->sendmsg() so that SELinux checks sock_has_perm() for only once when multiple different destination's addresses are passed to sendmmsg()? static int selinux_socket_sendmsg(struct socket *sock, struct msghdr *msg, int size, int nosec) { return nosec ? 0 : sock_has_perm(current, sock->sk, SOCKET__WRITE); } ---------------------------------------- [PATCH] net: Fix security_socket_sendmsg() bypass problem. The sendmmsg() introduced by commit 228e548e "net: Add sendmmsg socket system call" is capable of sending to multiple different destinations. However, security_socket_sendmsg() is called for only once even if multiple different destination's addresses are passed to sendmmsg(). SMACK is using destination's address for checking sendmsg() permission. Therefore, we need to call security_socket_sendmsg() for each destination address rather than only the first destination address. Fix this problem by removing nosec flags. Also, remove sock_sendmsg_nosec() because it is no longer used. Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Cc: stable <stable@kernel.org> [3.0+] --- net/socket.c | 28 +++++++--------------------- 1 file changed, 7 insertions(+), 21 deletions(-) -- 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