* Re: [PATCH v7 01/16] hashtable: introduce a small and naive hashtable
From: Sasha Levin @ 2012-10-29 16:06 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: torvalds, tj, akpm, linux-kernel, linux-mm, paul.gortmaker, davem,
rostedt, mingo, ebiederm, aarcange, ericvh, netdev, josh,
eric.dumazet, axboe, agk, dm-devel, neilb, ccaulfie, teigland,
Trond.Myklebust, bfields, fweisbec, jesse, venkat.x.venkatsubra,
ejt, snitzer, edumazet, linux-nfs, dev, rds-devel, lw
In-Reply-To: <20121029112907.GA9115@Krystal>
On Mon, Oct 29, 2012 at 7:29 AM, Mathieu Desnoyers
<mathieu.desnoyers@efficios.com> wrote:
> * Sasha Levin (levinsasha928@gmail.com) wrote:
>> +
>> + for (i = 0; i < sz; i++)
>> + INIT_HLIST_HEAD(&ht[sz]);
>
> ouch. How did this work ? Has it been tested at all ?
>
> sz -> i
Funny enough, it works perfectly. Generally as a test I boot the
kernel in a VM and let it fuzz with trinity for a bit, doing that with
the code above worked flawlessly.
While it works, it's obviously wrong. Why does it work though? Usually
there's a list op happening pretty soon after that which brings the
list into proper state.
I've been playing with a patch that adds a magic value into list_head
if CONFIG_DEBUG_LIST is set, and checks that magic in the list debug
code in lib/list_debug.c.
Does it sound like something useful? If so I'll send that patch out.
Thanks,
Sasha
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH v7 15/16] openvswitch: use new hashtable implementation
From: Mathieu Desnoyers @ 2012-10-29 15:59 UTC (permalink / raw)
To: Sasha Levin
Cc: torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
tj-DgEjT+Ai2ygdnm+yROfE0A, akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
paul.gortmaker-CWA4WttNNZF54TAoqtyWWQ,
davem-fT/PcQaiUtIeIZ0/mPfg9Q, rostedt-nx8X9YLhiw1AfugRpC6u6w,
mingo-X9Un+BFzKDI, ebiederm-aS9lmoZGLiVWk0Htik3J/w,
aarcange-H+wXaHxf7aLQT0dZR+AlfA, ericvh-Re5JQEeQqe8AvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA, josh-iaAMLnmF4UmaiuxdJuQwMA,
eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w, axboe-tSWWG44O7X1aa/9Udqfwiw,
agk-H+wXaHxf7aLQT0dZR+AlfA, dm-devel-H+wXaHxf7aLQT0dZR+AlfA,
neilb-l3A5Bk7waGM, ccaulfie-H+wXaHxf7aLQT0dZR+AlfA,
teigland-H+wXaHxf7aLQT0dZR+AlfA,
Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA,
bfields-uC3wQj2KruNg9hUCZPvPmw, fweisbec-Re5JQEeQqe8AvxtiuMwx3w,
jesse-l0M0P4e3n4LQT0dZR+AlfA,
venkat.x.venkatsubra-QHcLZuEGTsvQT0dZR+AlfA,
ejt-H+wXaHxf7aLQT0dZR+AlfA, snitzer-H+wXaHxf7aLQT0dZR+AlfA,
edumazet-hpIqsD4AKlfQT0dZR+AlfA, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
dev-yBygre7rU0TnMu66kgdUjQ, rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
lw-BthXqXjhjHXQFUHtdCDX3A
In-Reply-To: <CA+1xoqfRGhPaBEVh228O5_295bWh8FmcyLSOwq8VE5Dm7i3JHg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
* Sasha Levin (levinsasha928-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org) wrote:
> Hi Mathieu,
>
> On Mon, Oct 29, 2012 at 9:29 AM, Mathieu Desnoyers
> <mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org> wrote:
> > * Sasha Levin (levinsasha928-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org) wrote:
> > [...]
> >> -static struct hlist_head *hash_bucket(struct net *net, const char *name)
> >> -{
> >> - unsigned int hash = jhash(name, strlen(name), (unsigned long) net);
> >> - return &dev_table[hash & (VPORT_HASH_BUCKETS - 1)];
> >> -}
> >> -
> >> /**
> >> * ovs_vport_locate - find a port that has already been created
> >> *
> >> @@ -84,13 +76,12 @@ static struct hlist_head *hash_bucket(struct net *net, const char *name)
> >> */
> >> struct vport *ovs_vport_locate(struct net *net, const char *name)
> >> {
> >> - struct hlist_head *bucket = hash_bucket(net, name);
> >> struct vport *vport;
> >> struct hlist_node *node;
> >> + int key = full_name_hash(name, strlen(name));
> >>
> >> - hlist_for_each_entry_rcu(vport, node, bucket, hash_node)
> >> - if (!strcmp(name, vport->ops->get_name(vport)) &&
> >> - net_eq(ovs_dp_get_net(vport->dp), net))
> >> + hash_for_each_possible_rcu(dev_table, vport, node, hash_node, key)
> >
> > Is applying hash_32() on top of full_name_hash() needed and expected ?
>
> Since this was pointed out in several of the patches, I'll answer it
> just once here.
>
> I've intentionally "allowed" double hashing with hash_32 to keep the
> code simple.
>
> hash_32() is pretty simple and gcc optimizes it to be almost nothing,
> so doing that costs us a multiplication and a shift. On the other
> hand, we benefit from keeping our code simple - how would we avoid
> doing this double hash? adding a different hashtable function for
> strings? or a new function for already hashed keys? I think we benefit
> a lot from having to mul/shr instead of adding extra lines of code
> here.
This could be done, as I pointed out in another email within this
thread, by changing the "key" argument from add/for_each_possible to an
expected "hash" value, and let the caller invoke hash_32() if they want.
I doubt this would add a significant amount of complexity for users of
this API, but would allow much more flexibility to choose hash
functions.
Thanks,
Mathieu
--
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" 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
* Re: [PATCH v7 10/16] dlm: use new hashtable implementation
From: Sasha Levin @ 2012-10-29 15:53 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: snitzer-H+wXaHxf7aLQT0dZR+AlfA, neilb-l3A5Bk7waGM,
fweisbec-Re5JQEeQqe8AvxtiuMwx3w,
Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA,
bfields-uC3wQj2KruNg9hUCZPvPmw,
paul.gortmaker-CWA4WttNNZF54TAoqtyWWQ,
dm-devel-H+wXaHxf7aLQT0dZR+AlfA, agk-H+wXaHxf7aLQT0dZR+AlfA,
aarcange-H+wXaHxf7aLQT0dZR+AlfA, rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
venkat.x.venkatsubra-QHcLZuEGTsvQT0dZR+AlfA,
ccaulfie-H+wXaHxf7aLQT0dZR+AlfA, mingo-X9Un+BFzKDI,
dev-yBygre7rU0TnMu66kgdUjQ, ericvh-Re5JQEeQqe8AvxtiuMwx3w,
josh-iaAMLnmF4UmaiuxdJuQwMA, rostedt-nx8X9YLhiw1AfugRpC6u6w,
lw-BthXqXjhjHXQFUHtdCDX3A, teigland-H+wXaHxf7aLQT0dZR+AlfA,
axboe-tSWWG44O7X1aa/9Udqfwiw, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
edumazet-hpIqsD4AKlfQT0dZR+AlfA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, ejt-H+wXaHxf7aLQT0dZR+AlfA,
ebiederm-aS9lmoZGLiVWk0Htik3J/w, tj-DgEjT+Ai2ygdnm+yROfE0A,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <20121029130736.GF11733@Krystal>
On Mon, Oct 29, 2012 at 9:07 AM, Mathieu Desnoyers
<mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org> wrote:
> * Mathieu Desnoyers (mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org) wrote:
>> * Sasha Levin (levinsasha928-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org) wrote:
>> [...]
>> > @@ -158,34 +159,21 @@ static int dlm_allow_conn;
>> > static struct workqueue_struct *recv_workqueue;
>> > static struct workqueue_struct *send_workqueue;
>> >
>> > -static struct hlist_head connection_hash[CONN_HASH_SIZE];
>> > +static struct hlist_head connection_hash[CONN_HASH_BITS];
>> > static DEFINE_MUTEX(connections_lock);
>> > static struct kmem_cache *con_cache;
>> >
>> > static void process_recv_sockets(struct work_struct *work);
>> > static void process_send_sockets(struct work_struct *work);
>> >
>> > -
>> > -/* This is deliberately very simple because most clusters have simple
>> > - sequential nodeids, so we should be able to go straight to a connection
>> > - struct in the array */
>> > -static inline int nodeid_hash(int nodeid)
>> > -{
>> > - return nodeid & (CONN_HASH_SIZE-1);
>> > -}
>>
>> There is one thing I dislike about this change: you remove a useful
>> comment. It's good to be informed of the reason why a direct mapping
>> "value -> hash" without any dispersion function is preferred here.
Yes, I've removed the comment because it's no longer true with the patch :)
> And now that I come to think of it: you're changing the behavior : you
> will now use a dispersion function on the key, which goes against the
> intent expressed in this comment.
The comment gave us the information that nodeids are mostly
sequential, we no longer need to rely on that.
> It might be good to change hash_add(), hash_add_rcu(),
> hash_for_each_possible*() key parameter for a "hash" parameter, and let
> the caller provide the hash value computed by the function they like as
> parameter, rather than enforcing hash_32/hash_64.
Why? We already proved that hash_32() is more than enough as a hashing
function, why complicate things?
Even doing hash_32() on top of another hash is probably a good idea to
keep things simple.
Thanks,
Sasha
^ permalink raw reply
* [PATCH net-next 7/8] vhost-net: select tx zero copy dynamically
From: Michael S. Tsirkin @ 2012-10-29 15:50 UTC (permalink / raw)
Cc: Michael S. Tsirkin, David S. Miller, Eric Dumazet, Andrew Morton,
Alexander Duyck, Ian Campbell, kvm, virtualization, netdev,
linux-kernel
In-Reply-To: <cover.1351524501.git.mst@redhat.com>
Even when vhost-net is in zero-copy transmit mode,
net core might still decide to copy the skb later
which is somewhat slower than a copy in user
context: data copy overhead is added to the cost of
page pin/unpin. The result is that enabling tx zero copy
option leads to higher CPU utilization for guest to guest
and guest to host traffic.
To fix this, suppress zero copy tx after a given number of
packets triggered late data copy. Re-enable periodically
to detect workload changes.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/vhost/net.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 50 insertions(+), 5 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 532fc88..8e9de79 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -42,6 +42,21 @@ MODULE_PARM_DESC(experimental_zcopytx, "Enable Experimental Zero Copy TX");
#define VHOST_MAX_PEND 128
#define VHOST_GOODCOPY_LEN 256
+/*
+ * For transmit, used buffer len is unused; we override it to track buffer
+ * status internally; used for zerocopy tx only.
+ */
+/* Lower device DMA failed */
+#define VHOST_DMA_FAILED_LEN 3
+/* Lower device DMA done */
+#define VHOST_DMA_DONE_LEN 2
+/* Lower device DMA in progress */
+#define VHOST_DMA_IN_PROGRESS 1
+/* Buffer unused */
+#define VHOST_DMA_CLEAR_LEN 0
+
+#define VHOST_DMA_IS_DONE(len) ((len) >= VHOST_DMA_DONE_LEN)
+
enum {
VHOST_NET_VQ_RX = 0,
VHOST_NET_VQ_TX = 1,
@@ -62,8 +77,33 @@ struct vhost_net {
* We only do this when socket buffer fills up.
* Protected by tx vq lock. */
enum vhost_net_poll_state tx_poll_state;
+ /* Number of TX recently submitted.
+ * Protected by tx vq lock. */
+ unsigned tx_packets;
+ /* Number of times zerocopy TX recently failed.
+ * Protected by tx vq lock. */
+ unsigned tx_zcopy_err;
};
+static void vhost_net_tx_packet(struct vhost_net *net)
+{
+ ++net->tx_packets;
+ if (net->tx_packets < 1024)
+ return;
+ net->tx_packets = 0;
+ net->tx_zcopy_err = 0;
+}
+
+static void vhost_net_tx_err(struct vhost_net *net)
+{
+ ++net->tx_zcopy_err;
+}
+
+static bool vhost_net_tx_select_zcopy(struct vhost_net *net)
+{
+ return net->tx_packets / 64 >= net->tx_zcopy_err;
+}
+
static bool vhost_sock_zcopy(struct socket *sock)
{
return unlikely(experimental_zcopytx) &&
@@ -131,12 +171,15 @@ static void tx_poll_start(struct vhost_net *net, struct socket *sock)
* of used idx. Once lower device DMA done contiguously, we will signal KVM
* guest used idx.
*/
-int vhost_zerocopy_signal_used(struct vhost_virtqueue *vq)
+static int vhost_zerocopy_signal_used(struct vhost_net *net,
+ struct vhost_virtqueue *vq)
{
int i;
int j = 0;
for (i = vq->done_idx; i != vq->upend_idx; i = (i + 1) % UIO_MAXIOV) {
+ if (vq->heads[i].len == VHOST_DMA_FAILED_LEN)
+ vhost_net_tx_err(net);
if (VHOST_DMA_IS_DONE(vq->heads[i].len)) {
vq->heads[i].len = VHOST_DMA_CLEAR_LEN;
vhost_add_used_and_signal(vq->dev, vq,
@@ -208,7 +251,7 @@ static void handle_tx(struct vhost_net *net)
for (;;) {
/* Release DMAs done buffers first */
if (zcopy)
- vhost_zerocopy_signal_used(vq);
+ vhost_zerocopy_signal_used(net, vq);
head = vhost_get_vq_desc(&net->dev, vq, vq->iov,
ARRAY_SIZE(vq->iov),
@@ -263,7 +306,8 @@ static void handle_tx(struct vhost_net *net)
/* use msg_control to pass vhost zerocopy ubuf info to skb */
if (zcopy) {
vq->heads[vq->upend_idx].id = head;
- if (len < VHOST_GOODCOPY_LEN) {
+ if (!vhost_net_tx_select_zcopy(net) ||
+ len < VHOST_GOODCOPY_LEN) {
/* copy don't need to wait for DMA done */
vq->heads[vq->upend_idx].len =
VHOST_DMA_DONE_LEN;
@@ -305,8 +349,9 @@ static void handle_tx(struct vhost_net *net)
if (!zcopy)
vhost_add_used_and_signal(&net->dev, vq, head, 0);
else
- vhost_zerocopy_signal_used(vq);
+ vhost_zerocopy_signal_used(net, vq);
total_len += len;
+ vhost_net_tx_packet(net);
if (unlikely(total_len >= VHOST_NET_WEIGHT)) {
vhost_poll_queue(&vq->poll);
break;
@@ -774,7 +819,7 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
if (oldubufs) {
vhost_ubuf_put_and_wait(oldubufs);
mutex_lock(&vq->mutex);
- vhost_zerocopy_signal_used(vq);
+ vhost_zerocopy_signal_used(n, vq);
mutex_unlock(&vq->mutex);
}
--
MST
^ permalink raw reply related
* [PATCH net-next 6/8] vhost: move -net specific code out
From: Michael S. Tsirkin @ 2012-10-29 15:50 UTC (permalink / raw)
Cc: Michael S. Tsirkin, David S. Miller, Eric Dumazet, Andrew Morton,
Alexander Duyck, Ian Campbell, kvm, virtualization, netdev,
linux-kernel
In-Reply-To: <cover.1351524501.git.mst@redhat.com>
Zerocopy handling code is vhost-net specific.
Move it from vhost.c/vhost.h out to net.c
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/vhost/net.c | 45 ++++++++++++++++++++++++++++++++++++++++
drivers/vhost/tcm_vhost.c | 1 +
drivers/vhost/vhost.c | 53 +++++++----------------------------------------
drivers/vhost/vhost.h | 21 +++----------------
4 files changed, 56 insertions(+), 64 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index f80ae5f..532fc88 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -126,6 +126,42 @@ static void tx_poll_start(struct vhost_net *net, struct socket *sock)
net->tx_poll_state = VHOST_NET_POLL_STARTED;
}
+/* In case of DMA done not in order in lower device driver for some reason.
+ * upend_idx is used to track end of used idx, done_idx is used to track head
+ * of used idx. Once lower device DMA done contiguously, we will signal KVM
+ * guest used idx.
+ */
+int vhost_zerocopy_signal_used(struct vhost_virtqueue *vq)
+{
+ int i;
+ int j = 0;
+
+ for (i = vq->done_idx; i != vq->upend_idx; i = (i + 1) % UIO_MAXIOV) {
+ if (VHOST_DMA_IS_DONE(vq->heads[i].len)) {
+ vq->heads[i].len = VHOST_DMA_CLEAR_LEN;
+ vhost_add_used_and_signal(vq->dev, vq,
+ vq->heads[i].id, 0);
+ ++j;
+ } else
+ break;
+ }
+ if (j)
+ vq->done_idx = i;
+ return j;
+}
+
+static void vhost_zerocopy_callback(struct ubuf_info *ubuf, int status)
+{
+ struct vhost_ubuf_ref *ubufs = ubuf->ctx;
+ struct vhost_virtqueue *vq = ubufs->vq;
+
+ vhost_poll_queue(&vq->poll);
+ /* set len to mark this desc buffers done DMA */
+ vq->heads[ubuf->desc].len = status ?
+ VHOST_DMA_FAILED_LEN : VHOST_DMA_DONE_LEN;
+ vhost_ubuf_put(ubufs);
+}
+
/* Expects to be always run from workqueue - which acts as
* read-size critical section for our kind of RCU. */
static void handle_tx(struct vhost_net *net)
@@ -594,9 +630,18 @@ static int vhost_net_release(struct inode *inode, struct file *f)
struct vhost_net *n = f->private_data;
struct socket *tx_sock;
struct socket *rx_sock;
+ int i;
vhost_net_stop(n, &tx_sock, &rx_sock);
vhost_net_flush(n);
+ vhost_dev_stop(&n->dev);
+ for (i = 0; i < n->dev.nvqs; ++i) {
+ /* Wait for all lower device DMAs done. */
+ if (n->dev.vqs[i].ubufs)
+ vhost_ubuf_put_and_wait(n->dev.vqs[i].ubufs);
+
+ vhost_zerocopy_signal_used(n, &n->dev.vqs[i]);
+ }
vhost_dev_cleanup(&n->dev, false);
if (tx_sock)
fput(tx_sock->file);
diff --git a/drivers/vhost/tcm_vhost.c b/drivers/vhost/tcm_vhost.c
index aa31692..23c138f 100644
--- a/drivers/vhost/tcm_vhost.c
+++ b/drivers/vhost/tcm_vhost.c
@@ -895,6 +895,7 @@ static int vhost_scsi_release(struct inode *inode, struct file *f)
vhost_scsi_clear_endpoint(s, &backend);
}
+ vhost_dev_stop(&s->dev);
vhost_dev_cleanup(&s->dev, false);
kfree(s);
return 0;
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 5affce3..ef8f598 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -26,10 +26,6 @@
#include <linux/kthread.h>
#include <linux/cgroup.h>
-#include <linux/net.h>
-#include <linux/if_packet.h>
-#include <linux/if_arp.h>
-
#include "vhost.h"
enum {
@@ -414,28 +410,16 @@ long vhost_dev_reset_owner(struct vhost_dev *dev)
return 0;
}
-/* In case of DMA done not in order in lower device driver for some reason.
- * upend_idx is used to track end of used idx, done_idx is used to track head
- * of used idx. Once lower device DMA done contiguously, we will signal KVM
- * guest used idx.
- */
-int vhost_zerocopy_signal_used(struct vhost_virtqueue *vq)
+void vhost_dev_stop(struct vhost_dev *dev)
{
int i;
- int j = 0;
-
- for (i = vq->done_idx; i != vq->upend_idx; i = (i + 1) % UIO_MAXIOV) {
- if (VHOST_DMA_IS_DONE(vq->heads[i].len)) {
- vq->heads[i].len = VHOST_DMA_CLEAR_LEN;
- vhost_add_used_and_signal(vq->dev, vq,
- vq->heads[i].id, 0);
- ++j;
- } else
- break;
+
+ for (i = 0; i < dev->nvqs; ++i) {
+ if (dev->vqs[i].kick && dev->vqs[i].handle_kick) {
+ vhost_poll_stop(&dev->vqs[i].poll);
+ vhost_poll_flush(&dev->vqs[i].poll);
+ }
}
- if (j)
- vq->done_idx = i;
- return j;
}
/* Caller should have device mutex if and only if locked is set */
@@ -444,17 +428,6 @@ void vhost_dev_cleanup(struct vhost_dev *dev, bool locked)
int i;
for (i = 0; i < dev->nvqs; ++i) {
- if (dev->vqs[i].kick && dev->vqs[i].handle_kick) {
- vhost_poll_stop(&dev->vqs[i].poll);
- vhost_poll_flush(&dev->vqs[i].poll);
- }
- /* Wait for all lower device DMAs done. */
- if (dev->vqs[i].ubufs)
- vhost_ubuf_put_and_wait(dev->vqs[i].ubufs);
-
- /* Signal guest as appropriate. */
- vhost_zerocopy_signal_used(&dev->vqs[i]);
-
if (dev->vqs[i].error_ctx)
eventfd_ctx_put(dev->vqs[i].error_ctx);
if (dev->vqs[i].error)
@@ -1599,15 +1572,3 @@ void vhost_ubuf_put_and_wait(struct vhost_ubuf_ref *ubufs)
wait_event(ubufs->wait, !atomic_read(&ubufs->kref.refcount));
kfree(ubufs);
}
-
-void vhost_zerocopy_callback(struct ubuf_info *ubuf, int status)
-{
- struct vhost_ubuf_ref *ubufs = ubuf->ctx;
- struct vhost_virtqueue *vq = ubufs->vq;
-
- vhost_poll_queue(&vq->poll);
- /* set len to mark this desc buffers done DMA */
- vq->heads[ubuf->desc].len = status ?
- VHOST_DMA_FAILED_LEN : VHOST_DMA_DONE_LEN;
- kref_put(&ubufs->kref, vhost_zerocopy_done_signal);
-}
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 6fdf31d..5e19e3d 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -7,27 +7,11 @@
#include <linux/mutex.h>
#include <linux/poll.h>
#include <linux/file.h>
-#include <linux/skbuff.h>
#include <linux/uio.h>
#include <linux/virtio_config.h>
#include <linux/virtio_ring.h>
#include <linux/atomic.h>
-/*
- * For transmit, used buffer len is unused; we override it to track buffer
- * status internally; used for zerocopy tx only.
- */
-/* Lower device DMA failed */
-#define VHOST_DMA_FAILED_LEN 3
-/* Lower device DMA done */
-#define VHOST_DMA_DONE_LEN 2
-/* Lower device DMA in progress */
-#define VHOST_DMA_IN_PROGRESS 1
-/* Buffer unused */
-#define VHOST_DMA_CLEAR_LEN 0
-
-#define VHOST_DMA_IS_DONE(len) ((len) >= VHOST_DMA_DONE_LEN)
-
struct vhost_device;
struct vhost_work;
@@ -80,6 +64,8 @@ struct vhost_ubuf_ref *vhost_ubuf_alloc(struct vhost_virtqueue *, bool zcopy);
void vhost_ubuf_put(struct vhost_ubuf_ref *);
void vhost_ubuf_put_and_wait(struct vhost_ubuf_ref *);
+struct ubuf_info;
+
/* The virtqueue structure describes a queue attached to a device. */
struct vhost_virtqueue {
struct vhost_dev *dev;
@@ -177,6 +163,7 @@ long vhost_dev_init(struct vhost_dev *, struct vhost_virtqueue *vqs, int nvqs);
long vhost_dev_check_owner(struct vhost_dev *);
long vhost_dev_reset_owner(struct vhost_dev *);
void vhost_dev_cleanup(struct vhost_dev *, bool locked);
+void vhost_dev_stop(struct vhost_dev *);
long vhost_dev_ioctl(struct vhost_dev *, unsigned int ioctl, unsigned long arg);
int vhost_vq_access_ok(struct vhost_virtqueue *vq);
int vhost_log_access_ok(struct vhost_dev *);
@@ -201,8 +188,6 @@ bool vhost_enable_notify(struct vhost_dev *, struct vhost_virtqueue *);
int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log,
unsigned int log_num, u64 len);
-void vhost_zerocopy_callback(struct ubuf_info *, int);
-int vhost_zerocopy_signal_used(struct vhost_virtqueue *vq);
#define vq_err(vq, fmt, ...) do { \
pr_debug(pr_fmt(fmt), ##__VA_ARGS__); \
--
MST
^ permalink raw reply related
* [PATCH net-next 5/8] vhost: track zero copy failures using DMA length
From: Michael S. Tsirkin @ 2012-10-29 15:50 UTC (permalink / raw)
Cc: Alexander Duyck, Ian Campbell, kvm, Michael S. Tsirkin, netdev,
linux-kernel, virtualization, Eric Dumazet, Andrew Morton,
David S. Miller
In-Reply-To: <cover.1351524501.git.mst@redhat.com>
This will be used to disable zerocopy when error rate
is high.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/vhost/vhost.c | 7 ++++---
drivers/vhost/vhost.h | 4 ++++
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 906fd9f..5affce3 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -425,7 +425,7 @@ int vhost_zerocopy_signal_used(struct vhost_virtqueue *vq)
int j = 0;
for (i = vq->done_idx; i != vq->upend_idx; i = (i + 1) % UIO_MAXIOV) {
- if ((vq->heads[i].len == VHOST_DMA_DONE_LEN)) {
+ if (VHOST_DMA_IS_DONE(vq->heads[i].len)) {
vq->heads[i].len = VHOST_DMA_CLEAR_LEN;
vhost_add_used_and_signal(vq->dev, vq,
vq->heads[i].id, 0);
@@ -1600,13 +1600,14 @@ void vhost_ubuf_put_and_wait(struct vhost_ubuf_ref *ubufs)
kfree(ubufs);
}
-void vhost_zerocopy_callback(struct ubuf_info *ubuf, int zerocopy_status)
+void vhost_zerocopy_callback(struct ubuf_info *ubuf, int status)
{
struct vhost_ubuf_ref *ubufs = ubuf->ctx;
struct vhost_virtqueue *vq = ubufs->vq;
vhost_poll_queue(&vq->poll);
/* set len to mark this desc buffers done DMA */
- vq->heads[ubuf->desc].len = VHOST_DMA_DONE_LEN;
+ vq->heads[ubuf->desc].len = status ?
+ VHOST_DMA_FAILED_LEN : VHOST_DMA_DONE_LEN;
kref_put(&ubufs->kref, vhost_zerocopy_done_signal);
}
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index ad72a1f..6fdf31d 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -17,6 +17,8 @@
* For transmit, used buffer len is unused; we override it to track buffer
* status internally; used for zerocopy tx only.
*/
+/* Lower device DMA failed */
+#define VHOST_DMA_FAILED_LEN 3
/* Lower device DMA done */
#define VHOST_DMA_DONE_LEN 2
/* Lower device DMA in progress */
@@ -24,6 +26,8 @@
/* Buffer unused */
#define VHOST_DMA_CLEAR_LEN 0
+#define VHOST_DMA_IS_DONE(len) ((len) >= VHOST_DMA_DONE_LEN)
+
struct vhost_device;
struct vhost_work;
--
MST
^ permalink raw reply related
* [PATCH net-next 4/8] vhost-net: cleanup macros for DMA status tracking
From: Michael S. Tsirkin @ 2012-10-29 15:49 UTC (permalink / raw)
Cc: Alexander Duyck, Ian Campbell, kvm, Michael S. Tsirkin, netdev,
linux-kernel, virtualization, Eric Dumazet, Andrew Morton,
David S. Miller
In-Reply-To: <cover.1351524501.git.mst@redhat.com>
Better document macros for DMA tracking. Add an
explicit one for DMA in progress instead of
relying on user supplying len != 1.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/vhost/net.c | 3 ++-
drivers/vhost/vhost.c | 2 +-
drivers/vhost/vhost.h | 12 +++++++++---
3 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 072cbba..f80ae5f 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -237,7 +237,8 @@ static void handle_tx(struct vhost_net *net)
} else {
struct ubuf_info *ubuf = &vq->ubuf_info[head];
- vq->heads[vq->upend_idx].len = len;
+ vq->heads[vq->upend_idx].len =
+ VHOST_DMA_IN_PROGRESS;
ubuf->callback = vhost_zerocopy_callback;
ubuf->ctx = vq->ubufs;
ubuf->desc = vq->upend_idx;
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 92308b6..906fd9f 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -1606,7 +1606,7 @@ void vhost_zerocopy_callback(struct ubuf_info *ubuf, int zerocopy_status)
struct vhost_virtqueue *vq = ubufs->vq;
vhost_poll_queue(&vq->poll);
- /* set len = 1 to mark this desc buffers done DMA */
+ /* set len to mark this desc buffers done DMA */
vq->heads[ubuf->desc].len = VHOST_DMA_DONE_LEN;
kref_put(&ubufs->kref, vhost_zerocopy_done_signal);
}
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index eb7263c3..ad72a1f 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -13,9 +13,15 @@
#include <linux/virtio_ring.h>
#include <linux/atomic.h>
-/* This is for zerocopy, used buffer len is set to 1 when lower device DMA
- * done */
-#define VHOST_DMA_DONE_LEN 1
+/*
+ * For transmit, used buffer len is unused; we override it to track buffer
+ * status internally; used for zerocopy tx only.
+ */
+/* Lower device DMA done */
+#define VHOST_DMA_DONE_LEN 2
+/* Lower device DMA in progress */
+#define VHOST_DMA_IN_PROGRESS 1
+/* Buffer unused */
#define VHOST_DMA_CLEAR_LEN 0
struct vhost_device;
--
MST
^ permalink raw reply related
* [PATCH net-next 8/8] vhost-net: reduce vq polling on tx zerocopy
From: Michael S. Tsirkin @ 2012-10-29 15:49 UTC (permalink / raw)
Cc: Alexander Duyck, Ian Campbell, kvm, Michael S. Tsirkin, netdev,
linux-kernel, virtualization, Eric Dumazet, Andrew Morton,
David S. Miller
In-Reply-To: <cover.1351524501.git.mst@redhat.com>
It seems that to avoid deadlocks it is enough to poll vq before
we are going to use the last buffer. This should be faster than
c70aa540c7a9f67add11ad3161096fb95233aa2e.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/vhost/net.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 8e9de79..3967f82 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -197,8 +197,16 @@ static void vhost_zerocopy_callback(struct ubuf_info *ubuf, int status)
{
struct vhost_ubuf_ref *ubufs = ubuf->ctx;
struct vhost_virtqueue *vq = ubufs->vq;
-
- vhost_poll_queue(&vq->poll);
+ int cnt = atomic_read(&ubufs->kref.refcount);
+
+ /*
+ * Trigger polling thread if guest stopped submitting new buffers:
+ * in this case, the refcount after decrement will eventually reach 1
+ * so here it is 2.
+ * We also trigger polling periodically after each 16 packets.
+ */
+ if (cnt <= 2 || !(cnt % 16))
+ vhost_poll_queue(&vq->poll);
/* set len to mark this desc buffers done DMA */
vq->heads[ubuf->desc].len = status ?
VHOST_DMA_FAILED_LEN : VHOST_DMA_DONE_LEN;
--
MST
^ permalink raw reply related
* [PATCH net-next 3/8] tun: report orphan frags errors to zero copy callback
From: Michael S. Tsirkin @ 2012-10-29 15:49 UTC (permalink / raw)
Cc: Alexander Duyck, Ian Campbell, kvm, Michael S. Tsirkin, netdev,
linux-kernel, virtualization, Eric Dumazet, Andrew Morton,
David S. Miller
In-Reply-To: <cover.1351524501.git.mst@redhat.com>
When tun transmits a zero copy skb, it orphans the frags
which might need to allocate extra memory, in atomic context.
If that fails, notify ubufs callback before freeing the skb
as a hint that device should disable zerocopy mode.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/net/tun.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 3157519..613f826 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -433,6 +433,7 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
drop:
dev->stats.tx_dropped++;
+ skb_tx_error(skb, -ENOMEM);
kfree_skb(skb);
return NETDEV_TX_OK;
}
--
MST
^ permalink raw reply related
* [PATCH net-next 2/8] skb: api to report errors for zero copy skbs
From: Michael S. Tsirkin @ 2012-10-29 15:49 UTC (permalink / raw)
Cc: Alexander Duyck, Ian Campbell, kvm, Michael S. Tsirkin, netdev,
linux-kernel, virtualization, Eric Dumazet, Andrew Morton,
David S. Miller
In-Reply-To: <cover.1351524501.git.mst@redhat.com>
Orphaning frags for zero copy skbs needs to allocate data in atomic
context so is has a chance to fail. If it does we currently discard
the skb which is safe, but we don't report anything to the caller,
so it can not recover by e.g. disabling zero copy.
Add an API to free skb reporting such errors: this is used
by tun in case orphaning frags fails.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
include/linux/skbuff.h | 1 +
net/core/skbuff.c | 19 +++++++++++++++++++
2 files changed, 20 insertions(+)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 8bac11b..0644432 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -568,6 +568,7 @@ static inline struct rtable *skb_rtable(const struct sk_buff *skb)
}
extern void kfree_skb(struct sk_buff *skb);
+extern void skb_tx_error(struct sk_buff *skb, int err);
extern void consume_skb(struct sk_buff *skb);
extern void __kfree_skb(struct sk_buff *skb);
extern struct kmem_cache *skbuff_head_cache;
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index eb31f6e..ad99c64 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -635,6 +635,25 @@ void kfree_skb(struct sk_buff *skb)
EXPORT_SYMBOL(kfree_skb);
/**
+ * kfree_skb_on_error - report an sk_buff xmit error
+ * @skb: buffer that triggered an error
+ *
+ * Report xmit error if a device callback is tracking this skb.
+ */
+void skb_tx_error(struct sk_buff *skb, int err)
+{
+ if (skb_shinfo(skb)->tx_flags & SKBTX_DEV_ZEROCOPY) {
+ struct ubuf_info *uarg;
+
+ uarg = skb_shinfo(skb)->destructor_arg;
+ if (uarg->callback)
+ uarg->callback(uarg, err);
+ skb_shinfo(skb)->tx_flags &= ~SKBTX_DEV_ZEROCOPY;
+ }
+}
+EXPORT_SYMBOL(skb_tx_error);
+
+/**
* consume_skb - free an skbuff
* @skb: buffer to free
*
--
MST
^ permalink raw reply related
* [PATCH net-next 1/8] skb: report completion status for zero copy skbs
From: Michael S. Tsirkin @ 2012-10-29 15:49 UTC (permalink / raw)
Cc: Michael S. Tsirkin, David S. Miller, Eric Dumazet, Andrew Morton,
Alexander Duyck, Ian Campbell, kvm, virtualization, netdev,
linux-kernel
In-Reply-To: <cover.1351524501.git.mst@redhat.com>
Even if skb is marked for zero copy, net core might still decide
to copy it later which is somewhat slower than a copy in user context:
besides copying the data we need to pin/unpin the pages.
Add a parameter reporting such cases through zero copy callback:
if this happens a lot, device can take this into account
and switch to copying in user context.
This patch updates all users but ignores the passed value for now:
it will be used by follow-up patches.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/vhost/vhost.c | 2 +-
drivers/vhost/vhost.h | 2 +-
include/linux/skbuff.h | 4 +++-
net/core/skbuff.c | 4 ++--
4 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 99ac2cb..92308b6 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -1600,7 +1600,7 @@ void vhost_ubuf_put_and_wait(struct vhost_ubuf_ref *ubufs)
kfree(ubufs);
}
-void vhost_zerocopy_callback(struct ubuf_info *ubuf)
+void vhost_zerocopy_callback(struct ubuf_info *ubuf, int zerocopy_status)
{
struct vhost_ubuf_ref *ubufs = ubuf->ctx;
struct vhost_virtqueue *vq = ubufs->vq;
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 1125af3..eb7263c3 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -191,7 +191,7 @@ bool vhost_enable_notify(struct vhost_dev *, struct vhost_virtqueue *);
int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log,
unsigned int log_num, u64 len);
-void vhost_zerocopy_callback(struct ubuf_info *);
+void vhost_zerocopy_callback(struct ubuf_info *, int);
int vhost_zerocopy_signal_used(struct vhost_virtqueue *vq);
#define vq_err(vq, fmt, ...) do { \
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 6a2c34e..8bac11b 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -235,11 +235,13 @@ enum {
/*
* The callback notifies userspace to release buffers when skb DMA is done in
* lower device, the skb last reference should be 0 when calling this.
+ * The zerocopy_status argument is 0 if zero copy transmit occurred,
+ * 1 on successful data copy; < 0 on out of memory error.
* The ctx field is used to track device context.
* The desc field is used to track userspace buffer index.
*/
struct ubuf_info {
- void (*callback)(struct ubuf_info *);
+ void (*callback)(struct ubuf_info *, int zerocopy_status);
void *ctx;
unsigned long desc;
};
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 6e04b1f..eb31f6e 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -519,7 +519,7 @@ static void skb_release_data(struct sk_buff *skb)
uarg = skb_shinfo(skb)->destructor_arg;
if (uarg->callback)
- uarg->callback(uarg);
+ uarg->callback(uarg, 0);
}
if (skb_has_frag_list(skb))
@@ -797,7 +797,7 @@ int skb_copy_ubufs(struct sk_buff *skb, gfp_t gfp_mask)
for (i = 0; i < num_frags; i++)
skb_frag_unref(skb, i);
- uarg->callback(uarg);
+ uarg->callback(uarg, 1);
/* skb frags point to kernel buffers */
for (i = num_frags - 1; i >= 0; i--) {
--
MST
^ permalink raw reply related
* [PATCH net-next 0/8] enable/disable zero copy tx dynamically
From: Michael S. Tsirkin @ 2012-10-29 15:49 UTC (permalink / raw)
Cc: Alexander Duyck, Ian Campbell, kvm, Michael S. Tsirkin, netdev,
linux-kernel, virtualization, Eric Dumazet, Andrew Morton,
David S. Miller
tun supports zero copy transmit since 0690899b4d4501b3505be069b9a687e68ccbe15b,
however you can only enable this mode if you know your workload does not
trigger heavy guest to host/host to guest traffic - otherwise you
get a (minor) performance regression.
This patchset addresses this problem by notifying the owner
device when callback is invoked because of a data copy.
This makes it possible to detect whether zero copy is appropriate
dynamically: we start in zero copy mode, when we detect
data copied we disable zero copy for a while.
With this patch applied, I get the same performance for
guest to host and guest to guest both with and without zero copy tx.
Michael S. Tsirkin (8):
skb: report completion status for zero copy skbs
skb: api to report errors for zero copy skbs
tun: report orphan frags errors to zero copy callback
vhost-net: cleanup macros for DMA status tracking
vhost: track zero copy failures using DMA length
vhost: move -net specific code out
vhost-net: select tx zero copy dynamically
vhost-net: reduce vq polling on tx zerocopy
drivers/net/tun.c | 1 +
drivers/vhost/net.c | 109 +++++++++++++++++++++++++++++++++++++++++++---
drivers/vhost/tcm_vhost.c | 1 +
drivers/vhost/vhost.c | 52 +++-------------------
drivers/vhost/vhost.h | 11 ++---
include/linux/skbuff.h | 5 ++-
net/core/skbuff.c | 23 +++++++++-
7 files changed, 141 insertions(+), 61 deletions(-)
--
MST
^ permalink raw reply
* Re: [PATCH v3 4/6] net/core: apply pm_runtime_set_memalloc_noio on network devices
From: Alan Stern @ 2012-10-29 15:44 UTC (permalink / raw)
To: Ming Lei
Cc: linux-kernel, Oliver Neukum, Minchan Kim, Greg Kroah-Hartman,
Rafael J. Wysocki, Jens Axboe, David S. Miller, Andrew Morton,
netdev, linux-usb, linux-pm, linux-mm, Eric Dumazet,
David Decotigny, Tom Herbert, Ingo Molnar
In-Reply-To: <1351513440-9286-5-git-send-email-ming.lei@canonical.com>
On Mon, 29 Oct 2012, Ming Lei wrote:
> Deadlock might be caused by allocating memory with GFP_KERNEL in
> runtime_resume callback of network devices in iSCSI situation, so
> mark network devices and its ancestor as 'memalloc_noio_resume'
> with the introduced pm_runtime_set_memalloc_noio().
> @@ -1411,6 +1414,8 @@ int netdev_register_kobject(struct net_device *net)
> *groups++ = &netstat_group;
> #endif /* CONFIG_SYSFS */
>
> + pm_runtime_set_memalloc_noio(dev, true);
> +
> error = device_add(dev);
> if (error)
> return error;
This is an example of what I described earlier. The
pm_runtime_set_memalloc_noio() call should come after device_add(), not
before.
(Not to mention that this version of the code doesn't correctly handle
the case where device_add() fails.)
Alan Stern
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH v7 15/16] openvswitch: use new hashtable implementation
From: Sasha Levin @ 2012-10-29 15:43 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: snitzer-H+wXaHxf7aLQT0dZR+AlfA, neilb-l3A5Bk7waGM,
fweisbec-Re5JQEeQqe8AvxtiuMwx3w,
Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA,
bfields-uC3wQj2KruNg9hUCZPvPmw,
paul.gortmaker-CWA4WttNNZF54TAoqtyWWQ,
dm-devel-H+wXaHxf7aLQT0dZR+AlfA, agk-H+wXaHxf7aLQT0dZR+AlfA,
aarcange-H+wXaHxf7aLQT0dZR+AlfA, rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
venkat.x.venkatsubra-QHcLZuEGTsvQT0dZR+AlfA,
ccaulfie-H+wXaHxf7aLQT0dZR+AlfA, mingo-X9Un+BFzKDI,
dev-yBygre7rU0TnMu66kgdUjQ, ericvh-Re5JQEeQqe8AvxtiuMwx3w,
josh-iaAMLnmF4UmaiuxdJuQwMA, rostedt-nx8X9YLhiw1AfugRpC6u6w,
lw-BthXqXjhjHXQFUHtdCDX3A, teigland-H+wXaHxf7aLQT0dZR+AlfA,
axboe-tSWWG44O7X1aa/9Udqfwiw, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
edumazet-hpIqsD4AKlfQT0dZR+AlfA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, ejt-H+wXaHxf7aLQT0dZR+AlfA,
ebiederm-aS9lmoZGLiVWk0Htik3J/w, tj-DgEjT+Ai2ygdnm+yROfE0A,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <20121029132931.GC16391@Krystal>
Hi Mathieu,
On Mon, Oct 29, 2012 at 9:29 AM, Mathieu Desnoyers
<mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org> wrote:
> * Sasha Levin (levinsasha928-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org) wrote:
> [...]
>> -static struct hlist_head *hash_bucket(struct net *net, const char *name)
>> -{
>> - unsigned int hash = jhash(name, strlen(name), (unsigned long) net);
>> - return &dev_table[hash & (VPORT_HASH_BUCKETS - 1)];
>> -}
>> -
>> /**
>> * ovs_vport_locate - find a port that has already been created
>> *
>> @@ -84,13 +76,12 @@ static struct hlist_head *hash_bucket(struct net *net, const char *name)
>> */
>> struct vport *ovs_vport_locate(struct net *net, const char *name)
>> {
>> - struct hlist_head *bucket = hash_bucket(net, name);
>> struct vport *vport;
>> struct hlist_node *node;
>> + int key = full_name_hash(name, strlen(name));
>>
>> - hlist_for_each_entry_rcu(vport, node, bucket, hash_node)
>> - if (!strcmp(name, vport->ops->get_name(vport)) &&
>> - net_eq(ovs_dp_get_net(vport->dp), net))
>> + hash_for_each_possible_rcu(dev_table, vport, node, hash_node, key)
>
> Is applying hash_32() on top of full_name_hash() needed and expected ?
Since this was pointed out in several of the patches, I'll answer it
just once here.
I've intentionally "allowed" double hashing with hash_32 to keep the
code simple.
hash_32() is pretty simple and gcc optimizes it to be almost nothing,
so doing that costs us a multiplication and a shift. On the other
hand, we benefit from keeping our code simple - how would we avoid
doing this double hash? adding a different hashtable function for
strings? or a new function for already hashed keys? I think we benefit
a lot from having to mul/shr instead of adding extra lines of code
here.
Thanks,
Sasha
^ permalink raw reply
* Re: [PATCH v7 09/16] SUNRPC/cache: use new hashtable implementation
From: Mathieu Desnoyers @ 2012-10-29 15:41 UTC (permalink / raw)
To: J. Bruce Fields
Cc: snitzer-H+wXaHxf7aLQT0dZR+AlfA, neilb-l3A5Bk7waGM,
fweisbec-Re5JQEeQqe8AvxtiuMwx3w,
Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA,
paul.gortmaker-CWA4WttNNZF54TAoqtyWWQ,
dm-devel-H+wXaHxf7aLQT0dZR+AlfA, agk-H+wXaHxf7aLQT0dZR+AlfA,
aarcange-H+wXaHxf7aLQT0dZR+AlfA, rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
venkat.x.venkatsubra-QHcLZuEGTsvQT0dZR+AlfA,
ccaulfie-H+wXaHxf7aLQT0dZR+AlfA, mingo-X9Un+BFzKDI,
dev-yBygre7rU0TnMu66kgdUjQ, ericvh-Re5JQEeQqe8AvxtiuMwx3w,
josh-iaAMLnmF4UmaiuxdJuQwMA, rostedt-nx8X9YLhiw1AfugRpC6u6w,
lw-BthXqXjhjHXQFUHtdCDX3A, teigland-H+wXaHxf7aLQT0dZR+AlfA,
Sasha Levin, axboe-tSWWG44O7X1aa/9Udqfwiw,
linux-nfs-u79uwXL29TY76Z2rM5mHXA, edumazet-hpIqsD4AKlfQT0dZR+AlfA,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, ejt-H+wXaHxf7aLQT0dZR+AlfA,
ebiederm-aS9lmoZGLiVWk0Htik3J/w, tj-DgEjT+Ai2ygdnm+yROfE0A,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b, Linus Torvalds,
davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <20121029151653.GC9502-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>
* J. Bruce Fields (bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org) wrote:
> On Mon, Oct 29, 2012 at 11:13:43AM -0400, Mathieu Desnoyers wrote:
> > * Linus Torvalds (torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org) wrote:
> > > On Mon, Oct 29, 2012 at 5:42 AM, Mathieu Desnoyers
> > > <mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org> wrote:
> > > >
> > > > So defining e.g.:
> > > >
> > > > #include <linux/log2.h>
> > > >
> > > > #define DFR_HASH_BITS (PAGE_SHIFT - ilog2(BITS_PER_LONG))
> > > >
> > > > would keep the intended behavior in all cases: use one page for the hash
> > > > array.
> > >
> > > Well, since that wasn't true before either because of the long-time
> > > bug you point out, clearly the page size isn't all that important. I
> > > think it's more important to have small and simple code, and "9" is
> > > certainly that, compared to playing ilog2 games with not-so-obvious
> > > things.
> > >
> > > Because there's no reason to believe that '9' is in any way a worse
> > > random number than something page-shift-related, is there? And getting
> > > away from *previous* overly-complicated size calculations that had
> > > been broken because they were too complicated and random, sounds like
> > > a good idea.
> >
> > Good point. I agree that unless we really care about the precise number
> > of TLB entries and cache lines used by this hash table, we might want to
> > stay away from page-size and pointer-size based calculation.
> >
> > It might not hurt to explain this in the patch changelog though.
>
> I'd also be happy to take that as a separate patch now.
FYIW: I've made a nice boo-boo above. It should have been:
#define DFR_HASH_BITS (PAGE_SHIFT - ilog2(sizeof(struct hlist_head)))
Because we happen to have a memory indexed in bytes, not in bits. I
guess this goes a long way proving Linus' point about virtues of trivial
code. ;-)
Thanks,
Mathieu
--
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* Re: [PATCH v3 2/6] PM / Runtime: introduce pm_runtime_set[get]_memalloc_noio()
From: Alan Stern @ 2012-10-29 15:41 UTC (permalink / raw)
To: Ming Lei
Cc: linux-kernel, Oliver Neukum, Minchan Kim, Greg Kroah-Hartman,
Rafael J. Wysocki, Jens Axboe, David S. Miller, Andrew Morton,
netdev, linux-usb, linux-pm, linux-mm
In-Reply-To: <1351513440-9286-3-git-send-email-ming.lei@canonical.com>
On Mon, 29 Oct 2012, Ming Lei wrote:
> The patch introduces the flag of memalloc_noio_resume in
> 'struct dev_pm_info' to help PM core to teach mm not allocating
> memory with GFP_KERNEL flag for avoiding probable deadlock
> problem.
>
> As explained in the comment, any GFP_KERNEL allocation inside
> runtime_resume on any one of device in the path from one block
> or network device to the root device in the device tree may cause
> deadlock, the introduced pm_runtime_set_memalloc_noio() sets or
> clears the flag on device of the path recursively.
>
> This patch also introduces pm_runtime_get_memalloc_noio() because
> the flag may be accessed in block device's error handling path
> (for example, usb device reset)
> +/*
> + * pm_runtime_get_memalloc_noio - Get a device's memalloc_noio flag.
> + * @dev: Device to handle.
> + *
> + * Return the device's memalloc_noio flag.
> + *
> + * The device power lock is held because bitfield is not SMP-safe.
> + */
> +bool pm_runtime_get_memalloc_noio(struct device *dev)
> +{
> + bool ret;
> + spin_lock_irq(&dev->power.lock);
> + ret = dev->power.memalloc_noio_resume;
> + spin_unlock_irq(&dev->power.lock);
> + return ret;
> +}
You don't need to acquire and release a spinlock just to read the
value. Reading bitfields _is_ SMP-safe; writing them is not.
> +/*
> + * pm_runtime_set_memalloc_noio - Set a device's memalloc_noio flag.
> + * @dev: Device to handle.
> + * @enable: True for setting the flag and False for clearing the flag.
> + *
> + * Set the flag for all devices in the path from the device to the
> + * root device in the device tree if @enable is true, otherwise clear
> + * the flag for devices in the path which sibliings don't set the flag.
s/which/whose/
s/ii/i
> + *
> + * The function should only be called by block device, or network
> + * device driver for solving the deadlock problem during runtime
> + * resume:
> + * if memory allocation with GFP_KERNEL is called inside runtime
> + * resume callback of any one of its ancestors(or the block device
> + * itself), the deadlock may be triggered inside the memory
> + * allocation since it might not complete until the block device
> + * becomes active and the involed page I/O finishes. The situation
> + * is pointed out first by Alan Stern. Network device are involved
> + * in iSCSI kind of situation.
> + *
> + * The lock of dev_hotplug_mutex is held in the function for handling
> + * hotplug race because pm_runtime_set_memalloc_noio() may be called
> + * in async probe().
> + */
> +void pm_runtime_set_memalloc_noio(struct device *dev, bool enable)
> +{
> + static DEFINE_MUTEX(dev_hotplug_mutex);
> +
> + mutex_lock(&dev_hotplug_mutex);
> + while (dev) {
Unless you think somebody is likely to call this function with dev
equal to NULL, this can simply be
for (;;) {
> + /* hold power lock since bitfield is not SMP-safe. */
> + spin_lock_irq(&dev->power.lock);
> + dev->power.memalloc_noio_resume = enable;
> + spin_unlock_irq(&dev->power.lock);
> +
> + dev = dev->parent;
> +
> + /* only clear the flag for one device if all
> + * children of the device don't set the flag.
> + */
> + if (!dev || (!enable &&
... thanks to this test.
> + device_for_each_child(dev, NULL,
> + dev_memalloc_noio)))
> + break;
> + }
> + mutex_unlock(&dev_hotplug_mutex);
> +}
This might not work if somebody calls pm_runtime_set_memalloc_noio(dev,
true) and then afterwards registers dev at the same time as someone
else calls pm_runtime_set_memalloc_noio(dev2, false), if dev and dev2
have the same parent.
Perhaps the kerneldoc should mention that this function must not be
called until after dev is registered.
Alan Stern
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH v7 09/16] SUNRPC/cache: use new hashtable implementation
From: J. Bruce Fields @ 2012-10-29 15:16 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Linus Torvalds, Sasha Levin, tj, akpm, linux-kernel, linux-mm,
paul.gortmaker, davem, rostedt, mingo, ebiederm, aarcange, ericvh,
netdev, josh, eric.dumazet, axboe, agk, dm-devel, neilb, ccaulfie,
teigland, Trond.Myklebust, fweisbec, jesse, venkat.x.venkatsubra,
ejt, snitzer, edumazet, linux-nfs, dev, rds-devel, lw
In-Reply-To: <20121029151343.GA17722@Krystal>
On Mon, Oct 29, 2012 at 11:13:43AM -0400, Mathieu Desnoyers wrote:
> * Linus Torvalds (torvalds@linux-foundation.org) wrote:
> > On Mon, Oct 29, 2012 at 5:42 AM, Mathieu Desnoyers
> > <mathieu.desnoyers@efficios.com> wrote:
> > >
> > > So defining e.g.:
> > >
> > > #include <linux/log2.h>
> > >
> > > #define DFR_HASH_BITS (PAGE_SHIFT - ilog2(BITS_PER_LONG))
> > >
> > > would keep the intended behavior in all cases: use one page for the hash
> > > array.
> >
> > Well, since that wasn't true before either because of the long-time
> > bug you point out, clearly the page size isn't all that important. I
> > think it's more important to have small and simple code, and "9" is
> > certainly that, compared to playing ilog2 games with not-so-obvious
> > things.
> >
> > Because there's no reason to believe that '9' is in any way a worse
> > random number than something page-shift-related, is there? And getting
> > away from *previous* overly-complicated size calculations that had
> > been broken because they were too complicated and random, sounds like
> > a good idea.
>
> Good point. I agree that unless we really care about the precise number
> of TLB entries and cache lines used by this hash table, we might want to
> stay away from page-size and pointer-size based calculation.
>
> It might not hurt to explain this in the patch changelog though.
I'd also be happy to take that as a separate patch now.
--b.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH v7 09/16] SUNRPC/cache: use new hashtable implementation
From: Mathieu Desnoyers @ 2012-10-29 15:13 UTC (permalink / raw)
To: Linus Torvalds
Cc: Sasha Levin, tj, akpm, linux-kernel, linux-mm, paul.gortmaker,
davem, rostedt, mingo, ebiederm, aarcange, ericvh, netdev, josh,
eric.dumazet, axboe, agk, dm-devel, neilb, ccaulfie, teigland,
Trond.Myklebust, bfields, fweisbec, jesse, venkat.x.venkatsubra,
ejt, snitzer, edumazet, linux-nfs, dev, rds-devel, lw
In-Reply-To: <CA+55aFzO8DJJP3HBfgqXFac9r3=bYK+_nYe4cuXiNFg-623s6w@mail.gmail.com>
* Linus Torvalds (torvalds@linux-foundation.org) wrote:
> On Mon, Oct 29, 2012 at 5:42 AM, Mathieu Desnoyers
> <mathieu.desnoyers@efficios.com> wrote:
> >
> > So defining e.g.:
> >
> > #include <linux/log2.h>
> >
> > #define DFR_HASH_BITS (PAGE_SHIFT - ilog2(BITS_PER_LONG))
> >
> > would keep the intended behavior in all cases: use one page for the hash
> > array.
>
> Well, since that wasn't true before either because of the long-time
> bug you point out, clearly the page size isn't all that important. I
> think it's more important to have small and simple code, and "9" is
> certainly that, compared to playing ilog2 games with not-so-obvious
> things.
>
> Because there's no reason to believe that '9' is in any way a worse
> random number than something page-shift-related, is there? And getting
> away from *previous* overly-complicated size calculations that had
> been broken because they were too complicated and random, sounds like
> a good idea.
Good point. I agree that unless we really care about the precise number
of TLB entries and cache lines used by this hash table, we might want to
stay away from page-size and pointer-size based calculation.
It might not hurt to explain this in the patch changelog though.
Thanks,
Mathieu
--
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* RE: [PATCH] sctp: Clean up type-punning in sctp_cmd_t union
From: David Laight @ 2012-10-29 15:07 UTC (permalink / raw)
To: David Miller, nhorman; +Cc: vyasevich, netdev, linux-sctp
In-Reply-To: <20121026.171019.702275326710216395.davem@davemloft.net>
> > On Fri, Oct 26, 2012 at 03:12:11PM -0400, Vlad Yasevich wrote:
> >> Look at commit 19c7e9ee that introduced this. I don't remember all
> >> the details any more, but the problem only occurred on ia64
> >> (probably due its speculative load handling).
> >>
> >> -vlad
> >>
> > Thanks Vlad, I'll have a look see.
>
> Ok, so this IA64 issue is all about accesses to uninitialized memory.
>
> I think Neil's change is thus the most desirable thing to do. Simple
> memset the object to zero.
I'd guess it an artifact of the C memory aliasing rules and unions.
If you write to a location as a one type but read it as another type
(and neither type is char) then the compiler is generally allowed to
schedule the read before the write (and may even optimise the write away).
This shouldn't happen if the fields are both members of the same union,
but casting some other address to a 'union *' separately for the
write and read isn't enough.
This probably means that this entire type-punning argument passing
scheme is actually broken.
David
^ permalink raw reply
* [PATCH net] tcp-repair: Handle zero-length data put in rcv queue
From: Pavel Emelyanov @ 2012-10-29 15:05 UTC (permalink / raw)
To: David Miller, Linux Netdev List; +Cc: Giorgos Mavrikas
When sending data into a tcp socket in repair state we should check
for the amount of data being 0 explicitly. Otherwise we'll have an skb
with seq == end_seq in rcv queue, but tcp doesn't expect this to happen
(in particular a warn_on in tcp_recvmsg shoots).
Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
Reported-by: Giorgos Mavrikas <gmavrikas@gmail.com>
---
>From 8f70f4ea4f509a3772ee7eb5d9d5c2571a86652a Mon Sep 17 00:00:00 2001
From: Pavel Emelyanov <xemul@parallels.com>
Date: Mon, 29 Oct 2012 18:12:41 +0400
Subject: [PATCH] fix for repair queue getback
Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
---
net/ipv4/tcp_input.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 432c366..ff5b746 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -4529,6 +4529,9 @@ int tcp_send_rcvq(struct sock *sk, struct msghdr *msg, size_t size)
struct tcphdr *th;
bool fragstolen;
+ if (size == 0)
+ return 0;
+
skb = alloc_skb(size + sizeof(*th), sk->sk_allocation);
if (!skb)
goto err;
--
1.7.6.5
^ permalink raw reply related
* Re: [PATCH v7 09/16] SUNRPC/cache: use new hashtable implementation
From: Linus Torvalds @ 2012-10-29 14:49 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: snitzer-H+wXaHxf7aLQT0dZR+AlfA, neilb-l3A5Bk7waGM,
fweisbec-Re5JQEeQqe8AvxtiuMwx3w,
Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA,
bfields-uC3wQj2KruNg9hUCZPvPmw,
paul.gortmaker-CWA4WttNNZF54TAoqtyWWQ,
dm-devel-H+wXaHxf7aLQT0dZR+AlfA, agk-H+wXaHxf7aLQT0dZR+AlfA,
aarcange-H+wXaHxf7aLQT0dZR+AlfA, rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
venkat.x.venkatsubra-QHcLZuEGTsvQT0dZR+AlfA,
ccaulfie-H+wXaHxf7aLQT0dZR+AlfA, mingo-X9Un+BFzKDI,
dev-yBygre7rU0TnMu66kgdUjQ, ericvh-Re5JQEeQqe8AvxtiuMwx3w,
josh-iaAMLnmF4UmaiuxdJuQwMA, rostedt-nx8X9YLhiw1AfugRpC6u6w,
lw-BthXqXjhjHXQFUHtdCDX3A, teigland-H+wXaHxf7aLQT0dZR+AlfA,
Sasha Levin, axboe-tSWWG44O7X1aa/9Udqfwiw,
linux-nfs-u79uwXL29TY76Z2rM5mHXA, edumazet-hpIqsD4AKlfQT0dZR+AlfA,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, ejt-H+wXaHxf7aLQT0dZR+AlfA,
ebiederm-aS9lmoZGLiVWk0Htik3J/w, tj-DgEjT+Ai2ygdnm+yROfE0A,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <20121029124229.GC11733@Krystal>
On Mon, Oct 29, 2012 at 5:42 AM, Mathieu Desnoyers
<mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org> wrote:
>
> So defining e.g.:
>
> #include <linux/log2.h>
>
> #define DFR_HASH_BITS (PAGE_SHIFT - ilog2(BITS_PER_LONG))
>
> would keep the intended behavior in all cases: use one page for the hash
> array.
Well, since that wasn't true before either because of the long-time
bug you point out, clearly the page size isn't all that important. I
think it's more important to have small and simple code, and "9" is
certainly that, compared to playing ilog2 games with not-so-obvious
things.
Because there's no reason to believe that '9' is in any way a worse
random number than something page-shift-related, is there? And getting
away from *previous* overly-complicated size calculations that had
been broken because they were too complicated and random, sounds like
a good idea.
Linus
^ permalink raw reply
* [PATCH net-next 1/1] rtnl/ipv4: use netconf msg to advertise rp_filter status
From: Nicolas Dichtel @ 2012-10-29 14:53 UTC (permalink / raw)
To: davem; +Cc: netdev, Nicolas Dichtel
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
include/uapi/linux/netconf.h | 1 +
net/ipv4/devinet.c | 24 ++++++++++++++++++++++++
2 files changed, 25 insertions(+)
diff --git a/include/uapi/linux/netconf.h b/include/uapi/linux/netconf.h
index d051372..75dcbc5 100644
--- a/include/uapi/linux/netconf.h
+++ b/include/uapi/linux/netconf.h
@@ -12,6 +12,7 @@ enum {
NETCONFA_UNSPEC,
NETCONFA_IFINDEX,
NETCONFA_FORWARDING,
+ NETCONFA_RP_FILTER,
__NETCONFA_MAX
};
#define NETCONFA_MAX (__NETCONFA_MAX - 1)
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index f8b1e04..f6db227 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -1451,6 +1451,8 @@ static int inet_netconf_msgsize_devconf(int type)
/* type -1 is used for ALL */
if (type == -1 || type == NETCONFA_FORWARDING)
size += nla_total_size(4);
+ if (type == -1 || type == NETCONFA_RP_FILTER)
+ size += nla_total_size(4);
return size;
}
@@ -1479,6 +1481,10 @@ static int inet_netconf_fill_devconf(struct sk_buff *skb, int ifindex,
nla_put_s32(skb, NETCONFA_FORWARDING,
IPV4_DEVCONF(*devconf, FORWARDING)) < 0)
goto nla_put_failure;
+ if ((type == -1 || type == NETCONFA_RP_FILTER) &&
+ nla_put_s32(skb, NETCONFA_RP_FILTER,
+ IPV4_DEVCONF(*devconf, RP_FILTER)) < 0)
+ goto nla_put_failure;
return nlmsg_end(skb, nlh);
@@ -1515,6 +1521,7 @@ errout:
static const struct nla_policy devconf_ipv4_policy[NETCONFA_MAX+1] = {
[NETCONFA_IFINDEX] = { .len = sizeof(int) },
[NETCONFA_FORWARDING] = { .len = sizeof(int) },
+ [NETCONFA_RP_FILTER] = { .len = sizeof(int) },
};
static int inet_netconf_get_devconf(struct sk_buff *in_skb,
@@ -1647,6 +1654,23 @@ static int devinet_conf_proc(ctl_table *ctl, int write,
i == IPV4_DEVCONF_ROUTE_LOCALNET - 1)
if ((new_value == 0) && (old_value != 0))
rt_cache_flush(net);
+ if (i == IPV4_DEVCONF_RP_FILTER - 1 &&
+ new_value != old_value) {
+ int ifindex;
+
+ if (cnf == net->ipv4.devconf_dflt)
+ ifindex = NETCONFA_IFINDEX_DEFAULT;
+ else if (cnf == net->ipv4.devconf_all)
+ ifindex = NETCONFA_IFINDEX_ALL;
+ else {
+ struct in_device *idev =
+ container_of(cnf, struct in_device,
+ cnf);
+ ifindex = idev->dev->ifindex;
+ }
+ inet_netconf_notify_devconf(net, NETCONFA_RP_FILTER,
+ ifindex, cnf);
+ }
}
return ret;
--
1.7.12
^ permalink raw reply related
* Re: 3.7-rc3 oops on reboot
From: Roland Stigge @ 2012-10-29 14:41 UTC (permalink / raw)
To: Alexandre Pereira da Silva
Cc: David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, netdev, linux-kernel
In-Reply-To: <CAAAP30HWhz_h4AVwjkw9APw6pv0ydO=GZ89uaBhKbmjw60ym4Q@mail.gmail.com>
On 10/29/2012 02:51 PM, Alexandre Pereira da Silva wrote:
>> I just tried both on a PHY3250 reference machine, and on a custom board
>> and couldn't reproduce sth. like this. Booted the kernel and connected
>> the ethernet cable later on, as well as with cable connected on boot.
>
> This is happening on my custom board. I didn't try this on the
> reference board yet.
>
>> Any related patches applied? Can you reproduce this on a reference
>> machine? Can you provide a .config for this? Any more detail about how
>> this could be reproduced?
>
> No patches applied. I'm trying to bisect this. This happens on a clean
> -rc1 as well. Doesn't happen on v3.6.
>
> Attached my .config
>
> To reproduce this, I am just running "reboot". I have the ethernet
> cable always connected here.
Just tried this with your .config on the PHY3250 (w/o your initrd
thought) but couldn't reproduce the issue.
Maybe it's related to your Micrel ethernet PHY (activated in your
config) which I don't have hardware for?
Otherwise, we would need to have a look at your devicetree.
Roland
^ permalink raw reply
* Re: switching network namespace midway
From: Stephen Hemminger @ 2012-10-29 14:23 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: Benjamin LaHaise, rsa, netdev
In-Reply-To: <877gqb883y.fsf@xmission.com>
On Sat, 27 Oct 2012 22:43:13 -0700
ebiederm@xmission.com (Eric W. Biederman) wrote:
> Stephen Hemminger <shemminger@vyatta.com> writes:
>
> > I noticed that the L2TP sockets are not being moved to the correct name
> > space.
> >
> > Something like this is probably needed.
>
> This is almost right.
>
> There needs to be a line in l2tp_tunnel_create that verifies
> the network namespace of the socket derived from a file descriptor
> and the passed in network namespace match.
>
> For the l2tp_tunnel_sock_create case where we have a socket that is not
> exported to userspace using sk_change_net seems appropriate to avoid
> reference counting problems. And it may be worth moving that work into
> sk_create_kern. But we need a network namespace hook that will lookup
> all l2tp tunnel sockets when a network namespace is being destroyed and
> remove them. I think we can hit this bug with rmmod as well.
Since I don't use netns or L2TP for real, someone else needs to take
up the crusade here.
^ permalink raw reply
* Re: [PATCH] vxlan nits
From: Stephen Hemminger @ 2012-10-29 14:20 UTC (permalink / raw)
To: Vincent Bernat; +Cc: David L Stevens, David Miller, netdev
In-Reply-To: <m3objm45n7.fsf@neo.luffy.cx>
On Sun, 28 Oct 2012 23:02:20 +0100
Vincent Bernat <bernat@luffy.cx> wrote:
> ❦ 19 octobre 2012 22:28 CEST, Stephen Hemminger <shemminger@vyatta.com> :
>
> > The plan is to add IPV6 support, in which case group address could
> > be IPV6.
>
> Hi Stephen!
>
> Is it something that you plan to add soon?
The only real question is whether to have one driver do dual stack with
one forwarding table, or separate drivers (like GRE). I prefer the former.
Since the RFC does not include IPv6 yet, there is no rush and the final
standard may address some of the corner case issues like fragmentation.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox