Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH v5 00/15] some netpoll and netconsole fixes
From: David Miller @ 2012-08-14  4:01 UTC (permalink / raw)
  To: amwang; +Cc: netdev
In-Reply-To: <1344851127.22116.5.camel@cr0>

From: Cong Wang <amwang@redhat.com>
Date: Mon, 13 Aug 2012 17:45:27 +0800

> Does this patchset look good to you now?

I'm backlogged and haven't had a chance to look at it yet.

^ permalink raw reply

* [PATCH] TCP/IP stack bypass for loopback connections fix
From: Weiping Pan @ 2012-08-14  3:12 UTC (permalink / raw)
  To: netdev; +Cc: brutus, Weiping Pan
In-Reply-To: <1344559958-29162-1-git-send-email-brutus@google.com>

I found two problems then I tested Bruce Curtis <brutus@google.com> loopback
patch, http://patchwork.ozlabs.org/patch/176304/

1 ‘friends’ undeclared in function ‘tcp_recvmsg’
I think it should use sysctl_tcp_friends

2 scheduling while atomic

BUG: scheduling while atomic: netperf/3820/0x10000200
Modules linked in: bridge stp llc autofs4 sunrpc ipv6 uinput
iTCO_wdt iTCO_vendor_suppo]
Pid: 3820, comm: netperf Tainted: G        W    3.5.0+ #11
Call Trace:
[<ffffffff810861e2>] __schedule_bug+0x52/0x60
[<ffffffff8151e7de>] __schedule+0x68e/0x710
[<ffffffff8108a80a>] __cond_resched+0x2a/0x40
[<ffffffff8151e8f0>] _cond_resched+0x30/0x40
[<ffffffff8149b68e>] tcp_sendmsg+0x96e/0x12c0
[<ffffffff814bf928>] inet_sendmsg+0x48/0xb0
[<ffffffff812081a3>] ? selinux_socket_sendmsg+0x23/0x30
[<ffffffff8143c4be>] sock_sendmsg+0xbe/0xf0
[<ffffffff81078958>] ? finish_wait+0x68/0x90
[<ffffffff81172660>] ? fget_light+0x50/0xc0
[<ffffffff8143c629>] sys_sendto+0x139/0x190
[<ffffffff810d21cc>] ?  __audit_syscall_entry+0xcc/0x210
[<ffffffff810d20a6>] ?  __audit_syscall_exit+0x3d6/0x430
[<ffffffff81527c29>] system_call_fastpath+0x16/0x1b

The reason is that in tcp_friend_tail(), it holds a spinlock,
spin_lock_bh(&friend->sk_lock.slock);
but skb_add_data_nocache() can cause schedule, so this warning happends.

Wit this patch, the original patch can apply on top of net-next tree
commit 79cda75a107da(fib: use __fls() on non null argument),
and I test it works fine on x86_64 system.

Signed-off-by: Weiping Pan <wpan@redhat.com>
---
 net/ipv4/tcp.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 6dc267c..bab15da 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -628,7 +628,7 @@ static inline struct sk_buff *tcp_friend_tail(struct sock *sk, int *copy)
 	int		sz = 0;
 
 	if (skb_peek_tail(&friend->sk_receive_queue)) {
-		spin_lock_bh(&friend->sk_lock.slock);
+		lock_sock(friend);
 		skb = skb_peek_tail(&friend->sk_receive_queue);
 		if (skb && skb->friend) {
 			if (!*copy)
@@ -637,7 +637,7 @@ static inline struct sk_buff *tcp_friend_tail(struct sock *sk, int *copy)
 				sz = *copy - skb->len;
 		}
 		if (!skb || sz <= 0)
-			spin_unlock_bh(&friend->sk_lock.slock);
+			release_sock(friend);
 	}
 
 	*copy = sz;
@@ -655,7 +655,7 @@ static inline void tcp_friend_seq(struct sock *sk, int copy, int charge)
 	}
 	tp->rcv_nxt += copy;
 	tp->rcv_wup += copy;
-	spin_unlock_bh(&friend->sk_lock.slock);
+	release_sock(friend);
 
 	friend->sk_data_ready(friend, copy);
 
@@ -676,7 +676,7 @@ static inline int tcp_friend_push(struct sock *sk, struct sk_buff *skb)
 		return -ECONNRESET;
 	}
 
-	spin_lock_bh(&friend->sk_lock.slock);
+	lock_sock(friend);
 	skb->friend = sk;
 	skb_set_owner_r(skb, friend);
 	__skb_queue_tail(&friend->sk_receive_queue, skb);
@@ -1505,7 +1505,7 @@ out:
 
 do_fault:
 	if (friend_tail)
-		spin_unlock_bh(&friend->sk_lock.slock);
+		release_sock(friend);
 	else if (!skb->len) {
 		if (friend)
 			__kfree_skb(skb);
@@ -1932,7 +1932,7 @@ int tcp_recvmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
 			available = TCP_SKB_CB(skb)->seq + skb->len - (*seq);
 		if ((available < target) &&
 		    (len > sysctl_tcp_dma_copybreak) && !(flags & MSG_PEEK) &&
-		    !sysctl_tcp_low_latency && !friends &&
+		    !sysctl_tcp_low_latency && !sysctl_tcp_friends &&
 		    net_dma_find_channel()) {
 			preempt_enable_no_resched();
 			tp->ucopy.pinned_list =
-- 
1.7.1

^ permalink raw reply related

* Re: [net PATCH v1 2/2] net: netprio: fd passed in SCM_RIGHTS datagram not set correctly
From: John Fastabend @ 2012-08-14  3:00 UTC (permalink / raw)
  To: Al Viro; +Cc: nhorman, netdev, davem, linux-kernel, joe
In-Reply-To: <20120814023128.GG23464@ZenIV.linux.org.uk>

On 8/13/2012 7:31 PM, Al Viro wrote:
> On Mon, Aug 13, 2012 at 06:39:45PM -0700, John Fastabend wrote:
>>   		/* Bump the usage count and install the file. */
>>   		get_file(fp[i]);
>>   		fd_install(new_fd, fp[i]);
>> +		sock = sock_from_file(fp[i], &err);
>> +		if (sock)
>> +			sock->sk->sk_cgrp_prioidx = prioidx;
>
> Better to do that before fd_install() exposes the sucker; it's not
> a serious issue, but it's more obviously correct that way.
>

Right. Moved it in v2.

^ permalink raw reply

* Re: [net PATCH v1 1/2] net: netprio: fix files lock and remove useless d_path bits
From: John Fastabend @ 2012-08-14  2:59 UTC (permalink / raw)
  To: Al Viro; +Cc: nhorman, netdev, davem, linux-kernel, joe
In-Reply-To: <20120814022951.GF23464@ZenIV.linux.org.uk>

On 8/13/2012 7:29 PM, Al Viro wrote:
> On Mon, Aug 13, 2012 at 06:39:39PM -0700, John Fastabend wrote:
>> -			path = d_path(&file->f_path, tmp, PAGE_SIZE);
>
> What's using tmp after that patch and do you still need to bother
> allocating it?
>

nothing removed it in v2.

^ permalink raw reply

* [net PATCH v2 2/2] net: netprio: fd passed in SCM_RIGHTS datagram not set correctly
From: John Fastabend @ 2012-08-14  2:43 UTC (permalink / raw)
  To: viro, nhorman; +Cc: netdev, davem, linux-kernel, joe
In-Reply-To: <20120814024320.6983.92222.stgit@jf-dev1-dcblab>

A socket fd passed in a SCM_RIGHTS datagram was not getting
updated with the new tasks cgrp prioidx. This leaves IO on
the socket tagged with the old tasks priority.

To fix this add a check in the scm recvmsg path to update the
sock cgrp prioidx with the new tasks value.

Thanks to Al Viro for catching this.

CC: Neil Horman <nhorman@tuxdriver.com>
Reported-by: Al Viro <viro@ZenIV.linux.org.uk>
Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
---

 net/core/scm.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/net/core/scm.c b/net/core/scm.c
index 8f6ccfd..a14d9e2 100644
--- a/net/core/scm.c
+++ b/net/core/scm.c
@@ -249,6 +249,7 @@ void scm_detach_fds(struct msghdr *msg, struct scm_cookie *scm)
 	struct file **fp = scm->fp->fp;
 	int __user *cmfptr;
 	int err = 0, i;
+	__u32 prioidx = task_netprioidx(current);
 
 	if (MSG_CMSG_COMPAT & msg->msg_flags) {
 		scm_detach_fds_compat(msg, scm);
@@ -265,6 +266,7 @@ void scm_detach_fds(struct msghdr *msg, struct scm_cookie *scm)
 	for (i=0, cmfptr=(__force int __user *)CMSG_DATA(cm); i<fdmax;
 	     i++, cmfptr++)
 	{
+		struct socket *sock;
 		int new_fd;
 		err = security_file_receive(fp[i]);
 		if (err)
@@ -281,6 +283,9 @@ void scm_detach_fds(struct msghdr *msg, struct scm_cookie *scm)
 		}
 		/* Bump the usage count and install the file. */
 		get_file(fp[i]);
+		sock = sock_from_file(fp[i], &err);
+		if (sock)
+			sock->sk->sk_cgrp_prioidx = prioidx;
 		fd_install(new_fd, fp[i]);
 	}
 

^ permalink raw reply related

* [net PATCH v2 1/2] net: netprio: fix files lock and remove useless d_path bits
From: John Fastabend @ 2012-08-14  2:43 UTC (permalink / raw)
  To: viro, nhorman; +Cc: netdev, davem, linux-kernel, joe

Add lock to prevent a race with a file closing and also remove
useless and ugly sscanf code. The extra code was never needed
and the case it supposedly protected against is in fact handled
correctly by sock_from_file as pointed out by Al Viro.

CC: Neil Horman <nhorman@tuxdriver.com>
Reported-by: Al Viro <viro@ZenIV.linux.org.uk>
Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
---

 net/core/netprio_cgroup.c |   22 ++++------------------
 1 files changed, 4 insertions(+), 18 deletions(-)

diff --git a/net/core/netprio_cgroup.c b/net/core/netprio_cgroup.c
index ed0c043..f65dba3 100644
--- a/net/core/netprio_cgroup.c
+++ b/net/core/netprio_cgroup.c
@@ -277,12 +277,6 @@ out_free_devname:
 void net_prio_attach(struct cgroup *cgrp, struct cgroup_taskset *tset)
 {
 	struct task_struct *p;
-	char *tmp = kzalloc(sizeof(char) * PATH_MAX, GFP_KERNEL);
-
-	if (!tmp) {
-		pr_warn("Unable to attach cgrp due to alloc failure!\n");
-		return;
-	}
 
 	cgroup_taskset_for_each(p, cgrp, tset) {
 		unsigned int fd;
@@ -296,32 +290,24 @@ void net_prio_attach(struct cgroup *cgrp, struct cgroup_taskset *tset)
 			continue;
 		}
 
-		rcu_read_lock();
+		spin_lock(&files->file_lock);
 		fdt = files_fdtable(files);
 		for (fd = 0; fd < fdt->max_fds; fd++) {
-			char *path;
 			struct file *file;
 			struct socket *sock;
-			unsigned long s;
-			int rv, err = 0;
+			int err;
 
 			file = fcheck_files(files, fd);
 			if (!file)
 				continue;
 
-			path = d_path(&file->f_path, tmp, PAGE_SIZE);
-			rv = sscanf(path, "socket:[%lu]", &s);
-			if (rv <= 0)
-				continue;
-
 			sock = sock_from_file(file, &err);
-			if (!err)
+			if (sock)
 				sock_update_netprioidx(sock->sk, p);
 		}
-		rcu_read_unlock();
+		spin_unlock(&files->file_lock);
 		task_unlock(p);
 	}
-	kfree(tmp);
 }
 
 static struct cftype ss_files[] = {

^ permalink raw reply related

* Re: [net PATCH v1 2/2] net: netprio: fd passed in SCM_RIGHTS datagram not set correctly
From: Al Viro @ 2012-08-14  2:31 UTC (permalink / raw)
  To: John Fastabend; +Cc: nhorman, netdev, davem, linux-kernel, joe
In-Reply-To: <20120814013944.3144.53854.stgit@jf-dev1-dcblab>

On Mon, Aug 13, 2012 at 06:39:45PM -0700, John Fastabend wrote:
>  		/* Bump the usage count and install the file. */
>  		get_file(fp[i]);
>  		fd_install(new_fd, fp[i]);
> +		sock = sock_from_file(fp[i], &err);
> +		if (sock)
> +			sock->sk->sk_cgrp_prioidx = prioidx;

Better to do that before fd_install() exposes the sucker; it's not
a serious issue, but it's more obviously correct that way.

^ permalink raw reply

* Re: [net PATCH v1 1/2] net: netprio: fix files lock and remove useless d_path bits
From: Al Viro @ 2012-08-14  2:29 UTC (permalink / raw)
  To: John Fastabend; +Cc: nhorman, netdev, davem, linux-kernel, joe
In-Reply-To: <20120814013939.3144.13167.stgit@jf-dev1-dcblab>

On Mon, Aug 13, 2012 at 06:39:39PM -0700, John Fastabend wrote:
> -			path = d_path(&file->f_path, tmp, PAGE_SIZE);

What's using tmp after that patch and do you still need to bother
allocating it?

^ permalink raw reply

* [net PATCH v1 2/2] net: netprio: fd passed in SCM_RIGHTS datagram not set correctly
From: John Fastabend @ 2012-08-14  1:39 UTC (permalink / raw)
  To: viro, nhorman; +Cc: netdev, davem, linux-kernel, joe
In-Reply-To: <20120814013939.3144.13167.stgit@jf-dev1-dcblab>

A socket fd passed in a SCM_RIGHTS datagram was not getting
updated with the new tasks cgrp prioidx. This leaves IO on
the socket tagged with the old tasks priority.

To fix this add a check in the scm recvmsg path to update the
sock cgrp prioidx with the new tasks value.

Thanks to Al Viro for catching this.

CC: Neil Horman <nhorman@tuxdriver.com>
Reported-by: Al Viro <viro@ZenIV.linux.org.uk>
Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
---

 net/core/scm.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/net/core/scm.c b/net/core/scm.c
index 8f6ccfd..053bd43 100644
--- a/net/core/scm.c
+++ b/net/core/scm.c
@@ -249,6 +249,7 @@ void scm_detach_fds(struct msghdr *msg, struct scm_cookie *scm)
 	struct file **fp = scm->fp->fp;
 	int __user *cmfptr;
 	int err = 0, i;
+	__u32 prioidx = task_netprioidx(current);
 
 	if (MSG_CMSG_COMPAT & msg->msg_flags) {
 		scm_detach_fds_compat(msg, scm);
@@ -265,6 +266,7 @@ void scm_detach_fds(struct msghdr *msg, struct scm_cookie *scm)
 	for (i=0, cmfptr=(__force int __user *)CMSG_DATA(cm); i<fdmax;
 	     i++, cmfptr++)
 	{
+		struct socket *sock;
 		int new_fd;
 		err = security_file_receive(fp[i]);
 		if (err)
@@ -282,6 +284,9 @@ void scm_detach_fds(struct msghdr *msg, struct scm_cookie *scm)
 		/* Bump the usage count and install the file. */
 		get_file(fp[i]);
 		fd_install(new_fd, fp[i]);
+		sock = sock_from_file(fp[i], &err);
+		if (sock)
+			sock->sk->sk_cgrp_prioidx = prioidx;
 	}
 
 	if (i > 0)

^ permalink raw reply related

* [net PATCH v1 1/2] net: netprio: fix files lock and remove useless d_path bits
From: John Fastabend @ 2012-08-14  1:39 UTC (permalink / raw)
  To: viro, nhorman; +Cc: netdev, davem, linux-kernel, joe

Add lock to prevent a race with a file closing and also remove
useless and ugly sscanf code. The extra code was never needed
and the case it supposedly protected against is in fact handled
correctly by sock_from_file as pointed out by Al Viro.

CC: Neil Horman <nhorman@tuxdriver.com>
Reported-by: Al Viro <viro@ZenIV.linux.org.uk>
Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
---

 net/core/netprio_cgroup.c |   15 ++++-----------
 1 files changed, 4 insertions(+), 11 deletions(-)

diff --git a/net/core/netprio_cgroup.c b/net/core/netprio_cgroup.c
index ed0c043..0323ec3 100644
--- a/net/core/netprio_cgroup.c
+++ b/net/core/netprio_cgroup.c
@@ -296,29 +296,22 @@ void net_prio_attach(struct cgroup *cgrp, struct cgroup_taskset *tset)
 			continue;
 		}
 
-		rcu_read_lock();
+		spin_lock(&files->file_lock);
 		fdt = files_fdtable(files);
 		for (fd = 0; fd < fdt->max_fds; fd++) {
-			char *path;
 			struct file *file;
 			struct socket *sock;
-			unsigned long s;
-			int rv, err = 0;
+			int err;
 
 			file = fcheck_files(files, fd);
 			if (!file)
 				continue;
 
-			path = d_path(&file->f_path, tmp, PAGE_SIZE);
-			rv = sscanf(path, "socket:[%lu]", &s);
-			if (rv <= 0)
-				continue;
-
 			sock = sock_from_file(file, &err);
-			if (!err)
+			if (sock)
 				sock_update_netprioidx(sock->sk, p);
 		}
-		rcu_read_unlock();
+		spin_unlock(&files->file_lock);
 		task_unlock(p);
 	}
 	kfree(tmp);

^ permalink raw reply related

* Re: [RFC net-next 0/4] gianfar: Use separate NAPI for Tx confirmation processing
From: Paul Gortmaker @ 2012-08-14  1:15 UTC (permalink / raw)
  To: Claudiu Manoil; +Cc: Tomas Hruby, Eric Dumazet, netdev, David S. Miller
In-Reply-To: <502929ED.2050703@freescale.com>

[Re: [RFC net-next 0/4] gianfar: Use separate NAPI for Tx confirmation processing] On 13/08/2012 (Mon 19:23) Claudiu Manoil wrote:

> On 08/09/2012 06:07 PM, Claudiu Manoil wrote:
> >On 8/9/2012 2:06 AM, Tomas Hruby wrote:
> >>On Wed, Aug 8, 2012 at 9:44 AM, Eric Dumazet
> >><eric.dumazet@gmail.com> wrote:
> >>>On Wed, 2012-08-08 at 12:24 -0400, Paul Gortmaker wrote:
> >>>>[[RFC net-next 0/4] gianfar: Use separate NAPI for Tx
> >>>>confirmation processing] On 08/08/2012 (Wed 15:26) Claudiu
> >>>>Manoil wrote:
> >>>>
> >>>>>Hi all,
> >>>>>This set of patches basically splits the existing napi
> >>>>>poll routine into
> >>>>>two separate napi functions, one for Rx processing
> >>>>>(triggered by frame
> >>>>>receive interrupts only) and one for the Tx confirmation
> >>>>>path processing
> >>>>>(triggerred by Tx confirmation interrupts only). The
> >>>>>polling algorithm
> >>>>>behind remains much the same.
> >>>>>
> >>>>>Important throughput improvements have been noted on low
> >>>>>power boards with
> >>>>>this set of changes.
> >>>>>For instance, for the following netperf test:
> >>>>>netperf -l 20 -cC -H 192.168.10.1 -t TCP_STREAM -- -m 1500
> >>>>>yields a throughput gain from oscilating ~500-~700 Mbps to
> >>>>>steady ~940 Mbps,
> >>>>>(if the Rx/Tx paths are processed on different cores), w/
> >>>>>no increase in CPU%,
> >>>>>on a p1020rdb - 2 core machine featuring etsec2.0
> >>>>>(Multi-Queue Multi-Group
> >>>>>driver mode).
> >>>>
> >>>>It would be interesting to know more about what was causing that large
> >>>>an oscillation -- presumably you will have it reappear once one core
> >>>>becomes 100% utilized.  Also, any thoughts on how the change
> >>>>will change
> >>>>performance on an older low power single core gianfar system
> >>>>(e.g.  83xx)?
> >>>
> >>>I also was wondering if this low performance could be caused by BQL
> >>>
> >>>Since TCP stack is driven by incoming ACKS, a NAPI run could have to
> >>>handle 10 TCP acks in a row, and resulting xmits could hit BQL and
> >>>transit on qdisc (Because NAPI handler wont handle TX
> >>>completions in the
> >>>middle of RX handler)
> >>
> >>Does disabling BQL help? Is the BQL limit stable? To what value is it
> >>set? I would be very much interested in more data if the issue is BQL
> >>related.
> >>
> >>.
> >>
> >
> >I agree that more tests should be run to investigate why gianfar under-
> >performs on the low power p1020rdb platform, and BQL seems to be
> >a good starting point (thanks for the hint). What I can say now is that
> >the issue is not apparent on p2020rdb, for instance, which is a more
> >powerful platform: the CPUs - 1200 MHz instead of 800 MHz; twice the
> >size of L2 cache (512 KB), greater bus (CCB) frequency ... On this
> >board (p2020rdb) the netperf test reaches 940Mbps both w/ and w/o these
> >patches.
> >
> >For a single core system I'm not expecting any performance degradation,
> >simply because I don't see why the proposed napi poll implementation
> >would be slower than the existing one. I'll do some measurements on a
> >p1010rdb too (single core, CPU:800 MHz) and get back to you with the
> >results.
> >
> 
> Hi all,
> 
> Please find below the netperf measurements performed on a p1010rdb machine
> (single core, low power).  Three kernel images were used:
> 1) Linux version 3.5.0-20970-gaae06bf  -- net-next commit aae06bf
> 2) Linux version 3.5.0-20974-g2920464 -- commit aae06bf + Tx NAPI patches
> 3) Linux version 3.5.0-20970-gaae06bf-dirty -- commit aae06bf +
> CONFIG_BQL set to 'n'

