Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] xmit_compl_seq: information to reclaim vmsplice buffers
From: David Miller @ 2010-09-20  0:35 UTC (permalink / raw)
  To: therbert; +Cc: netdev, sridharr
In-Reply-To: <alpine.DEB.1.00.1009191451090.5120@pokey.mtv.corp.google.com>

From: Tom Herbert <therbert@google.com>
Date: Sun, 19 Sep 2010 14:55:40 -0700 (PDT)

> diff --git a/include/asm-generic/socket.h b/include/asm-generic/socket.h
> index 9a6115e..6dc1ed8 100644
> --- a/include/asm-generic/socket.h
> +++ b/include/asm-generic/socket.h
> @@ -64,4 +64,7 @@
>  #define SO_DOMAIN		39
>  
>  #define SO_RXQ_OVFL             40
> +
> +#define SO_XMIT_COMPL_SEQ	41
> +#define SCM_XMIT_COMPL_SEQ	SO_XMIT_COMPL_SEQ
>  #endif /* __ASM_GENERIC_SOCKET_H */

Tom, I just noticed that you're going to have to update all of
the arch specific arch/*/include/asm/socket.h files that don't
make use of asm-generic/socket.h before I can apply this.

Please fix this up and resubmit, thanks!

^ permalink raw reply

* Re: [autofs] Remaining BKL users, what to do
From: Ian Kent @ 2010-09-20  1:25 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: codalist, autofs, Samuel Ortiz, Jan Kara,
	Arnaldo Carvalho de Melo, netdev, Anders Larsen, Trond Myklebust,
	linux-kernel, dri-devel, Christoph Hellwig, Petr Vandrovec,
	Mikulas Patocka, linux-fsdevel, Evgeniy Dushistov, Ingo Molnar,
	Andrew Hendry, linux-media
In-Reply-To: <201009161632.59210.arnd@arndb.de>

On Thu, 2010-09-16 at 16:32 +0200, Arnd Bergmann wrote:
> The big kernel lock is gone from almost all code in linux-next, this is
> the status of what I think will happen to the remaining users:
> 

...

> fs/autofs:
> 	Pretty much dead, replaced by autofs4. I'd suggest moving this
> 	to drivers/staging in 2.6.37 and letting it die there.

Not sure that's what we need.
What actually needs to happen is that autofs4 needs to be renamed to
autofs.

Ian




^ permalink raw reply

* Re: [PATCH] xmit_compl_seq: information to reclaim vmsplice buffers
From: Changli Gao @ 2010-09-20  3:22 UTC (permalink / raw)
  To: Tom Herbert; +Cc: netdev, davem, sridharr
In-Reply-To: <alpine.DEB.1.00.1009191451090.5120@pokey.mtv.corp.google.com>

On Mon, Sep 20, 2010 at 5:55 AM, Tom Herbert <therbert@google.com> wrote:
> In this patch we propose to adds some socket API to retrieve the
>  "transmit completion sequence number", essentially a byte counter
> for the number of bytes that have been transmitted and will not be
> retransmitted.  In the case of TCP, this should correspond to snd_una.
>
> The purpose of this API is to provide information to userspace about
> which buffers can be reclaimed when sending with vmsplice() on a
> socket.
>
> There are two methods for retrieving the completed sequence number:
> through a simple getsockopt (implemented here for TCP), as well as
> returning the value in the ancilary data of a recvmsg.
>
> The expected flow would be something like:
>   - Connect is created
>   - Initial completion seq # is retrieved through the sockopt, and is
>     stored in userspace "compl_seq" variable for the connection.
>   - Whenever a send is done, compl_seq += # bytes sent.
>   - When doing a vmsplice the completion sequence number is saved
>     for each user space buffer, buffer_compl_seq = compl_seq.
>   - When recvmsg returns with a completion sequence number in
>     ancillary data, any buffers cover by that sequence number
>     (where buffer_compl_seq < recvmsg_compl_seq) are reclaimed
>     and can be written to again.
>   - If no data is receieved on a connection (recvmsg does not
>     return), a timeout can be used to call the getsockopt and
>     reclaim buffers as a fallback.
>
> Using recvmsg data in this manner is sort of a cheap way to get a
> "callback" for when a vmspliced buffer is consumed.  It will work
> well for a client where the response causes recvmsg to return.

In this case, we don't need it. The response is sufficient to indicate
the request is sent out, and the corresponding pages are available for
the other use.

> On the server side it works well if there are a sufficient
> number of requests coming on the connection (resorting to the
> timeout if necessary as described above).

If no response is sent out by the client, the timeout mechanism may
slow down the data transfer.

I think we should not mix RX and TX. Maybe we can add a new epoll
event to indicate that the snd bufer is advanced.

-- 
Regards,
Changli Gao(xiaosuo@gmail.com)

^ permalink raw reply

* Re: Re: Fwd: LVS on local node
From: Simon Horman @ 2010-09-20  5:56 UTC (permalink / raw)
  To: Franchoze Eric; +Cc: wensong, lvs-devel, netdev, netfilter-devel
In-Reply-To: <111551279817984@web84.yandex.ru>

On Thu, Jul 22, 2010 at 08:59:44PM +0400, Franchoze Eric wrote:
> 
> 
> 22.07.10, 17:25, "Simon Horman" <horms@verge.net.au>:
> 
> > On Thu, Jul 22, 2010 at 07:51:20AM +0400, Franchoze Eric wrote:
> >  > Hello,
> >  > 
> >  > I'm trying to do load balancing of incoming traffic to my applications. This applications are not very  smp friendly, and I want try to run some instances according to number of cpus on single machine. And balance load of incoming traffic/connections to this applications.
> >  > Looks like is should be similar to http://www.austintek.com/LVS/LVS-HOWTO/HOWTO/LVS-HOWTO.localnode.html
> >  > 
> >  >  linux kernel 2.6.32 with  or without hide interface patches.  Tried different configurations but could not see packets on application layer.
> >  > 
> >  > 192.168.1.165 - eth0 - interface for external connections
> >  > 195.0.0.1 - dummy0 - virtual interface, real application is binded to that address.
> >  > 
> >  > Configuration is:
> >  > -A -t 192.168.1.165:1234 -s wlc
> >  > -a -t 192.168.1.165:1234 -r 195.0.0.1:1234 -g -w
> >  > 
> >  > #ipvsadm -L -n
> >  > IP Virtual Server version 1.2.1 (size=4096)
> >  > Prot LocalAddress:Port Scheduler Flags
> >  >   -> RemoteAddress:Port           Forward Weight ActiveConn InActConn
> >  > TCP  192.168.1.165:1234 wlc
> >  >   -> 195.0.0.1:1234               Local   1      0          0        
> >  > #
> >  > 
> >  > Log:
> >  > [ 2106.897409] IPVS: lookup/out TCP 192.168.1.165:44847->192.168.1.165:1234 not hit
> >  > [ 2106.897412] IPVS: lookup service: fwm 0 TCP 192.168.1.165:1234 hit
> >  > [ 2106.897414] IPVS: ip_vs_wlc_schedule(): Scheduling...
> >  > [ 2106.897416] IPVS: WLC: server 195.0.0.1:1234 activeconns 0 refcnt 2 weight 1 overhead 1
> >  > [ 2106.897418] IPVS: Enter: ip_vs_conn_new, net/netfilter/ipvs/ip_vs_conn.c line 693
> >  > [ 2106.897421] IPVS: Bind-dest TCP c:192.168.1.165:44847 v:192.168.1.165:1234 d:195.0.0.1:1234 fwd:L s:0 conn->flags:181 conn->refcnt:1 dest->refcnt:3
> >  > [ 2106.897425] IPVS: Schedule fwd:L c:192.168.1.165:44847 v:192.168.1.165:1234 d:195.0.0.1:1234 conn->flags:1C1 conn->refcnt:2
> >  > [ 2106.897429] IPVS: TCP input  [S...] 195.0.0.1:1234->192.168.1.165:44847 state: NONE->SYN_RECV conn->refcnt:2
> >  > [ 2106.897431] IPVS: Enter: ip_vs_null_xmit, net/netfilter/ipvs/ip_vs_xmit.c line 212
> >  > [ 2106.897439] IPVS: lookup/in TCP 192.168.1.165:1234->192.168.1.165:44847 not hit
> >  > [ 2106.897441] IPVS: lookup/out TCP 192.168.1.165:1234->192.168.1.165:44847 not hit
> >  > [ 2107.277535] IPVS: packet type=1 proto=17 daddr=255.255.255.255 ignored
> >  > [ 2108.542691] IPVS: packet type=1 proto=17 daddr=192.168.1.255 ignored
> >  > 
> >  > As the result, server application does receive anything on accept(). I tried to make dummy0 a hidden device and play with arp settings. But without result.
> >  > 
> >  > I will be happy to hear any idea how to do connection in this environment.
> >  
> >  Hi,
> >  
> >  while others have suggested not using LVS for this task for various reasons.
> >  I would just like to comment that this should work and this smells
> >  like a bug to me. I will try and confirm that. But it won't be today.
> >  
> >  
> >  
> 
> With the latest kernel I see that: LVS accepts connections, selects right destination (if round robin is selected destination changes accoring it), then it detects that it is local node and do:
> net/netfilter/ipvs/ip_vs_xmit.c:
>    ip_vs_null_xmit(struct sk_buff *skb, struct ip_vs_conn *cp,
>  struct ip_vs_protocol *pp)
> 
> Which does nothing with skb. (here I do not understand what happens with that packet then)
> I think if VLS could change destination for packets which go from local node to local node then connection can be established. Is it reasonable?

Hi,

sorry for taking a very long time to look into this more closely.
I believe that the problem is that you need to make sure that
the source IP address used by the client (application) is not the VIP
as the VIP also exists on the real-server and thus the real-server will
ignore packets from the VIP.

You also need to make sure that the source address can be receive
packets from the real-server. So you can't use 127.0.0.1.

What I suggest is have a different 192.168.1.x address as the primary
address on eth0 (or whatever the interface in question is) on the
client/linux-director. And add 192.168.1.165 as a secondary address on the
same interface.


^ permalink raw reply

* Re: [PATCH net-next-2.6] net: pskb_expand_head() optimization
From: Jarek Poplawski @ 2010-09-20  7:21 UTC (permalink / raw)
  To: David Miller; +Cc: eric.dumazet, netdev
In-Reply-To: <20100919.171725.245378386.davem@davemloft.net>

On Sun, Sep 19, 2010 at 05:17:25PM -0700, David Miller wrote:
> From: Jarek Poplawski <jarkao2@gmail.com>
> Date: Sat, 11 Sep 2010 14:31:40 +0200
> 
> > Otherwise seems OK, but I still would like to know the scenario
> > demanding this change.
> 
> Ok Jarek, after some more consideration I agree with you.
> Removing this kind of sharing would be unwise, ho hum...
> 
> While pondering over this I thought about why we even need
> frag lists at all.  We need them for two reasons:
> 
> 1) Because we have an inability to turn kmalloc data references into
>    page based ones easily.
> 
>    We've run into this sort of problem with socket splice() too.
> 
> 2) For recording the segmentation points of a fragmented packet.
> 
> We definitely don't use frag lists for performance, if you look
> in the GRO history the GRO code specifically has been changed
> to prefer accumulating into the page vector whenever possible
> because using frag lists is a lot slower.

Yes, and GRO made frag lists more popular btw.

> Anyways, even if we somehow solved #1 we'd still have a lot of
> code (even bluetooth) using it for the sake of issue #2.

Since there exist drivers using entirely paged skbs and napi_gro_frags
path I can't see why #1 or #2 could be unsolvable elsewhere.

> So for the time being there is no clear path for trying to
> eliminate frag lists altogether if we wanted to do that either.

Probably we could start from enhacing moving drivers to paged skbs
where possible. And maybe simplifying the skb model by not allowing
frags and frag lists together?

Btw, I wonder what is the exact reason we can't use only
NET_SKBUFF_DATA_USES_OFFSET?

Jarek P.

^ permalink raw reply

* Re: [RFC PATCH v9 12/16] Add mp(mediate passthru) device.
From: xiaohui.xin @ 2010-09-20  8:08 UTC (permalink / raw)
  To: netdev, kvm, linux-kernel, mingo, davem, herbert, jdike, mst; +Cc: Xin Xiaohui
In-Reply-To: <20100915112811.GB29267@redhat.com>

From: Xin Xiaohui <xiaohui.xin@intel.com>

---
Michael,
I have move the ioctl to configure the locked memory to vhost and 
check the limit with mm->locked_vm. please have a look.

Thanks
Xiaohui

 drivers/vhost/mpassthru.c |   74 +++++++++----------------------------------
 drivers/vhost/net.c       |   78 ++++++++++++++++++++++++++++++++++++++------
 include/linux/vhost.h     |    3 ++
 3 files changed, 85 insertions(+), 70 deletions(-)

diff --git a/drivers/vhost/mpassthru.c b/drivers/vhost/mpassthru.c
index d86d94c..fd3827b 100644
--- a/drivers/vhost/mpassthru.c
+++ b/drivers/vhost/mpassthru.c
@@ -109,9 +109,6 @@ struct page_ctor {
 	int			wq_len;
 	int			rq_len;
 	spinlock_t		read_lock;
-	/* record the locked pages */
-	int			lock_pages;
-	struct rlimit		o_rlim;
 	struct net_device	*dev;
 	struct mpassthru_port	port;
 	struct page_info	**hash_table;
@@ -231,7 +228,6 @@ static int page_ctor_attach(struct mp_struct *mp)
 	ctor->port.ctor = page_ctor;
 	ctor->port.sock = &mp->socket;
 	ctor->port.hash = mp_lookup;
-	ctor->lock_pages = 0;
 
 	/* locked by mp_mutex */
 	dev->mp_port = &ctor->port;
@@ -264,37 +260,6 @@ struct page_info *info_dequeue(struct page_ctor *ctor)
 	return info;
 }
 
-static int set_memlock_rlimit(struct page_ctor *ctor, int resource,
-			      unsigned long cur, unsigned long max)
-{
-	struct rlimit new_rlim, *old_rlim;
-	int retval;
-
-	if (resource != RLIMIT_MEMLOCK)
-		return -EINVAL;
-	new_rlim.rlim_cur = cur;
-	new_rlim.rlim_max = max;
-
-	old_rlim = current->signal->rlim + resource;
-
-	/* remember the old rlimit value when backend enabled */
-	ctor->o_rlim.rlim_cur = old_rlim->rlim_cur;
-	ctor->o_rlim.rlim_max = old_rlim->rlim_max;
-
-	if ((new_rlim.rlim_max > old_rlim->rlim_max) &&
-			!capable(CAP_SYS_RESOURCE))
-		return -EPERM;
-
-	retval = security_task_setrlimit(resource, &new_rlim);
-	if (retval)
-		return retval;
-
-	task_lock(current->group_leader);
-	*old_rlim = new_rlim;
-	task_unlock(current->group_leader);
-	return 0;
-}
-
 static void relinquish_resource(struct page_ctor *ctor)
 {
 	if (!(ctor->dev->flags & IFF_UP) &&
@@ -322,8 +287,6 @@ static void mp_ki_dtor(struct kiocb *iocb)
 		info->ctor->rq_len--;
 	} else
 		info->ctor->wq_len--;
-	/* Decrement the number of locked pages */
-	info->ctor->lock_pages -= info->pnum;
 	kmem_cache_free(ext_page_info_cache, info);
 	relinquish_resource(info->ctor);
 
@@ -349,7 +312,7 @@ static struct kiocb *create_iocb(struct page_info *info, int size)
 	iocb->ki_dtor(iocb);
 	iocb->private = (void *)info;
 	iocb->ki_dtor = mp_ki_dtor;
-
+	iocb->ki_user_data = info->pnum;
 	return iocb;
 }
 
@@ -375,10 +338,6 @@ static int page_ctor_detach(struct mp_struct *mp)
 
 	relinquish_resource(ctor);
 
-	set_memlock_rlimit(ctor, RLIMIT_MEMLOCK,
-			   ctor->o_rlim.rlim_cur,
-			   ctor->o_rlim.rlim_max);
-
 	/* locked by mp_mutex */
 	ctor->dev->mp_port = NULL;
 	dev_put(ctor->dev);
