* [PATCH 17/18] vhost: don't bother copying iovecs in handle_rx(), kill memcpy_toiovecend()
From: Al Viro @ 2015-01-31 3:56 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20150131035513.GK29656@ZenIV.linux.org.uk>
From: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
drivers/vhost/net.c | 79 ++++++++++++++---------------------------------------
include/linux/uio.h | 3 --
lib/iovec.c | 26 ------------------
3 files changed, 20 insertions(+), 88 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index d86cc9b..73c0ebf 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -84,10 +84,6 @@ struct vhost_net_ubuf_ref {
struct vhost_net_virtqueue {
struct vhost_virtqueue vq;
- /* hdr is used to store the virtio header.
- * Since each iovec has >= 1 byte length, we never need more than
- * header length entries to store the header. */
- struct iovec hdr[sizeof(struct virtio_net_hdr_mrg_rxbuf)];
size_t vhost_hlen;
size_t sock_hlen;
/* vhost zerocopy support fields below: */
@@ -235,44 +231,6 @@ static bool vhost_sock_zcopy(struct socket *sock)
sock_flag(sock->sk, SOCK_ZEROCOPY);
}
-/* Pop first len bytes from iovec. Return number of segments used. */
-static int move_iovec_hdr(struct iovec *from, struct iovec *to,
- size_t len, int iov_count)
-{
- int seg = 0;
- size_t size;
-
- while (len && seg < iov_count) {
- size = min(from->iov_len, len);
- to->iov_base = from->iov_base;
- to->iov_len = size;
- from->iov_len -= size;
- from->iov_base += size;
- len -= size;
- ++from;
- ++to;
- ++seg;
- }
- return seg;
-}
-/* Copy iovec entries for len bytes from iovec. */
-static void copy_iovec_hdr(const struct iovec *from, struct iovec *to,
- size_t len, int iovcount)
-{
- int seg = 0;
- size_t size;
-
- while (len && seg < iovcount) {
- size = min(from->iov_len, len);
- to->iov_base = from->iov_base;
- to->iov_len = size;
- len -= size;
- ++from;
- ++to;
- ++seg;
- }
-}
-
/* 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
@@ -570,9 +528,9 @@ static void handle_rx(struct vhost_net *net)
.msg_controllen = 0,
.msg_flags = MSG_DONTWAIT,
};
- struct virtio_net_hdr_mrg_rxbuf hdr = {
- .hdr.flags = 0,
- .hdr.gso_type = VIRTIO_NET_HDR_GSO_NONE
+ struct virtio_net_hdr hdr = {
+ .flags = 0,
+ .gso_type = VIRTIO_NET_HDR_GSO_NONE
};
size_t total_len = 0;
int err, mergeable;
@@ -580,6 +538,7 @@ static void handle_rx(struct vhost_net *net)
size_t vhost_hlen, sock_hlen;
size_t vhost_len, sock_len;
struct socket *sock;
+ struct iov_iter fixup;
mutex_lock(&vq->mutex);
sock = vq->private_data;
@@ -624,14 +583,17 @@ static void handle_rx(struct vhost_net *net)
break;
}
/* We don't need to be notified again. */
- if (unlikely((vhost_hlen)))
- /* Skip header. TODO: support TSO. */
- move_iovec_hdr(vq->iov, nvq->hdr, vhost_hlen, in);
- else
- /* Copy the header for use in VIRTIO_NET_F_MRG_RXBUF:
- * needed because recvmsg can modify msg_iov. */
- copy_iovec_hdr(vq->iov, nvq->hdr, sock_hlen, in);
- iov_iter_init(&msg.msg_iter, READ, vq->iov, in, sock_len);
+ iov_iter_init(&msg.msg_iter, READ, vq->iov, in, vhost_len);
+ fixup = msg.msg_iter;
+ if (unlikely((vhost_hlen))) {
+ /* We will supply the header ourselves
+ * TODO: support TSO. */
+ iov_iter_advance(&msg.msg_iter, vhost_hlen);
+ } else {
+ /* It'll come from socket; we'll need to patch
+ * ->num_buffers over if VIRTIO_NET_F_MRG_RXBUF */
+ iov_iter_advance(&fixup, sizeof(hdr));
+ }
err = sock->ops->recvmsg(NULL, sock, &msg,
sock_len, MSG_DONTWAIT | MSG_TRUNC);
/* Userspace might have consumed the packet meanwhile:
@@ -643,18 +605,17 @@ static void handle_rx(struct vhost_net *net)
vhost_discard_vq_desc(vq, headcount);
continue;
}
+ /* Supply virtio_net_hdr if VHOST_NET_F_VIRTIO_NET_HDR */
if (unlikely(vhost_hlen) &&
- memcpy_toiovecend(nvq->hdr, (unsigned char *)&hdr, 0,
- vhost_hlen)) {
+ copy_to_iter(&hdr, sizeof(hdr), &fixup) != sizeof(hdr)) {
vq_err(vq, "Unable to write vnet_hdr at addr %p\n",
vq->iov->iov_base);
break;
}
- /* TODO: Should check and handle checksum. */
+ /* Supply (or replace) ->num_buffers if VIRTIO_NET_F_MRG_RXBUF
+ * TODO: Should check and handle checksum. */
if (likely(mergeable) &&
- memcpy_toiovecend(nvq->hdr, (unsigned char *)&headcount,
- offsetof(typeof(hdr), num_buffers),
- sizeof hdr.num_buffers)) {
+ copy_to_iter(&headcount, 2, &fixup) != 2) {
vq_err(vq, "Failed num_buffers write");
vhost_discard_vq_desc(vq, headcount);
break;
diff --git a/include/linux/uio.h b/include/linux/uio.h
index af3439f..02bd8a9 100644
--- a/include/linux/uio.h
+++ b/include/linux/uio.h
@@ -137,7 +137,4 @@ size_t csum_and_copy_from_iter(void *addr, size_t bytes, __wsum *csum, struct io
int memcpy_fromiovecend(unsigned char *kdata, const struct iovec *iov,
int offset, int len);
-int memcpy_toiovecend(const struct iovec *v, unsigned char *kdata,
- int offset, int len);
-
#endif
diff --git a/lib/iovec.c b/lib/iovec.c
index 4a90875..d8f17a9 100644
--- a/lib/iovec.c
+++ b/lib/iovec.c
@@ -3,32 +3,6 @@
#include <linux/uio.h>
/*
- * Copy kernel to iovec. Returns -EFAULT on error.
- */
-
-int memcpy_toiovecend(const struct iovec *iov, unsigned char *kdata,
- int offset, int len)
-{
- int copy;
- for (; len > 0; ++iov) {
- /* Skip over the finished iovecs */
- if (unlikely(offset >= iov->iov_len)) {
- offset -= iov->iov_len;
- continue;
- }
- copy = min_t(unsigned int, iov->iov_len - offset, len);
- if (copy_to_user(iov->iov_base + offset, kdata, copy))
- return -EFAULT;
- offset = 0;
- kdata += copy;
- len -= copy;
- }
-
- return 0;
-}
-EXPORT_SYMBOL(memcpy_toiovecend);
-
-/*
* Copy iovec to kernel. Returns -EFAULT on error.
*/
--
2.1.4
^ permalink raw reply related
* [PATCH 18/18] vhost: vhost_scsi_handle_vq() should just use copy_from_user()
From: Al Viro @ 2015-01-31 3:56 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20150131035513.GK29656@ZenIV.linux.org.uk>
From: Al Viro <viro@zeniv.linux.org.uk>
it has just verified that it asks no more than the length of the
first segment of iovec.
And with that the last user of stuff in lib/iovec.c is gone.
RIP.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
drivers/vhost/scsi.c | 2 +-
include/linux/uio.h | 2 --
lib/Makefile | 2 +-
lib/iovec.c | 36 ------------------------------------
4 files changed, 2 insertions(+), 40 deletions(-)
delete mode 100644 lib/iovec.c
diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index d695b16..dc78d87 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -1079,7 +1079,7 @@ vhost_scsi_handle_vq(struct vhost_scsi *vs, struct vhost_virtqueue *vq)
req_size, vq->iov[0].iov_len);
break;
}
- ret = memcpy_fromiovecend(req, &vq->iov[0], 0, req_size);
+ ret = copy_from_user(req, vq->iov[0].iov_base, req_size);
if (unlikely(ret)) {
vq_err(vq, "Faulted on virtio_scsi_cmd_req\n");
break;
diff --git a/include/linux/uio.h b/include/linux/uio.h
index 02bd8a9..3e0cb4e 100644
--- a/include/linux/uio.h
+++ b/include/linux/uio.h
@@ -135,6 +135,4 @@ static inline void iov_iter_reexpand(struct iov_iter *i, size_t count)
size_t csum_and_copy_to_iter(void *addr, size_t bytes, __wsum *csum, struct iov_iter *i);
size_t csum_and_copy_from_iter(void *addr, size_t bytes, __wsum *csum, struct iov_iter *i);
-int memcpy_fromiovecend(unsigned char *kdata, const struct iovec *iov,
- int offset, int len);
#endif
diff --git a/lib/Makefile b/lib/Makefile
index 3c3b30b..1071d06 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -24,7 +24,7 @@ obj-y += lockref.o
obj-y += bcd.o div64.o sort.o parser.o halfmd4.o debug_locks.o random32.o \
bust_spinlocks.o hexdump.o kasprintf.o bitmap.o scatterlist.o \
- gcd.o lcm.o list_sort.o uuid.o flex_array.o iovec.o clz_ctz.o \
+ gcd.o lcm.o list_sort.o uuid.o flex_array.o clz_ctz.o \
bsearch.o find_last_bit.o find_next_bit.o llist.o memweight.o kfifo.o \
percpu-refcount.o percpu_ida.o rhashtable.o reciprocal_div.o
obj-y += string_helpers.o
diff --git a/lib/iovec.c b/lib/iovec.c
deleted file mode 100644
index d8f17a9..0000000
--- a/lib/iovec.c
+++ /dev/null
@@ -1,36 +0,0 @@
-#include <linux/uaccess.h>
-#include <linux/export.h>
-#include <linux/uio.h>
-
-/*
- * Copy iovec to kernel. Returns -EFAULT on error.
- */
-
-int memcpy_fromiovecend(unsigned char *kdata, const struct iovec *iov,
- int offset, int len)
-{
- /* No data? Done! */
- if (len == 0)
- return 0;
-
- /* Skip over the finished iovecs */
- while (offset >= iov->iov_len) {
- offset -= iov->iov_len;
- iov++;
- }
-
- while (len > 0) {
- u8 __user *base = iov->iov_base + offset;
- int copy = min_t(unsigned int, len, iov->iov_len - offset);
-
- offset = 0;
- if (copy_from_user(kdata, base, copy))
- return -EFAULT;
- len -= copy;
- kdata += copy;
- iov++;
- }
-
- return 0;
-}
-EXPORT_SYMBOL(memcpy_fromiovecend);
--
2.1.4
^ permalink raw reply related
* [PATCH 16/18] vhost: don't bother with copying iovec in handle_tx()
From: Al Viro @ 2015-01-31 3:56 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20150131035513.GK29656@ZenIV.linux.org.uk>
From: Al Viro <viro@zeniv.linux.org.uk>
just advance the msg.msg_iter and be done with that.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
drivers/vhost/net.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 6906f76..d86cc9b 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -336,7 +336,7 @@ static void handle_tx(struct vhost_net *net)
{
struct vhost_net_virtqueue *nvq = &net->vqs[VHOST_NET_VQ_TX];
struct vhost_virtqueue *vq = &nvq->vq;
- unsigned out, in, s;
+ unsigned out, in;
int head;
struct msghdr msg = {
.msg_name = NULL,
@@ -395,16 +395,17 @@ static void handle_tx(struct vhost_net *net)
break;
}
/* Skip header. TODO: support TSO. */
- s = move_iovec_hdr(vq->iov, nvq->hdr, hdr_size, out);
len = iov_length(vq->iov, out);
iov_iter_init(&msg.msg_iter, WRITE, vq->iov, out, len);
+ iov_iter_advance(&msg.msg_iter, hdr_size);
/* Sanity check */
- if (!len) {
+ if (!iov_iter_count(&msg.msg_iter)) {
vq_err(vq, "Unexpected header len for TX: "
"%zd expected %zd\n",
- iov_length(nvq->hdr, s), hdr_size);
+ len, hdr_size);
break;
}
+ len = iov_iter_count(&msg.msg_iter);
zcopy_used = zcopy && len >= VHOST_GOODCOPY_LEN
&& (nvq->upend_idx + 1) % UIO_MAXIOV !=
--
2.1.4
^ permalink raw reply related
* [PATCH 15/18] switch vhost get_indirect() to iov_iter, kill memcpy_fromiovec()
From: Al Viro @ 2015-01-31 3:56 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20150131035513.GK29656@ZenIV.linux.org.uk>
From: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
drivers/vhost/vhost.c | 6 ++++--
include/linux/uio.h | 1 -
lib/iovec.c | 25 -------------------------
3 files changed, 4 insertions(+), 28 deletions(-)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index cb807d0..2ee2826 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -1125,6 +1125,7 @@ static int get_indirect(struct vhost_virtqueue *vq,
struct vring_desc desc;
unsigned int i = 0, count, found = 0;
u32 len = vhost32_to_cpu(vq, indirect->len);
+ struct iov_iter from;
int ret;
/* Sanity check */
@@ -1142,6 +1143,7 @@ static int get_indirect(struct vhost_virtqueue *vq,
vq_err(vq, "Translation failure %d in indirect.\n", ret);
return ret;
}
+ iov_iter_init(&from, READ, vq->indirect, ret, len);
/* We will use the result as an address to read from, so most
* architectures only need a compiler barrier here. */
@@ -1164,8 +1166,8 @@ static int get_indirect(struct vhost_virtqueue *vq,
i, count);
return -EINVAL;
}
- if (unlikely(memcpy_fromiovec((unsigned char *)&desc,
- vq->indirect, sizeof desc))) {
+ if (unlikely(copy_from_iter(&desc, sizeof(desc), &from) !=
+ sizeof(desc))) {
vq_err(vq, "Failed indirect descriptor: idx %d, %zx\n",
i, (size_t)vhost64_to_cpu(vq, indirect->addr) + i * sizeof desc);
return -EINVAL;
diff --git a/include/linux/uio.h b/include/linux/uio.h
index 1c5e453..af3439f 100644
--- a/include/linux/uio.h
+++ b/include/linux/uio.h
@@ -135,7 +135,6 @@ static inline void iov_iter_reexpand(struct iov_iter *i, size_t count)
size_t csum_and_copy_to_iter(void *addr, size_t bytes, __wsum *csum, struct iov_iter *i);
size_t csum_and_copy_from_iter(void *addr, size_t bytes, __wsum *csum, struct iov_iter *i);
-int memcpy_fromiovec(unsigned char *kdata, struct iovec *iov, int len);
int memcpy_fromiovecend(unsigned char *kdata, const struct iovec *iov,
int offset, int len);
int memcpy_toiovecend(const struct iovec *v, unsigned char *kdata,
diff --git a/lib/iovec.c b/lib/iovec.c
index 2d99cb4..4a90875 100644
--- a/lib/iovec.c
+++ b/lib/iovec.c
@@ -3,31 +3,6 @@
#include <linux/uio.h>
/*
- * Copy iovec to kernel. Returns -EFAULT on error.
- *
- * Note: this modifies the original iovec.
- */
-
-int memcpy_fromiovec(unsigned char *kdata, struct iovec *iov, int len)
-{
- while (len > 0) {
- if (iov->iov_len) {
- int copy = min_t(unsigned int, len, iov->iov_len);
- if (copy_from_user(kdata, iov->iov_base, copy))
- return -EFAULT;
- len -= copy;
- kdata += copy;
- iov->iov_base += copy;
- iov->iov_len -= copy;
- }
- iov++;
- }
-
- return 0;
-}
-EXPORT_SYMBOL(memcpy_fromiovec);
-
-/*
* Copy kernel to iovec. Returns -EFAULT on error.
*/
--
2.1.4
^ permalink raw reply related
* Re: [RFC][PATCHSET] more iov_iter conversion in net/*
From: Al Viro @ 2015-01-31 4:03 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20150131035513.GK29656@ZenIV.linux.org.uk>
On Sat, Jan 31, 2015 at 03:55:13AM +0000, Al Viro wrote:
> ->sendmsg() side of that business, now. By the end of it, we
> get all ->sendmsg() instances leaving iovec unchanged and ->msg_iter -
> drained.
>
> 1/18: netlink: make the check for "send from tx_ring" deterministic
> As discussed last year.
> 2/18: raw_send_hdrinc(): pass msghdr
> Switch from passing msg->iov_iter.iov to passing msg itself
> 3/18: rawv6_send_hdrinc(): pass msghdr
> Ditto
> 4/18: propagate msghdr all way down to __qp_memcpy_to_queue()
> Ditto
> 5/18: switch rxrpc_send_data() to iov_iter primitives
> Convert skb_add_data() to iov_iter; allows to get rid of the explicit
> messing with iovec in its only caller - skb_add_data() will keep advancing
> ->msg_iter for us, so there's no need to similate that manually.
> 6/18: make the users of rxrpc_kernel_send_data() set kvec-backed msg_iter
> properly
> Use iov_iter_kvec() there, get rid of set_fs() games - now that
> rxrpc_send_data() uses iov_iter primitives, it'll handle ITER_KVEC just
> fine.
> 7/18: stash a pointer to msghdr in struct ping_fakehdr
> ... instead of storing its ->mgs_iter.iov there
> 8/18: convert tcp_sendmsg() to iov_iter primitives
> There's one potentially subtle change here: in case of short
> copy from userland, mainline tcp_send_syn_data() discards the skb it
> has allocated and falls back to normal path, where we'll send as much
> as possible after rereading the same data again. This patch trims
> SYN+data skb instead - that way we don't need to copy from the same
> place twice. I _think_ it's correct, but I'd really appreciate a review
> of that one.
> 9/18: switch memcpy_fromiovec()/memcpy_fromiovecend() users to
> copy_from_iter()
> That takes care of the majority of ->sendmsg() instances.
> 10/18: tipc ->sendmsg() conversion
> This one needs to copy the same data from user potentially more than
> once. Sadly, MTU changes can trigger that ;-/
> 11/18: bury net/core/iovec.c - nothing in there is used anymore
> 12/18: switch af_alg_make_sg() to iov_iter
> With that, all ->sendmsg() instances are converted to iov_iter
> primitives and are agnostic wrt the kind of iov_iter they are working with.
> So's the last remaining ->recvmsg() instance that wasn't kind-agnostic yet.
> All ->sendmsg() and ->recvmsg() advance ->msg_iter by the amount actually
> copied and none of them modifies the underlying iovec, etc.
> 13/18: net/socket.c: fold do_sock_{read,write} into callers
> 14/18: switch sockets to ->read_iter/->write_iter
> 15/18: switch vhost get_indirect() to iov_iter, kill memcpy_fromiovec()
> 16/18: vhost: don't bother with copying iovec in handle_tx()
> 17/18: vhost: don't bother copying iovecs in handle_rx(), kill
> memcpy_toiovecend()
> 18/18: vhost: vhost_scsi_handle_vq() should just use copy_from_user()
> ... and with that lib/iovec.c is gone - nothing in there has callers
> left.
>
> The pile after that one will be dealing with the kernel_sendmsg and
> kernel_recvmg callers - at that point we can start reaping benefits of
> consistent way ->msg_iter is handled. Note that after these changes if
> iov_iter_kvec() is used to initialize ->msg_iter, we don't need the games
> with get_fs()/set_fs() anymore; just sock_sendmsg()/sock_recvmsg() will do,
> so quite a few of those kernel_{send,recv}msg() callers will turn into
> sock_{send,recv}msg() ones.
FWIW, for those who prefer to review stuff in git, this pile is in
git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git for-davem, and
diffstat is
crypto/af_alg.c | 40 ++----
crypto/algif_hash.c | 45 +++---
crypto/algif_skcipher.c | 74 +++++-----
drivers/misc/vmw_vmci/vmci_queue_pair.c | 16 +--
drivers/vhost/net.c | 88 ++++--------
drivers/vhost/scsi.c | 2 +-
drivers/vhost/vhost.c | 6 +-
fs/afs/rxrpc.c | 14 +-
include/crypto/if_alg.h | 3 +-
include/linux/skbuff.h | 14 +-
include/linux/socket.h | 7 -
include/linux/uio.h | 6 -
include/linux/vmw_vmci_api.h | 2 +-
include/net/ping.h | 2 +-
include/net/sock.h | 18 ++-
include/net/udplite.h | 3 +-
lib/Makefile | 2 +-
lib/iovec.c | 87 ------------
net/core/Makefile | 2 +-
net/core/iovec.c | 137 -------------------
net/ipv4/ip_output.c | 6 +-
net/ipv4/ping.c | 17 ++-
net/ipv4/raw.c | 7 +-
net/ipv4/tcp.c | 233 +++++++++++++++-----------------
net/ipv4/tcp_output.c | 11 +-
net/ipv6/ping.c | 3 +-
net/ipv6/raw.c | 7 +-
net/netlink/af_netlink.c | 4 +
net/rxrpc/ar-output.c | 46 ++-----
net/socket.c | 76 ++++-------
net/tipc/msg.c | 7 +-
net/tipc/socket.c | 14 +-
net/vmw_vsock/vmci_transport.c | 3 +-
33 files changed, 316 insertions(+), 686 deletions(-)
Please, review.
^ permalink raw reply
* Is sch_teql still useful?
From: Cong Wang @ 2015-01-31 4:23 UTC (permalink / raw)
To: Jamal Hadi Salim; +Cc: netdev, David Miller, Stephen Hemminger
Hi,
Since we have bonding alb, is it still needed to have sch_teql since
they do the same thing, load balancing over multiple interfaces? Also
sch_teql mixes netdevice with qdisc, which somewhat breaks the
abstraction?
I don't dig the history so I could easily miss something here.
Thanks!
^ permalink raw reply
* Re: [PATCH 7/9] netfilter: nft_hash: add support for timeouts
From: Herbert Xu @ 2015-01-31 4:29 UTC (permalink / raw)
To: Patrick McHardy
Cc: tgraf, davem, David.Laight, ying.xue, paulmck, netdev,
netfilter-devel
In-Reply-To: <1422603994-5836-8-git-send-email-kaber@trash.net>
On Fri, Jan 30, 2015 at 07:46:32AM +0000, Patrick McHardy wrote:
>
> + mutex_lock(&priv->ht.mutex);
> + tbl = rht_dereference(priv->ht.tbl, &priv->ht);
> + for (i = 0; i < tbl->size; i++) {
> + rht_for_each_entry_safe(he, pos, next, tbl, i, node) {
> + if (!nft_set_ext_exists(&he->ext, NFT_SET_EXT_TIMEOUT))
> + continue;
> + timeout = *nft_set_ext_timeout(&he->ext);
> + if (time_before(jiffies, timeout))
> + continue;
> +
> + rhashtable_remove(&priv->ht, &he->node);
> + nft_hash_elem_destroy(set, he);
> + }
> + }
> + mutex_unlock(&priv->ht.mutex);
What if somebody is currently walking over the table? Shouldn't
you do an RCU free here instead of immediately destroying the
element?
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* netfilter: Use rhashtable walk iterator
From: Herbert Xu @ 2015-01-31 4:31 UTC (permalink / raw)
To: David Miller; +Cc: tgraf, ying.xue, kaber, paulmck, netdev, netfilter-devel
In-Reply-To: <20150131031356.GA26781@gondor.apana.org.au>
On Sat, Jan 31, 2015 at 02:13:56PM +1100, Herbert Xu wrote:
>
> No problems. Here is a new version of these two patches which
> hopefully should work on netfilter as well.
And here is the patch for netfilter.
-- >8 --
This patch gets rid of the manual rhashtable walk in nft_hash
which touches rhashtable internals that should not be exposed.
It does so by using the rhashtable iterator primitives.
Note that I'm leaving nft_hash_destroy alone since it's only
invoked on shutdown and it shouldn't be affected by changes
to rhashtable internals (or at least not what I'm planning to
change).
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
diff --git a/net/netfilter/nft_hash.c b/net/netfilter/nft_hash.c
index 75887d7..61e6c40 100644
--- a/net/netfilter/nft_hash.c
+++ b/net/netfilter/nft_hash.c
@@ -130,31 +130,50 @@ static void nft_hash_walk(const struct nft_ctx *ctx, const struct nft_set *set,
struct nft_set_iter *iter)
{
struct rhashtable *priv = nft_set_priv(set);
- const struct bucket_table *tbl;
const struct nft_hash_elem *he;
+ struct rhashtable_iter hti;
struct nft_set_elem elem;
- unsigned int i;
+ int err;
- tbl = rht_dereference_rcu(priv->tbl, priv);
- for (i = 0; i < tbl->size; i++) {
- struct rhash_head *pos;
+ err = rhashtable_walk_init(priv, &hti);
+ iter->err = err;
+ if (err)
+ return;
+
+ err = rhashtable_walk_start(&hti);
+ if (err && err != -EAGAIN) {
+ iter->err = err;
+ goto out;
+ }
- rht_for_each_entry_rcu(he, pos, tbl, i, node) {
- if (iter->count < iter->skip)
- goto cont;
+ while ((he = rhashtable_walk_next(&hti))) {
+ if (IS_ERR(he)) {
+ err = PTR_ERR(he);
+ if (err != -EAGAIN) {
+ iter->err = err;
+ goto out;
+ }
+ }
+
+ if (iter->count < iter->skip)
+ goto cont;
+
+ memcpy(&elem.key, &he->key, sizeof(elem.key));
+ if (set->flags & NFT_SET_MAP)
+ memcpy(&elem.data, he->data, sizeof(elem.data));
+ elem.flags = 0;
- memcpy(&elem.key, &he->key, sizeof(elem.key));
- if (set->flags & NFT_SET_MAP)
- memcpy(&elem.data, he->data, sizeof(elem.data));
- elem.flags = 0;
+ iter->err = iter->fn(ctx, set, iter, &elem);
+ if (iter->err < 0)
+ goto out;
- iter->err = iter->fn(ctx, set, iter, &elem);
- if (iter->err < 0)
- return;
cont:
- iter->count++;
- }
+ iter->count++;
}
+
+out:
+ rhashtable_walk_stop(&hti);
+ rhashtable_walk_exit(&hti);
}
static unsigned int nft_hash_privsize(const struct nlattr * const nla[])
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply related
* Re: [PATCH 2/6] rhashtable: Use a single bucket lock for sibling buckets
From: Herbert Xu @ 2015-01-31 4:34 UTC (permalink / raw)
To: Thomas Graf; +Cc: davem, netdev, ying.xue
In-Reply-To: <14d537500868b4ff0396168cf9d5db95d327c128.1422576760.git.tgraf@suug.ch>
Thomas Graf <tgraf@suug.ch> wrote:
> rhashtable currently allows to use a bucket lock per bucket. This
> requires multiple levels of complicated nested locking because when
> resizing, a single bucket of the smaller table will map to two
> buckets in the larger table. So far rhashtable has explicitly locked
> both buckets in the larger table.
>
> By excluding the highest bit of the hash from the bucket lock map and
> thus only allowing locks to buckets in a ratio of 1:2, the locking
> can be simplified a lot without losing the benefits of multiple locks.
> Larger tables which benefit from multiple locks will not have a single
> lock per bucket anyway.
>
> Signed-off-by: Thomas Graf <tgraf@suug.ch>
Thomas, could you please hold off on these changes? They totally
conflict with my rehash work, which is going to render these changes
moot anyway since it'll completely change how expansion/shrinking
works.
Thanks,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Please let me know if you need to print color box, display box and labels
From: Jinghao Printing - CHINA @ 2015-01-31 8:00 UTC (permalink / raw)
Hi, this is David Wu from Shanghai, China.
Please let me know if you need color box, display box, corrugated box,
label, hang tag etc.
I will send you the website.
Best regards,
David Wu
^ permalink raw reply
* Re: [PATCH 2/6] rhashtable: Use a single bucket lock for sibling buckets
From: Thomas Graf @ 2015-01-31 8:41 UTC (permalink / raw)
To: Herbert Xu; +Cc: davem, netdev, ying.xue
In-Reply-To: <20150131043436.GA27564@gondor.apana.org.au>
On 01/31/15 at 03:34pm, Herbert Xu wrote:
> Thomas, could you please hold off on these changes? They totally
> conflict with my rehash work, which is going to render these changes
> moot anyway since it'll completely change how expansion/shrinking
> works.
Can you share that work?
^ permalink raw reply
* rhashtable: Fix potential crash on destroy in rhashtable_shrink
From: Herbert Xu @ 2015-01-31 9:36 UTC (permalink / raw)
To: Thomas Graf, netdev
The current being_destroyed check in rhashtable_expand is not
enough since if we start a shrinking process after freeing all
elements in the table that's also going to crash.
This patch adds a being_destroyed check to the deferred worker
thread so that we bail out as soon as we take the lock.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index 69a4eb0..4c3da1f 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -489,6 +489,9 @@ static void rht_deferred_worker(struct work_struct *work)
ht = container_of(work, struct rhashtable, run_work);
mutex_lock(&ht->mutex);
+ if (ht->being_destroyed)
+ goto unlock;
+
tbl = rht_dereference(ht->tbl, ht);
list_for_each_entry(walker, &ht->walkers, list)
@@ -499,6 +502,7 @@ static void rht_deferred_worker(struct work_struct *work)
else if (ht->p.shrink_decision && ht->p.shrink_decision(ht, tbl->size))
rhashtable_shrink(ht);
+unlock:
mutex_unlock(&ht->mutex);
}
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply related
* Re: [PATCH 2/6] rhashtable: Use a single bucket lock for sibling buckets
From: Herbert Xu @ 2015-01-31 9:38 UTC (permalink / raw)
To: Thomas Graf; +Cc: davem, netdev, ying.xue
In-Reply-To: <20150131084127.GA13841@casper.infradead.org>
On Sat, Jan 31, 2015 at 08:41:27AM +0000, Thomas Graf wrote:
> On 01/31/15 at 03:34pm, Herbert Xu wrote:
> > Thomas, could you please hold off on these changes? They totally
> > conflict with my rehash work, which is going to render these changes
> > moot anyway since it'll completely change how expansion/shrinking
> > works.
>
> Can you share that work?
I will post it when it's ready. But if you keep moving the goal
posts then I'll never get there.
Thanks,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* rhashtable: Move hash_rnd into bucket_table
From: Herbert Xu @ 2015-01-31 10:21 UTC (permalink / raw)
To: Thomas Graf, netdev
Currently hash_rnd is a parameter that users can set. However,
no existing users set this parameter. It is also something that
people are unlikely to want to set directly since it's just a
random number.
In preparation for allowing the reseeding/rehashing of rhashtable,
this patch moves hash_rnd into bucket_table so that it's now an
internal state rather than a parameter.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
diff --git a/include/linux/rhashtable.h b/include/linux/rhashtable.h
index a2562ed..6d7e840 100644
--- a/include/linux/rhashtable.h
+++ b/include/linux/rhashtable.h
@@ -55,6 +55,7 @@ struct rhash_head {
struct bucket_table {
size_t size;
unsigned int locks_mask;
+ u32 hash_rnd;
spinlock_t *locks;
struct rhash_head __rcu *buckets[];
};
@@ -70,7 +71,6 @@ struct rhashtable;
* @key_len: Length of key
* @key_offset: Offset of key in struct to be hashed
* @head_offset: Offset of rhash_head in struct to be hashed
- * @hash_rnd: Seed to use while hashing
* @max_shift: Maximum number of shifts while expanding
* @min_shift: Minimum number of shifts while shrinking
* @nulls_base: Base value to generate nulls marker
@@ -89,7 +89,6 @@ struct rhashtable_params {
size_t key_len;
size_t key_offset;
size_t head_offset;
- u32 hash_rnd;
size_t max_shift;
size_t min_shift;
u32 nulls_base;
diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index 84a78e3..71c6aa1 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -81,13 +81,14 @@ static u32 rht_bucket_index(const struct bucket_table *tbl, u32 hash)
static u32 obj_raw_hashfn(const struct rhashtable *ht, const void *ptr)
{
+ struct bucket_table *tbl = rht_dereference_rcu(ht->tbl, ht);
u32 hash;
if (unlikely(!ht->p.key_len))
- hash = ht->p.obj_hashfn(ptr, ht->p.hash_rnd);
+ hash = ht->p.obj_hashfn(ptr, tbl->hash_rnd);
else
hash = ht->p.hashfn(ptr + ht->p.key_offset, ht->p.key_len,
- ht->p.hash_rnd);
+ tbl->hash_rnd);
return hash >> HASH_RESERVED_SPACE;
}
@@ -97,7 +98,7 @@ static u32 key_hashfn(struct rhashtable *ht, const void *key, u32 len)
struct bucket_table *tbl = rht_dereference_rcu(ht->tbl, ht);
u32 hash;
- hash = ht->p.hashfn(key, len, ht->p.hash_rnd);
+ hash = ht->p.hashfn(key, len, tbl->hash_rnd);
hash >>= HASH_RESERVED_SPACE;
return rht_bucket_index(tbl, hash);
@@ -894,14 +895,13 @@ int rhashtable_init(struct rhashtable *ht, struct rhashtable_params *params)
if (tbl == NULL)
return -ENOMEM;
+ get_random_bytes(&tbl->hash_rnd, sizeof(tbl->hash_rnd));
+
atomic_set(&ht->nelems, 0);
atomic_set(&ht->shift, ilog2(tbl->size));
RCU_INIT_POINTER(ht->tbl, tbl);
RCU_INIT_POINTER(ht->future_tbl, tbl);
- if (!ht->p.hash_rnd)
- get_random_bytes(&ht->p.hash_rnd, sizeof(ht->p.hash_rnd));
-
if (ht->p.grow_decision || ht->p.shrink_decision)
INIT_WORK(&ht->run_work, rht_deferred_worker);
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply related
* [RFC] rhashtable: rhashtable_rehash
From: Herbert Xu @ 2015-01-31 10:23 UTC (permalink / raw)
To: Thomas Graf, netdev
In-Reply-To: <20150131102150.GA29498@gondor.apana.org.au>
Here is a totally untested patch that illustrates what I want to
do with rehash.
Note that this depends on the hash rnd patch I just sent.
diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index 4c3da1f..ebd5e44 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -79,9 +79,9 @@ static u32 rht_bucket_index(const struct bucket_table *tbl, u32 hash)
return hash & (tbl->size - 1);
}
-static u32 obj_raw_hashfn(const struct rhashtable *ht, const void *ptr)
+static u32 obj_raw_hashfn(const struct rhashtable *ht,
+ struct bucket_table *tbl, const void *ptr)
{
- struct bucket_table *tbl = rht_dereference_rcu(ht->tbl, ht);
u32 hash;
if (unlikely(!ht->p.key_len))
@@ -93,9 +93,9 @@ static u32 obj_raw_hashfn(const struct rhashtable *ht, const void *ptr)
return hash >> HASH_RESERVED_SPACE;
}
-static u32 key_hashfn(struct rhashtable *ht, const void *key, u32 len)
+static u32 key_hashfn(struct rhashtable *ht, struct bucket_table *tbl,
+ const void *key, u32 len)
{
- struct bucket_table *tbl = rht_dereference_rcu(ht->tbl, ht);
u32 hash;
hash = ht->p.hashfn(key, len, tbl->hash_rnd);
@@ -295,6 +295,89 @@ static void link_old_to_new(struct bucket_table *new_tbl,
spin_unlock_bh(new_bucket_lock);
}
+static void rhashtable_rehash_one(struct rhashtable *ht, unsigned int old_hash)
+{
+ struct *old_tbl = rht_dereference(ht->tbl, ht);
+ struct *new_tbl = rht_dereference(ht->future_tbl, ht);
+ struct rhash_head *head, *next, *entry;
+ struct rhash_head __rcu **pprev;
+ spinlock_t *new_bucket_lock;
+ unsigned int new_hash;
+
+ pprev = &old_tbl->buckets[old_hash];
+ rht_for_each(entry, old_tbl, old_hash) {
+ next = rht_dereference_bucket(entry->next, old_tbl, old_hash);
+
+ if (rht_is_a_nulls(next))
+ break;
+
+ pprev = &entry->next;
+ }
+
+ new_hash = head_hashfn(ht, new_tbl, entry);
+
+ new_bucket_lock = bucket_lock(new_tbl, new_hash);
+
+ spin_lock(new_bucket_lock);
+ head = rht_dereference_bucket(new_tbl->buckets[new_hash],
+ new_tbl, new_hash);
+
+ if (rht_is_a_nulls(head))
+ INIT_RHT_NULLS_HEAD(entry->next, ht, hash);
+ else
+ RCU_INIT_POINTER(entry->next, head);
+
+ rcu_assign_pointer(new_tbl->buckets[new_hash], entry);
+ spin_unlock(new_bucket_lock);
+
+ rcu_assign_pointer(*pprev, next);
+}
+
+static void rhashtable_rehash_chain(struct rhashtable *ht,
+ unsigned int old_hash)
+{
+ struct *old_tbl = rht_dereference(ht->tbl, ht);
+ spinlock_t *old_bucket_lock;
+
+ old_bucket_lock = bucket_lock(old_tbl, old_hash);
+
+ spin_lock_bh(old_bucket_lock);
+ while (!rht_is_a_nulls(rht_dereference_bucket(
+ old_tbl->buckets[old_hash], old_tbl, old_hash)))
+ rhashtable_rehash_one(ht, old_hash);
+ spin_unlock_bh(old_bucket_lock);
+}
+
+static void rhashtable_rehash(struct rhashtable *ht,
+ struct bucket_table *new_tbl)
+{
+ struct *old_tbl = rht_dereference(ht->tbl, ht);
+
+ get_random_bytes(&new_tbl->hash_rnd, sizeof(new_tbl->hash_rnd));
+
+ /* Make insertions go into the new, empty table right away. Deletions
+ * and lookups will be attempted in both tables until we synchronize.
+ * The synchronize_rcu() guarantees for the new table to be picked up
+ * so no new additions go into the old table while we relink.
+ */
+ rcu_assign_pointer(ht->future_tbl, new_tbl);
+ synchronize_rcu();
+
+ for (old_hash = 0; i < old_tbl->size; old_hash++)
+ rhashtable_rehash_chain(ht, old_hash);
+
+ /* Publish the new table pointer. */
+ rcu_assign_pointer(ht->tbl, new_tbl);
+
+ /* Wait for readers. All new readers will see the new
+ * table, and thus no references to the old table will
+ * remain.
+ */
+ synchronize_rcu();
+
+ bucket_table_free(old_tbl);
+}
+
/**
* rhashtable_expand - Expand hash table while allowing concurrent lookups
* @ht: the hash table to expand
@@ -327,72 +410,8 @@ int rhashtable_expand(struct rhashtable *ht)
atomic_inc(&ht->shift);
- /* Make insertions go into the new, empty table right away. Deletions
- * and lookups will be attempted in both tables until we synchronize.
- * The synchronize_rcu() guarantees for the new table to be picked up
- * so no new additions go into the old table while we relink.
- */
- rcu_assign_pointer(ht->future_tbl, new_tbl);
- synchronize_rcu();
-
- /* For each new bucket, search the corresponding old bucket for the
- * first entry that hashes to the new bucket, and link the end of
- * newly formed bucket chain (containing entries added to future
- * table) to that entry. Since all the entries which will end up in
- * the new bucket appear in the same old bucket, this constructs an
- * entirely valid new hash table, but with multiple buckets
- * "zipped" together into a single imprecise chain.
- */
- for (new_hash = 0; new_hash < new_tbl->size; new_hash++) {
- old_hash = rht_bucket_index(old_tbl, new_hash);
- old_bucket_lock = bucket_lock(old_tbl, old_hash);
-
- spin_lock_bh(old_bucket_lock);
- rht_for_each(he, old_tbl, old_hash) {
- if (head_hashfn(ht, new_tbl, he) == new_hash) {
- link_old_to_new(new_tbl, new_hash, he);
- break;
- }
- }
- spin_unlock_bh(old_bucket_lock);
- }
-
- /* Publish the new table pointer. Lookups may now traverse
- * the new table, but they will not benefit from any
- * additional efficiency until later steps unzip the buckets.
- */
- rcu_assign_pointer(ht->tbl, new_tbl);
-
- /* Unzip interleaved hash chains */
- while (!complete && !ht->being_destroyed) {
- /* Wait for readers. All new readers will see the new
- * table, and thus no references to the old table will
- * remain.
- */
- synchronize_rcu();
-
- /* For each bucket in the old table (each of which
- * contains items from multiple buckets of the new
- * table): ...
- */
- complete = true;
- for (old_hash = 0; old_hash < old_tbl->size; old_hash++) {
- struct rhash_head *head;
-
- old_bucket_lock = bucket_lock(old_tbl, old_hash);
- spin_lock_bh(old_bucket_lock);
-
- hashtable_chain_unzip(ht, new_tbl, old_tbl, old_hash);
- head = rht_dereference_bucket(old_tbl->buckets[old_hash],
- old_tbl, old_hash);
- if (!rht_is_a_nulls(head))
- complete = false;
-
- spin_unlock_bh(old_bucket_lock);
- }
- }
+ rhashtable_rehash(ht, new_tbl);
- bucket_table_free(old_tbl);
return 0;
}
EXPORT_SYMBOL_GPL(rhashtable_expand);
@@ -425,57 +444,7 @@ int rhashtable_shrink(struct rhashtable *ht)
if (new_tbl == NULL)
return -ENOMEM;
- rcu_assign_pointer(ht->future_tbl, new_tbl);
- synchronize_rcu();
-
- /* Link the first entry in the old bucket to the end of the
- * bucket in the new table. As entries are concurrently being
- * added to the new table, lock down the new bucket. As we
- * always divide the size in half when shrinking, each bucket
- * in the new table maps to exactly two buckets in the old
- * table.
- *
- * As removals can occur concurrently on the old table, we need
- * to lock down both matching buckets in the old table.
- */
- for (new_hash = 0; new_hash < new_tbl->size; new_hash++) {
- old_bucket_lock1 = bucket_lock(tbl, new_hash);
- old_bucket_lock2 = bucket_lock(tbl, new_hash + new_tbl->size);
- new_bucket_lock = bucket_lock(new_tbl, new_hash);
-
- spin_lock_bh(old_bucket_lock1);
-
- /* Depending on the lock per buckets mapping, the bucket in
- * the lower and upper region may map to the same lock.
- */
- if (old_bucket_lock1 != old_bucket_lock2) {
- spin_lock_bh_nested(old_bucket_lock2, RHT_LOCK_NESTED);
- spin_lock_bh_nested(new_bucket_lock, RHT_LOCK_NESTED2);
- } else {
- spin_lock_bh_nested(new_bucket_lock, RHT_LOCK_NESTED);
- }
-
- rcu_assign_pointer(*bucket_tail(new_tbl, new_hash),
- tbl->buckets[new_hash]);
- rcu_assign_pointer(*bucket_tail(new_tbl, new_hash),
- tbl->buckets[new_hash + new_tbl->size]);
-
- spin_unlock_bh(new_bucket_lock);
- if (old_bucket_lock1 != old_bucket_lock2)
- spin_unlock_bh(old_bucket_lock2);
- spin_unlock_bh(old_bucket_lock1);
- }
-
- /* Publish the new, valid hash table */
- rcu_assign_pointer(ht->tbl, new_tbl);
- atomic_dec(&ht->shift);
-
- /* Wait for readers. No new readers will have references to the
- * old hash table.
- */
- synchronize_rcu();
-
- bucket_table_free(tbl);
+ rhashtable_rehash(ht, new_tbl);
return 0;
}
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply related
* Re: rhashtable: Fix potential crash on destroy in rhashtable_shrink
From: Thomas Graf @ 2015-01-31 11:16 UTC (permalink / raw)
To: Herbert Xu; +Cc: netdev
In-Reply-To: <20150131093637.GA29106@gondor.apana.org.au>
On 01/31/15 at 08:36pm, Herbert Xu wrote:
> The current being_destroyed check in rhashtable_expand is not
> enough since if we start a shrinking process after freeing all
> elements in the table that's also going to crash.
(The check in expand() is just an optimization to drop out of
work cycles if it does not make sense to continue anymore.)
>
> This patch adds a being_destroyed check to the deferred worker
> thread so that we bail out as soon as we take the lock.
Shouldn't the cancel_work_sync() in rhashtable_destroy() block
until the deferred worker is done and cancelled?
^ permalink raw reply
* Re: rhashtable: Move hash_rnd into bucket_table
From: Thomas Graf @ 2015-01-31 11:17 UTC (permalink / raw)
To: Herbert Xu; +Cc: netdev
In-Reply-To: <20150131102150.GA29498@gondor.apana.org.au>
On 01/31/15 at 09:21pm, Herbert Xu wrote:
> Currently hash_rnd is a parameter that users can set. However,
> no existing users set this parameter. It is also something that
> people are unlikely to want to set directly since it's just a
> random number.
>
> In preparation for allowing the reseeding/rehashing of rhashtable,
> this patch moves hash_rnd into bucket_table so that it's now an
> internal state rather than a parameter.
>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
For net-next
Acked-by: Thomas Graf <tgraf@suug.ch>
^ permalink raw reply
* Re: rhashtable: Fix potential crash on destroy in rhashtable_shrink
From: Herbert Xu @ 2015-01-31 11:22 UTC (permalink / raw)
To: Thomas Graf; +Cc: netdev
In-Reply-To: <20150131111652.GA22448@casper.infradead.org>
On Sat, Jan 31, 2015 at 11:16:52AM +0000, Thomas Graf wrote:
> On 01/31/15 at 08:36pm, Herbert Xu wrote:
> > The current being_destroyed check in rhashtable_expand is not
> > enough since if we start a shrinking process after freeing all
> > elements in the table that's also going to crash.
>
> (The check in expand() is just an optimization to drop out of
> work cycles if it does not make sense to continue anymore.)
>
> >
> > This patch adds a being_destroyed check to the deferred worker
> > thread so that we bail out as soon as we take the lock.
>
> Shouldn't the cancel_work_sync() in rhashtable_destroy() block
> until the deferred worker is done and cancelled?
That's too late. nft_hash will have freed all the elements
before rhashtable_destroy gets called.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: rhashtable: Fix potential crash on destroy in rhashtable_shrink
From: Thomas Graf @ 2015-01-31 12:15 UTC (permalink / raw)
To: Herbert Xu; +Cc: netdev
In-Reply-To: <20150131112209.GA29869@gondor.apana.org.au>
On 01/31/15 at 10:22pm, Herbert Xu wrote:
> That's too late. nft_hash will have freed all the elements
> before rhashtable_destroy gets called.
I see, so this is to accomodate nft_hash which doesn't remove
the elements from the hash but just frees them.
Acked-by: Thomas Graf <tgraf@suug.ch>
^ permalink raw reply
* Re: [PATCH 7/9] netfilter: nft_hash: add support for timeouts
From: Patrick McHardy @ 2015-01-31 12:16 UTC (permalink / raw)
To: Herbert Xu
Cc: tgraf, davem, David.Laight, ying.xue, paulmck, netdev,
netfilter-devel
In-Reply-To: <20150131042920.GA27480@gondor.apana.org.au>
On 31.01, Herbert Xu wrote:
> On Fri, Jan 30, 2015 at 07:46:32AM +0000, Patrick McHardy wrote:
> >
> > + mutex_lock(&priv->ht.mutex);
> > + tbl = rht_dereference(priv->ht.tbl, &priv->ht);
> > + for (i = 0; i < tbl->size; i++) {
> > + rht_for_each_entry_safe(he, pos, next, tbl, i, node) {
> > + if (!nft_set_ext_exists(&he->ext, NFT_SET_EXT_TIMEOUT))
> > + continue;
> > + timeout = *nft_set_ext_timeout(&he->ext);
> > + if (time_before(jiffies, timeout))
> > + continue;
> > +
> > + rhashtable_remove(&priv->ht, &he->node);
> > + nft_hash_elem_destroy(set, he);
> > + }
> > + }
> > + mutex_unlock(&priv->ht.mutex);
>
> What if somebody is currently walking over the table? Shouldn't
> you do an RCU free here instead of immediately destroying the
> element?
Yes, that's what I meant in mail 0/x regarding the existing races.
Probably will add some fixed sized batching here.
^ permalink raw reply
* [PATCH net-next 0/4] enic: improve rq buff allocation and reduce dma mapping
From: Govindarajulu Varadarajan @ 2015-01-31 12:28 UTC (permalink / raw)
To: davem, netdev; +Cc: ssujith, benve, edumazet, ben, Govindarajulu Varadarajan
The following series tries to address these two problem in rq buff allocation.
* Memory wastage because of large 9k allocation using kmalloc:
For 9k mtu buffer, netdev_alloc_skb_ip_align internally calls kmalloc for
size > 4096. In case of 9k buff, kmalloc returns pages for order 2, 16k.
And we use only ~9k of 16k. 7k memory wasted. Using the frag the frag
allocator in patch 1/2, we can allocate three 9k buffs in a 32k page size.
Typical enic configuration has 8 rq, and desc ring of size 4096.
Thats 8 * 4096 * (16*1024) = 512 MB. Using this frag allocator:
8 * 4096 * (32*1024/3) = 341 MB. Thats 171 MB of memory save.
* frequent dma_map() calls:
we call dma_map() for every buff we allocate. When iommu is on, This is very
cpu time consuming. From my testing, most of the cpu cycles are wasted
spinning on spin_lock_irqsave(&iovad->iova_rbtree_lock, flags) in
intel_map_page() .. -> ..__alloc_and_insert_iova_range()
With this patch, we call dma_map() once for 32k page. i.e once for every three
9k desc, and once every twenty 1500 bytes desc.
Here are testing result with 8 rq, 4096 ring size and 9k mtu. irq of each rq
is affinitized with different CPU. Ran iperf with 32 threads. Link is 10G.
iommu is on.
CPU utilization throughput
without patch 100% 1.8 Gbps
with patch 13% 9.8 Gbps
Govindarajulu Varadarajan (4):
enic: implement frag allocator
enic: Add rq allocation failure stats
ethtool: add RX_ALLOC_ORDER to tunable
enic: add ethtool support for changing alloc order
drivers/net/ethernet/cisco/enic/enic.h | 16 +++
drivers/net/ethernet/cisco/enic/enic_ethtool.c | 17 +++
drivers/net/ethernet/cisco/enic/enic_main.c | 177 +++++++++++++++++++++----
drivers/net/ethernet/cisco/enic/vnic_rq.c | 13 ++
drivers/net/ethernet/cisco/enic/vnic_rq.h | 2 +
drivers/net/ethernet/cisco/enic/vnic_stats.h | 2 +
include/uapi/linux/ethtool.h | 1 +
net/core/ethtool.c | 5 +
8 files changed, 209 insertions(+), 24 deletions(-)
--
2.2.2
^ permalink raw reply
* [PATCH net-next 1/4] enic: implement frag allocator
From: Govindarajulu Varadarajan @ 2015-01-31 12:28 UTC (permalink / raw)
To: davem, netdev; +Cc: ssujith, benve, edumazet, ben, Govindarajulu Varadarajan
In-Reply-To: <1422707290-939-1-git-send-email-_govind@gmx.com>
This patch implements frag allocator for rq buffer. This is based on
__alloc_page_frag & __page_frag_refill implementation in net/core/skbuff.c
In addition to frag allocation from order(3) page in __alloc_page_frag,
we also maintain dma address of the page. While allocating a frag for rx buffer
we return va + offset for virtual address of the frag, and pa + offset for
dma address of the frag. This reduces the number of calls to dma_map()
by 1/3 for 9k mtu and by 1/20 for 1500 mtu.
__alloc_page_frag is limited to max buffer size of PAGE_SIZE, i.e 4096 in most
of the cases. So 9k buffer allocation goes through kmalloc which return
page of order 2, 16k. We waste 7k bytes for every 9k buffer.
We maintain dma_count variable which is incremented when we allocate a frag.
enic_unmap_dma will decrement the dma_count and unmap it when there is no user
of that page in rx ring.
This reduces the memory utilization for 9k mtu by 33%.
enic_alloc_cache struct, which stores the page details, is declared per rq.
And all calls to allocation, free, dmap_unmap is serialized. So we do not need
locks.
Signed-off-by: Govindarajulu Varadarajan <_govind@gmx.com>
---
drivers/net/ethernet/cisco/enic/enic.h | 16 +++
drivers/net/ethernet/cisco/enic/enic_main.c | 156 +++++++++++++++++++++++-----
drivers/net/ethernet/cisco/enic/vnic_rq.c | 13 +++
drivers/net/ethernet/cisco/enic/vnic_rq.h | 2 +
4 files changed, 163 insertions(+), 24 deletions(-)
diff --git a/drivers/net/ethernet/cisco/enic/enic.h b/drivers/net/ethernet/cisco/enic/enic.h
index 84b6a2b..7fd3db1 100644
--- a/drivers/net/ethernet/cisco/enic/enic.h
+++ b/drivers/net/ethernet/cisco/enic/enic.h
@@ -20,6 +20,11 @@
#ifndef _ENIC_H_
#define _ENIC_H_
+#include <linux/if.h>
+#include <linux/if_link.h>
+#include <linux/if_ether.h>
+#include <linux/netdevice.h>
+
#include "vnic_enet.h"
#include "vnic_dev.h"
#include "vnic_wq.h"
@@ -176,6 +181,7 @@ struct enic {
u64 rq_truncated_pkts;
u64 rq_bad_fcs;
struct napi_struct napi[ENIC_RQ_MAX + ENIC_WQ_MAX];
+ u8 alloc_order;
/* interrupt resource cache line section */
____cacheline_aligned struct vnic_intr intr[ENIC_INTR_MAX];
@@ -191,6 +197,16 @@ struct enic {
struct vnic_gen_stats gen_stats;
};
+#define ENIC_ALLOC_ORDER get_order(32 * 1024)
+
+struct enic_alloc_cache {
+ struct page_frag frag;
+ unsigned int pagecnt_bias;
+ int dma_count;
+ void *va;
+ dma_addr_t pa;
+};
+
static inline struct device *enic_get_dev(struct enic *enic)
{
return &(enic->pdev->dev);
diff --git a/drivers/net/ethernet/cisco/enic/enic_main.c b/drivers/net/ethernet/cisco/enic/enic_main.c
index ee44c82..d9cad93 100644
--- a/drivers/net/ethernet/cisco/enic/enic_main.c
+++ b/drivers/net/ethernet/cisco/enic/enic_main.c
@@ -950,6 +950,105 @@ nla_put_failure:
return -EMSGSIZE;
}
+struct enic_alloc_cache *enic_page_refill(struct enic *enic, size_t sz,
+ gfp_t gfp)
+{
+ struct enic_alloc_cache *ec;
+ gfp_t gfp_comp = gfp | __GFP_COMP | __GFP_NOWARN | __GFP_NORETRY;
+ u8 order = enic->alloc_order;
+
+ ec = kzalloc(sizeof(*ec), GFP_ATOMIC);
+ if (unlikely(!ec))
+ goto no_ec;
+ ec->frag.page = alloc_pages_node(NUMA_NO_NODE, gfp_comp, order);
+ if (unlikely(!ec->frag.page)) {
+ order = get_order(sz);
+ ec->frag.page = alloc_pages_node(NUMA_NO_NODE, gfp, order);
+ if (!ec->frag.page)
+ goto free_ec;
+ }
+
+ ec->frag.size = (PAGE_SIZE << order);
+ ec->va = page_address(ec->frag.page);
+ ec->pa = pci_map_single(enic->pdev, ec->va, ec->frag.size,
+ PCI_DMA_FROMDEVICE);
+ if (unlikely(enic_dma_map_check(enic, ec->pa)))
+ goto free_page;
+ atomic_add(ec->frag.size - 1, &ec->frag.page->_count);
+ ec->pagecnt_bias = ec->frag.size;
+ ec->frag.offset = ec->frag.size;
+
+ return ec;
+
+free_page:
+ __free_pages(ec->frag.page, order);
+free_ec:
+ kfree(ec);
+no_ec:
+ return NULL;
+}
+
+struct enic_alloc_cache *enic_alloc_frag(struct vnic_rq *rq, size_t sz)
+{
+ struct enic *enic = vnic_dev_priv(rq->vdev);
+ struct enic_alloc_cache *ec = rq->ec;
+ int offset;
+
+ if (unlikely(!ec)) {
+refill:
+ ec = enic_page_refill(enic, sz, GFP_ATOMIC);
+ rq->ec = ec;
+
+ if (unlikely(!ec))
+ return NULL;
+ }
+
+ offset = ec->frag.offset - sz;
+ if (offset < 0) {
+ if (!atomic_sub_and_test(ec->pagecnt_bias,
+ &ec->frag.page->_count)) {
+ /* rq cleanup service has processed all the frags
+ * belonging to this page. Since page->_count is not 0
+ * and ec->dma_count is 0 these frags should be in
+ * stack. We should unmap the page here.
+ */
+ if (!ec->dma_count) {
+ pci_unmap_single(enic->pdev, ec->pa,
+ ec->frag.size,
+ PCI_DMA_FROMDEVICE);
+ kfree(ec);
+ } else {
+ /* frags from this page are still in rx queue. Let the
+ * rx cleanup service unmap the page in enic_unmap_dma.
+ */
+ ec->pagecnt_bias = 0;
+ }
+ goto refill;
+ }
+ WARN_ON(ec->dma_count);
+ atomic_set(&ec->frag.page->_count, ec->frag.size);
+ ec->pagecnt_bias = ec->frag.size;
+ offset = ec->frag.size - sz;
+ }
+ ec->pagecnt_bias--;
+ ec->dma_count++;
+ ec->frag.offset = offset;
+
+ return ec;
+}
+
+void enic_unmap_dma(struct enic *enic, struct enic_alloc_cache *ec)
+{
+ /* enic_alloc_frag is done using this page. We should be free to unmap
+ * the page if there are no pending frags in the queue.
+ */
+ if (!--ec->dma_count && !ec->pagecnt_bias) {
+ pci_unmap_single(enic->pdev, ec->pa, ec->frag.size,
+ PCI_DMA_FROMDEVICE);
+ kfree(ec);
+ }
+}
+
static void enic_free_rq_buf(struct vnic_rq *rq, struct vnic_rq_buf *buf)
{
struct enic *enic = vnic_dev_priv(rq->vdev);
@@ -957,8 +1056,7 @@ static void enic_free_rq_buf(struct vnic_rq *rq, struct vnic_rq_buf *buf)
if (!buf->os_buf)
return;
- pci_unmap_single(enic->pdev, buf->dma_addr,
- buf->len, PCI_DMA_FROMDEVICE);
+ enic_unmap_dma(enic, buf->ec);
dev_kfree_skb_any(buf->os_buf);
buf->os_buf = NULL;
}
@@ -968,10 +1066,12 @@ static int enic_rq_alloc_buf(struct vnic_rq *rq)
struct enic *enic = vnic_dev_priv(rq->vdev);
struct net_device *netdev = enic->netdev;
struct sk_buff *skb;
- unsigned int len = netdev->mtu + VLAN_ETH_HLEN;
+ unsigned int len;
unsigned int os_buf_index = 0;
dma_addr_t dma_addr;
struct vnic_rq_buf *buf = rq->to_use;
+ struct enic_alloc_cache *ec;
+ void *va;
if (buf->os_buf) {
enic_queue_rq_desc(rq, buf->os_buf, os_buf_index, buf->dma_addr,
@@ -979,21 +1079,33 @@ static int enic_rq_alloc_buf(struct vnic_rq *rq)
return 0;
}
- skb = netdev_alloc_skb_ip_align(netdev, len);
- if (!skb)
- return -ENOMEM;
- dma_addr = pci_map_single(enic->pdev, skb->data, len,
- PCI_DMA_FROMDEVICE);
- if (unlikely(enic_dma_map_check(enic, dma_addr))) {
- dev_kfree_skb(skb);
- return -ENOMEM;
- }
+ len = netdev->mtu + VLAN_ETH_HLEN + NET_IP_ALIGN + NET_SKB_PAD;
+ len = SKB_DATA_ALIGN(len) +
+ SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
- enic_queue_rq_desc(rq, skb, os_buf_index,
- dma_addr, len);
+ ec = enic_alloc_frag(rq, len);
+ if (unlikely(!ec))
+ goto alloc_fail;
+ va = ec->va + ec->frag.offset;
+ skb = build_skb(va, len);
+ if (unlikely(!skb)) {
+ ec->pagecnt_bias++;
+ ec->frag.offset += len;
+ ec->dma_count--;
+
+ goto alloc_fail;
+ }
+ skb_reserve(skb, NET_SKB_PAD + NET_IP_ALIGN);
+ dma_addr = ec->pa + ec->frag.offset + NET_SKB_PAD + NET_IP_ALIGN;
+ buf->ec = ec;
+ enic_queue_rq_desc(rq, skb, os_buf_index, dma_addr,
+ netdev->mtu + VLAN_ETH_HLEN);
return 0;
+
+alloc_fail:
+ return -ENOMEM;
}
static void enic_intr_update_pkt_size(struct vnic_rx_bytes_counter *pkt_size,
@@ -1016,8 +1128,6 @@ static bool enic_rxcopybreak(struct net_device *netdev, struct sk_buff **skb,
new_skb = netdev_alloc_skb_ip_align(netdev, len);
if (!new_skb)
return false;
- pci_dma_sync_single_for_cpu(enic->pdev, buf->dma_addr, len,
- DMA_FROM_DEVICE);
memcpy(new_skb->data, (*skb)->data, len);
*skb = new_skb;
@@ -1065,8 +1175,7 @@ static void enic_rq_indicate_buf(struct vnic_rq *rq,
enic->rq_truncated_pkts++;
}
- pci_unmap_single(enic->pdev, buf->dma_addr, buf->len,
- PCI_DMA_FROMDEVICE);
+ enic_unmap_dma(enic, buf->ec);
dev_kfree_skb_any(skb);
buf->os_buf = NULL;
@@ -1077,11 +1186,11 @@ static void enic_rq_indicate_buf(struct vnic_rq *rq,
/* Good receive
*/
-
+ pci_dma_sync_single_for_cpu(enic->pdev, buf->dma_addr,
+ bytes_written, DMA_FROM_DEVICE);
if (!enic_rxcopybreak(netdev, &skb, buf, bytes_written)) {
buf->os_buf = NULL;
- pci_unmap_single(enic->pdev, buf->dma_addr, buf->len,
- PCI_DMA_FROMDEVICE);
+ enic_unmap_dma(enic, buf->ec);
}
prefetch(skb->data - NET_IP_ALIGN);
@@ -1122,9 +1231,7 @@ static void enic_rq_indicate_buf(struct vnic_rq *rq,
/* Buffer overflow
*/
-
- pci_unmap_single(enic->pdev, buf->dma_addr, buf->len,
- PCI_DMA_FROMDEVICE);
+ enic_unmap_dma(enic, buf->ec);
dev_kfree_skb_any(skb);
buf->os_buf = NULL;
}
@@ -2637,6 +2744,7 @@ static int enic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
goto err_out_dev_deinit;
}
enic->rx_copybreak = RX_COPYBREAK_DEFAULT;
+ enic->alloc_order = ENIC_ALLOC_ORDER;
return 0;
diff --git a/drivers/net/ethernet/cisco/enic/vnic_rq.c b/drivers/net/ethernet/cisco/enic/vnic_rq.c
index 36a2ed6..c31669f 100644
--- a/drivers/net/ethernet/cisco/enic/vnic_rq.c
+++ b/drivers/net/ethernet/cisco/enic/vnic_rq.c
@@ -26,6 +26,7 @@
#include "vnic_dev.h"
#include "vnic_rq.h"
+#include "enic.h"
static int vnic_rq_alloc_bufs(struct vnic_rq *rq)
{
@@ -199,6 +200,18 @@ void vnic_rq_clean(struct vnic_rq *rq,
rq->ring.desc_avail++;
}
+ if (rq->ec) {
+ struct enic *enic = vnic_dev_priv(rq->vdev);
+ struct enic_alloc_cache *ec = rq->ec;
+
+ WARN_ON(ec->dma_count);
+ pci_unmap_single(enic->pdev, ec->pa, ec->frag.size,
+ PCI_DMA_FROMDEVICE);
+ atomic_sub(ec->pagecnt_bias - 1, &ec->frag.page->_count);
+ __free_pages(ec->frag.page, get_order(ec->frag.size));
+ kfree(ec);
+ rq->ec = NULL;
+ }
/* Use current fetch_index as the ring starting point */
fetch_index = ioread32(&rq->ctrl->fetch_index);
diff --git a/drivers/net/ethernet/cisco/enic/vnic_rq.h b/drivers/net/ethernet/cisco/enic/vnic_rq.h
index 8111d52..2e4815a 100644
--- a/drivers/net/ethernet/cisco/enic/vnic_rq.h
+++ b/drivers/net/ethernet/cisco/enic/vnic_rq.h
@@ -73,6 +73,7 @@ struct vnic_rq_buf {
unsigned int index;
void *desc;
uint64_t wr_id;
+ struct enic_alloc_cache *ec;
};
struct vnic_rq {
@@ -100,6 +101,7 @@ struct vnic_rq {
unsigned int bpoll_state;
spinlock_t bpoll_lock;
#endif /* CONFIG_NET_RX_BUSY_POLL */
+ struct enic_alloc_cache *ec;
};
static inline unsigned int vnic_rq_desc_avail(struct vnic_rq *rq)
--
2.2.2
^ permalink raw reply related
* [PATCH net-next 2/4] enic: Add rq allocation failure stats
From: Govindarajulu Varadarajan @ 2015-01-31 12:28 UTC (permalink / raw)
To: davem, netdev; +Cc: ssujith, benve, edumazet, ben, Govindarajulu Varadarajan
In-Reply-To: <1422707290-939-1-git-send-email-_govind@gmx.com>
This patch adds rq buff allocation failure stats.
cache_alloc_err: incremented when higher order page allocation fails.
enic_rq_alloc_buf: incremented rq buff fails. Either due to page alloc
failure or build_skb.
Signed-off-by: Govindarajulu Varadarajan <_govind@gmx.com>
---
drivers/net/ethernet/cisco/enic/enic_ethtool.c | 2 ++
drivers/net/ethernet/cisco/enic/enic_main.c | 3 +++
drivers/net/ethernet/cisco/enic/vnic_stats.h | 2 ++
3 files changed, 7 insertions(+)
diff --git a/drivers/net/ethernet/cisco/enic/enic_ethtool.c b/drivers/net/ethernet/cisco/enic/enic_ethtool.c
index 0c396c1..3f9d91b 100644
--- a/drivers/net/ethernet/cisco/enic/enic_ethtool.c
+++ b/drivers/net/ethernet/cisco/enic/enic_ethtool.c
@@ -86,6 +86,8 @@ static const struct enic_stat enic_rx_stats[] = {
static const struct enic_stat enic_gen_stats[] = {
ENIC_GEN_STAT(dma_map_error),
+ ENIC_GEN_STAT(cache_alloc_err),
+ ENIC_GEN_STAT(rq_alloc_err),
};
static const unsigned int enic_n_tx_stats = ARRAY_SIZE(enic_tx_stats);
diff --git a/drivers/net/ethernet/cisco/enic/enic_main.c b/drivers/net/ethernet/cisco/enic/enic_main.c
index d9cad93..f15687d 100644
--- a/drivers/net/ethernet/cisco/enic/enic_main.c
+++ b/drivers/net/ethernet/cisco/enic/enic_main.c
@@ -962,6 +962,7 @@ struct enic_alloc_cache *enic_page_refill(struct enic *enic, size_t sz,
goto no_ec;
ec->frag.page = alloc_pages_node(NUMA_NO_NODE, gfp_comp, order);
if (unlikely(!ec->frag.page)) {
+ enic->gen_stats.cache_alloc_err++;
order = get_order(sz);
ec->frag.page = alloc_pages_node(NUMA_NO_NODE, gfp, order);
if (!ec->frag.page)
@@ -1105,6 +1106,8 @@ static int enic_rq_alloc_buf(struct vnic_rq *rq)
return 0;
alloc_fail:
+ enic->gen_stats.rq_alloc_err++;
+
return -ENOMEM;
}
diff --git a/drivers/net/ethernet/cisco/enic/vnic_stats.h b/drivers/net/ethernet/cisco/enic/vnic_stats.h
index 74c81ed..b2a4528 100644
--- a/drivers/net/ethernet/cisco/enic/vnic_stats.h
+++ b/drivers/net/ethernet/cisco/enic/vnic_stats.h
@@ -65,6 +65,8 @@ struct vnic_rx_stats {
/* Generic statistics */
struct vnic_gen_stats {
u64 dma_map_error;
+ u64 cache_alloc_err; /* alloc_pages(enic->order) failures */
+ u64 rq_alloc_err; /* rq buf + skb alloc failures */
};
struct vnic_stats {
--
2.2.2
^ permalink raw reply related
* [PATCH net-next 3/4] ethtool: add RX_ALLOC_ORDER to tunable
From: Govindarajulu Varadarajan @ 2015-01-31 12:28 UTC (permalink / raw)
To: davem, netdev; +Cc: ssujith, benve, edumazet, ben, Govindarajulu Varadarajan
In-Reply-To: <1422707290-939-1-git-send-email-_govind@gmx.com>
Signed-off-by: Govindarajulu Varadarajan <_govind@gmx.com>
---
include/uapi/linux/ethtool.h | 1 +
net/core/ethtool.c | 5 +++++
2 files changed, 6 insertions(+)
diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index 5f66d9c..59362f8 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -213,6 +213,7 @@ enum tunable_id {
ETHTOOL_ID_UNSPEC,
ETHTOOL_RX_COPYBREAK,
ETHTOOL_TX_COPYBREAK,
+ ETHTOOL_RX_ALLOC_ORDER,
};
enum tunable_type_id {
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 91f74f3..5fd7ebf 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -1670,6 +1670,11 @@ static int ethtool_tunable_valid(const struct ethtool_tunable *tuna)
tuna->type_id != ETHTOOL_TUNABLE_U32)
return -EINVAL;
break;
+ case ETHTOOL_RX_ALLOC_ORDER:
+ if (tuna->len != sizeof(u8) ||
+ tuna->type_id != ETHTOOL_TUNABLE_U8)
+ return -EINVAL;
+ break;
default:
return -EINVAL;
}
--
2.2.2
^ permalink raw reply related
* [PATCH net-next 4/4] enic: add ethtool support for changing alloc order
From: Govindarajulu Varadarajan @ 2015-01-31 12:28 UTC (permalink / raw)
To: davem, netdev; +Cc: ssujith, benve, edumazet, ben, Govindarajulu Varadarajan
In-Reply-To: <1422707290-939-1-git-send-email-_govind@gmx.com>
Adds support for changing page order of enic frag allocator.
In case of changing mtu, if size of new mtu is greater than size of compound
page allocated in enic frag allocator, we change the order to min order required
for the new mtu. We would like to give high priority for changing mtu than order
value.
Signed-off-by: Govindarajulu Varadarajan <_govind@gmx.com>
---
drivers/net/ethernet/cisco/enic/enic_ethtool.c | 15 +++++++++++++++
drivers/net/ethernet/cisco/enic/enic_main.c | 18 ++++++++++++++++++
2 files changed, 33 insertions(+)
diff --git a/drivers/net/ethernet/cisco/enic/enic_ethtool.c b/drivers/net/ethernet/cisco/enic/enic_ethtool.c
index 3f9d91b..e29423c 100644
--- a/drivers/net/ethernet/cisco/enic/enic_ethtool.c
+++ b/drivers/net/ethernet/cisco/enic/enic_ethtool.c
@@ -18,6 +18,7 @@
#include <linux/netdevice.h>
#include <linux/ethtool.h>
+#include <linux/if_vlan.h>
#include "enic_res.h"
#include "enic.h"
@@ -409,6 +410,9 @@ static int enic_get_tunable(struct net_device *dev,
case ETHTOOL_RX_COPYBREAK:
*(u32 *)data = enic->rx_copybreak;
break;
+ case ETHTOOL_RX_ALLOC_ORDER:
+ *(u8 *)data = enic->alloc_order;
+ break;
default:
ret = -EINVAL;
break;
@@ -428,6 +432,17 @@ static int enic_set_tunable(struct net_device *dev,
case ETHTOOL_RX_COPYBREAK:
enic->rx_copybreak = *(u32 *)data;
break;
+ case ETHTOOL_RX_ALLOC_ORDER:
+ ret = dev->mtu + VLAN_ETH_HLEN + NET_IP_ALIGN + NET_SKB_PAD;
+ ret = SKB_DATA_ALIGN(ret) +
+ SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
+ if (*(u8 *)data < get_order(ret)) {
+ ret = -EINVAL;
+ break;
+ }
+ ret = 0;
+ enic->alloc_order = *(u8 *)data;
+ break;
default:
ret = -EINVAL;
break;
diff --git a/drivers/net/ethernet/cisco/enic/enic_main.c b/drivers/net/ethernet/cisco/enic/enic_main.c
index f15687d..4a759a0 100644
--- a/drivers/net/ethernet/cisco/enic/enic_main.c
+++ b/drivers/net/ethernet/cisco/enic/enic_main.c
@@ -1872,7 +1872,11 @@ static int enic_change_mtu(struct net_device *netdev, int new_mtu)
{
struct enic *enic = netdev_priv(netdev);
int running = netif_running(netdev);
+ size_t len;
+ len = new_mtu + VLAN_ETH_HLEN + NET_IP_ALIGN + NET_SKB_PAD;
+ len = SKB_DATA_ALIGN(len) +
+ SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
if (new_mtu < ENIC_MIN_MTU || new_mtu > ENIC_MAX_MTU)
return -EINVAL;
@@ -1884,6 +1888,11 @@ static int enic_change_mtu(struct net_device *netdev, int new_mtu)
netdev->mtu = new_mtu;
+ if (len > (PAGE_SIZE << enic->alloc_order)) {
+ enic->alloc_order = get_order(len);
+ netdev_warn(netdev, "new mtu is greater than size of rx alloc_page, resetting enic->alloc_order to :%d\n",
+ enic->alloc_order);
+ }
if (netdev->mtu > enic->port_mtu)
netdev_warn(netdev,
"interface MTU (%d) set higher than port MTU (%d)\n",
@@ -1902,8 +1911,12 @@ static void enic_change_mtu_work(struct work_struct *work)
int new_mtu = vnic_dev_mtu(enic->vdev);
int err;
unsigned int i;
+ size_t len;
new_mtu = max_t(int, ENIC_MIN_MTU, min_t(int, ENIC_MAX_MTU, new_mtu));
+ len = new_mtu + VLAN_ETH_HLEN + NET_IP_ALIGN + NET_SKB_PAD;
+ len = SKB_DATA_ALIGN(len) +
+ SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
rtnl_lock();
@@ -1927,6 +1940,11 @@ static void enic_change_mtu_work(struct work_struct *work)
/* Fill RQ with new_mtu-sized buffers */
netdev->mtu = new_mtu;
+ if (len > (PAGE_SIZE << enic->alloc_order)) {
+ enic->alloc_order = get_order(len);
+ netdev_warn(netdev, "new mtu is greater than size of rx alloc_page, resetting enic->alloc_order to :%d\n",
+ enic->alloc_order);
+ }
vnic_rq_fill(&enic->rq[0], enic_rq_alloc_buf);
/* Need at least one buffer on ring to get going */
if (vnic_rq_desc_used(&enic->rq[0]) == 0) {
--
2.2.2
^ permalink raw reply related
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