For future reference, you don't need to dirty the tree to disable
CONFIG_BQL at compile time; there is a runtime disable:

http://permalink.gmane.org/gmane.linux.network/223107

> 
> The results show that, on *Image 1)*, by adjusting
> tcp_limit_output_bytes no substantial
> improvements are seen, as the throughput stays in the 580-60x Mbps range .

This is a lot lower variation than what you reported earlier (20 versus
200, I think).  It was the variation that raised a red flag for me...

> By changing the coalescing settings from default* (rx coalescing off,
> tx-usecs: 10, tx-frames: 16) to:
> "ethtool -C eth1 rx-frames 22 tx-frames 22 rx-usecs 32 tx-usecs 32"
> we get a throughput of ~710 Mbps.
> 
> For *Image 2)*, using the default tcp_limit_output_bytes value
> (131072) - I've noticed
> that "tweaking" tcp_limit_output_bytes does not improve the
> throughput -, we get the
> following performance numbers:
> * default coalescing settings: ~650 Mbps
> * rx-frames tx-frames 22 rx-usecs 32 tx-usecs 32: ~860-880 Mbps
> 
> For *Image 3)*, by disabling BQL (CONFIG_BQL = n), there's *no*
> relevant performance
> improvement compared to Image 1).
> (note:
> For all the measurements, rx and tx BD ring sizes have been set to
> 64, for best performance.)
> 
> So, I really tend to believe that the performance degradation comes
> primarily from the driver,
> and the napi poll processing turns out to be an important source for
> that. The proposed patches

This would make sense, if the CPU was slammed at 100% load in dealing
with the tx processing, and the change made the driver considerably more
efficient.  But is that really the case?  Is the p1010 really going flat
out just to handle the Tx processing?  Have you done any sort of
profiling to confirm/deny where the CPU is spending its time?

> show substantial improvement, especially for SMP systems where Tx
> and Rx processing may be
> done in parallel.
> What do you think?
> Is it ok to proceed by re-spinning the patches? Do you recommend
> additional measurements?

Unfortunately Eric is out this week, so we will be without his input for
a while.  However, we are only at 3.6-rc1 -- meaning net-next will be
open for quite some time, hence no need to rush to try and jam stuff in.

Also, I have two targets I'm interested in testing your patches on.  The
1st is a 500MHz mpc8349 board -- which should replicate what you see on
your p1010 (slow, single core).  The other is an 8641D, which is
interesting since it will give us the SMP tx/rx as separate threads, but
without the MQ_MG_MODE support (is that a correct assumption?)

I don't have any fundamental problem with your patches (although 4/4
might be better as two patches) -- the above targets/tests are only
of interest, since I'm not convinced we yet understand _why_ your
changes give a performance boost, and there might be something
interesting hiding in there.

So, while Eric is out, let me see if I can collect some more data on
those two targets sometime this week.

Thanks,
Paul.
--