@@ -565,21 +524,23 @@ static struct page_info *alloc_page_info(struct page_ctor *ctor,
 	int rc;
 	int i, j, n = 0;
 	int len;
-	unsigned long base, lock_limit;
+	unsigned long base, lock_limit, locked;
 	struct page_info *info = NULL;
 
-	lock_limit = current->signal->rlim[RLIMIT_MEMLOCK].rlim_cur;
-	lock_limit >>= PAGE_SHIFT;
+	down_write(&current->mm->mmap_sem);
+	locked     = count + current->mm->locked_vm;
+	lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
 
-	if (ctor->lock_pages + count > lock_limit && npages) {
-		printk(KERN_INFO "exceed the locked memory rlimit.");
-		return NULL;
-	}
+	if ((locked > lock_limit) && !capable(CAP_IPC_LOCK))
+		goto out;
 
 	info = kmem_cache_alloc(ext_page_info_cache, GFP_KERNEL);
 	
 	if (!info)
-		return NULL;
+		goto out;
+
+	up_write(&current->mm->mmap_sem);
+
 	info->skb = NULL;
 	info->next = info->prev = NULL;
 
@@ -633,8 +594,7 @@ static struct page_info *alloc_page_info(struct page_ctor *ctor,
 		for (i = 0; i < j; i++)
 			mp_hash_insert(ctor, info->pages[i], info);
 	}
-	/* increment the number of locked pages */
-	ctor->lock_pages += j;
+
 	return info;
 
 failed:
@@ -642,7 +602,9 @@ failed:
 		put_page(info->pages[i]);
 
 	kmem_cache_free(ext_page_info_cache, info);
-
+	return NULL;
+out:
+	up(&current->mm->mmap_sem);
 	return NULL;
 }
 
@@ -1006,12 +968,6 @@ proceed:
 		count--;
 	}
 
-	if (!ctor->lock_pages || !ctor->rq_len) {
-		set_memlock_rlimit(ctor, RLIMIT_MEMLOCK,
-				iocb->ki_user_data * 4096 * 2,
-				iocb->ki_user_data * 4096 * 2);
-	}
-
 	/* Translate address to kernel */
 	info = alloc_page_info(ctor, iocb, iov, count, frags, npages, 0);
 	if (!info)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index c4bc815..da78837 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -42,6 +42,7 @@ enum {
 };
 
 static struct kmem_cache *notify_cache;
+static struct rlimit orig_rlim;
 
 enum vhost_net_poll_state {
 	VHOST_NET_POLL_DISABLED = 0,
@@ -136,13 +137,7 @@ static void handle_async_rx_events_notify(struct vhost_net *net,
 	struct vhost_log *vq_log = NULL;
 	int rx_total_len = 0;
 	unsigned int head, log, in, out;
-	int size;
-	int count;
-
-	struct virtio_net_hdr_mrg_rxbuf hdr = {
-		.hdr.flags = 0,
-		.hdr.gso_type = VIRTIO_NET_HDR_GSO_NONE
-	};
+	int size, free = 0;
 
 	if (!is_async_vq(vq))
 		return;
@@ -160,7 +155,7 @@ static void handle_async_rx_events_notify(struct vhost_net *net,
 			size = iocb->ki_nbytes;
 			head = iocb->ki_pos;
 			rx_total_len += iocb->ki_nbytes;
-
+			free += iocb->ki_user_data;
 			if (iocb->ki_dtor)
 				iocb->ki_dtor(iocb);
 			kmem_cache_free(net->cache, iocb);
@@ -192,6 +187,7 @@ static void handle_async_rx_events_notify(struct vhost_net *net,
 					size = iocb->ki_nbytes;
 					head = iocb->ki_pos;
 					rx_total_len += iocb->ki_nbytes;
+					free += iocb->ki_user_data;
 
 					if (iocb->ki_dtor)
 						iocb->ki_dtor(iocb);
@@ -211,7 +207,6 @@ static void handle_async_rx_events_notify(struct vhost_net *net,
 					break;
 
 				i++;
-				iocb == NULL;
 				if (count)
 					iocb = notify_dequeue(vq);
 			}
@@ -219,6 +214,10 @@ static void handle_async_rx_events_notify(struct vhost_net *net,
 					&net->dev, vq, vq->heads, hc);
 		}
 	}
+	/* record locked memroy */
+	down_write(&current->mm->mmap_sem);
+	current->mm->locked_vm -= free;
+	up_write(&current->mm->mmap_sem);
 }
 
 static void handle_async_tx_events_notify(struct vhost_net *net,
@@ -227,7 +226,7 @@ static void handle_async_tx_events_notify(struct vhost_net *net,
 	struct kiocb *iocb = NULL;
 	struct list_head *entry, *tmp;
 	unsigned long flags;
-	int tx_total_len = 0;
+	int tx_total_len = 0, free = 0;
 
 	if (!is_async_vq(vq))
 		return;
@@ -242,7 +241,7 @@ static void handle_async_tx_events_notify(struct vhost_net *net,
 		vhost_add_used_and_signal(&net->dev, vq,
 				iocb->ki_pos, 0);
 		tx_total_len += iocb->ki_nbytes;
-
+		free += iocb->ki_user_data;
 		if (iocb->ki_dtor)
 			iocb->ki_dtor(iocb);
 
@@ -253,6 +252,10 @@ static void handle_async_tx_events_notify(struct vhost_net *net,
 		}
 	}
 	spin_unlock_irqrestore(&vq->notify_lock, flags);
+	/* record locked memroy */
+	down_write(&current->mm->mmap_sem);
+	current->mm->locked_vm -= free;
+	up_write(&current->mm->mmap_sem);
 }
 
 static struct kiocb *create_iocb(struct vhost_net *net,
@@ -581,6 +584,7 @@ static void handle_rx_net(struct work_struct *work)
 static int vhost_net_open(struct inode *inode, struct file *f)
 {
 	struct vhost_net *n = kmalloc(sizeof *n, GFP_KERNEL);
+	struct rlimit *old_rlim;
 	int r;
 	if (!n)
 		return -ENOMEM;
@@ -597,6 +601,12 @@ static int vhost_net_open(struct inode *inode, struct file *f)
 	n->tx_poll_state = VHOST_NET_POLL_DISABLED;
 	n->cache = NULL;
 
+	old_rlim = current->signal->rlim + RLIMIT_MEMLOCK;
+
+	/* remember the old rlimit value when backend enabled */
+	orig_rlim.rlim_cur = old_rlim->rlim_cur;
+	orig_rlim.rlim_max = old_rlim->rlim_max;
+
 	f->private_data = n;
 
 	return 0;
@@ -659,6 +669,39 @@ static void vhost_net_flush(struct vhost_net *n)
 	vhost_net_flush_vq(n, VHOST_NET_VQ_RX);
 }
 
+static long vhost_net_set_mem_locked(struct vhost_net *n,
+				     unsigned long cur,
+				     unsigned long max)
+{
+	struct rlimit new_rlim, *old_rlim;
+	int retval = 0;
+
+	mutex_lock(&n->dev.mutex);
+	new_rlim.rlim_cur = cur;
+	new_rlim.rlim_max = max;
+
+	old_rlim = current->signal->rlim + RLIMIT_MEMLOCK;
+
+	if ((new_rlim.rlim_max > old_rlim->rlim_max) &&
+			!capable(CAP_SYS_RESOURCE)) {
+		retval = -EPERM;
+		goto err;
+	}
+
+	retval = security_task_setrlimit(RLIMIT_MEMLOCK, &new_rlim);
+	if (retval) {
+		retval = retval;
+		goto err;
+	}
+
+	task_lock(current->group_leader);
+	*old_rlim = new_rlim;
+	task_unlock(current->group_leader);
+err:
+	mutex_unlock(&n->dev.mutex);
+	return retval;
+}
+
 static void vhost_async_cleanup(struct vhost_net *n)
 {
 	/* clean the notifier */
@@ -691,6 +734,10 @@ static int vhost_net_release(struct inode *inode, struct file *f)
 	 * since jobs can re-queue themselves. */
 	vhost_net_flush(n);
 	vhost_async_cleanup(n);
+	/* return back the rlimit */
+	vhost_net_set_mem_locked(n,
+				 orig_rlim.rlim_cur,
+				 orig_rlim.rlim_max);
 	kfree(n);
 	return 0;
 }
@@ -846,6 +893,7 @@ err:
 	return r;
 }
 
+
 static long vhost_net_reset_owner(struct vhost_net *n)
 {
 	struct socket *tx_sock = NULL;
@@ -913,6 +961,7 @@ static long vhost_net_ioctl(struct file *f, unsigned int ioctl,
 	void __user *argp = (void __user *)arg;
 	u64 __user *featurep = argp;
 	struct vhost_vring_file backend;
+	struct rlimit rlim;
 	u64 features;
 	int r;
 	switch (ioctl) {
@@ -933,6 +982,13 @@ static long vhost_net_ioctl(struct file *f, unsigned int ioctl,
 		return vhost_net_set_features(n, features);
 	case VHOST_RESET_OWNER:
 		return vhost_net_reset_owner(n);
+	case VHOST_SET_MEM_LOCKED:
+		r = copy_from_user(&rlim, argp, sizeof rlim);
+		if (r < 0)
+			return r;
+		return vhost_net_set_mem_locked(n,
+						rlim.rlim_cur,
+						rlim.rlim_max);
 	default:
 		mutex_lock(&n->dev.mutex);
 		r = vhost_dev_ioctl(&n->dev, ioctl, arg);
diff --git a/include/linux/vhost.h b/include/linux/vhost.h
index e847f1e..df93f5a 100644
--- a/include/linux/vhost.h
+++ b/include/linux/vhost.h
@@ -92,6 +92,9 @@ struct vhost_memory {
 /* Specify an eventfd file descriptor to signal on log write. */
 #define VHOST_SET_LOG_FD _IOW(VHOST_VIRTIO, 0x07, int)
 
+/* Specify how much locked memory can be used */
+#define VHOST_SET_MEM_LOCKED	_IOW(VHOST_VIRTIO, 0x08, struct rlimit)
+
 /* Ring setup. */
 /* Set number of descriptors in ring. This parameter can not
  * be modified while ring is running (bound to a device). */
-- 
1.5.4.4


^ permalink raw reply related

* Re: [PATCH net-next-2.6] net: pskb_expand_head() optimization
From: Eric Dumazet @ 2010-09-20  9:02 UTC (permalink / raw)
  To: Jarek Poplawski; +Cc: David Miller, netdev
In-Reply-To: <20100920072149.GA6353@ff.dom.local>

Le lundi 20 septembre 2010 à 07:21 +0000, Jarek Poplawski a écrit :

> Probably we could start from enhacing moving drivers to paged skbs
> where possible. And maybe simplifying the skb model by not allowing
> frags and frag lists together?
> 

Sure. I believe current model, pre-allocating skb in huge tx rings is a
waste of mem bandwidth anyway. (I am refering to the struct sk_buff
itself, not the payload part)

Of course some drivers are doing it right, using netdev_alloc_skb()
right before feeding this skb to network stack, not an old one.

> Btw, I wonder what is the exact reason we can't use only
> NET_SKBUFF_DATA_USES_OFFSET?

I see no real reason.

On 32bit arches, it might be faster to manipulate pointers, and not
'base+offset' values.




^ permalink raw reply

* [rfc] IPVS: Masq local real-servers
From: Simon Horman @ 2010-09-20  9:09 UTC (permalink / raw)
  To: lvs-devel, netfilter-devel, netdev
  Cc: Patrick McHardy, Wensong Zhang, Julian Anastasov

IPVS has a special Local forwarding mechanism that is used if the
real-server is a local IP address. Like the Route and Tunnel forwarding
mechanism Local does not allow port mapping, and thus the port of the
real-server is always set to be the same as the virtual service.

The Masq forwarding mechanism does allow port mapping, and this causes some
confusion when the real-server happens to be local.

This patch addresses this confusion by not using the Local forwarding
mechanism if the masq forwarding mechanism is requested. That is, the masq
forwarding mechanism will be used, and the real-servers may have a
different port to the virtual service.

Signed-off-by: Simon Horman <horms@verge.net.au>

--- 

I considered using Local for the case where the real-server and virtual
service ports are the same. However, this would require updating the
real-servers if the port of the virtual-service was changed, however
editing the forwarding mechanism of a real-server currently isn't
supported and the extra complexity for an unmeasured performance gain seems
to be at best left for another patch.

Index: nf-next-2.6/net/netfilter/ipvs/ip_vs_core.c
===================================================================
--- nf-next-2.6.orig/net/netfilter/ipvs/ip_vs_core.c	2010-09-19 20:51:30.000000000 +0900
+++ nf-next-2.6/net/netfilter/ipvs/ip_vs_core.c	2010-09-20 16:30:59.000000000 +0900
@@ -1496,6 +1496,22 @@ static struct nf_hook_ops ip_vs_ops[] __
 		.hooknum        = NF_INET_FORWARD,
 		.priority       = 100,
 	},
+	/* change source only for local VS/NAT */
+	{
+	       .hook           = ip_vs_out,
+	       .owner          = THIS_MODULE,
+	       .pf             = PF_INET,
+	       .hooknum        = NF_INET_LOCAL_OUT,
+	       .priority       = 100,
+	},
+	/* change source only for local VS/NAT */
+	{
+	       .hook           = ip_vs_out,
+	       .owner          = THIS_MODULE,
+	       .pf             = PF_INET,
+	       .hooknum        = NF_INET_LOCAL_OUT,
+	       .priority       = 100,
+	},
 	/* After packet filtering (but before ip_vs_out_icmp), catch icmp
 	 * destined for 0.0.0.0/0, which is for incoming IPVS connections */
 	{
Index: nf-next-2.6/net/netfilter/ipvs/ip_vs_ctl.c
===================================================================
--- nf-next-2.6.orig/net/netfilter/ipvs/ip_vs_ctl.c	2010-09-20 15:07:27.000000000 +0900
+++ nf-next-2.6/net/netfilter/ipvs/ip_vs_ctl.c	2010-09-20 17:45:46.000000000 +0900
@@ -766,7 +766,7 @@ ip_vs_zero_stats(struct ip_vs_stats *sta
  *	Update a destination in the given service
  */
 static void
-__ip_vs_update_dest(struct ip_vs_service *svc, struct ip_vs_dest *dest,
+_ip_vs_update_dest(struct ip_vs_service *svc, struct ip_vs_dest *dest,
 		    struct ip_vs_dest_user_kern *udest, int add)
 {
 	int conn_flags;
@@ -777,18 +777,22 @@ __ip_vs_update_dest(struct ip_vs_service
 	conn_flags |= IP_VS_CONN_F_INACTIVE;
 
 	/* check if local node and update the flags */
+	if ((conn_flags & IP_VS_CONN_F_FWD_MASK) != IP_VS_CONN_F_MASQ ||
+	    svc->fwmark) {
 #ifdef CONFIG_IP_VS_IPV6
-	if (svc->af == AF_INET6) {
-		if (__ip_vs_addr_is_local_v6(&udest->addr.in6)) {
-			conn_flags = (conn_flags & ~IP_VS_CONN_F_FWD_MASK)
-				| IP_VS_CONN_F_LOCALNODE;
-		}
-	} else
+		if (svc->af == AF_INET6) {
+			if (__ip_vs_addr_is_local_v6(&udest->addr.in6)) {
+				conn_flags = (conn_flags &
+					      ~IP_VS_CONN_F_FWD_MASK) |
+					IP_VS_CONN_F_LOCALNODE;
+			}
+		} else
 #endif
 		if (inet_addr_type(&init_net, udest->addr.ip) == RTN_LOCAL) {
 			conn_flags = (conn_flags & ~IP_VS_CONN_F_FWD_MASK)
 				| IP_VS_CONN_F_LOCALNODE;
 		}
+	}
 
 	/* set the IP_VS_CONN_F_NOOUTPUT flag if not masquerading/NAT */
 	if ((conn_flags & IP_VS_CONN_F_FWD_MASK) != IP_VS_CONN_F_MASQ) {

^ permalink raw reply

* Re: [PATCH net-next-2.6] net: pskb_expand_head() optimization
From: Jarek Poplawski @ 2010-09-20  9:14 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, netdev
In-Reply-To: <1284973320.3420.49.camel@edumazet-laptop>

On Mon, Sep 20, 2010 at 11:02:00AM +0200, Eric Dumazet wrote:
> Le lundi 20 septembre 2010 ?? 07:21 +0000, Jarek Poplawski a écrit :
> 
> > Probably we could start from enhacing moving drivers to paged skbs
> > where possible. And maybe simplifying the skb model by not allowing
> > frags and frag lists together?
> > 
> 
> Sure. I believe current model, pre-allocating skb in huge tx rings is a
> waste of mem bandwidth anyway. (I am refering to the struct sk_buff
> itself, not the payload part)
> 
> Of course some drivers are doing it right, using netdev_alloc_skb()
> right before feeding this skb to network stack, not an old one.
> 
> > Btw, I wonder what is the exact reason we can't use only
> > NET_SKBUFF_DATA_USES_OFFSET?
> 
> I see no real reason.
> 
> On 32bit arches, it might be faster to manipulate pointers, and not
> 'base+offset' values.

The only reason is code readability and maintenance. Did anybody check
how much faster?

Jarek P.

^ permalink raw reply

* [PATCH] net: ipv4,nodefrag: check socket type before touching nodefrag flag
From: Jiri Olsa @ 2010-09-20  9:27 UTC (permalink / raw)
  To: eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
	jengelh-nopoi9nDyk+ELgA04lAiVw, kaber-dcUjhNyLwpNeoWH0uzbU5w,
	davem-H+wXaHxf7aLQT0dZR+AlfA
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	netfilter-devel-u79uwXL29TY76Z2rM5mHXA,
	linux-man-u79uwXL29TY76Z2rM5mHXA, Jiri Olsa

hi,
we need to check proper socket type within ipv4_conntrack_defrag
function before referencing the nodefrag flag.

For example the tun driver receive path produces skbs with
AF_UNSPEC socket type, and so current code is causing unwanted
fragmented packets going out.

wbr,
jirka



Signed-off-by: Jiri Olsa <jolsa-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 net/ipv4/netfilter/nf_defrag_ipv4.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/net/ipv4/netfilter/nf_defrag_ipv4.c b/net/ipv4/netfilter/nf_defrag_ipv4.c
index eab8de3..f3a9b42 100644
--- a/net/ipv4/netfilter/nf_defrag_ipv4.c
+++ b/net/ipv4/netfilter/nf_defrag_ipv4.c
@@ -66,9 +66,11 @@ static unsigned int ipv4_conntrack_defrag(unsigned int hooknum,
 					  const struct net_device *out,
 					  int (*okfn)(struct sk_buff *))
 {
+	struct sock *sk = skb->sk;
 	struct inet_sock *inet = inet_sk(skb->sk);
 
-	if (inet && inet->nodefrag)
+	if (sk && (sk->sk_family == PF_INET) &&
+	    inet->nodefrag)
 		return NF_ACCEPT;
 
 #if defined(CONFIG_NF_CONNTRACK) || defined(CONFIG_NF_CONNTRACK_MODULE)
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH net-next-2.6] net: rx_dropped accounting
From: Eric Dumazet @ 2010-09-20 10:12 UTC (permalink / raw)
  To: David Miller; +Cc: netdev

Under load, netif_rx() can drop incoming packets but administrators dont
have a chance to spot which device needs some tuning (RPS activation for
example)

This patch adds rx_dropped accounting in vlans and tunnels.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 net/8021q/vlan.h      |    2 ++
 net/8021q/vlan_dev.c  |    9 +++++++--
 net/ipv4/ip_gre.c     |    6 ++++--
 net/ipv4/ipip.c       |    5 ++++-
 net/ipv6/ip6_tunnel.c |    5 ++++-
 net/ipv6/ip6mr.c      |    4 +++-
 net/ipv6/sit.c        |    5 ++++-
 7 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/net/8021q/vlan.h b/net/8021q/vlan.h
index 8d9503a..b26ce34 100644
--- a/net/8021q/vlan.h
+++ b/net/8021q/vlan.h
@@ -25,6 +25,7 @@ struct vlan_priority_tci_mapping {
  *	@rx_multicast: number of received multicast packets
  *	@syncp: synchronization point for 64bit counters
  *	@rx_errors: number of errors
+ *	@rx_dropped: number of dropped packets
  */
 struct vlan_rx_stats {
 	u64			rx_packets;
@@ -32,6 +33,7 @@ struct vlan_rx_stats {
 	u64			rx_multicast;
 	struct u64_stats_sync	syncp;
 	unsigned long		rx_errors;
+	unsigned long		rx_dropped;
 };
 
 /**
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index 3bccdd1..94a1fed 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -225,7 +225,10 @@ int vlan_skb_recv(struct sk_buff *skb, struct net_device *dev,
 		}
 	}
 
-	netif_rx(skb);
+	if (unlikely(netif_rx(skb) == NET_RX_DROP)) {
+		if (rx_stats)
+			rx_stats->rx_dropped++;
+	}
 	rcu_read_unlock();
 	return NET_RX_SUCCESS;
 
@@ -843,13 +846,15 @@ static struct rtnl_link_stats64 *vlan_dev_get_stats64(struct net_device *dev, st
 			accum.rx_packets += rxpackets;
 			accum.rx_bytes   += rxbytes;
 			accum.rx_multicast += rxmulticast;
-			/* rx_errors is an ulong, not protected by syncp */
+			/* rx_errors, rx_dropped are ulong, not protected by syncp */
 			accum.rx_errors  += p->rx_errors;
+			accum.rx_dropped += p->rx_dropped;
 		}
 		stats->rx_packets = accum.rx_packets;
 		stats->rx_bytes   = accum.rx_bytes;
 		stats->rx_errors  = accum.rx_errors;
 		stats->multicast  = accum.rx_multicast;
+		stats->rx_dropped = accum.rx_dropped;
 	}
 	return stats;
 }
diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index fc20e68..8d53362 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -647,9 +647,11 @@ static int ipgre_rcv(struct sk_buff *skb)
 		skb_reset_network_header(skb);
 		ipgre_ecn_decapsulate(iph, skb);
 
-		netif_rx(skb);
+		if (netif_rx(skb) == NET_RX_DROP)
+			stats->rx_dropped++;
+
 		rcu_read_unlock();
-		return(0);
+		return 0;
 	}
 	icmp_send(skb, ICMP_DEST_UNREACH, ICMP_PORT_UNREACH, 0);
 
diff --git a/net/ipv4/ipip.c b/net/ipv4/ipip.c
index 8de8888..babd252 100644
--- a/net/ipv4/ipip.c
+++ b/net/ipv4/ipip.c
@@ -377,7 +377,10 @@ static int ipip_rcv(struct sk_buff *skb)
 		skb_tunnel_rx(skb, tunnel->dev);
 
 		ipip_ecn_decapsulate(iph, skb);
-		netif_rx(skb);
+
+		if (netif_rx(skb) == NET_RX_DROP)
+			tunnel->dev->stats.rx_dropped++;
+
 		rcu_read_unlock();
 		return 0;
 	}
diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
index 9289cec..f6d9f68 100644
--- a/net/ipv6/ip6_tunnel.c
+++ b/net/ipv6/ip6_tunnel.c
@@ -725,7 +725,10 @@ static int ip6_tnl_rcv(struct sk_buff *skb, __u16 protocol,
 		skb_tunnel_rx(skb, t->dev);
 
 		dscp_ecn_decapsulate(t, ipv6h, skb);
-		netif_rx(skb);
+
+		if (netif_rx(skb) == NET_RX_DROP)
+			t->dev->stats.rx_dropped++;
+
 		rcu_read_unlock();
 		return 0;
 	}
diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
index 66078da..2640c9b 100644
--- a/net/ipv6/ip6mr.c
+++ b/net/ipv6/ip6mr.c
@@ -666,7 +666,9 @@ static int pim6_rcv(struct sk_buff *skb)
 
 	skb_tunnel_rx(skb, reg_dev);
 
-	netif_rx(skb);
+	if (netif_rx(skb) == NET_RX_DROP)
+		reg_dev->stats.rx_dropped++;
+
 	dev_put(reg_dev);
 	return 0;
  drop:
diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
index 6822481..8a03998 100644
--- a/net/ipv6/sit.c
+++ b/net/ipv6/sit.c
@@ -564,7 +564,10 @@ static int ipip6_rcv(struct sk_buff *skb)
 		skb_tunnel_rx(skb, tunnel->dev);
 
 		ipip6_ecn_decapsulate(iph, skb);
-		netif_rx(skb);
+
+		if (netif_rx(skb) == NET_RX_DROP)
+			tunnel->dev->stats.rx_dropped++;
+
 		rcu_read_unlock();
 		return 0;
 	}



^ permalink raw reply related

* RE: [PATCH] qlcnic: dont assume NET_IP_ALIGN is 2
From: Amit Salecha @ 2010-09-20 11:16 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: netdev@vger.kernel.org, Ameen Rahman, Anirban Chakraborty,
	David Miller
In-Reply-To: <1284717448.3391.75.camel@edumazet-laptop>



> -----Original Message-----
> From: Eric Dumazet [mailto:eric.dumazet@gmail.com]
> Sent: Friday, September 17, 2010 3:27 PM
> To: Amit Salecha
> Cc: netdev@vger.kernel.org; Ameen Rahman; Anirban Chakraborty; David
> Miller
> Subject: [PATCH] qlcnic: dont assume NET_IP_ALIGN is 2
> 
> Amit, I noticed following bug in qlnic driver.
> 
> Also, skb->truesize should not be changed by drivers, unless dealing
> with fragments.
> 
> When you have :
> 	skb->truesize = skb->len + sizeof(struct sk_buff);
> 
> you are lying to stack that can queue many "small" UDP packets,
> accouting for small packets in socket rcvbuf, while the truesize was
> really 1532 + sizeof(struct sk_buff)
> 
> Could you take a look ?
> 
> 
This can be cleaned up. 
Though I have one doubt. We are allocating larger packets than the actual data used.
Doesn't it will break accounting ?

-Amit


^ permalink raw reply

* Re: [RFC PATCH v9 12/16] Add mp(mediate passthru) device.
From: Michael S. Tsirkin @ 2010-09-20 11:36 UTC (permalink / raw)
  To: xiaohui.xin; +Cc: netdev, kvm, linux-kernel, mingo, davem, herbert, jdike
In-Reply-To: <1284970128-7343-1-git-send-email-xiaohui.xin@intel.com>

On Mon, Sep 20, 2010 at 04:08:48PM +0800, xiaohui.xin@intel.com wrote:
> From: Xin Xiaohui <xiaohui.xin@intel.com>
> 
> ---
> Michael,
> I have move the ioctl to configure the locked memory to vhost

It's ok to move this to vhost but vhost does not
know how much memory is needed by the backend.
So I think we'll need another ioctl in the backend
to tell userspace how much memory is needed?

It seems a bit cleaner as a backend ioctl as vhost
does not lock memory itself, but I am not
principally opposed.

> and 
> check the limit with mm->locked_vm. please have a look.
> 
> Thanks
> Xiaohui
> 
>  drivers/vhost/mpassthru.c |   74 +++++++++----------------------------------
>  drivers/vhost/net.c       |   78 ++++++++++++++++++++++++++++++++++++++------
>  include/linux/vhost.h     |    3 ++
>  3 files changed, 85 insertions(+), 70 deletions(-)
> 
> diff --git a/drivers/vhost/mpassthru.c b/drivers/vhost/mpassthru.c
> index d86d94c..fd3827b 100644
> --- a/drivers/vhost/mpassthru.c
> +++ b/drivers/vhost/mpassthru.c
> @@ -109,9 +109,6 @@ struct page_ctor {
>  	int			wq_len;
>  	int			rq_len;
>  	spinlock_t		read_lock;
> -	/* record the locked pages */
> -	int			lock_pages;
> -	struct rlimit		o_rlim;
>  	struct net_device	*dev;
>  	struct mpassthru_port	port;
>  	struct page_info	**hash_table;
> @@ -231,7 +228,6 @@ static int page_ctor_attach(struct mp_struct *mp)
>  	ctor->port.ctor = page_ctor;
>  	ctor->port.sock = &mp->socket;
>  	ctor->port.hash = mp_lookup;
> -	ctor->lock_pages = 0;
>  
>  	/* locked by mp_mutex */
>  	dev->mp_port = &ctor->port;
> @@ -264,37 +260,6 @@ struct page_info *info_dequeue(struct page_ctor *ctor)
>  	return info;
>  }
>  
> -static int set_memlock_rlimit(struct page_ctor *ctor, int resource,
> -			      unsigned long cur, unsigned long max)
> -{
> -	struct rlimit new_rlim, *old_rlim;
> -	int retval;
> -
> -	if (resource != RLIMIT_MEMLOCK)
> -		return -EINVAL;
> -	new_rlim.rlim_cur = cur;
> -	new_rlim.rlim_max = max;
> -
> -	old_rlim = current->signal->rlim + resource;
> -
> -	/* remember the old rlimit value when backend enabled */
> -	ctor->o_rlim.rlim_cur = old_rlim->rlim_cur;
> -	ctor->o_rlim.rlim_max = old_rlim->rlim_max;
> -
> -	if ((new_rlim.rlim_max > old_rlim->rlim_max) &&
> -			!capable(CAP_SYS_RESOURCE))
> -		return -EPERM;
> -
> -	retval = security_task_setrlimit(resource, &new_rlim);
> -	if (retval)
> -		return retval;
> -
> -	task_lock(current->group_leader);
> -	*old_rlim = new_rlim;
> -	task_unlock(current->group_leader);
> -	return 0;
> -}
> -
>  static void relinquish_resource(struct page_ctor *ctor)
>  {
>  	if (!(ctor->dev->flags & IFF_UP) &&
> @@ -322,8 +287,6 @@ static void mp_ki_dtor(struct kiocb *iocb)
>  		info->ctor->rq_len--;
>  	} else
>  		info->ctor->wq_len--;
> -	/* Decrement the number of locked pages */
> -	info->ctor->lock_pages -= info->pnum;
>  	kmem_cache_free(ext_page_info_cache, info);
>  	relinquish_resource(info->ctor);
>  
> @@ -349,7 +312,7 @@ static struct kiocb *create_iocb(struct page_info *info, int size)
>  	iocb->ki_dtor(iocb);
>  	iocb->private = (void *)info;
>  	iocb->ki_dtor = mp_ki_dtor;
> -
> +	iocb->ki_user_data = info->pnum;
>  	return iocb;
>  }
>  
> @@ -375,10 +338,6 @@ static int page_ctor_detach(struct mp_struct *mp)
>  
>  	relinquish_resource(ctor);
>  
> -	set_memlock_rlimit(ctor, RLIMIT_MEMLOCK,
> -			   ctor->o_rlim.rlim_cur,
> -			   ctor->o_rlim.rlim_max);
> -
>  	/* locked by mp_mutex */
>  	ctor->dev->mp_port = NULL;
>  	dev_put(ctor->dev);
> @@ -565,21 +524,23 @@ static struct page_info *alloc_page_info(struct page_ctor *ctor,
>  	int rc;
>  	int i, j, n = 0;
>  	int len;
> -	unsigned long base, lock_limit;
> +	unsigned long base, lock_limit, locked;
>  	struct page_info *info = NULL;
>  
> -	lock_limit = current->signal->rlim[RLIMIT_MEMLOCK].rlim_cur;
> -	lock_limit >>= PAGE_SHIFT;
> +	down_write(&current->mm->mmap_sem);
> +	locked     = count + current->mm->locked_vm;
> +	lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
>  
> -	if (ctor->lock_pages + count > lock_limit && npages) {
> -		printk(KERN_INFO "exceed the locked memory rlimit.");
> -		return NULL;
> -	}
> +	if ((locked > lock_limit) && !capable(CAP_IPC_LOCK))
> +		goto out;
>  
>  	info = kmem_cache_alloc(ext_page_info_cache, GFP_KERNEL);
>  	
>  	if (!info)
> -		return NULL;
> +		goto out;
> +
> +	up_write(&current->mm->mmap_sem);
> +
>  	info->skb = NULL;
>  	info->next = info->prev = NULL;
>  

Sorry I wasnt clear, I didn't really mean we copy everything
from infiniband, just the capability checks and locked_mm use.
These guys don't do registration on data path so
they can play games with locked_mm etc on registration.
But lock on data path, taking mmap sem and doing security checks
on data path would be bad for performance.
I would expect this to cause contention especially
as we'll go for multiqueue.

Here's what I really meant:
	SET_MEM_LOCKED gets a 32 bit integer (or a 64 bit
	if you like - just not long).
	the meaning of which is "this is how much
	memory device can lock".
	This ioctl does rlim_cur and capability checks,
	if passed immediately increments locked_vm counter
	by the *maximum amount specified*.
	Device must store the value by which we incremented
	locked_vm and the mm pointer (if this is vhost ioctl
	it has the owner already). Let's call this
	field lock_limit.


	Lock limit can also take into account
	e.g. device tx queue depth and our queue size.
	Either we give another ioctl that tells userspace
	about these and let it make the decision,
	or simply cap lock_limit ourselves
	depending on these parameters.

	If another SET_MEM_LOCKED ioctl is made,
	decrement lcoked_vm in the stored mm,
	and redo the operation on current->mm
	(note: might be different!).

	This ioctl should probably fail if backend is active
	(already has locked some pages), such an
	approach makes it easy as we do not need to
	find and unlock pages.

	Each time you want to lock some memory you check that
	1. current->mm matches the stored mm.
	2. (number of pages locked + amount we want to lock) * PAGE_SIZE <= lock_limit.


	close and RESET_OWNER decrement and drop mm reference
	(note: on close
	we decrement owner's locked_vm, not current->mm
	which might be different).




> @@ -633,8 +594,7 @@ static struct page_info *alloc_page_info(struct page_ctor *ctor,
>  		for (i = 0; i < j; i++)
>  			mp_hash_insert(ctor, info->pages[i], info);
>  	}
> -	/* increment the number of locked pages */
> -	ctor->lock_pages += j;
> +
>  	return info;
>  
>  failed:
> @@ -642,7 +602,9 @@ failed:
>  		put_page(info->pages[i]);
>  
>  	kmem_cache_free(ext_page_info_cache, info);
> -
> +	return NULL;
> +out:
> +	up(&current->mm->mmap_sem);
>  	return NULL;
>  }
>  
> @@ -1006,12 +968,6 @@ proceed:
>  		count--;
>  	}
>  
> -	if (!ctor->lock_pages || !ctor->rq_len) {
> -		set_memlock_rlimit(ctor, RLIMIT_MEMLOCK,
> -				iocb->ki_user_data * 4096 * 2,
> -				iocb->ki_user_data * 4096 * 2);
> -	}
> -
>  	/* Translate address to kernel */
>  	info = alloc_page_info(ctor, iocb, iov, count, frags, npages, 0);
>  	if (!info)
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index c4bc815..da78837 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -42,6 +42,7 @@ enum {
>  };
>  
>  static struct kmem_cache *notify_cache;
> +static struct rlimit orig_rlim;
>  
>  enum vhost_net_poll_state {
>  	VHOST_NET_POLL_DISABLED = 0,
> @@ -136,13 +137,7 @@ static void handle_async_rx_events_notify(struct vhost_net *net,
>  	struct vhost_log *vq_log = NULL;
>  	int rx_total_len = 0;
>  	unsigned int head, log, in, out;
> -	int size;
> -	int count;
> -
> -	struct virtio_net_hdr_mrg_rxbuf hdr = {
> -		.hdr.flags = 0,
> -		.hdr.gso_type = VIRTIO_NET_HDR_GSO_NONE
> -	};
> +	int size, free = 0;
>  
>  	if (!is_async_vq(vq))
>  		return;
> @@ -160,7 +155,7 @@ static void handle_async_rx_events_notify(struct vhost_net *net,
>  			size = iocb->ki_nbytes;
>  			head = iocb->ki_pos;
>  			rx_total_len += iocb->ki_nbytes;
> -
> +			free += iocb->ki_user_data;
>  			if (iocb->ki_dtor)
>  				iocb->ki_dtor(iocb);
>  			kmem_cache_free(net->cache, iocb);
> @@ -192,6 +187,7 @@ static void handle_async_rx_events_notify(struct vhost_net *net,
>  					size = iocb->ki_nbytes;
>  					head = iocb->ki_pos;
>  					rx_total_len += iocb->ki_nbytes;
> +					free += iocb->ki_user_data;
>  
>  					if (iocb->ki_dtor)
>  						iocb->ki_dtor(iocb);
> @@ -211,7 +207,6 @@ static void handle_async_rx_events_notify(struct vhost_net *net,
>  					break;
>  
>  				i++;
> -				iocb == NULL;
>  				if (count)
>  					iocb = notify_dequeue(vq);
>  			}
> @@ -219,6 +214,10 @@ static void handle_async_rx_events_notify(struct vhost_net *net,
>  					&net->dev, vq, vq->heads, hc);
>  		}
>  	}
> +	/* record locked memroy */
> +	down_write(&current->mm->mmap_sem);
> +	current->mm->locked_vm -= free;
> +	up_write(&current->mm->mmap_sem);
>  }
>  
>  static void handle_async_tx_events_notify(struct vhost_net *net,
> @@ -227,7 +226,7 @@ static void handle_async_tx_events_notify(struct vhost_net *net,
>  	struct kiocb *iocb = NULL;
>  	struct list_head *entry, *tmp;
>  	unsigned long flags;
> -	int tx_total_len = 0;
> +	int tx_total_len = 0, free = 0;
>  
>  	if (!is_async_vq(vq))
>  		return;
> @@ -242,7 +241,7 @@ static void handle_async_tx_events_notify(struct vhost_net *net,
>  		vhost_add_used_and_signal(&net->dev, vq,
>  				iocb->ki_pos, 0);
>  		tx_total_len += iocb->ki_nbytes;
> -
> +		free += iocb->ki_user_data;
>  		if (iocb->ki_dtor)
>  			iocb->ki_dtor(iocb);
>  
> @@ -253,6 +252,10 @@ static void handle_async_tx_events_notify(struct vhost_net *net,
>  		}
>  	}
>  	spin_unlock_irqrestore(&vq->notify_lock, flags);
> +	/* record locked memroy */
> +	down_write(&current->mm->mmap_sem);
> +	current->mm->locked_vm -= free;
> +	up_write(&current->mm->mmap_sem);
>  }
>  
>  static struct kiocb *create_iocb(struct vhost_net *net,
> @@ -581,6 +584,7 @@ static void handle_rx_net(struct work_struct *work)
>  static int vhost_net_open(struct inode *inode, struct file *f)
>  {
>  	struct vhost_net *n = kmalloc(sizeof *n, GFP_KERNEL);
> +	struct rlimit *old_rlim;
>  	int r;
>  	if (!n)
>  		return -ENOMEM;
> @@ -597,6 +601,12 @@ static int vhost_net_open(struct inode *inode, struct file *f)
>  	n->tx_poll_state = VHOST_NET_POLL_DISABLED;
>  	n->cache = NULL;
>  
> +	old_rlim = current->signal->rlim + RLIMIT_MEMLOCK;
> +
> +	/* remember the old rlimit value when backend enabled */
> +	orig_rlim.rlim_cur = old_rlim->rlim_cur;
> +	orig_rlim.rlim_max = old_rlim->rlim_max;
> +
>  	f->private_data = n;
>  
>  	return 0;
> @@ -659,6 +669,39 @@ static void vhost_net_flush(struct vhost_net *n)
>  	vhost_net_flush_vq(n, VHOST_NET_VQ_RX);
>  }
>  
> +static long vhost_net_set_mem_locked(struct vhost_net *n,
> +				     unsigned long cur,
> +				     unsigned long max)
> +{

So one issue here is that when this is called on close,
current might be different from owner, with bad results.

I really think avoiding modifying rlimit is
the simplest way to go for now.

> +	struct rlimit new_rlim, *old_rlim;
> +	int retval = 0;
> +
> +	mutex_lock(&n->dev.mutex);
> +	new_rlim.rlim_cur = cur;
> +	new_rlim.rlim_max = max;
> +
> +	old_rlim = current->signal->rlim + RLIMIT_MEMLOCK;
> +
> +	if ((new_rlim.rlim_max > old_rlim->rlim_max) &&
> +			!capable(CAP_SYS_RESOURCE)) {
> +		retval = -EPERM;
> +		goto err;
> +	}
> +
> +	retval = security_task_setrlimit(RLIMIT_MEMLOCK, &new_rlim);
> +	if (retval) {
> +		retval = retval;
> +		goto err;
> +	}
> +
> +	task_lock(current->group_leader);
> +	*old_rlim = new_rlim;
> +	task_unlock(current->group_leader);
> +err:
> +	mutex_unlock(&n->dev.mutex);
> +	return retval;
> +}
> +
>  static void vhost_async_cleanup(struct vhost_net *n)
>  {
>  	/* clean the notifier */
> @@ -691,6 +734,10 @@ static int vhost_net_release(struct inode *inode, struct file *f)
>  	 * since jobs can re-queue themselves. */
>  	vhost_net_flush(n);
>  	vhost_async_cleanup(n);
> +	/* return back the rlimit */
> +	vhost_net_set_mem_locked(n,
> +				 orig_rlim.rlim_cur,
> +				 orig_rlim.rlim_max);
>  	kfree(n);
>  	return 0;
>  }
> @@ -846,6 +893,7 @@ err:
>  	return r;
>  }
>  
> +
>  static long vhost_net_reset_owner(struct vhost_net *n)
>  {
>  	struct socket *tx_sock = NULL;
> @@ -913,6 +961,7 @@ static long vhost_net_ioctl(struct file *f, unsigned int ioctl,
>  	void __user *argp = (void __user *)arg;
>  	u64 __user *featurep = argp;
>  	struct vhost_vring_file backend;
> +	struct rlimit rlim;
>  	u64 features;
>  	int r;
>  	switch (ioctl) {
> @@ -933,6 +982,13 @@ static long vhost_net_ioctl(struct file *f, unsigned int ioctl,
>  		return vhost_net_set_features(n, features);
>  	case VHOST_RESET_OWNER:
>  		return vhost_net_reset_owner(n);
> +	case VHOST_SET_MEM_LOCKED:
> +		r = copy_from_user(&rlim, argp, sizeof rlim);
> +		if (r < 0)
> +			return r;
> +		return vhost_net_set_mem_locked(n,
> +						rlim.rlim_cur,
> +						rlim.rlim_max);
>  	default:
>  		mutex_lock(&n->dev.mutex);
>  		r = vhost_dev_ioctl(&n->dev, ioctl, arg);
> diff --git a/include/linux/vhost.h b/include/linux/vhost.h
> index e847f1e..df93f5a 100644
> --- a/include/linux/vhost.h
> +++ b/include/linux/vhost.h
> @@ -92,6 +92,9 @@ struct vhost_memory {
>  /* Specify an eventfd file descriptor to signal on log write. */
>  #define VHOST_SET_LOG_FD _IOW(VHOST_VIRTIO, 0x07, int)
>  
> +/* Specify how much locked memory can be used */
> +#define VHOST_SET_MEM_LOCKED	_IOW(VHOST_VIRTIO, 0x08, struct rlimit)
> +

This is not a good structure to use: its size varies between
64 and 32 bit. rlimit64 would be better.
Also, you will have to include resource.h from here.

>  /* Ring setup. */
>  /* Set number of descriptors in ring. This parameter can not
>   * be modified while ring is running (bound to a device). */
> -- 
> 1.5.4.4

^ permalink raw reply

* [PATCH] xfrm: Allow different selector family in temporary state
From: Steffen Klassert @ 2010-09-20 11:52 UTC (permalink / raw)
  To: David Miller, Herbert Xu, Thomas Egerer; +Cc: netdev

From: Thomas Egerer <thomas.egerer@secunet.com>

The family parameter xfrm_state_find is used to find a state matching a
certain policy. This value is set to the template's family
(encap_family) right before xfrm_state_find is called.
The family parameter is however also used to construct a temporary state
in xfrm_state_find itself which is wrong for inter-family scenarios
because it produces a selector for the wrong family. Since this selector
is included in the xfrm_user_acquire structure, user space programs
misinterpret IPv6 addresses as IPv4 and vice versa.
This patch splits up the original init_tempsel function into a part that
initializes the selector respectively the props and id of the temporary
state, to allow for differing ip address families whithin the state.

Signed-off-by: Thomas Egerer <thomas.egerer@secunet.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 include/net/xfrm.h     |    4 ++--
 net/ipv4/xfrm4_state.c |   33 +++++++++++++++++++--------------
 net/ipv6/xfrm6_state.c |   33 +++++++++++++++++++--------------
 net/xfrm/xfrm_policy.c |    5 ++---
 net/xfrm/xfrm_state.c  |   45 +++++++++++++++++++++++++++------------------
 5 files changed, 69 insertions(+), 51 deletions(-)

diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index fc8f36d..4f53532 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -298,8 +298,8 @@ struct xfrm_state_afinfo {
 	const struct xfrm_type	*type_map[IPPROTO_MAX];
 	struct xfrm_mode	*mode_map[XFRM_MODE_MAX];
 	int			(*init_flags)(struct xfrm_state *x);
-	void			(*init_tempsel)(struct xfrm_state *x, struct flowi *fl,
-						struct xfrm_tmpl *tmpl,
+	void			(*init_tempsel)(struct xfrm_selector *sel, struct flowi *fl);
+	void			(*init_temprop)(struct xfrm_state *x, struct xfrm_tmpl *tmpl,
 						xfrm_address_t *daddr, xfrm_address_t *saddr);
 	int			(*tmpl_sort)(struct xfrm_tmpl **dst, struct xfrm_tmpl **src, int n);
 	int			(*state_sort)(struct xfrm_state **dst, struct xfrm_state **src, int n);
diff --git a/net/ipv4/xfrm4_state.c b/net/ipv4/xfrm4_state.c
index 1ef1366..4794762 100644
--- a/net/ipv4/xfrm4_state.c
+++ b/net/ipv4/xfrm4_state.c
@@ -21,21 +21,25 @@ static int xfrm4_init_flags(struct xfrm_state *x)
 }
 
 static void
-__xfrm4_init_tempsel(struct xfrm_state *x, struct flowi *fl,
-		     struct xfrm_tmpl *tmpl,
-		     xfrm_address_t *daddr, xfrm_address_t *saddr)
+__xfrm4_init_tempsel(struct xfrm_selector *sel, struct flowi *fl)
+{
+	sel->daddr.a4 = fl->fl4_dst;
+	sel->saddr.a4 = fl->fl4_src;
+	sel->dport = xfrm_flowi_dport(fl);
+	sel->dport_mask = htons(0xffff);
+	sel->sport = xfrm_flowi_sport(fl);
+	sel->sport_mask = htons(0xffff);
+	sel->family = AF_INET;
+	sel->prefixlen_d = 32;
+	sel->prefixlen_s = 32;
+	sel->proto = fl->proto;
+	sel->ifindex = fl->oif;
+}
+
+static void
+xfrm4_init_temprop(struct xfrm_state *x, struct xfrm_tmpl *tmpl,
+		   xfrm_address_t *daddr, xfrm_address_t *saddr)
 {
-	x->sel.daddr.a4 = fl->fl4_dst;
-	x->sel.saddr.a4 = fl->fl4_src;
-	x->sel.dport = xfrm_flowi_dport(fl);
-	x->sel.dport_mask = htons(0xffff);
-	x->sel.sport = xfrm_flowi_sport(fl);
-	x->sel.sport_mask = htons(0xffff);
-	x->sel.family = AF_INET;
-	x->sel.prefixlen_d = 32;
-	x->sel.prefixlen_s = 32;
-	x->sel.proto = fl->proto;
-	x->sel.ifindex = fl->oif;
 	x->id = tmpl->id;
 	if (x->id.daddr.a4 == 0)
 		x->id.daddr.a4 = daddr->a4;
@@ -70,6 +74,7 @@ static struct xfrm_state_afinfo xfrm4_state_afinfo = {
 	.owner			= THIS_MODULE,
 	.init_flags		= xfrm4_init_flags,
 	.init_tempsel		= __xfrm4_init_tempsel,
+	.init_temprop		= xfrm4_init_temprop,
 	.output			= xfrm4_output,
 	.extract_input		= xfrm4_extract_input,
 	.extract_output		= xfrm4_extract_output,
diff --git a/net/ipv6/xfrm6_state.c b/net/ipv6/xfrm6_state.c
index f417b77..a67575d 100644
--- a/net/ipv6/xfrm6_state.c
+++ b/net/ipv6/xfrm6_state.c
@@ -20,23 +20,27 @@
 #include <net/addrconf.h>
 
 static void
-__xfrm6_init_tempsel(struct xfrm_state *x, struct flowi *fl,
-		     struct xfrm_tmpl *tmpl,
-		     xfrm_address_t *daddr, xfrm_address_t *saddr)
+__xfrm6_init_tempsel(struct xfrm_selector *sel, struct flowi *fl)
 {
 	/* Initialize temporary selector matching only
 	 * to current session. */
-	ipv6_addr_copy((struct in6_addr *)&x->sel.daddr, &fl->fl6_dst);
-	ipv6_addr_copy((struct in6_addr *)&x->sel.saddr, &fl->fl6_src);
-	x->sel.dport = xfrm_flowi_dport(fl);
-	x->sel.dport_mask = htons(0xffff);
-	x->sel.sport = xfrm_flowi_sport(fl);
-	x->sel.sport_mask = htons(0xffff);
-	x->sel.family = AF_INET6;
-	x->sel.prefixlen_d = 128;
-	x->sel.prefixlen_s = 128;
-	x->sel.proto = fl->proto;
-	x->sel.ifindex = fl->oif;
+	ipv6_addr_copy((struct in6_addr *)&sel->daddr, &fl->fl6_dst);
+	ipv6_addr_copy((struct in6_addr *)&sel->saddr, &fl->fl6_src);
+	sel->dport = xfrm_flowi_dport(fl);
+	sel->dport_mask = htons(0xffff);
+	sel->sport = xfrm_flowi_sport(fl);
+	sel->sport_mask = htons(0xffff);
+	sel->family = AF_INET6;
+	sel->prefixlen_d = 128;
+	sel->prefixlen_s = 128;
+	sel->proto = fl->proto;
+	sel->ifindex = fl->oif;
+}
+
+static void
+xfrm6_init_temprop(struct xfrm_state *x, struct xfrm_tmpl *tmpl,
+		   xfrm_address_t *daddr, xfrm_address_t *saddr)
+{
 	x->id = tmpl->id;
 	if (ipv6_addr_any((struct in6_addr*)&x->id.daddr))
 		memcpy(&x->id.daddr, daddr, sizeof(x->sel.daddr));
@@ -168,6 +172,7 @@ static struct xfrm_state_afinfo xfrm6_state_afinfo = {
 	.eth_proto		= htons(ETH_P_IPV6),
 	.owner			= THIS_MODULE,
 	.init_tempsel		= __xfrm6_init_tempsel,
+	.init_temprop		= xfrm6_init_temprop,
 	.tmpl_sort		= __xfrm6_tmpl_sort,
 	.state_sort		= __xfrm6_state_sort,
 	.output			= xfrm6_output,
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 2b3ed7a..cbab6e1 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -1175,9 +1175,8 @@ xfrm_tmpl_resolve_one(struct xfrm_policy *policy, struct flowi *fl,
 		    tmpl->mode == XFRM_MODE_BEET) {
 			remote = &tmpl->id.daddr;
 			local = &tmpl->saddr;
-			family = tmpl->encap_family;
-			if (xfrm_addr_any(local, family)) {
-				error = xfrm_get_saddr(net, &tmp, remote, family);
+			if (xfrm_addr_any(local, tmpl->encap_family)) {
+				error = xfrm_get_saddr(net, &tmp, remote, tmpl->encap_family);
 				if (error)
 					goto fail;
 				local = &tmp;
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index 5208b12..eb96ce5 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -656,15 +656,23 @@ void xfrm_sad_getinfo(struct net *net, struct xfrmk_sadinfo *si)
 EXPORT_SYMBOL(xfrm_sad_getinfo);
 
 static int
-xfrm_init_tempsel(struct xfrm_state *x, struct flowi *fl,
-		  struct xfrm_tmpl *tmpl,
-		  xfrm_address_t *daddr, xfrm_address_t *saddr,
-		  unsigned short family)
+xfrm_init_tempstate(struct xfrm_state *x, struct flowi *fl,
+		    struct xfrm_tmpl *tmpl,
+		    xfrm_address_t *daddr, xfrm_address_t *saddr,
+		    unsigned short family)
 {
 	struct xfrm_state_afinfo *afinfo = xfrm_state_get_afinfo(family);
 	if (!afinfo)
 		return -1;
-	afinfo->init_tempsel(x, fl, tmpl, daddr, saddr);
+	afinfo->init_tempsel(&x->sel, fl);
+
+	if (family != tmpl->encap_family) {
+		xfrm_state_put_afinfo(afinfo);
+		afinfo = xfrm_state_get_afinfo(tmpl->encap_family);
+		if (!afinfo)
+			return -1;
+	}
+	afinfo->init_temprop(x, tmpl, daddr, saddr);
 	xfrm_state_put_afinfo(afinfo);
 	return 0;
 }
@@ -790,37 +798,38 @@ xfrm_state_find(xfrm_address_t *daddr, xfrm_address_t *saddr,
 	int error = 0;
 	struct xfrm_state *best = NULL;
 	u32 mark = pol->mark.v & pol->mark.m;
+	unsigned short encap_family = tmpl->encap_family;
 
 	to_put = NULL;
 
 	spin_lock_bh(&xfrm_state_lock);
-	h = xfrm_dst_hash(net, daddr, saddr, tmpl->reqid, family);
+	h = xfrm_dst_hash(net, daddr, saddr, tmpl->reqid, encap_family);
 	hlist_for_each_entry(x, entry, net->xfrm.state_bydst+h, bydst) {
-		if (x->props.family == family &&
+		if (x->props.family == encap_family &&
 		    x->props.reqid == tmpl->reqid &&
 		    (mark & x->mark.m) == x->mark.v &&
 		    !(x->props.flags & XFRM_STATE_WILDRECV) &&
-		    xfrm_state_addr_check(x, daddr, saddr, family) &&
+		    xfrm_state_addr_check(x, daddr, saddr, encap_family) &&
 		    tmpl->mode == x->props.mode &&
 		    tmpl->id.proto == x->id.proto &&
 		    (tmpl->id.spi == x->id.spi || !tmpl->id.spi))
-			xfrm_state_look_at(pol, x, fl, family, daddr, saddr,
+			xfrm_state_look_at(pol, x, fl, encap_family, daddr, saddr,
 					   &best, &acquire_in_progress, &error);
 	}
 	if (best)
 		goto found;
 
-	h_wildcard = xfrm_dst_hash(net, daddr, &saddr_wildcard, tmpl->reqid, family);
+	h_wildcard = xfrm_dst_hash(net, daddr, &saddr_wildcard, tmpl->reqid, encap_family);
 	hlist_for_each_entry(x, entry, net->xfrm.state_bydst+h_wildcard, bydst) {
-		if (x->props.family == family &&
+		if (x->props.family == encap_family &&
 		    x->props.reqid == tmpl->reqid &&
 		    (mark & x->mark.m) == x->mark.v &&
 		    !(x->props.flags & XFRM_STATE_WILDRECV) &&
-		    xfrm_state_addr_check(x, daddr, saddr, family) &&
+		    xfrm_state_addr_check(x, daddr, saddr, encap_family) &&
 		    tmpl->mode == x->props.mode &&
 		    tmpl->id.proto == x->id.proto &&
 		    (tmpl->id.spi == x->id.spi || !tmpl->id.spi))
-			xfrm_state_look_at(pol, x, fl, family, daddr, saddr,
+			xfrm_state_look_at(pol, x, fl, encap_family, daddr, saddr,
 					   &best, &acquire_in_progress, &error);
 	}
 
@@ -829,7 +838,7 @@ found:
 	if (!x && !error && !acquire_in_progress) {
 		if (tmpl->id.spi &&
 		    (x0 = __xfrm_state_lookup(net, mark, daddr, tmpl->id.spi,
-					      tmpl->id.proto, family)) != NULL) {
+					      tmpl->id.proto, encap_family)) != NULL) {
 			to_put = x0;
 			error = -EEXIST;
 			goto out;
@@ -839,9 +848,9 @@ found:
 			error = -ENOMEM;
 			goto out;
 		}
-		/* Initialize temporary selector matching only
+		/* Initialize temporary state matching only
 		 * to current session. */
-		xfrm_init_tempsel(x, fl, tmpl, daddr, saddr, family);
+		xfrm_init_tempstate(x, fl, tmpl, daddr, saddr, family);
 		memcpy(&x->mark, &pol->mark, sizeof(x->mark));
 
 		error = security_xfrm_state_alloc_acquire(x, pol->security, fl->secid);
@@ -856,10 +865,10 @@ found:
 			x->km.state = XFRM_STATE_ACQ;
 			list_add(&x->km.all, &net->xfrm.state_all);
 			hlist_add_head(&x->bydst, net->xfrm.state_bydst+h);
-			h = xfrm_src_hash(net, daddr, saddr, family);
+			h = xfrm_src_hash(net, daddr, saddr, encap_family);
 			hlist_add_head(&x->bysrc, net->xfrm.state_bysrc+h);
 			if (x->id.spi) {
-				h = xfrm_spi_hash(net, &x->id.daddr, x->id.spi, x->id.proto, family);
+				h = xfrm_spi_hash(net, &x->id.daddr, x->id.spi, x->id.proto, encap_family);
 				hlist_add_head(&x->byspi, net->xfrm.state_byspi+h);
 			}
 			x->lft.hard_add_expires_seconds = net->xfrm.sysctl_acq_expires;
-- 
1.7.0.4


^ permalink raw reply related

* Re: [PATCH net-next-2.6] net: pskb_expand_head() optimization
From: Jarek Poplawski @ 2010-09-20 12:12 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, netdev
In-Reply-To: <20100920091425.GA8381@ff.dom.local>

On Mon, Sep 20, 2010 at 09:14:25AM +0000, Jarek Poplawski wrote:
> On Mon, Sep 20, 2010 at 11:02:00AM +0200, Eric Dumazet wrote:
> > Le lundi 20 septembre 2010 ?? 07:21 +0000, Jarek Poplawski a écrit :
> > > Btw, I wonder what is the exact reason we can't use only
> > > NET_SKBUFF_DATA_USES_OFFSET?
> > 
> > I see no real reason.
> > 
> > On 32bit arches, it might be faster to manipulate pointers, and not
> > 'base+offset' values.
> 
> The only reason is code readability and maintenance. Did anybody check
> how much faster?

Hmm... I probably misread your reasoning. So, if no real reason,
really, how about turning this NET_SKBUFF_DATA_USES_OFFSET on
unconditionally in net-next, until somebody spots the difference?

Jarek P.

^ permalink raw reply

* RE: [PATCH] qlcnic: dont assume NET_IP_ALIGN is 2
From: Eric Dumazet @ 2010-09-20 12:18 UTC (permalink / raw)
  To: Amit Salecha
  Cc: netdev@vger.kernel.org, Ameen Rahman, Anirban Chakraborty,
	David Miller
In-Reply-To: <99737F4847ED0A48AECC9F4A1974A4B80F86F8018D@MNEXMB2.qlogic.org>

Le lundi 20 septembre 2010 à 06:16 -0500, Amit Salecha a écrit :

> This can be cleaned up. 
> Though I have one doubt. We are allocating larger packets than the actual data used.
> Doesn't it will break accounting ?

truesize accounts for the real size of buffers, not the used part in
them.

IMHO, a driver not dealing with fragments should not touch skb->truesize

# grep truesize drivers/net/tg3.c
<nothing>

If a driver deals with fragments, it probably should use "+=" operator
only, not hardcoding sizeof(struct sk_buff) thing that only core network
has to deal with.

$ find drivers/net/bnx2x|xargs grep truesize
drivers/net/bnx2x/bnx2x_cmn.c:		skb->truesize += frag_len;

Almost all drivers are fine, they are some of them that should be
changed.




^ permalink raw reply

* [PATCH net-next-2.6] qlnic: dont set skb->truesize
From: Eric Dumazet @ 2010-09-20 12:28 UTC (permalink / raw)
  To: Amit Salecha
  Cc: netdev@vger.kernel.org, Ameen Rahman, Anirban Chakraborty,
	David Miller
In-Reply-To: <1284985118.3420.270.camel@edumazet-laptop>

Le lundi 20 septembre 2010 à 14:18 +0200, Eric Dumazet a écrit :
> Le lundi 20 septembre 2010 à 06:16 -0500, Amit Salecha a écrit :
> 
> > This can be cleaned up. 
> > Though I have one doubt. We are allocating larger packets than the actual data used.
> > Doesn't it will break accounting ?
> 
> truesize accounts for the real size of buffers, not the used part in
> them.
> 
> IMHO, a driver not dealing with fragments should not touch skb->truesize
> 
> # grep truesize drivers/net/tg3.c
> <nothing>
> 
> If a driver deals with fragments, it probably should use "+=" operator
> only, not hardcoding sizeof(struct sk_buff) thing that only core network
> has to deal with.
> 
> $ find drivers/net/bnx2x|xargs grep truesize
> drivers/net/bnx2x/bnx2x_cmn.c:		skb->truesize += frag_len;
> 
> Almost all drivers are fine, they are some of them that should be
> changed.
> 
> 

I suspect following patch should be fine :

[PATCH net-next-2.6] qlnic: dont set skb->truesize

skb->truesize is set in core network.

Dont change it unless dealing with fragments.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 drivers/net/qlcnic/qlcnic_init.c |    6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/net/qlcnic/qlcnic_init.c b/drivers/net/qlcnic/qlcnic_init.c
index e26fa95..16dd9eb 100644
--- a/drivers/net/qlcnic/qlcnic_init.c
+++ b/drivers/net/qlcnic/qlcnic_init.c
@@ -1418,8 +1418,6 @@ qlcnic_process_rcv(struct qlcnic_adapter *adapter,
 	if (pkt_offset)
 		skb_pull(skb, pkt_offset);
 
-	skb->truesize = skb->len + sizeof(struct sk_buff);
-
 	if (unlikely(qlcnic_check_rx_tagging(adapter, skb, &vid))) {
 		adapter->stats.rxdropped++;
 		dev_kfree_skb(skb);
@@ -1491,8 +1489,6 @@ qlcnic_process_lro(struct qlcnic_adapter *adapter,
 
 	skb_put(skb, lro_length + data_offset);
 
-	skb->truesize = skb->len + sizeof(struct sk_buff) + skb_headroom(skb);
-
 	skb_pull(skb, l2_hdr_offset);
 
 	if (unlikely(qlcnic_check_rx_tagging(adapter, skb, &vid))) {
@@ -1732,8 +1728,6 @@ qlcnic_process_rcv_diag(struct qlcnic_adapter *adapter,
 	if (pkt_offset)
 		skb_pull(skb, pkt_offset);
 
-	skb->truesize = skb->len + sizeof(struct sk_buff);
-
 	if (!qlcnic_check_loopback_buff(skb->data))
 		adapter->diag_cnt++;
 



^ permalink raw reply related

* Re: [PATCH net-next-2.6] net: pskb_expand_head() optimization
From: Eric Dumazet @ 2010-09-20 12:40 UTC (permalink / raw)
  To: Jarek Poplawski; +Cc: David Miller, netdev
In-Reply-To: <20100920121232.GA9931@ff.dom.local>

Le lundi 20 septembre 2010 à 12:12 +0000, Jarek Poplawski a écrit :

> Hmm... I probably misread your reasoning. So, if no real reason,
> really, how about turning this NET_SKBUFF_DATA_USES_OFFSET on
> unconditionally in net-next, until somebody spots the difference?

Yes, but most developpers use 64bit kernels anyway, I suspect nobody
will ever notice :(

Here (with a typical config), here is the vmlinux size before and after
this patch :

$ size vmlinux.old
   text	   data	    bss	    dec	    hex	filename
6061748	 640208	7285056	13987012	 d56cc4	vmlinux.old

$ size vmlinux
   text	   data	    bss	    dec	    hex	filename
6070420	 640208	7285056	13995684	 d58ea4	vmlinux


Thats 8672 bytes of text increase.

(1330326 instructions instead of 1328472 -> 1854 more instructions)




^ permalink raw reply

* [RFC][PATCH 1/3] IRQ: Add irq_get_numa_node()
From: Ben Hutchings @ 2010-09-20 12:44 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: David Miller, therbert, eric.dumazet, shemminger, netdev
In-Reply-To: <20100919172451.GA12878@redhat.com>

This will be used to support NUMA-aware memory allocation of
structures used in interrupt handlers, starting with network drivers.
---
irq_get_numa_node() probably needs to grab desc->lock.  Other than that,
this should work.

Ben.

 include/linux/interrupt.h |    9 +++++++++
 kernel/irq/manage.c       |   35 +++++++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 0 deletions(-)

diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index a0384a4..82e9a08 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -260,6 +260,15 @@ static inline int irq_set_affinity_hint(unsigned int irq,
 }
 #endif /* CONFIG_SMP && CONFIG_GENERIC_HARDIRQS */
 
+#if defined(CONFIG_NUMA) && defined(CONFIG_GENERIC_HARDIRQS)
+extern int irq_get_numa_node(unsigned int irq);
+#else
+static inline int irq_get_numa_node(unsigned int irq)
+{
+	return -1;
+}
+#endif
+
 #ifdef CONFIG_GENERIC_HARDIRQS
 /*
  * Special lockdep variants of irq disabling/enabling.
diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index c3003e9..03e683e 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -154,6 +154,41 @@ int irq_set_affinity_hint(unsigned int irq, const struct cpumask *m)
 }
 EXPORT_SYMBOL_GPL(irq_set_affinity_hint);
 
+#ifdef CONFIG_NUMA
+/**
+ *	irq_get_numa_node - get the NUMA node of a given IRQ
+ *	@irq:		Interrupt to get NUMA node for
+ *
+ *	If the current SMP affinity mask of the IRQ corresponds to a
+ *	single NUMA node, return the node number.  Otherwise return
+ *	%NUMA_NO_NODE.
+ */
+int irq_get_numa_node(unsigned int irq)
+{
+	struct irq_desc *desc = irq_to_desc(irq);
+	const struct cpumask *mask;
+	int cpu, node = NUMA_NO_NODE;
+
+	if (!desc)
+		return node;
+
+	mask = desc->affinity;
+#ifdef CONFIG_GENERIC_PENDING_IRQ
+	if (desc->status & IRQ_MOVE_PENDING)
+		mask = desc->pending_mask;
+#endif
+
+	for_each_cpu(cpu, mask) {
+		if (node < 0)
+			node = cpu_to_node(cpu);
+		else if (node != cpu_to_node(cpu))
+			return NUMA_NO_NODE;
+	}
+
+	return node;
+}
+#endif
+
 #ifndef CONFIG_AUTO_IRQ_AFFINITY
 /*
  * Generic version of the affinity autoselector.
-- 
1.7.2.1



-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


^ permalink raw reply related

* [RFC][PATCH 2/3] ethtool: NUMA affinity control
From: Ben Hutchings @ 2010-09-20 12:45 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: David Miller, therbert, eric.dumazet, shemminger, netdev
In-Reply-To: <20100919172451.GA12878@redhat.com>

Define operations to get and set the numbers of channels belonging to
a net device.  A channel is defined as the combination of an IRQ and
all the queues that can trigger that IRQ.  Channels are identified by
type and index, similarly to the naming scheme used for IRQ handlers.

Define operations to get and set the NUMA affinity of objects
associated with a channel.
---
 include/linux/ethtool.h |   56 ++++++++++++++++
 net/core/ethtool.c      |  165 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 221 insertions(+), 0 deletions(-)

diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index b67af60..0eaae5d 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -517,6 +517,47 @@ struct ethtool_flash {
 	char	data[ETHTOOL_FLASH_MAX_FILENAME];
 };
 
+/* Network channel information.  A network channel is the combination of
+ * an IRQ and all the queues that can trigger that IRQ. */
+struct ethtool_channels {
+	__u32	cmd;
+	__u32	combined_count;		/* number of multi-purpose channels */
+	__u32	rx_count;		/* number of RX-only channels */
+	__u32	tx_count;		/* number of TX-only channels */
+	__u32	other_count;		/* number of additional channels */
+};
+
+/* Channel ID is made up of a type and an index */
+enum ethtool_channel_id {
+	ETH_CHAN_INDEX_MASK	= 0x0fffffff,
+	ETH_CHAN_TYPE_MASK	= 0xc0000000,
+	ETH_CHAN_TYPE_COMBINED	= 0x00000000,
+	ETH_CHAN_TYPE_RX	= 0x40000000,
+	ETH_CHAN_TYPE_TX	= 0x80000000,
+	ETH_CHAN_TYPE_OTHER	= 0xc0000000,
+	ETH_CHAN_ALL		= 0xffffffff	/* special: operate on all */
+};
+
+/* Special NUMA node IDs */
+enum ethtool_numa_node {
+	ETH_NUMA_NODE_UNSET	= -1,	/* get/set: no affinity set */
+	ETH_NUMA_NODE_N_A	= -2,	/* get/set: not applicable; channel
+					 * doesn't have this object */
+	ETH_NUMA_NODE_IRQ	= -3,	/* set: match current IRQ affinity */
+	ETH_NUMA_NODE_DEV	= -4,	/* set: match device affinity */
+};
+
+struct ethtool_affinity {
+	__u32	cmd;
+	__u32	channel_id;		/* channel type and index; may be
+					 * ETH_CHAN_ALL when setting */
+	__s32	rx_ring_node;		/* affinity of RX descriptor ring */
+	__s32	tx_ring_node;		/* affinity of TX descriptor ring */
+	__s32	event_ring_node;	/* affinity of event/completion ring */
+	__s32	handler_data_node;	/* affinity of IRQ/NAPI handler's
+					 * software structures */
+};
+
 #ifdef __KERNEL__
 
 #include <linux/rculist.h>
@@ -551,6 +592,9 @@ int ethtool_op_set_ufo(struct net_device *dev, u32 data);
 u32 ethtool_op_get_flags(struct net_device *dev);
 int ethtool_op_set_flags(struct net_device *dev, u32 data, u32 supported);
 void ethtool_ntuple_flush(struct net_device *dev);
+#ifdef CONFIG_NUMA
+int ethtool_affinity_resolve(s32 node_id, struct net_device *dev, unsigned irq);
+#endif
 
 /**
  * &ethtool_ops - Alter and report network device settings
@@ -672,6 +716,14 @@ struct ethtool_ops {
 				  struct ethtool_rxfh_indir *);
 	int	(*set_rxfh_indir)(struct net_device *,
 				  const struct ethtool_rxfh_indir *);
+	int	(*get_channels)(struct net_device *, struct ethtool_channels *);
+	int	(*set_channels)(struct net_device *,
+				const struct ethtool_channels *);
+#ifdef CONFIG_NUMA
+	int	(*get_affinity)(struct net_device *, struct ethtool_affinity *);
+	int	(*set_affinity)(struct net_device *,
+				const struct ethtool_affinity *);
+#endif
 };
 #endif /* __KERNEL__ */
 
@@ -735,6 +787,10 @@ struct ethtool_ops {
 #define ETHTOOL_GSSET_INFO	0x00000037 /* Get string set info */
 #define ETHTOOL_GRXFHINDIR	0x00000038 /* Get RX flow hash indir'n table */
 #define ETHTOOL_SRXFHINDIR	0x00000039 /* Set RX flow hash indir'n table */
+#define ETHTOOL_GCHANNELS	0x0000003a /* Get numbers of channels */
+#define ETHTOOL_SCHANNELS	0x0000003b /* Set numbers of channels */
+#define ETHTOOL_GAFFINITY	0x0000003c /* Get NUMA affinity */
+#define ETHTOOL_SAFFINITY	0x0000003d /* Set NUMA affinity */
 
 /* compatibility with older code */
 #define SPARC_ETH_GSET		ETHTOOL_GSET
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 34fae15..753a186 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -20,6 +20,8 @@
 #include <linux/bitops.h>
 #include <linux/uaccess.h>
 #include <linux/slab.h>
+#include <linux/interrupt.h>
+#include <linux/numa.h>
 
 /*
  * Some useful ethtool_ops methods that're device independent.
@@ -1429,6 +1431,155 @@ static noinline_for_stack int ethtool_flash_device(struct net_device *dev,
 	return dev->ethtool_ops->flash_device(dev, &efl);
 }
 
+static noinline_for_stack int ethtool_get_channels(struct net_device *dev,
+						   char __user *useraddr)
+{
+	struct ethtool_channels channels;
+	int rc;
+
+	if (!dev->ethtool_ops->get_channels)
+		return -EOPNOTSUPP;
+
+	if (copy_from_user(&channels, useraddr, sizeof(channels)))
+		return -EFAULT;
+
+	rc = dev->ethtool_ops->get_channels(dev, &channels);
+	if (rc)
+		return rc;
+
+	if (copy_to_user(useraddr, &channels, sizeof(channels)))
+		return -EFAULT;
+
+	return 0;
+}
+
+static noinline_for_stack int ethtool_set_channels(struct net_device *dev,
+						   char __user *useraddr)
+{
+	struct ethtool_channels channels;
+
+	if (!dev->ethtool_ops->set_channels)
+		return -EOPNOTSUPP;
+
+	if (copy_from_user(&channels, useraddr, sizeof(channels)))
+		return -EFAULT;
+
+	return dev->ethtool_ops->set_channels(dev, &channels);
+}
+
+#ifdef CONFIG_NUMA
+
+static int ethtool_check_channel_id(struct net_device *dev, u32 channel_id)
+{
+	struct ethtool_channels channels;
+	u32 channel_count;
+	int rc;
+
+	if (!dev->ethtool_ops->get_channels)
+		return -EOPNOTSUPP;
+
+	memset(&channels, 0, sizeof(channels));
+	channels.cmd = ETHTOOL_GCHANNELS;
+	rc = dev->ethtool_ops->get_channels(dev, &channels);
+	if (rc)
+		return rc;
+
+	switch (channel_id & ETH_CHAN_TYPE_MASK) {
+	case ETH_CHAN_TYPE_COMBINED:
+		channel_count = channels.combined_count;
+		break;
+	case ETH_CHAN_TYPE_RX:
+		channel_count = channels.rx_count;
+		break;
+	case ETH_CHAN_TYPE_TX:
+		channel_count = channels.tx_count;
+		break;
+	case ETH_CHAN_TYPE_OTHER:
+	default:
+		channel_count = channels.other_count;
+		break;
+	}
+	if ((channel_id & ETH_CHAN_INDEX_MASK) >= channel_count)
+		return -EINVAL;
+
+	return 0;
+}
+
+static noinline_for_stack int ethtool_get_affinity(struct net_device *dev,
+						   char __user *useraddr)
+{
+	struct ethtool_affinity affin;
+	int rc;
+
+	if (!dev->ethtool_ops->get_affinity)
+		return -EOPNOTSUPP;
+
+	if (copy_from_user(&affin, useraddr, sizeof(affin)))
+		return -EFAULT;
+
+	rc = ethtool_check_channel_id(dev, affin.channel_id);
+	if (rc)
+		return rc;
+
+	rc = dev->ethtool_ops->get_affinity(dev, &affin);
+	if (rc)
+		return rc;
+
+	if (copy_to_user(useraddr, &affin, sizeof(affin)))
+		return -EFAULT;
+
+	return 0;
+}
+
+static noinline_for_stack int ethtool_set_affinity(struct net_device *dev,
+						   char __user *useraddr)
+{
+	struct ethtool_affinity affin;
+	int rc;
+
+	if (!dev->ethtool_ops->set_affinity)
+		return -EOPNOTSUPP;
+
+	if (copy_from_user(&affin, useraddr, sizeof(affin)))
+		return -EFAULT;
+
+	if (affin.channel_id != ETH_CHAN_ALL) {
+		rc = ethtool_check_channel_id(dev, affin.channel_id);
+		if (rc)
+			return rc;
+	}
+
+	return dev->ethtool_ops->set_affinity(dev, &affin);
+}
+
+/**
+ * ethtool_affinity_resolve - resolve the NUMA node ID for a channel
+ * @node_id:		User-specified node ID
+ * @dev:		The channel's net device
+ * @irq:		The channel's IRQ
+ *
+ * This resolves the special node IDs %ETH_NUMA_NODE_IRQ,
+ * %ETH_NUMA_NODE_DEV and %ETH_NUMA_NODE_UNSET and validates that any
+ * other specified node ID is in the valid range.  It returns either a
+ * specific node ID, %NUMA_NO_NODE or a negative error code (less than
+ * %NUMA_NO_NODE).
+ */
+int ethtool_affinity_resolve(s32 node_id, struct net_device *dev, unsigned irq)
+{
+	if (node_id == ETH_NUMA_NODE_IRQ)
+		return irq_get_numa_node(irq);
+	if (node_id == ETH_NUMA_NODE_DEV && dev->dev.parent)
+		return dev_to_node(dev->dev.parent);
+	if (node_id == ETH_NUMA_NODE_UNSET)
+		return NUMA_NO_NODE;
+	if (node_id >= 0 && node_id < MAX_NUMNODES && nr_cpus_node(node_id))
+		return node_id;
+	return -EINVAL;
+}
+EXPORT_SYMBOL(ethtool_affinity_resolve);
+
+#endif /* CONFIG_NUMA */
+
 /* The main entry point in this file.  Called from net/core/dev.c */
 
 int dev_ethtool(struct net *net, struct ifreq *ifr)
@@ -1673,6 +1824,20 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
 	case ETHTOOL_SRXFHINDIR:
 		rc = ethtool_set_rxfh_indir(dev, useraddr);
 		break;
+	case ETHTOOL_GCHANNELS:
+		rc = ethtool_get_channels(dev, useraddr);
+		break;
+	case ETHTOOL_SCHANNELS:
+		rc = ethtool_set_channels(dev, useraddr);
+		break;
+#ifdef CONFIG_NUMA
+	case ETHTOOL_GAFFINITY:
+		rc = ethtool_get_affinity(dev, useraddr);
+		break;
+	case ETHTOOL_SAFFINITY:
+		rc = ethtool_set_affinity(dev, useraddr);
+		break;
+#endif
 	default:
 		rc = -EOPNOTSUPP;
 	}
-- 
1.7.2.1



-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


^ permalink raw reply related

* [RFC][PATCH 3/3] sfc: Add support for NUMA affinity control
From: Ben Hutchings @ 2010-09-20 12:48 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: David Miller, therbert, eric.dumazet, shemminger, netdev
In-Reply-To: <20100919172451.GA12878@redhat.com>

Allow channel structures and hardware queues to be reallocated with
specific affinity.
---
The reallocation code is admittedly rather complicated and ugly.

Ben.

 drivers/net/sfc/efx.c        |  115 ++++++++++++++++++++++++++++++++++++------
 drivers/net/sfc/efx.h        |    4 +-
 drivers/net/sfc/ethtool.c    |   72 ++++++++++++++++++++++++++-
 drivers/net/sfc/net_driver.h |   12 ++++
 drivers/net/sfc/nic.c        |   47 ++++++++++++-----
 5 files changed, 220 insertions(+), 30 deletions(-)

diff --git a/drivers/net/sfc/efx.c b/drivers/net/sfc/efx.c
index f702f1f..a116d2a 100644
--- a/drivers/net/sfc/efx.c
+++ b/drivers/net/sfc/efx.c
@@ -416,10 +416,74 @@ static void efx_remove_eventq(struct efx_channel *channel)
  *
  *************************************************************************/
 
+static struct efx_channel *
+efx_clone_channel_with_affinity(struct efx_nic *efx,
+				struct efx_channel *old_channel,
+				const struct ethtool_affinity *affin)
+{
+#ifdef CONFIG_NUMA
+	struct net_device *net_dev = efx->net_dev;
+	struct efx_channel *channel;
+	unsigned irq = (efx->interrupt_mode == EFX_INT_MODE_LEGACY ?
+			efx->legacy_irq : old_channel->irq);
+	int node_id;
+
+	node_id = ethtool_affinity_resolve(affin->handler_data_node,
+					   net_dev, irq);
+	if (node_id < NUMA_NO_NODE)
+		return ERR_PTR(node_id);
+			
+	channel = kmalloc_node(sizeof(*channel), GFP_KERNEL, node_id);
+	if (!channel)
+		return ERR_PTR(-ENOMEM);
+	*channel = *old_channel;
+	channel->channel_node = node_id;
+
+	if (efx_channel_get_rx_queue(old_channel)) {
+		node_id = ethtool_affinity_resolve(affin->rx_ring_node,
+						   net_dev, irq);
+		if (node_id < NUMA_NO_NODE)
+			goto fail;
+		channel->rxq_node = node_id;
+	} else {
+		if (affin->rx_ring_node != ETH_NUMA_NODE_N_A &&
+		    affin->rx_ring_node != ETH_NUMA_NODE_UNSET)
+			return ERR_PTR(-EINVAL);
+	}
+
+	if (efx_channel_get_tx_queue(old_channel, 0)) {
+		node_id = ethtool_affinity_resolve(affin->tx_ring_node,
+						   net_dev, irq);
+		if (node_id < NUMA_NO_NODE)
+			goto fail;
+		channel->txq_node = node_id;
+	} else {
+		if (affin->tx_ring_node != ETH_NUMA_NODE_N_A &&
+		    affin->tx_ring_node != ETH_NUMA_NODE_UNSET)
+			return ERR_PTR(-EINVAL);
+	}
+
+	node_id = ethtool_affinity_resolve(affin->event_ring_node,
+					   net_dev, irq);
+	if (node_id < NUMA_NO_NODE)
+		goto fail;
+	channel->evq_node = node_id;
+
+	return channel;
+
+fail:
+	kfree(channel);
+	return ERR_PTR(node_id);
+#else /* !CONFIG_NUMA */
+	return ERR_PTR(-EOPNOTSUPP);
+#endif /* CONFIG_NUMA */
+}
+
 /* Allocate and initialise a channel structure, optionally copying
  * parameters (but not resources) from an old channel structure. */
 static struct efx_channel *
-efx_alloc_channel(struct efx_nic *efx, int i, struct efx_channel *old_channel)
+efx_alloc_channel(struct efx_nic *efx, int i, struct efx_channel *old_channel,
+		  const struct ethtool_affinity *new_affin)
 {
 	struct efx_channel *channel;
 	struct efx_rx_queue *rx_queue;
@@ -427,11 +491,18 @@ efx_alloc_channel(struct efx_nic *efx, int i, struct efx_channel *old_channel)
 	int j;
 
 	if (old_channel) {
-		channel = kmalloc(sizeof(*channel), GFP_KERNEL);
-		if (!channel)
-			return NULL;
-
-		*channel = *old_channel;
+		if (new_affin) {
+			channel = efx_clone_channel_with_affinity(
+				efx, old_channel, new_affin);
+			if (IS_ERR(channel))
+				return channel;
+		} else {
+			channel = kmalloc_node(sizeof(*channel), GFP_KERNEL,
+					       old_channel->channel_node);
+			if (!channel)
+				return ERR_PTR(-ENOMEM);
+			*channel = *old_channel;
+		}
 
 		memset(&channel->eventq, 0, sizeof(channel->eventq));
 
@@ -449,10 +520,13 @@ efx_alloc_channel(struct efx_nic *efx, int i, struct efx_channel *old_channel)
 	} else {
 		channel = kzalloc(sizeof(*channel), GFP_KERNEL);
 		if (!channel)
-			return NULL;
+			return ERR_PTR(-ENOMEM);
 
 		channel->efx = efx;
 		channel->channel = i;
+		channel->rxq_node = channel->txq_node =
+			dev_to_node(&efx->pci_dev->dev);
+		channel->evq_node = channel->channel_node = numa_node_id();
 
 		for (j = 0; j < EFX_TXQ_TYPES; j++) {
 			tx_queue = &channel->tx_queue[j];
@@ -707,7 +781,9 @@ static void efx_remove_channels(struct efx_nic *efx)
 }
 
 int
-efx_realloc_channels(struct efx_nic *efx, u32 rxq_entries, u32 txq_entries)
+efx_realloc_channels(struct efx_nic *efx, u32 rxq_entries, u32 txq_entries,
+		     struct efx_channel *affin_channel,
+		     const struct ethtool_affinity *affin)
 {
 	struct efx_channel *other_channel[EFX_MAX_CHANNELS], *channel;
 	u32 old_rxq_entries, old_txq_entries;
@@ -720,9 +796,13 @@ efx_realloc_channels(struct efx_nic *efx, u32 rxq_entries, u32 txq_entries)
 	/* Clone channels */
 	memset(other_channel, 0, sizeof(other_channel));
 	for (i = 0; i < efx->n_channels; i++) {
-		channel = efx_alloc_channel(efx, i, efx->channel[i]);
-		if (!channel) {
-			rc = -ENOMEM;
+		const struct ethtool_affinity *new_affin =
+			(affin_channel == NULL ||
+			 affin_channel == efx->channel[i]) ? affin : NULL;
+
+		channel = efx_alloc_channel(efx, i, efx->channel[i], new_affin);
+		if (IS_ERR(channel)) {
+			rc = PTR_ERR(channel);
 			goto out;
 		}
 		other_channel[i] = channel;
@@ -1281,13 +1361,16 @@ static void efx_set_channels(struct efx_nic *efx)
 	unsigned tx_channel_offset =
 		separate_tx_channels ? efx->n_channels - efx->n_tx_channels : 0;
 
-	/* Channel pointers were set in efx_init_struct() but we now
-	 * need to clear them for TX queues in any RX-only channels. */
+	/* Invalidate pointers and node IDs for unused RX and TX queue
+	 * structures */
 	efx_for_each_channel(channel, efx) {
+		if (channel->channel >= efx->n_rx_channels)
+			channel->rxq_node = ETH_NUMA_NODE_N_A;
 		if (channel->channel - tx_channel_offset >=
 		    efx->n_tx_channels) {
 			efx_for_each_channel_tx_queue(tx_queue, channel)
 				tx_queue->channel = NULL;
+			channel->txq_node = ETH_NUMA_NODE_N_A;
 		}
 	}
 }
@@ -2198,6 +2281,7 @@ static struct efx_phy_operations efx_dummy_phy_operations = {
 static int efx_init_struct(struct efx_nic *efx, struct efx_nic_type *type,
 			   struct pci_dev *pci_dev, struct net_device *net_dev)
 {
+	struct efx_channel *channel;
 	int i;
 
 	/* Initialise common structures */
@@ -2226,9 +2310,10 @@ static int efx_init_struct(struct efx_nic *efx, struct efx_nic_type *type,
 	INIT_WORK(&efx->mac_work, efx_mac_work);
 
 	for (i = 0; i < EFX_MAX_CHANNELS; i++) {
-		efx->channel[i] = efx_alloc_channel(efx, i, NULL);
-		if (!efx->channel[i])
+		channel = efx_alloc_channel(efx, i, NULL, NULL);
+		if (IS_ERR(channel))
 			goto fail;
+		efx->channel[i] = channel;
 	}
 
 	efx->type = type;
diff --git a/drivers/net/sfc/efx.h b/drivers/net/sfc/efx.h
index e783c0f..8baad6f 100644
--- a/drivers/net/sfc/efx.h
+++ b/drivers/net/sfc/efx.h
@@ -67,7 +67,9 @@ extern void efx_schedule_slow_fill(struct efx_rx_queue *rx_queue);
 /* Channels */
 extern void efx_process_channel_now(struct efx_channel *channel);
 extern int
-efx_realloc_channels(struct efx_nic *efx, u32 rxq_entries, u32 txq_entries);
+efx_realloc_channels(struct efx_nic *efx, u32 rxq_entries, u32 txq_entries,
+		     struct efx_channel *channel,
+		     const struct ethtool_affinity *affin);
 
 /* Ports */
 extern int efx_reconfigure_port(struct efx_nic *efx);
diff --git a/drivers/net/sfc/ethtool.c b/drivers/net/sfc/ethtool.c
index 7f735d8..4029819 100644
--- a/drivers/net/sfc/ethtool.c
+++ b/drivers/net/sfc/ethtool.c
@@ -775,7 +775,8 @@ static int efx_ethtool_set_ringparam(struct net_device *net_dev,
 		return -EINVAL;
 	}
 
-	return efx_realloc_channels(efx, ring->rx_pending, ring->tx_pending);
+	return efx_realloc_channels(efx, ring->rx_pending, ring->tx_pending,
+				    NULL, NULL);
 }
 
 static int efx_ethtool_set_pauseparam(struct net_device *net_dev,
@@ -993,6 +994,70 @@ static int efx_ethtool_set_rxfh_indir(struct net_device *net_dev,
 	return 0;
 }
 
+static int efx_ethtool_get_channels(struct net_device *net_dev,
+				    struct ethtool_channels *channels)
+{
+	struct efx_nic *efx = netdev_priv(net_dev);
+
+	channels->rx_count = efx->n_channels - efx->n_tx_channels;
+	channels->tx_count = efx->n_channels - efx->n_rx_channels;
+	channels->combined_count = (efx->n_channels - channels->rx_count -
+				    channels->tx_count);
+	channels->other_count = 0;
+	return 0;
+}
+
+#ifdef CONFIG_NUMA
+
+static struct efx_channel *
+efx_channel_from_id(struct efx_nic *efx, u32 channel_id)
+{
+	u32 channel_index = channel_id & ETH_CHAN_INDEX_MASK;
+
+	switch (channel_id & ETH_CHAN_TYPE_MASK) {
+	case ETH_CHAN_TYPE_RX:
+		return efx_get_channel(efx, channel_index);
+	case ETH_CHAN_TYPE_COMBINED:
+		return efx_get_channel(efx,
+				       efx->n_channels - efx->n_tx_channels +
+				       channel_index);
+	case ETH_CHAN_TYPE_TX:
+		return efx_get_channel(efx, efx->n_rx_channels + channel_index);
+	default:
+		BUG();
+	}
+}
+
+static int efx_ethtool_get_affinity(struct net_device *net_dev,
+				    struct ethtool_affinity *affin)
+{
+	struct efx_nic *efx = netdev_priv(net_dev);
+	struct efx_channel *channel =
+		efx_channel_from_id(efx, affin->channel_id);
+
+	affin->rx_ring_node = channel->rxq_node;
+	affin->tx_ring_node = channel->txq_node;
+	affin->event_ring_node = channel->evq_node;
+	affin->handler_data_node = channel->channel_node;
+	return 0;
+}
+
+static int efx_ethtool_set_affinity(struct net_device *net_dev,
+				    const struct ethtool_affinity *affin)
+{
+	struct efx_nic *efx = netdev_priv(net_dev);
+	struct efx_channel *channel;
+
+	if (affin->channel_id == ETH_CHAN_ALL)
+		channel = NULL;
+	else
+		channel = efx_channel_from_id(efx, affin->channel_id);
+	return efx_realloc_channels(efx, efx->rxq_entries, efx->txq_entries,
+				    channel, affin);
+}
+
+#endif /* CONFIG_NUMA */
+
 const struct ethtool_ops efx_ethtool_ops = {
 	.get_settings		= efx_ethtool_get_settings,
 	.set_settings		= efx_ethtool_set_settings,
@@ -1035,4 +1100,9 @@ const struct ethtool_ops efx_ethtool_ops = {
 	.get_rxnfc		= efx_ethtool_get_rxnfc,
 	.get_rxfh_indir		= efx_ethtool_get_rxfh_indir,
 	.set_rxfh_indir		= efx_ethtool_set_rxfh_indir,
+	.get_channels		= efx_ethtool_get_channels,
+#ifdef CONFIG_NUMA
+	.get_affinity		= efx_ethtool_get_affinity,
+	.set_affinity		= efx_ethtool_set_affinity,
+#endif
 };
diff --git a/drivers/net/sfc/net_driver.h b/drivers/net/sfc/net_driver.h
index 152342d..de69ac3 100644
--- a/drivers/net/sfc/net_driver.h
+++ b/drivers/net/sfc/net_driver.h
@@ -69,8 +69,10 @@
 
 /**
  * struct efx_special_buffer - An Efx special buffer
+ * @page: Page pointer, iff buffer was allocated with alloc_pages_node()
  * @addr: CPU base address of the buffer
  * @dma_addr: DMA base address of the buffer
+ * @dma_dir: Direction of DMA mapping
  * @len: Buffer length, in bytes
  * @index: Buffer index within controller;s buffer table
  * @entries: Number of buffer table entries
@@ -80,8 +82,10 @@
  * actual transmit and receive buffers.
  */
 struct efx_special_buffer {
+	struct page *page;
 	void *addr;
 	dma_addr_t dma_addr;
+	enum dma_data_direction dma_dir;
 	unsigned int len;
 	int index;
 	int entries;
@@ -299,6 +303,10 @@ enum efx_rx_alloc_method {
  *
  * @efx: Associated Efx NIC
  * @channel: Channel instance number
+ * @rxq_node: Hardware RX queue NUMA affinity
+ * @txq_node: Hardware TX queue NUMA affinity
+ * @evq_node: Hardware event queue NUMA affinity
+ * @channel_node: NUMA affinity for this structure
  * @enabled: Channel enabled indicator
  * @irq: IRQ number (MSI and MSI-X only)
  * @irq_moderation: IRQ moderation value (in hardware ticks)
@@ -332,6 +340,10 @@ enum efx_rx_alloc_method {
 struct efx_channel {
 	struct efx_nic *efx;
 	int channel;
+	int rxq_node;
+	int txq_node;
+	int evq_node;
+	int channel_node;
 	bool enabled;
 	int irq;
 	unsigned int irq_moderation;
diff --git a/drivers/net/sfc/nic.c b/drivers/net/sfc/nic.c
index 6c5c0ce..799e881 100644
--- a/drivers/net/sfc/nic.c
+++ b/drivers/net/sfc/nic.c
@@ -259,17 +259,31 @@ efx_fini_special_buffer(struct efx_nic *efx, struct efx_special_buffer *buffer)
  */
 static int efx_alloc_special_buffer(struct efx_nic *efx,
 				    struct efx_special_buffer *buffer,
-				    unsigned int len)
+				    unsigned int len,
+				    int node_id,
+				    enum dma_data_direction dma_dir)
 {
+	unsigned int order;
+
 	len = ALIGN(len, EFX_BUF_SIZE);
+	buffer->dma_dir = dma_dir;
 
-	buffer->addr = dma_alloc_coherent(&efx->pci_dev->dev, len,
-					  &buffer->dma_addr, GFP_KERNEL);
-	if (!buffer->addr)
+	order = order_base_2(DIV_ROUND_UP(len, PAGE_SIZE));
+	buffer->page = alloc_pages_node(node_id, GFP_KERNEL, order);
+	if (!buffer->page)
+		return -ENOMEM;
+
+	buffer->dma_addr = dma_map_page(&efx->pci_dev->dev, buffer->page, 0,
+					buffer->len, buffer->dma_dir);
+	if (unlikely(dma_mapping_error(&efx->pci_dev->dev, buffer->dma_addr))) {
+		__free_pages(buffer->page, order);
 		return -ENOMEM;
+	}
+	EFX_BUG_ON_PARANOID(buffer->dma_addr & (EFX_BUF_SIZE - 1));
+
+	buffer->addr = page_address(buffer->page);
 	buffer->len = len;
 	buffer->entries = len / EFX_BUF_SIZE;
-	BUG_ON(buffer->dma_addr & (EFX_BUF_SIZE - 1));
 
 	/* All zeros is a potentially valid event so memset to 0xff */
 	memset(buffer->addr, 0xff, len);
@@ -291,6 +305,8 @@ static int efx_alloc_special_buffer(struct efx_nic *efx,
 static void
 efx_free_special_buffer(struct efx_nic *efx, struct efx_special_buffer *buffer)
 {
+	unsigned int order;
+
 	if (!buffer->addr)
 		return;
 
@@ -301,8 +317,10 @@ efx_free_special_buffer(struct efx_nic *efx, struct efx_special_buffer *buffer)
 		  (u64)buffer->dma_addr, buffer->len,
 		  buffer->addr, (u64)virt_to_phys(buffer->addr));
 
-	dma_free_coherent(&efx->pci_dev->dev, buffer->len, buffer->addr,
-			  buffer->dma_addr);
+	order = order_base_2(DIV_ROUND_UP(buffer->len, PAGE_SIZE));
+	dma_unmap_page(&efx->pci_dev->dev, buffer->dma_addr,
+		       buffer->len, buffer->dma_dir);
+	__free_pages(buffer->page, order);
 	buffer->addr = NULL;
 	buffer->entries = 0;
 }
@@ -401,8 +419,9 @@ int efx_nic_probe_tx(struct efx_tx_queue *tx_queue)
 	unsigned entries;
 
 	entries = tx_queue->ptr_mask + 1;
-	return efx_alloc_special_buffer(efx, &tx_queue->txd,
-					entries * sizeof(efx_qword_t));
+	return efx_alloc_special_buffer(
+		efx, &tx_queue->txd, entries * sizeof(efx_qword_t),
+		tx_queue->channel->txq_node, DMA_TO_DEVICE);
 }
 
 void efx_nic_init_tx(struct efx_tx_queue *tx_queue)
@@ -551,8 +570,9 @@ int efx_nic_probe_rx(struct efx_rx_queue *rx_queue)
 	unsigned entries;
 
 	entries = rx_queue->ptr_mask + 1;
-	return efx_alloc_special_buffer(efx, &rx_queue->rxd,
-					entries * sizeof(efx_qword_t));
+	return efx_alloc_special_buffer(
+		efx, &rx_queue->rxd, entries * sizeof(efx_qword_t),
+		efx_rx_queue_channel(rx_queue)->rxq_node, DMA_TO_DEVICE);
 }
 
 void efx_nic_init_rx(struct efx_rx_queue *rx_queue)
@@ -1080,8 +1100,9 @@ int efx_nic_probe_eventq(struct efx_channel *channel)
 	unsigned entries;
 
 	entries = channel->eventq_mask + 1;
-	return efx_alloc_special_buffer(efx, &channel->eventq,
-					entries * sizeof(efx_qword_t));
+	return efx_alloc_special_buffer(
+		efx, &channel->eventq, entries * sizeof(efx_qword_t),
+		channel->evq_node, DMA_BIDIRECTIONAL);
 }
 
 void efx_nic_init_eventq(struct efx_channel *channel)
-- 
1.7.2.1


-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


^ permalink raw reply related

* [PATCH] ip_gre: CONFIG_IPV6_MODULE support
From: Eric Dumazet @ 2010-09-20 13:01 UTC (permalink / raw)
  To: David Miller; +Cc: netdev

ipv6 can be a module, we should test CONFIG_IPV6 and CONFIG_IPV6_MODULE
to enable ipv6 bits in ip_gre.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 net/ipv4/ip_gre.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index fc20e68..2af2fbd 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -46,7 +46,7 @@
 #include <net/rtnetlink.h>
 #include <net/gre.h>
 
-#ifdef CONFIG_IPV6
+#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
 #include <net/ipv6.h>
 #include <net/ip6_fib.h>
 #include <net/ip6_route.h>
@@ -701,7 +701,7 @@ static netdev_tx_t ipgre_tunnel_xmit(struct sk_buff *skb, struct net_device *dev
 			if ((dst = rt->rt_gateway) == 0)
 				goto tx_error_icmp;
 		}
-#ifdef CONFIG_IPV6
+#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
 		else if (skb->protocol == htons(ETH_P_IPV6)) {
 			struct in6_addr *addr6;
 			int addr_type;
@@ -776,7 +776,7 @@ static netdev_tx_t ipgre_tunnel_xmit(struct sk_buff *skb, struct net_device *dev
 			goto tx_error;
 		}
 	}
-#ifdef CONFIG_IPV6
+#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
 	else if (skb->protocol == htons(ETH_P_IPV6)) {
 		struct rt6_info *rt6 = (struct rt6_info *)skb_dst(skb);
 
@@ -852,7 +852,7 @@ static netdev_tx_t ipgre_tunnel_xmit(struct sk_buff *skb, struct net_device *dev
 	if ((iph->ttl = tiph->ttl) == 0) {
 		if (skb->protocol == htons(ETH_P_IP))
 			iph->ttl = old_iph->ttl;
-#ifdef CONFIG_IPV6
+#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
 		else if (skb->protocol == htons(ETH_P_IPV6))
 			iph->ttl = ((struct ipv6hdr *)old_iph)->hop_limit;
 #endif



^ permalink raw reply related

* Re: [RFC][PATCH 1/3] IRQ: Add irq_get_numa_node()
From: Eric Dumazet @ 2010-09-20 13:04 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: Michael S. Tsirkin, David Miller, therbert, shemminger, netdev
In-Reply-To: <1284986671.2282.41.camel@achroite.uk.solarflarecom.com>

Le lundi 20 septembre 2010 à 13:44 +0100, Ben Hutchings a écrit :
> This will be used to support NUMA-aware memory allocation of
> structures used in interrupt handlers, starting with network drivers.
> ---
> irq_get_numa_node() probably needs to grab desc->lock.  Other than that,
> this should work.
> 
> Ben.
> 
>  include/linux/interrupt.h |    9 +++++++++
>  kernel/irq/manage.c       |   35 +++++++++++++++++++++++++++++++++++
>  2 files changed, 44 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
> index a0384a4..82e9a08 100644
> --- a/include/linux/interrupt.h
> +++ b/include/linux/interrupt.h
> @@ -260,6 +260,15 @@ static inline int irq_set_affinity_hint(unsigned int irq,
>  }
>  #endif /* CONFIG_SMP && CONFIG_GENERIC_HARDIRQS */
>  
> +#if defined(CONFIG_NUMA) && defined(CONFIG_GENERIC_HARDIRQS)
> +extern int irq_get_numa_node(unsigned int irq);
> +#else
> +static inline int irq_get_numa_node(unsigned int irq)
> +{
> +	return -1;
> +}
> +#endif
> +
>  #ifdef CONFIG_GENERIC_HARDIRQS
>  /*
>   * Special lockdep variants of irq disabling/enabling.
> diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
> index c3003e9..03e683e 100644
> --- a/kernel/irq/manage.c
> +++ b/kernel/irq/manage.c
> @@ -154,6 +154,41 @@ int irq_set_affinity_hint(unsigned int irq, const struct cpumask *m)
>  }
>  EXPORT_SYMBOL_GPL(irq_set_affinity_hint);
>  
> +#ifdef CONFIG_NUMA
> +/**
> + *	irq_get_numa_node - get the NUMA node of a given IRQ
> + *	@irq:		Interrupt to get NUMA node for
> + *
> + *	If the current SMP affinity mask of the IRQ corresponds to a
> + *	single NUMA node, return the node number.  Otherwise return
> + *	%NUMA_NO_NODE.
> + */
> +int irq_get_numa_node(unsigned int irq)
> +{
> +	struct irq_desc *desc = irq_to_desc(irq);
> +	const struct cpumask *mask;
> +	int cpu, node = NUMA_NO_NODE;
> +
> +	if (!desc)
> +		return node;
> +
> +	mask = desc->affinity;
> +#ifdef CONFIG_GENERIC_PENDING_IRQ
> +	if (desc->status & IRQ_MOVE_PENDING)
> +		mask = desc->pending_mask;
> +#endif
> +
> +	for_each_cpu(cpu, mask) {
> +		if (node < 0)

or : if (node == NUMA_NO_NODE)

> +			node = cpu_to_node(cpu);
> +		else if (node != cpu_to_node(cpu))
> +			return NUMA_NO_NODE;
> +	}
> +
> +	return node;
> +}
> +#endif
> +
>  #ifndef CONFIG_AUTO_IRQ_AFFINITY
>  /*
>   * Generic version of the affinity autoselector.
> -- 
> 1.7.2.1
> 
> 
> 



^ permalink raw reply

* idr_get_new_exact ?
From: Ohad Ben-Cohen @ 2010-09-20 14:11 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jean Delvare (PC drivers, core), Ben Dooks (embedded platforms),
	Roland Dreier, Sean Hefty, Hal Rosenstock, Steve Wise, Neil Brown,
	Paul Mackerras, linux-i2c, linux-rdma, dm-devel, linux-raid,
	linux-ppp, netdev, akpm

Occasionally, drivers care about the value that idr associates with
their pointers.

Today we have idr_get_new_above() which allocates a new idr entry
above or equal to a given starting id, but sometimes drivers need to
force an exact value.

To overcome this small API gap, drivers are wrapping idr_get_new_above
and then either BUG_ON() or just call idr_remove() and returns -EBUSY
when idr allocates them an id which is different than their requested
value.

There are only a handful of users who need this (see below. especially
note the i2c comment :), but it might be nice to have such an API (a
bit less of code, and a bit less error prone).

Would something like the below be desirable/acceptable ?

(untested. and i just picked the simplest and straight-forward way to
implement this; obviously it's not optimal since there's no reason to
even allocate an id if we know it's not the id we're looking for. but
it's enough to get the idea, it's not a hot path, and it's what
drivers are doing today)

---
 drivers/i2c/i2c-core.c                 |    9 +------
 drivers/infiniband/core/cma.c          |    9 +------
 drivers/infiniband/hw/cxgb3/iwch.h     |    3 +-
 drivers/infiniband/hw/cxgb4/iw_cxgb4.h |    3 +-
 drivers/md/dm.c                        |    8 +-----
 drivers/net/ppp_generic.c              |    7 +-----
 include/linux/idr.h                    |    1 +
 lib/idr.c                              |   39 ++++++++++++++++++++++++++++++++
 8 files changed, 46 insertions(+), 33 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 6649176..6220900 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -964,14 +964,7 @@ retry:
 		return -ENOMEM;

 	mutex_lock(&core_lock);
-	/* "above" here means "above or equal to", sigh;
-	 * we need the "equal to" result to force the result
-	 */
-	status = idr_get_new_above(&i2c_adapter_idr, adap, adap->nr, &id);
-	if (status == 0 && id != adap->nr) {
-		status = -EBUSY;
-		idr_remove(&i2c_adapter_idr, id);
-	}
+	status = idr_get_new_exact(&i2c_adapter_idr, adap, adap->nr, &id);
 	mutex_unlock(&core_lock);
 	if (status == -EAGAIN)
 		goto retry;
diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index b930b81..1756c99 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -1945,23 +1945,16 @@ static int cma_alloc_port(struct idr *ps,
struct rdma_id_private *id_priv,
 		return -ENOMEM;

 	do {
-		ret = idr_get_new_above(ps, bind_list, snum, &port);
+		ret = idr_get_new_exact(ps, bind_list, snum, &port);
 	} while ((ret == -EAGAIN) && idr_pre_get(ps, GFP_KERNEL));

 	if (ret)
 		goto err1;

-	if (port != snum) {
-		ret = -EADDRNOTAVAIL;
-		goto err2;
-	}
-
 	bind_list->ps = ps;
 	bind_list->port = (unsigned short) port;
 	cma_bind_port(bind_list, id_priv);
 	return 0;
-err2:
-	idr_remove(ps, port);
 err1:
 	kfree(bind_list);
 	return ret;
diff --git a/drivers/infiniband/hw/cxgb3/iwch.h
b/drivers/infiniband/hw/cxgb3/iwch.h
index a1c4457..71d9cae 100644
--- a/drivers/infiniband/hw/cxgb3/iwch.h
+++ b/drivers/infiniband/hw/cxgb3/iwch.h
@@ -160,8 +160,7 @@ static inline int insert_handle(struct iwch_dev
*rhp, struct idr *idr,
 			return -ENOMEM;
 		}
 		spin_lock_irq(&rhp->lock);
-		ret = idr_get_new_above(idr, handle, id, &newid);
-		BUG_ON(newid != id);
+		ret = idr_get_new_exact(idr, handle, id, &newid);
 		spin_unlock_irq(&rhp->lock);
 	} while (ret == -EAGAIN);

diff --git a/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
b/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
index ed459b8..45567b1 100644
--- a/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
+++ b/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
@@ -190,8 +190,7 @@ static inline int insert_handle(struct c4iw_dev
*rhp, struct idr *idr,
 		if (!idr_pre_get(idr, GFP_KERNEL))
 			return -ENOMEM;
 		spin_lock_irq(&rhp->lock);
-		ret = idr_get_new_above(idr, handle, id, &newid);
-		BUG_ON(newid != id);
+		ret = idr_get_new_exact(idr, handle, id, &newid);
 		spin_unlock_irq(&rhp->lock);
 	} while (ret == -EAGAIN);

diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index ac384b2..dee497c 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -1873,16 +1873,10 @@ static int specific_minor(int minor)
 		goto out;
 	}

-	r = idr_get_new_above(&_minor_idr, MINOR_ALLOCED, minor, &m);
+	r = idr_get_new_exact(&_minor_idr, MINOR_ALLOCED, minor, &m);
 	if (r)
 		goto out;

-	if (m != minor) {
-		idr_remove(&_minor_idr, m);
-		r = -EBUSY;
-		goto out;
-	}
-
 out:
 	spin_unlock(&_minor_lock);
 	return r;
diff --git a/drivers/net/ppp_generic.c b/drivers/net/ppp_generic.c
index 736b917..3bb63e5 100644
--- a/drivers/net/ppp_generic.c
+++ b/drivers/net/ppp_generic.c
@@ -2865,15 +2865,10 @@ again:
 		return -ENOMEM;
 	}

-	err = idr_get_new_above(p, ptr, n, &unit);
+	err = idr_get_new_exact(p, ptr, n, &unit);
 	if (err == -EAGAIN)
 		goto again;

-	if (unit != n) {
-		idr_remove(p, unit);
-		return -EINVAL;
-	}
-
 	return unit;
 }

diff --git a/include/linux/idr.h b/include/linux/idr.h
index e968db7..748b67e 100644
--- a/include/linux/idr.h
+++ b/include/linux/idr.h
@@ -104,6 +104,7 @@ void *idr_find(struct idr *idp, int id);
 int idr_pre_get(struct idr *idp, gfp_t gfp_mask);
 int idr_get_new(struct idr *idp, void *ptr, int *id);
 int idr_get_new_above(struct idr *idp, void *ptr, int starting_id, int *id);
+int idr_get_new_exact(struct idr *idp, void *ptr, int requested_id, int *id);
 int idr_for_each(struct idr *idp,
 		 int (*fn)(int id, void *p, void *data), void *data);
 void *idr_get_next(struct idr *idp, int *nextid);
diff --git a/lib/idr.c b/lib/idr.c
index 7f1a4f0..53cc161 100644
--- a/lib/idr.c
+++ b/lib/idr.c
@@ -430,6 +430,45 @@ void idr_remove(struct idr *idp, int id)
 EXPORT_SYMBOL(idr_remove);

 /**
+ * idr_get_new_exact - allocate new idr entry equal to a requested id
+ * @idp: idr handle
+ * @ptr: pointer you want associated with the id
+ * @requested_id: id to search for
+ * @id: pointer to the allocated handle
+ *
+ * This is the allocate id function.  It should be called with any
+ * required locks.
+ *
+ * If memory is required, it will return -EAGAIN, you should unlock
+ * and go back to the idr_pre_get() call.  If the idr is full, it will
+ * return -ENOSPC. If the requested id is already taken, it will return
+ * -EBUSY.
+ *
+ * @id returns @requested_id on success
+ */
+int idr_get_new_exact(struct idr *idp, void *ptr, int requested_id, int *id)
+{
+	int rv;
+
+	rv = idr_get_new_above_int(idp, ptr, requested_id);
+	/*
+	 * This is a cheap hack until the IDR code can be fixed to
+	 * return proper error values.
+	 */
+	if (rv < 0)
+		return _idr_rc_to_errno(rv);
+
+	if (requested_id != rv) {
+		idr_remove(idp, rv);
+		return -EBUSY;
+	}
+
+	*id = rv;
+	return 0;
+}
+EXPORT_SYMBOL(idr_get_new_exact);
+
+/**
  * idr_remove_all - remove all ids from the given idr tree
  * @idp: idr handle
  *
-- 
1.7.0.4

^ permalink raw reply related

* Re: [PATCH v5 2/2] netfilter: save the hash of the tuple in the original direction for latter use
From: Changli Gao @ 2010-09-20 15:04 UTC (permalink / raw)
  To: Patrick McHardy
  Cc: David S. Miller, Eric Dumazet, Mathieu Desnoyers, akpm,
	netfilter-devel, netdev, linux-kernel
In-Reply-To: <4C91B6BE.8020300@trash.net>

On Thu, Sep 16, 2010 at 2:18 PM, Patrick McHardy <kaber@trash.net> wrote:
> On 21.08.2010 00:49, Changli Gao wrote:
>> Since we don't change the tuple in the original direction, we can save it
>> in ct->tuplehash[IP_CT_DIR_REPLY].hnode.pprev for __nf_conntrack_confirm()
>> use.
>
> I like this idea. We could actually do the same for the reply tuple
> and invalidate the saved hash in case the reply tuple is changed
> (nf_conntrack_alter_reply()), which only happens when NAT is used.
>

We can't do that, as the unconfirmed ct owned maybe dropped, and
pre-computing will wast CPU cycles in this case.

>> __hash_conntrack() is split into two steps: ____hash_conntrack() is used
>> to get the raw hash, and __hash_bucket() is used to get the bucket id.
>
> This patch uses underscores a bit excessively, how about renaming:
>
> - ____hash_conntrack() => hash_conntrack_raw()
> - __hash variables => hash
> - hash variables => bucket

OK. Thanks.

>
>> @@ -408,7 +438,8 @@ __nf_conntrack_confirm(struct sk_buff *skb)
>>               return NF_ACCEPT;
>>
>>       zone = nf_ct_zone(ct);
>> -     hash = hash_conntrack(net, zone, &ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple);
>> +     /* reuse the __hash saved before */
>> +     hash = hash_bucket(*(unsigned long *)&ct->tuplehash[IP_CT_DIR_REPLY].hnnode.pprev, net);
>
> Please try to stay at least close to the 80 characters limit.
>

OK. Thanks.

-- 
Regards,
Changli Gao(xiaosuo@gmail.com)

^ permalink raw reply


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