> 
> Regards,
> Claudiu
> 
> //=Image 1)================
> root@p1010rdb:~# cat /proc/version
> Linux version 3.5.0-20970-gaae06bf [...]
> 
> root@p1010rdb:~# zcat /proc/config.gz | grep BQL
> CONFIG_BQL=y
> root@p1010rdb:~# cat /proc/sys/net/ipv4/tcp_limit_output_bytes
> 131072
> 
> root@p1010rdb:~# netperf -l 20 -cC -H 192.168.10.1 -t TCP_STREAM -- -m 1500
> TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to
> 192.168.10.1 (192.168.10.1) port 0 AF_INET
> Recv   Send    Send                          Utilization Service Demand
> Socket Socket  Message  Elapsed              Send     Recv Send    Recv
> Size   Size    Size     Time     Throughput  local    remote local   remote
> bytes  bytes   bytes    secs.    10^6bits/s  % S      % S us/KB   us/KB
> 
>  87380  16384   1500    20.00       580.76   99.95    11.76 14.099  1.659
> root@p1010rdb:~# netperf -l 20 -cC -H 192.168.10.1 -t TCP_STREAM -- -m 1500
> TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to
> 192.168.10.1 (192.168.10.1) port 0 AF_INET
> Recv   Send    Send                          Utilization Service Demand
> Socket Socket  Message  Elapsed              Send     Recv Send    Recv
> Size   Size    Size     Time     Throughput  local    remote local   remote
> bytes  bytes   bytes    secs.    10^6bits/s  % S      % S us/KB   us/KB
> 
>  87380  16384   1500    20.00       598.21   99.95    10.91 13.687  1.493
> root@p1010rdb:~# netperf -l 20 -cC -H 192.168.10.1 -t TCP_STREAM -- -m 1500
> TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to
> 192.168.10.1 (192.168.10.1) port 0 AF_INET
> Recv   Send    Send                          Utilization Service Demand
> Socket Socket  Message  Elapsed              Send     Recv Send    Recv
> Size   Size    Size     Time     Throughput  local    remote local   remote
> bytes  bytes   bytes    secs.    10^6bits/s  % S      % S us/KB   us/KB
> 
>  87380  16384   1500    20.00       583.04   99.95    11.25 14.043  1.581
> 
> 
> root@p1010rdb:~# cat /proc/sys/net/ipv4/tcp_limit_output_bytes
> 65536
> 
> root@p1010rdb:~# netperf -l 20 -cC -H 192.168.10.1 -t TCP_STREAM -- -m 1500
> TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to
> 192.168.10.1 (192.168.10.1) port 0 AF_INET
> Recv   Send    Send                          Utilization Service Demand
> Socket Socket  Message  Elapsed              Send     Recv Send    Recv
> Size   Size    Size     Time     Throughput  local    remote local   remote
> bytes  bytes   bytes    secs.    10^6bits/s  % S      % S us/KB   us/KB
> 
>  87380  16384   1500    20.00       604.29   99.95    11.15 13.550  1.512
> root@p1010rdb:~# netperf -l 20 -cC -H 192.168.10.1 -t TCP_STREAM -- -m 1500
> TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to
> 192.168.10.1 (192.168.10.1) port 0 AF_INET
> Recv   Send    Send                          Utilization Service Demand
> Socket Socket  Message  Elapsed              Send     Recv Send    Recv
> Size   Size    Size     Time     Throughput  local    remote local   remote
> bytes  bytes   bytes    secs.    10^6bits/s  % S      % S us/KB   us/KB
> 
>  87380  16384   1500    20.00       603.52   99.50    12.57 13.506  1.706
> root@p1010rdb:~# netperf -l 20 -cC -H 192.168.10.1 -t TCP_STREAM -- -m 1500
> TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to
> 192.168.10.1 (192.168.10.1) port 0 AF_INET
> Recv   Send    Send                          Utilization Service Demand
> Socket Socket  Message  Elapsed              Send     Recv Send    Recv
> Size   Size    Size     Time     Throughput  local    remote local   remote
> bytes  bytes   bytes    secs.    10^6bits/s  % S      % S us/KB   us/KB
> 
>  87380  16384   1500    20.00       596.18   99.95    12.81 13.734  1.760
> 
> 
> 
> root@p1010rdb:~# cat /proc/sys/net/ipv4/tcp_limit_output_bytes
> 32768
> 
> root@p1010rdb:~# netperf -l 20 -cC -H 192.168.10.1 -t TCP_STREAM -- -m 1500
> TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to
> 192.168.10.1 (192.168.10.1) port 0 AF_INET
> Recv   Send    Send                          Utilization Service Demand
> Socket Socket  Message  Elapsed              Send     Recv Send    Recv
> Size   Size    Size     Time     Throughput  local    remote local   remote
> bytes  bytes   bytes    secs.    10^6bits/s  % S      % S us/KB   us/KB
> 
>  87380  16384   1500    20.00       582.32   99.95    12.96 14.061  1.824
> root@p1010rdb:~# netperf -l 20 -cC -H 192.168.10.1 -t TCP_STREAM -- -m 1500
> TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to
> 192.168.10.1 (192.168.10.1) port 0 AF_INET
> Recv   Send    Send                          Utilization Service Demand
> Socket Socket  Message  Elapsed              Send     Recv Send    Recv
> Size   Size    Size     Time     Throughput  local    remote local   remote
> bytes  bytes   bytes    secs.    10^6bits/s  % S      % S us/KB   us/KB
> 
>  87380  16384   1500    20.00       583.79   99.95    11.19 14.026  1.571
> root@p1010rdb:~# netperf -l 20 -cC -H 192.168.10.1 -t TCP_STREAM -- -m 1500
> TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to
> 192.168.10.1 (192.168.10.1) port 0 AF_INET
> Recv   Send    Send                          Utilization Service Demand
> Socket Socket  Message  Elapsed              Send     Recv Send    Recv
> Size   Size    Size     Time     Throughput  local    remote local   remote
> bytes  bytes   bytes    secs.    10^6bits/s  % S      % S us/KB   us/KB
> 
>  87380  16384   1500    20.00       584.16   99.95    11.36 14.016  1.592
> 
> 
> 
> root@p1010rdb:~# ethtool -C eth1 rx-frames 22 tx-frames 22 rx-usecs
> 32 tx-usecs 32
> 
> root@p1010rdb:~# netperf -l 20 -cC -H 192.168.10.1 -t TCP_STREAM -- -m 1500
> TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to
> 192.168.10.1 (192.168.10.1) port 0 AF_INET
> Recv   Send    Send                          Utilization Service Demand
> Socket Socket  Message  Elapsed              Send     Recv Send    Recv
> Size   Size    Size     Time     Throughput  local    remote local   remote
> bytes  bytes   bytes    secs.    10^6bits/s  % S      % S us/KB   us/KB
> 
>  87380  16384   1500    20.00       708.77   99.85    13.32 11.541  1.540
> root@p1010rdb:~# netperf -l 20 -cC -H 192.168.10.1 -t TCP_STREAM -- -m 1500
> TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to
> 192.168.10.1 (192.168.10.1) port 0 AF_INET
> Recv   Send    Send                          Utilization Service Demand
> Socket Socket  Message  Elapsed              Send     Recv Send    Recv
> Size   Size    Size     Time     Throughput  local    remote local   remote
> bytes  bytes   bytes    secs.    10^6bits/s  % S      % S us/KB   us/KB
> 
>  87380  16384   1500    20.00       710.50   99.95    12.46 11.524  1.437
> root@p1010rdb:~# netperf -l 20 -cC -H 192.168.10.1 -t TCP_STREAM -- -m 1500
> TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to
> 192.168.10.1 (192.168.10.1) port 0 AF_INET
> Recv   Send    Send                          Utilization Service Demand
> Socket Socket  Message  Elapsed              Send     Recv Send    Recv
> Size   Size    Size     Time     Throughput  local    remote local   remote
> bytes  bytes   bytes    secs.    10^6bits/s  % S      % S us/KB   us/KB
> 
>  87380  16384   1500    20.00       709.95   99.95    14.15 11.533  1.633
> 
> 
> //=Image 2)================
> 
> root@p1010rdb:~# cat /proc/version
> Linux version 3.5.0-20974-g2920464 [...]
> 
> root@p1010rdb:~# netperf -l 20 -cC -H 192.168.10.1 -t TCP_STREAM -- -m 1500
> TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to
> 192.168.10.1 (192.168.10.1) port 0 AF_INET
> Recv   Send    Send                          Utilization Service Demand
> Socket Socket  Message  Elapsed              Send     Recv Send    Recv
> Size   Size    Size     Time     Throughput  local    remote local   remote
> bytes  bytes   bytes    secs.    10^6bits/s  % S      % S us/KB   us/KB
> 
>  87380  16384   1500    20.00       652.60   99.95    13.05 12.547  1.638
> root@p1010rdb:~# netperf -l 20 -cC -H 192.168.10.1 -t TCP_STREAM -- -m 1500
> TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to
> 192.168.10.1 (192.168.10.1) port 0 AF_INET
> Recv   Send    Send                          Utilization Service Demand
> Socket Socket  Message  Elapsed              Send     Recv Send    Recv
> Size   Size    Size     Time     Throughput  local    remote local   remote
> bytes  bytes   bytes    secs.    10^6bits/s  % S      % S us/KB   us/KB
> 
>  87380  16384   1500    20.00       657.47   99.95    11.81 12.454  1.471
> root@p1010rdb:~# netperf -l 20 -cC -H 192.168.10.1 -t TCP_STREAM -- -m 1500
> TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to
> 192.168.10.1 (192.168.10.1) port 0 AF_INET
> Recv   Send    Send                          Utilization Service Demand
> Socket Socket  Message  Elapsed              Send     Recv Send    Recv
> Size   Size    Size     Time     Throughput  local    remote local   remote
> bytes  bytes   bytes    secs.    10^6bits/s  % S      % S us/KB   us/KB
> 
>  87380  16384   1500    20.00       655.77   99.95    11.80 12.486  1.474
> 
> 
> root@p1010rdb:~# ethtool -C eth1 rx-frames 22 rx-usecs 32 tx-frames
> 22 tx-usecs 32
> 
> root@p1010rdb:~# cat /proc/sys/net/ipv4/tcp_limit_output_bytes
> 131072
> root@p1010rdb:~# netperf -l 20 -cC -H 192.168.10.1 -t TCP_STREAM -- -m 1500
> TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to
> 192.168.10.1 (192.168.10.1) port 0 AF_INET
> Recv   Send    Send                          Utilization Service Demand
> Socket Socket  Message  Elapsed              Send     Recv Send    Recv
> Size   Size    Size     Time     Throughput  local    remote local   remote
> bytes  bytes   bytes    secs.    10^6bits/s  % S      % S us/KB   us/KB
> 
>  87380  16384   1500    20.01       882.42   99.20    18.06 9.209   1.676
> root@p1010rdb:~# netperf -l 20 -cC -H 192.168.10.1 -t TCP_STREAM -- -m 1500
> TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to
> 192.168.10.1 (192.168.10.1) port 0 AF_INET
> Recv   Send    Send                          Utilization Service Demand
> Socket Socket  Message  Elapsed              Send     Recv Send    Recv
> Size   Size    Size     Time     Throughput  local    remote local   remote
> bytes  bytes   bytes    secs.    10^6bits/s  % S      % S us/KB   us/KB
> 
>  87380  16384   1500    20.00       867.02   99.75    16.21 9.425   1.531
> root@p1010rdb:~# netperf -l 20 -cC -H 192.168.10.1 -t TCP_STREAM -- -m 1500
> TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to
> 192.168.10.1 (192.168.10.1) port 0 AF_INET
> Recv   Send    Send                          Utilization Service Demand
> Socket Socket  Message  Elapsed              Send     Recv Send    Recv
> Size   Size    Size     Time     Throughput  local    remote local   remote
> bytes  bytes   bytes    secs.    10^6bits/s  % S      % S us/KB   us/KB
> 
>  87380  16384   1500    20.01       874.29   99.85    15.25 9.356   1.429
> 
> 
> //=Image 3)================
> 
> Linux version 3.5.0-20970-gaae06bf-dirty [...] //CONFIG_BQL = n
> 
> root@p1010rdb:~# cat /proc/version
> Linux version 3.5.0-20970-gaae06bf-dirty
> (b08782@zro04-ws574.ea.freescale.net) (gcc version 4.6.2 (GCC) ) #3
> Mon Aug 13 13:58:25 EEST 2012
> root@p1010rdb:~# zcat /proc/config.gz | grep BQL
> # CONFIG_BQL is not set
> 
> root@p1010rdb:~# netperf -l 20 -cC -H 192.168.10.1 -t TCP_STREAM -- -m 1500
> TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to
> 192.168.10.1 (192.168.10.1) port 0 AF_INET
> Recv   Send    Send                          Utilization Service Demand
> Socket Socket  Message  Elapsed              Send     Recv Send    Recv
> Size   Size    Size     Time     Throughput  local    remote local   remote
> bytes  bytes   bytes    secs.    10^6bits/s  % S      % S us/KB   us/KB
> 
>  87380  16384   1500    20.00       595.08   99.95    12.51 13.759  1.722
> root@p1010rdb:~# netperf -l 20 -cC -H 192.168.10.1 -t TCP_STREAM -- -m 1500
> TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to
> 192.168.10.1 (192.168.10.1) port 0 AF_INET
> Recv   Send    Send                          Utilization Service Demand
> Socket Socket  Message  Elapsed              Send     Recv Send    Recv
> Size   Size    Size     Time     Throughput  local    remote local   remote
> bytes  bytes   bytes    secs.    10^6bits/s  % S      % S us/KB   us/KB
> 
>  87380  16384   1500    20.00       593.95   99.95    10.96 13.785  1.511
> root@p1010rdb:~# netperf -l 20 -cC -H 192.168.10.1 -t TCP_STREAM -- -m 1500
> TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to
> 192.168.10.1 (192.168.10.1) port 0 AF_INET
> Recv   Send    Send                          Utilization Service Demand
> Socket Socket  Message  Elapsed              Send     Recv Send    Recv
> Size   Size    Size     Time     Throughput  local    remote local   remote
> bytes  bytes   bytes    secs.    10^6bits/s  % S      % S us/KB   us/KB
> 
>  87380  16384   1500    20.00       595.30   99.90    11.11 13.747  1.528
> 
> root@p1010rdb:~# ethtool -C eth1 rx-frames 22 rx-usecs 32 tx-frames
> 22 tx-usecs 32
> root@p1010rdb:~# netperf -l 20 -cC -H 192.168.10.1 -t TCP_STREAM -- -m 1500
> TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to
> 192.168.10.1 (192.168.10.1) port 0 AF_INET
> Recv   Send    Send                          Utilization Service Demand
> Socket Socket  Message  Elapsed              Send     Recv Send    Recv
> Size   Size    Size     Time     Throughput  local    remote local   remote
> bytes  bytes   bytes    secs.    10^6bits/s  % S      % S us/KB   us/KB
> 
>  87380  16384   1500    20.00       710.46   99.95    12.46 11.525  1.437
> root@p1010rdb:~# netperf -l 20 -cC -H 192.168.10.1 -t TCP_STREAM -- -m 1500
> TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to
> 192.168.10.1 (192.168.10.1) port 0 AF_INET
> Recv   Send    Send                          Utilization Service Demand
> Socket Socket  Message  Elapsed              Send     Recv Send    Recv
> Size   Size    Size     Time     Throughput  local    remote local   remote
> bytes  bytes   bytes    secs.    10^6bits/s  % S      % S us/KB   us/KB
> 
>  87380  16384   1500    20.00       714.27   99.95    14.05 11.463  1.611
> root@p1010rdb:~# netperf -l 20 -cC -H 192.168.10.1 -t TCP_STREAM -- -m 1500
> TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to
> 192.168.10.1 (192.168.10.1) port 0 AF_INET
> Recv   Send    Send                          Utilization Service Demand
> Socket Socket  Message  Elapsed              Send     Recv Send    Recv
> Size   Size    Size     Time     Throughput  local    remote local   remote
> bytes  bytes   bytes    secs.    10^6bits/s  % S      % S us/KB   us/KB
> 
>  87380  16384   1500    20.00       717.69   99.95    12.56 11.409  1.433
> .
> 

^ permalink raw reply

* Re: [RFC net-next 4/4] gianfar: Use separate NAPIs for Tx and Rx processing
From: Paul Gortmaker @ 2012-08-14  0:51 UTC (permalink / raw)
  To: Claudiu Manoil; +Cc: netdev, David S. Miller, Pankaj Chauhan, Eric Dumazet
In-Reply-To: <1344428810-29923-5-git-send-email-claudiu.manoil@freescale.com>

[[RFC net-next 4/4] gianfar: Use separate NAPIs for Tx and Rx processing] On 08/08/2012 (Wed 15:26) Claudiu Manoil wrote:

> Add a separate napi poll routine for Tx cleanup, to be triggerred by Tx
> confirmation interrupts only. Existing poll function is modified to handle
> only the Rx path processing. This allows parallel processing of Rx and Tx
> confirmation paths on a smp machine (2 cores).
> The split also results in simpler/cleaner napi poll function implementations,
> where each processing path has its own budget, thus improving the fairness b/w
> the processing paths at the same time.
> 
> Signed-off-by: Pankaj Chauhan <pankaj.chauhan@freescale.com>
> Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>
> ---
>  drivers/net/ethernet/freescale/gianfar.c |  154 +++++++++++++++++++++++-------
>  drivers/net/ethernet/freescale/gianfar.h |   16 +++-
>  2 files changed, 130 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
> index 919acb3..2774961 100644
> --- a/drivers/net/ethernet/freescale/gianfar.c
> +++ b/drivers/net/ethernet/freescale/gianfar.c
> @@ -128,12 +128,14 @@ static void free_skb_resources(struct gfar_private *priv);
>  static void gfar_set_multi(struct net_device *dev);
>  static void gfar_set_hash_for_addr(struct net_device *dev, u8 *addr);
>  static void gfar_configure_serdes(struct net_device *dev);
> -static int gfar_poll(struct napi_struct *napi, int budget);
> +static int gfar_poll_rx(struct napi_struct *napi, int budget);
> +static int gfar_poll_tx(struct napi_struct *napi, int budget);
>  #ifdef CONFIG_NET_POLL_CONTROLLER
>  static void gfar_netpoll(struct net_device *dev);
>  #endif
>  int gfar_clean_rx_ring(struct gfar_priv_rx_q *rx_queue, int rx_work_limit);
> -static int gfar_clean_tx_ring(struct gfar_priv_tx_q *tx_queue);
> +static int gfar_clean_tx_ring(struct gfar_priv_tx_q *tx_queue,
> +			      int tx_work_limit);

I'm looking at this in a bit more detail now (was on vacation last wk).
With the above, you push a work limit down into the clean_tx_ring.
I'm wondering if the above is implicitly involved in the performance
difference you see, since...

>  static int gfar_process_frame(struct net_device *dev, struct sk_buff *skb,
>  			      int amount_pull, struct napi_struct *napi);
>  void gfar_halt(struct net_device *dev);
> @@ -543,16 +545,20 @@ static void disable_napi(struct gfar_private *priv)
>  {
>  	int i;
>  
> -	for (i = 0; i < priv->num_grps; i++)
> -		napi_disable(&priv->gfargrp[i].napi);
> +	for (i = 0; i < priv->num_grps; i++) {
> +		napi_disable(&priv->gfargrp[i].napi_rx);
> +		napi_disable(&priv->gfargrp[i].napi_tx);
> +	}
>  }
>  
>  static void enable_napi(struct gfar_private *priv)
>  {
>  	int i;
>  
> -	for (i = 0; i < priv->num_grps; i++)
> -		napi_enable(&priv->gfargrp[i].napi);
> +	for (i = 0; i < priv->num_grps; i++) {
> +		napi_enable(&priv->gfargrp[i].napi_rx);
> +		napi_enable(&priv->gfargrp[i].napi_tx);
> +	}
>  }
>  
>  static int gfar_parse_group(struct device_node *np,
> @@ -1028,9 +1034,12 @@ static int gfar_probe(struct platform_device *ofdev)
>  	dev->ethtool_ops = &gfar_ethtool_ops;
>  
>  	/* Register for napi ...We are registering NAPI for each grp */
> -	for (i = 0; i < priv->num_grps; i++)
> -		netif_napi_add(dev, &priv->gfargrp[i].napi, gfar_poll,
> -			       GFAR_DEV_WEIGHT);
> +	for (i = 0; i < priv->num_grps; i++) {
> +		netif_napi_add(dev, &priv->gfargrp[i].napi_rx, gfar_poll_rx,
> +			       GFAR_DEV_RX_WEIGHT);
> +		netif_napi_add(dev, &priv->gfargrp[i].napi_tx, gfar_poll_tx,
> +			       GFAR_DEV_TX_WEIGHT);
> +	}
>  
>  	if (priv->device_flags & FSL_GIANFAR_DEV_HAS_CSUM) {
>  		dev->hw_features = NETIF_F_IP_CSUM | NETIF_F_SG |
> @@ -2465,7 +2474,8 @@ static void gfar_align_skb(struct sk_buff *skb)
>  }
>  
>  /* Interrupt Handler for Transmit complete */
> -static int gfar_clean_tx_ring(struct gfar_priv_tx_q *tx_queue)
> +static int gfar_clean_tx_ring(struct gfar_priv_tx_q *tx_queue,
> +			      int tx_work_limit)
>  {
>  	struct net_device *dev = tx_queue->dev;
>  	struct netdev_queue *txq;
> @@ -2490,7 +2500,7 @@ static int gfar_clean_tx_ring(struct gfar_priv_tx_q *tx_queue)
>  	bdp = tx_queue->dirty_tx;
>  	skb_dirtytx = tx_queue->skb_dirtytx;
>  
> -	while ((skb = tx_queue->tx_skbuff[skb_dirtytx])) {
> +	while ((skb = tx_queue->tx_skbuff[skb_dirtytx]) && tx_work_limit--) {

...code like this provides a new exit point that did not exist before,
for the case of a massive transmit blast.  Do you have any data on how
often the work limit is hit?  The old Don Becker ether drivers which
originally introduced the idea of work limits (on IRQs) used to printk a
message when they hit it, since ideally it shouldn't be happening all
the time.

In any case, it might be worth while to split this change out into a
separate commit; something like:

   gianfar: push transmit cleanup work limit down to clean_tx_ring

The advantage being (1) we can test this change in isolation, and
(2) it makes your remaining rx/tx separate thread patch smaller and
easier to review.

Thanks,
Paul.
--

>  		unsigned long flags;
>  
>  		frags = skb_shinfo(skb)->nr_frags;
> @@ -2580,29 +2590,50 @@ static int gfar_clean_tx_ring(struct gfar_priv_tx_q *tx_queue)
>  	return howmany;
>  }
>  
> -static void gfar_schedule_cleanup(struct gfar_priv_grp *gfargrp)
> +static void gfar_schedule_rx_cleanup(struct gfar_priv_grp *gfargrp)
>  {
>  	unsigned long flags;
>  
> -	spin_lock_irqsave(&gfargrp->grplock, flags);
> -	if (napi_schedule_prep(&gfargrp->napi)) {
> -		gfar_write(&gfargrp->regs->imask, IMASK_RTX_DISABLED);
> -		__napi_schedule(&gfargrp->napi);
> +	if (napi_schedule_prep(&gfargrp->napi_rx)) {
> +		u32 imask;
> +		spin_lock_irqsave(&gfargrp->grplock, flags);
> +		imask = gfar_read(&gfargrp->regs->imask);
> +		imask &= ~(IMASK_RX_DEFAULT);
> +		gfar_write(&gfargrp->regs->imask, imask);
> +		__napi_schedule(&gfargrp->napi_rx);
> +		spin_unlock_irqrestore(&gfargrp->grplock, flags);
>  	}
>  
>  	/* Clear IEVENT, so interrupts aren't called again
>  	 * because of the packets that have already arrived.
>  	 */
> -	gfar_write(&gfargrp->regs->ievent, IEVENT_RTX_MASK);
> +	gfar_write(&gfargrp->regs->ievent, IEVENT_RX_MASK);
> +}
>  
> -	spin_unlock_irqrestore(&gfargrp->grplock, flags);
> +static void gfar_schedule_tx_cleanup(struct gfar_priv_grp *gfargrp)
> +{
> +	unsigned long flags;
> +
> +	if (napi_schedule_prep(&gfargrp->napi_tx)) {
> +		u32 imask;
> +		spin_lock_irqsave(&gfargrp->grplock, flags);
> +		imask = gfar_read(&gfargrp->regs->imask);
> +		imask &= ~(IMASK_TX_DEFAULT);
> +		gfar_write(&gfargrp->regs->imask, imask);
> +		__napi_schedule(&gfargrp->napi_tx);
> +		spin_unlock_irqrestore(&gfargrp->grplock, flags);
> +	}
>  
> +	/* Clear IEVENT, so interrupts aren't called again
> +	 * because of the packets that have already arrived.
> +	 */
> +	gfar_write(&gfargrp->regs->ievent, IEVENT_TX_MASK);
>  }
>  
>  /* Interrupt Handler for Transmit complete */
>  static irqreturn_t gfar_transmit(int irq, void *grp_id)
>  {
> -	gfar_schedule_cleanup((struct gfar_priv_grp *)grp_id);
> +	gfar_schedule_tx_cleanup((struct gfar_priv_grp *)grp_id);
>  	return IRQ_HANDLED;
>  }
>  
> @@ -2683,7 +2714,7 @@ static inline void count_errors(unsigned short status, struct net_device *dev)
>  
>  irqreturn_t gfar_receive(int irq, void *grp_id)
>  {
> -	gfar_schedule_cleanup((struct gfar_priv_grp *)grp_id);
> +	gfar_schedule_rx_cleanup((struct gfar_priv_grp *)grp_id);
>  	return IRQ_HANDLED;
>  }
>  
> @@ -2813,7 +2844,7 @@ int gfar_clean_rx_ring(struct gfar_priv_rx_q *rx_queue, int rx_work_limit)
>  				rx_queue->stats.rx_bytes += pkt_len;
>  				skb_record_rx_queue(skb, rx_queue->qindex);
>  				gfar_process_frame(dev, skb, amount_pull,
> -						   &rx_queue->grp->napi);
> +						   &rx_queue->grp->napi_rx);
>  
>  			} else {
>  				netif_warn(priv, rx_err, dev, "Missing skb!\n");
> @@ -2842,21 +2873,19 @@ int gfar_clean_rx_ring(struct gfar_priv_rx_q *rx_queue, int rx_work_limit)
>  	return howmany;
>  }
>  
> -static int gfar_poll(struct napi_struct *napi, int budget)
> +static int gfar_poll_rx(struct napi_struct *napi, int budget)
>  {
>  	struct gfar_priv_grp *gfargrp =
> -		container_of(napi, struct gfar_priv_grp, napi);
> +		container_of(napi, struct gfar_priv_grp, napi_rx);
>  	struct gfar_private *priv = gfargrp->priv;
>  	struct gfar __iomem *regs = gfargrp->regs;
> -	struct gfar_priv_tx_q *tx_queue = NULL;
>  	struct gfar_priv_rx_q *rx_queue = NULL;
> -	int rx_cleaned = 0, budget_per_queue = 0, rx_cleaned_per_queue = 0;
> -	int tx_cleaned = 0, i, left_over_budget = budget;
> +	int rx_cleaned = 0, budget_per_queue, rx_cleaned_per_queue;
> +	int i, left_over_budget = budget;
>  	unsigned long serviced_queues = 0;
>  	int num_queues = 0;
>  
>  	num_queues = gfargrp->num_rx_queues;
> -	budget_per_queue = budget/num_queues;
>  
>  	while (num_queues && left_over_budget) {
>  		budget_per_queue = left_over_budget/num_queues;
> @@ -2866,9 +2895,6 @@ static int gfar_poll(struct napi_struct *napi, int budget)
>  			if (test_bit(i, &serviced_queues))
>  				continue;
>  			rx_queue = priv->rx_queue[i];
> -			tx_queue = priv->tx_queue[rx_queue->qindex];
> -
> -			tx_cleaned += gfar_clean_tx_ring(tx_queue);
>  			rx_cleaned_per_queue =
>  				gfar_clean_rx_ring(rx_queue, budget_per_queue);
>  			rx_cleaned += rx_cleaned_per_queue;
> @@ -2882,27 +2908,83 @@ static int gfar_poll(struct napi_struct *napi, int budget)
>  		}
>  	}
>  
> -	if (tx_cleaned)
> -		return budget;
> -
>  	if (rx_cleaned < budget) {
> +		u32 imask;
>  		napi_complete(napi);
>  
>  		/* Clear the halt bit in RSTAT */
>  		gfar_write(&regs->rstat, gfargrp->rstat);
>  
> -		gfar_write(&regs->imask, IMASK_DEFAULT);
> +		spin_lock_irq(&gfargrp->grplock);
> +		imask = gfar_read(&regs->imask);
> +		imask |= IMASK_RX_DEFAULT;
> +		gfar_write(&regs->imask, imask);
> +		spin_unlock_irq(&gfargrp->grplock);
>  
>  		/* If we are coalescing interrupts, update the timer
>  		 * Otherwise, clear it
>  		 */
> -		gfar_configure_coalescing(priv, gfargrp->rx_bit_map,
> -					  gfargrp->tx_bit_map);
> +		gfar_configure_rx_coalescing(priv, gfargrp->rx_bit_map);
>  	}
>  
>  	return rx_cleaned;
>  }
>  
> +static int gfar_poll_tx(struct napi_struct *napi, int budget)
> +{
> +	struct gfar_priv_grp *gfargrp =
> +		container_of(napi, struct gfar_priv_grp, napi_tx);
> +	struct gfar_private *priv = gfargrp->priv;
> +	struct gfar __iomem *regs = gfargrp->regs;
> +	struct gfar_priv_tx_q *tx_queue = NULL;
> +	int tx_cleaned = 0, budget_per_queue, tx_cleaned_per_queue;
> +	int i, left_over_budget = budget;
> +	unsigned long serviced_queues = 0;
> +	int num_queues = 0;
> +
> +	num_queues = gfargrp->num_tx_queues;
> +
> +	while (num_queues && left_over_budget) {
> +		budget_per_queue = left_over_budget/num_queues;
> +		left_over_budget = 0;
> +
> +		for_each_set_bit(i, &gfargrp->tx_bit_map, priv->num_tx_queues) {
> +			if (test_bit(i, &serviced_queues))
> +				continue;
> +			tx_queue = priv->tx_queue[i];
> +			tx_cleaned_per_queue =
> +				gfar_clean_tx_ring(tx_queue, budget_per_queue);
> +			tx_cleaned += tx_cleaned_per_queue;
> +			if (tx_cleaned_per_queue < budget_per_queue) {
> +				left_over_budget = left_over_budget +
> +					(budget_per_queue -
> +					 tx_cleaned_per_queue);
> +				set_bit(i, &serviced_queues);
> +				num_queues--;
> +			}
> +		}
> +	}
> +
> +	if (tx_cleaned < budget) {
> +		u32 imask;
> +		napi_complete(napi);
> +
> +		gfar_write(&regs->imask, IMASK_DEFAULT);
> +		spin_lock_irq(&gfargrp->grplock);
> +		imask = gfar_read(&regs->imask);
> +		imask |= IMASK_TX_DEFAULT;
> +		gfar_write(&regs->imask, imask);
> +		spin_unlock_irq(&gfargrp->grplock);
> +
> +		/* If we are coalescing interrupts, update the timer
> +		 * Otherwise, clear it
> +		 */
> +		gfar_configure_tx_coalescing(priv, gfargrp->tx_bit_map);
> +	}
> +
> +	return tx_cleaned;
> +}
> +
>  #ifdef CONFIG_NET_POLL_CONTROLLER
>  /* Polling 'interrupt' - used by things like netconsole to send skbs
>   * without having to re-enable interrupts. It's not called while
> diff --git a/drivers/net/ethernet/freescale/gianfar.h b/drivers/net/ethernet/freescale/gianfar.h
> index 2136c7f..f5be234 100644
> --- a/drivers/net/ethernet/freescale/gianfar.h
> +++ b/drivers/net/ethernet/freescale/gianfar.h
> @@ -57,8 +57,10 @@ struct ethtool_rx_list {
>  	unsigned int count;
>  };
>  
> -/* The maximum number of packets to be handled in one call of gfar_poll */
> -#define GFAR_DEV_WEIGHT 64
> +/* The maximum number of packets to be handled in one call of gfar_poll_rx */
> +#define GFAR_DEV_RX_WEIGHT 64
> +/* The maximum number of packets to be handled in one call of gfar_poll_tx */
> +#define GFAR_DEV_TX_WEIGHT 64
>  
>  /* Length for FCB */
>  #define GMAC_FCB_LEN 8
> @@ -366,6 +368,10 @@ extern const char gfar_driver_version[];
>  		| IMASK_PERR)
>  #define IMASK_RTX_DISABLED ((~(IMASK_RXFEN0 | IMASK_TXFEN | IMASK_BSY)) \
>  			   & IMASK_DEFAULT)
> +#define IMASK_RX_DEFAULT  (IMASK_RXFEN0 | IMASK_BSY)
> +#define IMASK_TX_DEFAULT  (IMASK_TXFEN)
> +#define IMASK_RX_DISABLED ((~(IMASK_RX_DEFAULT)) & IMASK_DEFAULT)
> +#define IMASK_TX_DISABLED ((~(IMASK_TX_DEFAULT)) & IMASK_DEFAULT)
>  
>  /* Fifo management */
>  #define FIFO_TX_THR_MASK	0x01ff
> @@ -993,7 +999,8 @@ struct gfar_priv_rx_q {
>  
>  /**
>   *	struct gfar_priv_grp - per group structure
> - *	@napi: the napi poll function
> + *	@napi_rx: the RX napi poll function
> + *	@napi_tx: the TX confirmation napi poll function
>   *	@priv: back pointer to the priv structure
>   *	@regs: the ioremapped register space for this group
>   *	@grp_id: group id for this group
> @@ -1007,7 +1014,8 @@ struct gfar_priv_rx_q {
>  
>  struct gfar_priv_grp {
>  	spinlock_t grplock __attribute__ ((aligned (SMP_CACHE_BYTES)));
> -	struct	napi_struct napi;
> +	struct	napi_struct napi_rx;
> +	struct	napi_struct napi_tx;
>  	struct gfar_private *priv;
>  	struct gfar __iomem *regs;
>  	unsigned int grp_id;
> -- 
> 1.6.6
> 
> 

^ permalink raw reply

* Re: [PATCH 03/19] netfilter: nf_nat_sip: fix via header translation with multiple parameters
From: Pablo Neira Ayuso @ 2012-08-14  0:28 UTC (permalink / raw)
  To: kaber; +Cc: netfilter-devel, netdev
In-Reply-To: <1344542943-11588-4-git-send-email-kaber@trash.net>

On Thu, Aug 09, 2012 at 10:08:47PM +0200, kaber@trash.net wrote:
> From: Patrick McHardy <kaber@trash.net>
> 
> Via-headers are parsed beginning at the first character after the Via-address.
> When the address is translated first and its length decreases, the offset to
> start parsing at is incorrect and header parameters might be missed.
> 
> Update the offset after translating the Via-address to fix this.

Applied, thanks Patrick.

^ permalink raw reply

* Re: [PATCH 02/19] netfilter: nf_ct_sip: fix IPv6 address parsing
From: Pablo Neira Ayuso @ 2012-08-14  0:19 UTC (permalink / raw)
  To: kaber; +Cc: netfilter-devel, netdev
In-Reply-To: <1344542943-11588-3-git-send-email-kaber@trash.net>

On Thu, Aug 09, 2012 at 10:08:46PM +0200, kaber@trash.net wrote:
> From: Patrick McHardy <kaber@trash.net>
> 
> Within SIP messages IPv6 addresses are enclosed in square brackets in most
> cases, with the exception of the "received=" header parameter. Currently
> the helper fails to parse enclosed addresses.
> 
> This patch:
> 
> - changes the SIP address parsing function to enforce square brackets
>   when required, and accept them when not required but present, as
>   recommended by RFC 5118.
> 
> - adds a new SDP address parsing function that never accepts square
>   brackets since SDP doesn't use them.
> 
> With these changes, the SIP helper correctly parses all test messages
> from RFC 5118 (Session Initiation Protocol (SIP) Torture Test Messages
> for Internet Protocol Version 6 (IPv6)).

Applied, thanks Patrick.

^ permalink raw reply

* Re: [PATCH 01/19] netfilter: nf_ct_sip: fix helper name
From: Pablo Neira Ayuso @ 2012-08-14  0:00 UTC (permalink / raw)
  To: kaber; +Cc: netfilter-devel, netdev
In-Reply-To: <1344542943-11588-2-git-send-email-kaber@trash.net>

On Thu, Aug 09, 2012 at 10:08:45PM +0200, kaber@trash.net wrote:
> From: Patrick McHardy <kaber@trash.net>
> 
> Commit 3a8fc53a (netfilter: nf_ct_helper: allocate 16 bytes for the helper
> and policy names) introduced a bug in the SIP helper, the helper name is
> sprinted to the sip_names array instead of instead of into the helper
> structure. This breaks the helper match and the /proc/net/nf_conntrack_expect
> output.

Applied, thanks Patrick.

^ permalink raw reply

* Re: [PATCH 06/21] userns: Print out socket uids in a user namespace aware fashion.
From: Rémi Denis-Courmont @ 2012-08-13 20:26 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1344889115-21610-6-git-send-email-ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>

Le lundi 13 août 2012 23:18:20, vous avez écrit :
> From: "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
> 
> Cc: David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
> Cc: Alexey Kuznetsov <kuznet-v/Mj1YrvjDBInbfyfbPRSQ@public.gmane.org>
> Cc: James Morris <jmorris-gx6/JNMH7DfYtjvyW6yDsg@public.gmane.org>
> Cc: Hideaki YOSHIFUJI <yoshfuji-VfPWfsRibaP+Ru+s062T9g@public.gmane.org>
> Cc: Patrick McHardy <kaber-dcUjhNyLwpNeoWH0uzbU5w@public.gmane.org>
> Cc: Arnaldo Carvalho de Melo <acme-f8uhVLnGfZaxAyOMLChx1axOck334EZe@public.gmane.org>
> Cc: Vlad Yasevich <vyasevich-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: Sridhar Samudrala <sri-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
> Acked-by: Serge Hallyn <serge.hallyn-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
> Signed-off-by: Eric W. Biederman <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>

FWIW, ...

Acked-By: Rémi Denis-Courmont <remi-AzDNUFsAnHasTnJN9+BGXg@public.gmane.org>

-- 
Rémi Denis-Courmont
C/C++ software engineer looking for a job
http://www.linkedin.com/in/remidenis

^ permalink raw reply

* [PATCH 10/21] userns: Convert net/ax25 to use kuid_t where appropriate
From: Eric W. Biederman @ 2012-08-13 20:18 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, containers, David Miller, Serge Hallyn,
	Eric W. Biederman, Ralf Baechle
In-Reply-To: <1344889115-21610-1-git-send-email-ebiederm@xmission.com>

From: "Eric W. Biederman" <ebiederm@xmission.com>

Cc: Ralf Baechle <ralf@linux-mips.org>
Acked-by: Serge Hallyn <serge.hallyn@canonical.com>
Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
 include/net/ax25.h  |    4 ++--
 init/Kconfig        |    1 -
 net/ax25/ax25_uid.c |   21 ++++++++++++++-------
 3 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/include/net/ax25.h b/include/net/ax25.h
index 5d23521..53539ac 100644
--- a/include/net/ax25.h
+++ b/include/net/ax25.h
@@ -157,7 +157,7 @@ enum {
 typedef struct ax25_uid_assoc {
 	struct hlist_node	uid_node;
 	atomic_t		refcount;
-	uid_t			uid;
+	kuid_t			uid;
 	ax25_address		call;
 } ax25_uid_assoc;
 
@@ -434,7 +434,7 @@ extern unsigned long ax25_display_timer(struct timer_list *);
 
 /* ax25_uid.c */
 extern int  ax25_uid_policy;
-extern ax25_uid_assoc *ax25_findbyuid(uid_t);
+extern ax25_uid_assoc *ax25_findbyuid(kuid_t);
 extern int __must_check ax25_uid_ioctl(int, struct sockaddr_ax25 *);
 extern const struct file_operations ax25_uid_fops;
 extern void ax25_uid_free(void);
diff --git a/init/Kconfig b/init/Kconfig
index 64ff9ce..8447e0c 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -952,7 +952,6 @@ config UIDGID_CONVERTED
 	depends on NET_KEY = n
 	depends on INET_DIAG = n
 	depends on DNS_RESOLVER = n
-	depends on AX25 = n
 
 	# Filesystems
 	depends on USB_GADGETFS = n
diff --git a/net/ax25/ax25_uid.c b/net/ax25/ax25_uid.c
index e3c579b..957999e 100644
--- a/net/ax25/ax25_uid.c
+++ b/net/ax25/ax25_uid.c
@@ -51,14 +51,14 @@ int ax25_uid_policy;
 
 EXPORT_SYMBOL(ax25_uid_policy);
 
-ax25_uid_assoc *ax25_findbyuid(uid_t uid)
+ax25_uid_assoc *ax25_findbyuid(kuid_t uid)
 {
 	ax25_uid_assoc *ax25_uid, *res = NULL;
 	struct hlist_node *node;
 
 	read_lock(&ax25_uid_lock);
 	ax25_uid_for_each(ax25_uid, node, &ax25_uid_list) {
-		if (ax25_uid->uid == uid) {
+		if (uid_eq(ax25_uid->uid, uid)) {
 			ax25_uid_hold(ax25_uid);
 			res = ax25_uid;
 			break;
@@ -84,7 +84,7 @@ int ax25_uid_ioctl(int cmd, struct sockaddr_ax25 *sax)
 		read_lock(&ax25_uid_lock);
 		ax25_uid_for_each(ax25_uid, node, &ax25_uid_list) {
 			if (ax25cmp(&sax->sax25_call, &ax25_uid->call) == 0) {
-				res = ax25_uid->uid;
+				res = from_kuid_munged(current_user_ns(), ax25_uid->uid);
 				break;
 			}
 		}
@@ -93,9 +93,14 @@ int ax25_uid_ioctl(int cmd, struct sockaddr_ax25 *sax)
 		return res;
 
 	case SIOCAX25ADDUID:
+	{
+		kuid_t sax25_kuid;
 		if (!capable(CAP_NET_ADMIN))
 			return -EPERM;
-		user = ax25_findbyuid(sax->sax25_uid);
+		sax25_kuid = make_kuid(current_user_ns(), sax->sax25_uid);
+		if (!uid_valid(sax25_kuid))
+			return -EINVAL;
+		user = ax25_findbyuid(sax25_kuid);
 		if (user) {
 			ax25_uid_put(user);
 			return -EEXIST;
@@ -106,7 +111,7 @@ int ax25_uid_ioctl(int cmd, struct sockaddr_ax25 *sax)
 			return -ENOMEM;
 
 		atomic_set(&ax25_uid->refcount, 1);
-		ax25_uid->uid  = sax->sax25_uid;
+		ax25_uid->uid  = sax25_kuid;
 		ax25_uid->call = sax->sax25_call;
 
 		write_lock(&ax25_uid_lock);
@@ -114,7 +119,7 @@ int ax25_uid_ioctl(int cmd, struct sockaddr_ax25 *sax)
 		write_unlock(&ax25_uid_lock);
 
 		return 0;
-
+	}
 	case SIOCAX25DELUID:
 		if (!capable(CAP_NET_ADMIN))
 			return -EPERM;
@@ -172,7 +177,9 @@ static int ax25_uid_seq_show(struct seq_file *seq, void *v)
 		struct ax25_uid_assoc *pt;
 
 		pt = hlist_entry(v, struct ax25_uid_assoc, uid_node);
-		seq_printf(seq, "%6d %s\n", pt->uid, ax2asc(buf, &pt->call));
+		seq_printf(seq, "%6d %s\n",
+			from_kuid_munged(seq_user_ns(seq), pt->uid),
+			ax2asc(buf, &pt->call));
 	}
 	return 0;
 }
-- 
1.7.5.4

^ permalink raw reply related

* [PATCH 21/21] userns: Convert tun/tap to use kuid and kgid where appropriate
From: Eric W. Biederman @ 2012-08-13 20:18 UTC (permalink / raw)
  To: netdev-u79uwXL29TY76Z2rM5mHXA
  Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Eric W. Biederman,
	Maxim Krasnyansky, David Miller
In-Reply-To: <1344889115-21610-1-git-send-email-ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>

From: "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>

Cc: Maxim Krasnyansky <maxk-zC7DfRvBq/JWk0Htik3J/w@public.gmane.org>
Acked-by: Serge Hallyn <serge.hallyn-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
Signed-off-by: Eric W. Biederman <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
---
 drivers/net/tun.c |   46 ++++++++++++++++++++++++++++++++--------------
 init/Kconfig      |    1 -
 2 files changed, 32 insertions(+), 15 deletions(-)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 926d4db..a9bd9f3 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -120,8 +120,8 @@ struct tun_sock;
 struct tun_struct {
 	struct tun_file		*tfile;
 	unsigned int 		flags;
-	uid_t			owner;
-	gid_t			group;
+	kuid_t			owner;
+	kgid_t			group;
 
 	struct net_device	*dev;
 	netdev_features_t	set_features;
@@ -1032,8 +1032,8 @@ static void tun_setup(struct net_device *dev)
 {
 	struct tun_struct *tun = netdev_priv(dev);
 
-	tun->owner = -1;
-	tun->group = -1;
+	tun->owner = INVALID_UID;
+	tun->group = INVALID_GID;
 
 	dev->ethtool_ops = &tun_ethtool_ops;
 	dev->destructor = tun_free_netdev;
@@ -1156,14 +1156,20 @@ static ssize_t tun_show_owner(struct device *dev, struct device_attribute *attr,
 			      char *buf)
 {
 	struct tun_struct *tun = netdev_priv(to_net_dev(dev));
-	return sprintf(buf, "%d\n", tun->owner);
+	return uid_valid(tun->owner)?
+		sprintf(buf, "%u\n",
+			from_kuid_munged(current_user_ns(), tun->owner)):
+		sprintf(buf, "-1\n");
 }
 
 static ssize_t tun_show_group(struct device *dev, struct device_attribute *attr,
 			      char *buf)
 {
 	struct tun_struct *tun = netdev_priv(to_net_dev(dev));
-	return sprintf(buf, "%d\n", tun->group);
+	return gid_valid(tun->group) ?
+		sprintf(buf, "%u\n",
+			from_kgid_munged(current_user_ns(), tun->group)):
+		sprintf(buf, "-1\n");
 }
 
 static DEVICE_ATTR(tun_flags, 0444, tun_show_flags, NULL);
@@ -1190,8 +1196,8 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
 		else
 			return -EINVAL;
 
-		if (((tun->owner != -1 && cred->euid != tun->owner) ||
-		     (tun->group != -1 && !in_egroup_p(tun->group))) &&
+		if (((uid_valid(tun->owner) && !uid_eq(cred->euid, tun->owner)) ||
+		     (gid_valid(tun->group) && !in_egroup_p(tun->group))) &&
 		    !capable(CAP_NET_ADMIN))
 			return -EPERM;
 		err = security_tun_dev_attach(tun->socket.sk);
@@ -1375,6 +1381,8 @@ static long __tun_chr_ioctl(struct file *file, unsigned int cmd,
 	void __user* argp = (void __user*)arg;
 	struct sock_fprog fprog;
 	struct ifreq ifr;
+	kuid_t owner;
+	kgid_t group;
 	int sndbuf;
 	int vnet_hdr_sz;
 	int ret;
@@ -1448,16 +1456,26 @@ static long __tun_chr_ioctl(struct file *file, unsigned int cmd,
 
 	case TUNSETOWNER:
 		/* Set owner of the device */
-		tun->owner = (uid_t) arg;
-
-		tun_debug(KERN_INFO, tun, "owner set to %d\n", tun->owner);
+		owner = make_kuid(current_user_ns(), arg);
+		if (!uid_valid(owner)) {
+			ret = -EINVAL;
+			break;
+		}
+		tun->owner = owner;
+		tun_debug(KERN_INFO, tun, "owner set to %d\n",
+			  from_kuid(&init_user_ns, tun->owner));
 		break;
 
 	case TUNSETGROUP:
 		/* Set group of the device */
-		tun->group= (gid_t) arg;
-
-		tun_debug(KERN_INFO, tun, "group set to %d\n", tun->group);
+		group = make_kgid(current_user_ns(), arg);
+		if (!gid_valid(group)) {
+			ret = -EINVAL;
+			break;
+		}
+		tun->group = group;
+		tun_debug(KERN_INFO, tun, "group set to %d\n",
+			  from_kgid(&init_user_ns, tun->group));
 		break;
 
 	case TUNSETLINK:
diff --git a/init/Kconfig b/init/Kconfig
index eac23a6..b445d6f 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1003,7 +1003,6 @@ config UIDGID_CONVERTED
 	depends on !UML || HOSTFS = n
 
 	# The rare drivers that won't build
-	depends on TUN = n
 	depends on INFINIBAND_QIB = n
 	depends on BLK_DEV_LOOP = n
 	depends on ANDROID_BINDER_IPC = n
-- 
1.7.5.4

^ permalink raw reply related

* [PATCH 20/21] userns: Make the airo wireless driver use kuids for proc uids and gids
From: Eric W. Biederman @ 2012-08-13 20:18 UTC (permalink / raw)
  To: netdev-u79uwXL29TY76Z2rM5mHXA
  Cc: John W. Linville,
	containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Alexey Dobriyan,
	Eric W. Biederman, David Miller, Dan Carpenter
In-Reply-To: <1344889115-21610-1-git-send-email-ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>

From: "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>

Cc: Dan Carpenter <dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
Cc: Alexey Dobriyan <adobriyan-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: John W. Linville <linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
Acked-by: Serge Hallyn <serge.hallyn-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
Signed-off-by: Eric W. Biederman <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
---
 drivers/net/wireless/airo.c |   48 +++++++++++++++++++++++++-----------------
 init/Kconfig                |    2 -
 2 files changed, 28 insertions(+), 22 deletions(-)

diff --git a/drivers/net/wireless/airo.c b/drivers/net/wireless/airo.c
index f9f15bb..c586f78 100644
--- a/drivers/net/wireless/airo.c
+++ b/drivers/net/wireless/airo.c
@@ -232,8 +232,10 @@ static int adhoc;
 
 static int probe = 1;
 
+static kuid_t proc_kuid;
 static int proc_uid /* = 0 */;
 
+static kgid_t proc_kgid;
 static int proc_gid /* = 0 */;
 
 static int airo_perm = 0555;
@@ -4499,78 +4501,79 @@ struct proc_data {
 static int setup_proc_entry( struct net_device *dev,
 			     struct airo_info *apriv ) {
 	struct proc_dir_entry *entry;
+
 	/* First setup the device directory */
 	strcpy(apriv->proc_name,dev->name);
 	apriv->proc_entry = proc_mkdir_mode(apriv->proc_name, airo_perm,
 					    airo_entry);
 	if (!apriv->proc_entry)
 		goto fail;
-	apriv->proc_entry->uid = proc_uid;
-	apriv->proc_entry->gid = proc_gid;
+	apriv->proc_entry->uid = proc_kuid;
+	apriv->proc_entry->gid = proc_kgid;
 
 	/* Setup the StatsDelta */
 	entry = proc_create_data("StatsDelta", S_IRUGO & proc_perm,
 				 apriv->proc_entry, &proc_statsdelta_ops, dev);
 	if (!entry)
 		goto fail_stats_delta;
-	entry->uid = proc_uid;
-	entry->gid = proc_gid;
+	entry->uid = proc_kuid;
+	entry->gid = proc_kgid;
 
 	/* Setup the Stats */
 	entry = proc_create_data("Stats", S_IRUGO & proc_perm,
 				 apriv->proc_entry, &proc_stats_ops, dev);
 	if (!entry)
 		goto fail_stats;
-	entry->uid = proc_uid;
-	entry->gid = proc_gid;
+	entry->uid = proc_kuid;
+	entry->gid = proc_kgid;
 
 	/* Setup the Status */
 	entry = proc_create_data("Status", S_IRUGO & proc_perm,
 				 apriv->proc_entry, &proc_status_ops, dev);
 	if (!entry)
 		goto fail_status;
-	entry->uid = proc_uid;
-	entry->gid = proc_gid;
+	entry->uid = proc_kuid;
+	entry->gid = proc_kgid;
 
 	/* Setup the Config */
 	entry = proc_create_data("Config", proc_perm,
 				 apriv->proc_entry, &proc_config_ops, dev);
 	if (!entry)
 		goto fail_config;
-	entry->uid = proc_uid;
-	entry->gid = proc_gid;
+	entry->uid = proc_kuid;
+	entry->gid = proc_kgid;
 
 	/* Setup the SSID */
 	entry = proc_create_data("SSID", proc_perm,
 				 apriv->proc_entry, &proc_SSID_ops, dev);
 	if (!entry)
 		goto fail_ssid;
-	entry->uid = proc_uid;
-	entry->gid = proc_gid;
+	entry->uid = proc_kuid;
+	entry->gid = proc_kgid;
 
 	/* Setup the APList */
 	entry = proc_create_data("APList", proc_perm,
 				 apriv->proc_entry, &proc_APList_ops, dev);
 	if (!entry)
 		goto fail_aplist;
-	entry->uid = proc_uid;
-	entry->gid = proc_gid;
+	entry->uid = proc_kuid;
+	entry->gid = proc_kgid;
 
 	/* Setup the BSSList */
 	entry = proc_create_data("BSSList", proc_perm,
 				 apriv->proc_entry, &proc_BSSList_ops, dev);
 	if (!entry)
 		goto fail_bsslist;
-	entry->uid = proc_uid;
-	entry->gid = proc_gid;
+	entry->uid = proc_kuid;
+	entry->gid = proc_kgid;
 
 	/* Setup the WepKey */
 	entry = proc_create_data("WepKey", proc_perm,
 				 apriv->proc_entry, &proc_wepkey_ops, dev);
 	if (!entry)
 		goto fail_wepkey;
-	entry->uid = proc_uid;
-	entry->gid = proc_gid;
+	entry->uid = proc_kuid;
+	entry->gid = proc_kgid;
 
 	return 0;
 
@@ -5697,11 +5700,16 @@ static int __init airo_init_module( void )
 {
 	int i;
 
+	proc_kuid = make_kuid(&init_user_ns, proc_uid);
+	proc_kgid = make_kgid(&init_user_ns, proc_gid);
+	if (!uid_valid(proc_kuid) || !gid_valid(proc_kgid))
+		return -EINVAL;
+
 	airo_entry = proc_mkdir_mode("driver/aironet", airo_perm, NULL);
 
 	if (airo_entry) {
-		airo_entry->uid = proc_uid;
-		airo_entry->gid = proc_gid;
+		airo_entry->uid = proc_kuid;
+		airo_entry->gid = proc_kgid;
 	}
 
 	for (i = 0; i < 4 && io[i] && irq[i]; i++) {
diff --git a/init/Kconfig b/init/Kconfig
index 76ffca9..eac23a6 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1003,8 +1003,6 @@ config UIDGID_CONVERTED
 	depends on !UML || HOSTFS = n
 
 	# The rare drivers that won't build
-	depends on AIRO = n
-	depends on AIRO_CS = n
 	depends on TUN = n
 	depends on INFINIBAND_QIB = n
 	depends on BLK_DEV_LOOP = n
-- 
1.7.5.4

^ permalink raw reply related

* [PATCH 19/21] userns: xt_owner: Add basic user namespace support.
From: Eric W. Biederman @ 2012-08-13 20:18 UTC (permalink / raw)
  To: netdev-u79uwXL29TY76Z2rM5mHXA
  Cc: Patrick McHardy,
	containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Jan Engelhardt,
	Eric W. Biederman, David Miller, Pablo Neira Ayuso
In-Reply-To: <1344889115-21610-1-git-send-email-ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>

From: "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>

- Only allow adding matches from the initial user namespace
- Add the appropriate conversion functions to handle matches
  against sockets in other user namespaces.

Cc: Jan Engelhardt <jengelh-nopoi9nDyk+ELgA04lAiVw@public.gmane.org>
Cc: Patrick McHardy <kaber-dcUjhNyLwpNeoWH0uzbU5w@public.gmane.org>
Cc: Pablo Neira Ayuso <pablo-Cap9r6Oaw4JrovVCs/uTlw@public.gmane.org>
Acked-by: Serge Hallyn <serge.hallyn-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
Signed-off-by: Eric W. Biederman <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
---
 init/Kconfig             |    1 -
 net/netfilter/xt_owner.c |   30 ++++++++++++++++++++++++------
 2 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/init/Kconfig b/init/Kconfig
index 40f5020..76ffca9 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -943,7 +943,6 @@ config UIDGID_CONVERTED
 
 	# Networking
 	depends on NET_9P = n
-	depends on NETFILTER_XT_MATCH_OWNER = n
 	depends on AF_RXRPC = n
 	depends on NET_KEY = n
 	depends on DNS_RESOLVER = n
diff --git a/net/netfilter/xt_owner.c b/net/netfilter/xt_owner.c
index 772d738..ca2e577 100644
--- a/net/netfilter/xt_owner.c
+++ b/net/netfilter/xt_owner.c
@@ -17,6 +17,17 @@
 #include <linux/netfilter/x_tables.h>
 #include <linux/netfilter/xt_owner.h>
 
+static int owner_check(const struct xt_mtchk_param *par)
+{
+	struct xt_owner_match_info *info = par->matchinfo;
+
+	/* For now only allow adding matches from the initial user namespace */
+	if ((info->match & (XT_OWNER_UID|XT_OWNER_GID)) &&
+	    (current_user_ns() != &init_user_ns))
+		return -EINVAL;
+	return 0;
+}
+
 static bool
 owner_mt(const struct sk_buff *skb, struct xt_action_param *par)
 {
@@ -37,17 +48,23 @@ owner_mt(const struct sk_buff *skb, struct xt_action_param *par)
 		return ((info->match ^ info->invert) &
 		       (XT_OWNER_UID | XT_OWNER_GID)) == 0;
 
-	if (info->match & XT_OWNER_UID)
-		if ((filp->f_cred->fsuid >= info->uid_min &&
-		    filp->f_cred->fsuid <= info->uid_max) ^
+	if (info->match & XT_OWNER_UID) {
+		kuid_t uid_min = make_kuid(&init_user_ns, info->uid_min);
+		kuid_t uid_max = make_kuid(&init_user_ns, info->uid_max);
+		if ((uid_gte(filp->f_cred->fsuid, uid_min) &&
+		     uid_lte(filp->f_cred->fsuid, uid_max)) ^
 		    !(info->invert & XT_OWNER_UID))
 			return false;
+	}
 
-	if (info->match & XT_OWNER_GID)
-		if ((filp->f_cred->fsgid >= info->gid_min &&
-		    filp->f_cred->fsgid <= info->gid_max) ^
+	if (info->match & XT_OWNER_GID) {
+		kgid_t gid_min = make_kgid(&init_user_ns, info->gid_min);
+		kgid_t gid_max = make_kgid(&init_user_ns, info->gid_max);
+		if ((gid_gte(filp->f_cred->fsgid, gid_min) &&
+		     gid_lte(filp->f_cred->fsgid, gid_max)) ^
 		    !(info->invert & XT_OWNER_GID))
 			return false;
+	}
 
 	return true;
 }
@@ -56,6 +73,7 @@ static struct xt_match owner_mt_reg __read_mostly = {
 	.name       = "owner",
 	.revision   = 1,
 	.family     = NFPROTO_UNSPEC,
+	.checkentry = owner_check,
 	.match      = owner_mt,
 	.matchsize  = sizeof(struct xt_owner_match_info),
 	.hooks      = (1 << NF_INET_LOCAL_OUT) |
-- 
1.7.5.4

^ permalink raw reply related

* [PATCH 18/21] userns xt_recent: Specify the owner/group of ip_list_perms in the initial user namespace
From: Eric W. Biederman @ 2012-08-13 20:18 UTC (permalink / raw)
  To: netdev-u79uwXL29TY76Z2rM5mHXA
  Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Patrick McHardy,
	Eric W. Biederman, Jan Engelhardt, David Miller,
	Pablo Neira Ayuso
In-Reply-To: <1344889115-21610-1-git-send-email-ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>

From: "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>

xt_recent creates a bunch of proc files and initializes their uid
and gids to the values of ip_list_uid and ip_list_gid.  When
initialize those proc files convert those values to kuids so they
can continue to reside on the /proc inode.

Cc: Pablo Neira Ayuso <pablo-Cap9r6Oaw4JrovVCs/uTlw@public.gmane.org>
Cc: Patrick McHardy <kaber-dcUjhNyLwpNeoWH0uzbU5w@public.gmane.org>
Cc: Jan Engelhardt <jengelh-nopoi9nDyk+ELgA04lAiVw@public.gmane.org>
Acked-by: Serge Hallyn <serge.hallyn-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
Signed-off-by: Eric W. Biederman <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
---
 init/Kconfig              |    1 -
 net/netfilter/xt_recent.c |   13 +++++++++++--
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/init/Kconfig b/init/Kconfig
index c8911eb..40f5020 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -944,7 +944,6 @@ config UIDGID_CONVERTED
 	# Networking
 	depends on NET_9P = n
 	depends on NETFILTER_XT_MATCH_OWNER = n
-	depends on NETFILTER_XT_MATCH_RECENT = n
 	depends on AF_RXRPC = n
 	depends on NET_KEY = n
 	depends on DNS_RESOLVER = n
diff --git a/net/netfilter/xt_recent.c b/net/netfilter/xt_recent.c
index ae2ad1e..4635c9b 100644
--- a/net/netfilter/xt_recent.c
+++ b/net/netfilter/xt_recent.c
@@ -317,6 +317,8 @@ static int recent_mt_check(const struct xt_mtchk_param *par,
 	struct recent_table *t;
 #ifdef CONFIG_PROC_FS
 	struct proc_dir_entry *pde;
+	kuid_t uid;
+	kgid_t gid;
 #endif
 	unsigned int i;
 	int ret = -EINVAL;
@@ -372,6 +374,13 @@ static int recent_mt_check(const struct xt_mtchk_param *par,
 	for (i = 0; i < ip_list_hash_size; i++)
 		INIT_LIST_HEAD(&t->iphash[i]);
 #ifdef CONFIG_PROC_FS
+	uid = make_kuid(&init_user_ns, ip_list_uid);
+	gid = make_kgid(&init_user_ns, ip_list_gid);
+	if (!uid_valid(uid) || !gid_valid(gid)) {
+		kfree(t);
+		ret = -EINVAL;
+		goto out;
+	}
 	pde = proc_create_data(t->name, ip_list_perms, recent_net->xt_recent,
 		  &recent_mt_fops, t);
 	if (pde == NULL) {
@@ -379,8 +388,8 @@ static int recent_mt_check(const struct xt_mtchk_param *par,
 		ret = -ENOMEM;
 		goto out;
 	}
-	pde->uid = ip_list_uid;
-	pde->gid = ip_list_gid;
+	pde->uid = uid;
+	pde->gid = gid;
 #endif
 	spin_lock_bh(&recent_lock);
 	list_add_tail(&t->list, &recent_net->tables);
-- 
1.7.5.4

^ permalink raw reply related

* [PATCH 17/21] userns: Convert xt_LOG to print socket kuids and kgids as uids and gids
From: Eric W. Biederman @ 2012-08-13 20:18 UTC (permalink / raw)
  To: netdev-u79uwXL29TY76Z2rM5mHXA
  Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Patrick McHardy,
	Eric W. Biederman, David Miller, Pablo Neira Ayuso
In-Reply-To: <1344889115-21610-1-git-send-email-ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>

From: "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>

xt_LOG always writes messages via sb_add via printk.  Therefore when
xt_LOG logs the uid and gid of a socket a packet came from the
values should be converted to be in the initial user namespace.

Thus making xt_LOG as user namespace safe as possible.

Cc: Pablo Neira Ayuso <pablo-Cap9r6Oaw4JrovVCs/uTlw@public.gmane.org>
Cc: Patrick McHardy <kaber-dcUjhNyLwpNeoWH0uzbU5w@public.gmane.org>
Acked-by: Serge Hallyn <serge.hallyn-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
Signed-off-by: Eric W. Biederman <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
---
 init/Kconfig           |    1 -
 net/netfilter/xt_LOG.c |   16 ++++++++++------
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/init/Kconfig b/init/Kconfig
index b44c3a3..c8911eb 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -945,7 +945,6 @@ config UIDGID_CONVERTED
 	depends on NET_9P = n
 	depends on NETFILTER_XT_MATCH_OWNER = n
 	depends on NETFILTER_XT_MATCH_RECENT = n
-	depends on NETFILTER_XT_TARGET_LOG = n
 	depends on AF_RXRPC = n
 	depends on NET_KEY = n
 	depends on DNS_RESOLVER = n
diff --git a/net/netfilter/xt_LOG.c b/net/netfilter/xt_LOG.c
index ff5f75f..02a2bf4 100644
--- a/net/netfilter/xt_LOG.c
+++ b/net/netfilter/xt_LOG.c
@@ -363,10 +363,12 @@ static void dump_ipv4_packet(struct sbuff *m,
 	/* Max length: 15 "UID=4294967295 " */
 	if ((logflags & XT_LOG_UID) && !iphoff && skb->sk) {
 		read_lock_bh(&skb->sk->sk_callback_lock);
-		if (skb->sk->sk_socket && skb->sk->sk_socket->file)
+		if (skb->sk->sk_socket && skb->sk->sk_socket->file) {
+			const struct cred *cred = skb->sk->sk_socket->file->f_cred;
 			sb_add(m, "UID=%u GID=%u ",
-				skb->sk->sk_socket->file->f_cred->fsuid,
-				skb->sk->sk_socket->file->f_cred->fsgid);
+				from_kuid_munged(&init_user_ns, cred->fsuid),
+				from_kgid_munged(&init_user_ns, cred->fsgid));
+		}
 		read_unlock_bh(&skb->sk->sk_callback_lock);
 	}
 
@@ -719,10 +721,12 @@ static void dump_ipv6_packet(struct sbuff *m,
 	/* Max length: 15 "UID=4294967295 " */
 	if ((logflags & XT_LOG_UID) && recurse && skb->sk) {
 		read_lock_bh(&skb->sk->sk_callback_lock);
-		if (skb->sk->sk_socket && skb->sk->sk_socket->file)
+		if (skb->sk->sk_socket && skb->sk->sk_socket->file) {
+			const struct cred *cred = skb->sk->sk_socket->file->f_cred;
 			sb_add(m, "UID=%u GID=%u ",
-				skb->sk->sk_socket->file->f_cred->fsuid,
-				skb->sk->sk_socket->file->f_cred->fsgid);
+				from_kuid_munged(&init_user_ns, cred->fsuid),
+				from_kgid_munged(&init_user_ns, cred->fsgid));
+		}
 		read_unlock_bh(&skb->sk->sk_callback_lock);
 	}
 
-- 
1.7.5.4

^ permalink raw reply related

* [PATCH 16/21] userns: Convert cls_flow to work with user namespaces enabled
From: Eric W. Biederman @ 2012-08-13 20:18 UTC (permalink / raw)
  To: netdev-u79uwXL29TY76Z2rM5mHXA
  Cc: Eric Dumazet,
	containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, David Miller,
	Jamal Hadi Salim, Eric W. Biederman, Changli Gao, Patrick McHardy
In-Reply-To: <1344889115-21610-1-git-send-email-ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>

From: "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>

The flow classifier can use uids and gids of the sockets that
are transmitting packets and do insert those uids and gids
into the packet classification calcuation.  I don't fully
understand the details but it appears that we can depend
on specific uids and gids when making traffic classification
decisions.

To work with user namespaces enabled map from kuids and kgids
into uids and gids in the initial user namespace giving raw
integer values the code can play with and depend on.

To avoid issues of userspace depending on uids and gids in
packet classifiers installed from other user namespaces
and getting confused deny all packet classifiers that
use uids or gids that are not comming from a netlink socket
in the initial user namespace.

Cc: Patrick McHardy <kaber-dcUjhNyLwpNeoWH0uzbU5w@public.gmane.org>
Cc: Eric Dumazet <eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Jamal Hadi Salim <jhs-jkUAjuhPggJWk0Htik3J/w@public.gmane.org>
Cc: Changli Gao <xiaosuo-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Acked-by: Serge Hallyn <serge.hallyn-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
Signed-off-by: Eric W. Biederman <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
---
 init/Kconfig         |    1 -
 net/sched/cls_flow.c |   16 ++++++++++++----
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/init/Kconfig b/init/Kconfig
index 2660b31..b44c3a3 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -943,7 +943,6 @@ config UIDGID_CONVERTED
 
 	# Networking
 	depends on NET_9P = n
-	depends on NET_CLS_FLOW = n
 	depends on NETFILTER_XT_MATCH_OWNER = n
 	depends on NETFILTER_XT_MATCH_RECENT = n
 	depends on NETFILTER_XT_TARGET_LOG = n
diff --git a/net/sched/cls_flow.c b/net/sched/cls_flow.c
index ae854f3..ce82d0c 100644
--- a/net/sched/cls_flow.c
+++ b/net/sched/cls_flow.c
@@ -193,15 +193,19 @@ static u32 flow_get_rtclassid(const struct sk_buff *skb)
 
 static u32 flow_get_skuid(const struct sk_buff *skb)
 {
-	if (skb->sk && skb->sk->sk_socket && skb->sk->sk_socket->file)
-		return skb->sk->sk_socket->file->f_cred->fsuid;
+	if (skb->sk && skb->sk->sk_socket && skb->sk->sk_socket->file) {
+		kuid_t skuid = skb->sk->sk_socket->file->f_cred->fsuid;
+		return from_kuid(&init_user_ns, skuid);
+	}
 	return 0;
 }
 
 static u32 flow_get_skgid(const struct sk_buff *skb)
 {
-	if (skb->sk && skb->sk->sk_socket && skb->sk->sk_socket->file)
-		return skb->sk->sk_socket->file->f_cred->fsgid;
+	if (skb->sk && skb->sk->sk_socket && skb->sk->sk_socket->file) {
+		kgid_t skgid = skb->sk->sk_socket->file->f_cred->fsgid;
+		return from_kgid(&init_user_ns, skgid);
+	}
 	return 0;
 }
 
@@ -387,6 +391,10 @@ static int flow_change(struct sk_buff *in_skb,
 
 		if (fls(keymask) - 1 > FLOW_KEY_MAX)
 			return -EOPNOTSUPP;
+
+		if ((keymask & (FLOW_KEY_SKUID|FLOW_KEY_SKGID)) &&
+		    sk_user_ns(NETLINK_CB(in_skb).ssk) != &init_user_ns)
+			return -EOPNOTSUPP;
 	}
 
 	err = tcf_exts_validate(tp, tb, tca[TCA_RATE], &e, &flow_ext_map);
-- 
1.7.5.4

^ permalink raw reply related

* [PATCH 15/21] net sched: Pass the skb into change so it can access NETLINK_CB
From: Eric W. Biederman @ 2012-08-13 20:18 UTC (permalink / raw)
  To: netdev-u79uwXL29TY76Z2rM5mHXA
  Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Jamal Hadi Salim,
	Eric W. Biederman, David Miller
In-Reply-To: <1344889115-21610-1-git-send-email-ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>

From: "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>

cls_flow.c plays with uids and gids.  Unless I misread that
code it is possible for classifiers to depend on the specific uid and
gid values.  Therefore I need to know the user namespace of the
netlink socket that is installing the packet classifiers.  Pass
in the rtnetlink skb so I can access the NETLINK_CB of the passed
packet.  In particular I want access to sk_user_ns(NETLINK_CB(in_skb).ssk).

Pass in not the user namespace but the incomming rtnetlink skb into
the the classifier change routines as that is generally the more useful
parameter.

Cc: Jamal Hadi Salim <jhs-jkUAjuhPggJWk0Htik3J/w@public.gmane.org>
Acked-by: Serge Hallyn <serge.hallyn-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
Signed-off-by: Eric W. Biederman <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
---
 include/net/sch_generic.h |    3 ++-
 net/sched/cls_api.c       |    2 +-
 net/sched/cls_basic.c     |    3 ++-
 net/sched/cls_cgroup.c    |    3 ++-
 net/sched/cls_flow.c      |    3 ++-
 net/sched/cls_fw.c        |    3 ++-
 net/sched/cls_route.c     |    3 ++-
 net/sched/cls_rsvp.h      |    3 ++-
 net/sched/cls_tcindex.c   |    3 ++-
 net/sched/cls_u32.c       |    3 ++-
 10 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index d9611e0..4616f46 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -188,7 +188,8 @@ struct tcf_proto_ops {
 
 	unsigned long		(*get)(struct tcf_proto*, u32 handle);
 	void			(*put)(struct tcf_proto*, unsigned long);
-	int			(*change)(struct tcf_proto*, unsigned long,
+	int			(*change)(struct sk_buff *,
+					struct tcf_proto*, unsigned long,
 					u32 handle, struct nlattr **,
 					unsigned long *);
 	int			(*delete)(struct tcf_proto*, unsigned long);
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 6dd1131..dc3ef5a 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -319,7 +319,7 @@ replay:
 		}
 	}
 
-	err = tp->ops->change(tp, cl, t->tcm_handle, tca, &fh);
+	err = tp->ops->change(skb, tp, cl, t->tcm_handle, tca, &fh);
 	if (err == 0) {
 		if (tp_created) {
 			spin_lock_bh(root_lock);
diff --git a/net/sched/cls_basic.c b/net/sched/cls_basic.c
index 590960a..344a11b 100644
--- a/net/sched/cls_basic.c
+++ b/net/sched/cls_basic.c
@@ -162,7 +162,8 @@ errout:
 	return err;
 }
 
-static int basic_change(struct tcf_proto *tp, unsigned long base, u32 handle,
+static int basic_change(struct sk_buff *in_skb,
+			struct tcf_proto *tp, unsigned long base, u32 handle,
 			struct nlattr **tca, unsigned long *arg)
 {
 	int err;
diff --git a/net/sched/cls_cgroup.c b/net/sched/cls_cgroup.c
index 7743ea8..91de666 100644
--- a/net/sched/cls_cgroup.c
+++ b/net/sched/cls_cgroup.c
@@ -151,7 +151,8 @@ static const struct nla_policy cgroup_policy[TCA_CGROUP_MAX + 1] = {
 	[TCA_CGROUP_EMATCHES]	= { .type = NLA_NESTED },
 };
 
-static int cls_cgroup_change(struct tcf_proto *tp, unsigned long base,
+static int cls_cgroup_change(struct sk_buff *in_skb,
+			     struct tcf_proto *tp, unsigned long base,
 			     u32 handle, struct nlattr **tca,
 			     unsigned long *arg)
 {
diff --git a/net/sched/cls_flow.c b/net/sched/cls_flow.c
index ccd08c8..ae854f3 100644
--- a/net/sched/cls_flow.c
+++ b/net/sched/cls_flow.c
@@ -347,7 +347,8 @@ static const struct nla_policy flow_policy[TCA_FLOW_MAX + 1] = {
 	[TCA_FLOW_PERTURB]	= { .type = NLA_U32 },
 };
 
-static int flow_change(struct tcf_proto *tp, unsigned long base,
+static int flow_change(struct sk_buff *in_skb, 
+		       struct tcf_proto *tp, unsigned long base,
 		       u32 handle, struct nlattr **tca,
 		       unsigned long *arg)
 {
diff --git a/net/sched/cls_fw.c b/net/sched/cls_fw.c
index 8384a47..4075a0a 100644
--- a/net/sched/cls_fw.c
+++ b/net/sched/cls_fw.c
@@ -233,7 +233,8 @@ errout:
 	return err;
 }
 
-static int fw_change(struct tcf_proto *tp, unsigned long base,
+static int fw_change(struct sk_buff *in_skb,
+		     struct tcf_proto *tp, unsigned long base,
 		     u32 handle,
 		     struct nlattr **tca,
 		     unsigned long *arg)
diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c
index 44f405c..c10d57b 100644
--- a/net/sched/cls_route.c
+++ b/net/sched/cls_route.c
@@ -427,7 +427,8 @@ errout:
 	return err;
 }
 
-static int route4_change(struct tcf_proto *tp, unsigned long base,
+static int route4_change(struct sk_buff *in_skb,
+		       struct tcf_proto *tp, unsigned long base,
 		       u32 handle,
 		       struct nlattr **tca,
 		       unsigned long *arg)
diff --git a/net/sched/cls_rsvp.h b/net/sched/cls_rsvp.h
index 18ab93e..494bbb9 100644
--- a/net/sched/cls_rsvp.h
+++ b/net/sched/cls_rsvp.h
@@ -416,7 +416,8 @@ static const struct nla_policy rsvp_policy[TCA_RSVP_MAX + 1] = {
 	[TCA_RSVP_PINFO]	= { .len = sizeof(struct tc_rsvp_pinfo) },
 };
 
-static int rsvp_change(struct tcf_proto *tp, unsigned long base,
+static int rsvp_change(struct sk_buff *in_skb,
+		       struct tcf_proto *tp, unsigned long base,
 		       u32 handle,
 		       struct nlattr **tca,
 		       unsigned long *arg)
diff --git a/net/sched/cls_tcindex.c b/net/sched/cls_tcindex.c
index fe29420..a1293b4 100644
--- a/net/sched/cls_tcindex.c
+++ b/net/sched/cls_tcindex.c
@@ -332,7 +332,8 @@ errout:
 }
 
 static int
-tcindex_change(struct tcf_proto *tp, unsigned long base, u32 handle,
+tcindex_change(struct sk_buff *in_skb,
+	       struct tcf_proto *tp, unsigned long base, u32 handle,
 	       struct nlattr **tca, unsigned long *arg)
 {
 	struct nlattr *opt = tca[TCA_OPTIONS];
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index d45373f..c7c27bc 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -544,7 +544,8 @@ errout:
 	return err;
 }
 
-static int u32_change(struct tcf_proto *tp, unsigned long base, u32 handle,
+static int u32_change(struct sk_buff *in_skb,
+		      struct tcf_proto *tp, unsigned long base, u32 handle,
 		      struct nlattr **tca,
 		      unsigned long *arg)
 {
-- 
1.7.5.4

^ permalink raw reply related

* [PATCH 14/21] userns: nfnetlink_log: Report socket uids in the log sockets user namespace
From: Eric W. Biederman @ 2012-08-13 20:18 UTC (permalink / raw)
  To: netdev-u79uwXL29TY76Z2rM5mHXA
  Cc: Eric W. Biederman,
	containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, David Miller
In-Reply-To: <1344889115-21610-1-git-send-email-ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>

From: "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>

At logging instance creation capture the peer netlink socket's user
namespace. Use the captured peer user namespace when reporting socket
uids to the peer.

The peer socket's user namespace is guaranateed to be valid until the user
closes the netlink socket.  nfnetlink_log removes instances during the final
close of a socket.  __build_packet_message does not get called after an
instance is destroyed.   Therefore it is safe to let the peer netlink socket
take care of the user namespace reference counting for us.

Acked-by: Serge Hallyn <serge.hallyn-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
Signed-off-by: Eric W. Biederman <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
---
 init/Kconfig                  |    1 -
 net/netfilter/nfnetlink_log.c |   14 ++++++++++----
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/init/Kconfig b/init/Kconfig
index 07435e0..2660b31 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -947,7 +947,6 @@ config UIDGID_CONVERTED
 	depends on NETFILTER_XT_MATCH_OWNER = n
 	depends on NETFILTER_XT_MATCH_RECENT = n
 	depends on NETFILTER_XT_TARGET_LOG = n
-	depends on NETFILTER_NETLINK_LOG = n
 	depends on AF_RXRPC = n
 	depends on NET_KEY = n
 	depends on DNS_RESOLVER = n
diff --git a/net/netfilter/nfnetlink_log.c b/net/netfilter/nfnetlink_log.c
index 169ab59..4142aac 100644
--- a/net/netfilter/nfnetlink_log.c
+++ b/net/netfilter/nfnetlink_log.c
@@ -55,6 +55,7 @@ struct nfulnl_instance {
 	unsigned int qlen;		/* number of nlmsgs in skb */
 	struct sk_buff *skb;		/* pre-allocatd skb */
 	struct timer_list timer;
+	struct user_namespace *peer_user_ns;	/* User namespace of the peer process */
 	int peer_pid;			/* PID of the peer process */
 
 	/* configurable parameters */
@@ -132,7 +133,7 @@ instance_put(struct nfulnl_instance *inst)
 static void nfulnl_timer(unsigned long data);
 
 static struct nfulnl_instance *
-instance_create(u_int16_t group_num, int pid)
+instance_create(u_int16_t group_num, int pid, struct user_namespace *user_ns)
 {
 	struct nfulnl_instance *inst;
 	int err;
@@ -162,6 +163,7 @@ instance_create(u_int16_t group_num, int pid)
 
 	setup_timer(&inst->timer, nfulnl_timer, (unsigned long)inst);
 
+	inst->peer_user_ns = user_ns;
 	inst->peer_pid = pid;
 	inst->group_num = group_num;
 
@@ -503,8 +505,11 @@ __build_packet_message(struct nfulnl_instance *inst,
 		read_lock_bh(&skb->sk->sk_callback_lock);
 		if (skb->sk->sk_socket && skb->sk->sk_socket->file) {
 			struct file *file = skb->sk->sk_socket->file;
-			__be32 uid = htonl(file->f_cred->fsuid);
-			__be32 gid = htonl(file->f_cred->fsgid);
+			__be32 uid = htonl(from_kuid_munged(inst->peer_user_ns,
+							    file->f_cred->fsuid));
+			__be32 gid = htonl(from_kgid_munged(inst->peer_user_ns,
+							    file->f_cred->fsgid));
+			/* need to unlock here since NLA_PUT may goto */
 			read_unlock_bh(&skb->sk->sk_callback_lock);
 			if (nla_put_be32(inst->skb, NFULA_UID, uid) ||
 			    nla_put_be32(inst->skb, NFULA_GID, gid))
@@ -783,7 +788,8 @@ nfulnl_recv_config(struct sock *ctnl, struct sk_buff *skb,
 			}
 
 			inst = instance_create(group_num,
-					       NETLINK_CB(skb).pid);
+					       NETLINK_CB(skb).pid,
+					       sk_user_ns(NETLINK_CB(skb).ssk));
 			if (IS_ERR(inst)) {
 				ret = PTR_ERR(inst);
 				goto out;
-- 
1.7.5.4

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox