* [PATCH v2 net 0/3] virtio: ctrl buffer fixes
@ 2018-04-19 5:30 Michael S. Tsirkin
2018-04-19 5:30 ` Michael S. Tsirkin
` (5 more replies)
0 siblings, 6 replies; 20+ messages in thread
From: Michael S. Tsirkin @ 2018-04-19 5:30 UTC (permalink / raw)
To: linux-kernel
Cc: Mikulas Patocka, Eric Dumazet, David Miller, Thomas Huth, Cornelia Huck
Here are a couple of fixes related to the virtio control buffer.
Lightly tested on x86 only.
Michael S. Tsirkin (3):
virtio_net: split out ctrl buffer
virtio_net: fix adding vids on big-endian
virtio_net: sparse annotation fix
drivers/net/virtio_net.c | 68 +++++++++++++++++++++++++++---------------------
1 file changed, 39 insertions(+), 29 deletions(-)
--
MST
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 net 1/3] virtio_net: split out ctrl buffer
2018-04-19 5:30 [PATCH v2 net 0/3] virtio: ctrl buffer fixes Michael S. Tsirkin
@ 2018-04-19 5:30 ` Michael S. Tsirkin
2018-04-19 5:30 ` [PATCH v2 net 2/3] virtio_net: fix adding vids on big-endian Michael S. Tsirkin
` (4 subsequent siblings)
5 siblings, 0 replies; 20+ messages in thread
From: Michael S. Tsirkin @ 2018-04-19 5:30 UTC (permalink / raw)
To: linux-kernel
Cc: Mikulas Patocka, Eric Dumazet, David Miller, Thomas Huth,
Cornelia Huck, Jason Wang, virtualization, netdev
When sending control commands, virtio net sets up several buffers for
DMA. The buffers are all part of the net device which means it's
actually allocated by kvmalloc so it's in theory (on extreme memory
pressure) possible to get a vmalloc'ed buffer which on some platforms
means we can't DMA there.
Fix up by moving the DMA buffers into a separate structure.
Reported-by: Mikulas Patocka <mpatocka@redhat.com>
Suggested-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
Changes from v1:
build fix
drivers/net/virtio_net.c | 68 +++++++++++++++++++++++++++---------------------
1 file changed, 39 insertions(+), 29 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 7b187ec..3d0eff53 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -147,6 +147,17 @@ struct receive_queue {
struct xdp_rxq_info xdp_rxq;
};
+/* Control VQ buffers: protected by the rtnl lock */
+struct control_buf {
+ struct virtio_net_ctrl_hdr hdr;
+ virtio_net_ctrl_ack status;
+ struct virtio_net_ctrl_mq mq;
+ u8 promisc;
+ u8 allmulti;
+ u16 vid;
+ u64 offloads;
+};
+
struct virtnet_info {
struct virtio_device *vdev;
struct virtqueue *cvq;
@@ -192,14 +203,7 @@ struct virtnet_info {
struct hlist_node node;
struct hlist_node node_dead;
- /* Control VQ buffers: protected by the rtnl lock */
- struct virtio_net_ctrl_hdr ctrl_hdr;
- virtio_net_ctrl_ack ctrl_status;
- struct virtio_net_ctrl_mq ctrl_mq;
- u8 ctrl_promisc;
- u8 ctrl_allmulti;
- u16 ctrl_vid;
- u64 ctrl_offloads;
+ struct control_buf *ctrl;
/* Ethtool settings */
u8 duplex;
@@ -1454,25 +1458,25 @@ static bool virtnet_send_command(struct virtnet_info *vi, u8 class, u8 cmd,
/* Caller should know better */
BUG_ON(!virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ));
- vi->ctrl_status = ~0;
- vi->ctrl_hdr.class = class;
- vi->ctrl_hdr.cmd = cmd;
+ vi->ctrl->status = ~0;
+ vi->ctrl->hdr.class = class;
+ vi->ctrl->hdr.cmd = cmd;
/* Add header */
- sg_init_one(&hdr, &vi->ctrl_hdr, sizeof(vi->ctrl_hdr));
+ sg_init_one(&hdr, &vi->ctrl->hdr, sizeof(vi->ctrl->hdr));
sgs[out_num++] = &hdr;
if (out)
sgs[out_num++] = out;
/* Add return status. */
- sg_init_one(&stat, &vi->ctrl_status, sizeof(vi->ctrl_status));
+ sg_init_one(&stat, &vi->ctrl->status, sizeof(vi->ctrl->status));
sgs[out_num] = &stat;
BUG_ON(out_num + 1 > ARRAY_SIZE(sgs));
virtqueue_add_sgs(vi->cvq, sgs, out_num, 1, vi, GFP_ATOMIC);
if (unlikely(!virtqueue_kick(vi->cvq)))
- return vi->ctrl_status == VIRTIO_NET_OK;
+ return vi->ctrl->status == VIRTIO_NET_OK;
/* Spin for a response, the kick causes an ioport write, trapping
* into the hypervisor, so the request should be handled immediately.
@@ -1481,7 +1485,7 @@ static bool virtnet_send_command(struct virtnet_info *vi, u8 class, u8 cmd,
!virtqueue_is_broken(vi->cvq))
cpu_relax();
- return vi->ctrl_status == VIRTIO_NET_OK;
+ return vi->ctrl->status == VIRTIO_NET_OK;
}
static int virtnet_set_mac_address(struct net_device *dev, void *p)
@@ -1593,8 +1597,8 @@ static int _virtnet_set_queues(struct virtnet_info *vi, u16 queue_pairs)
if (!vi->has_cvq || !virtio_has_feature(vi->vdev, VIRTIO_NET_F_MQ))
return 0;
- vi->ctrl_mq.virtqueue_pairs = cpu_to_virtio16(vi->vdev, queue_pairs);
- sg_init_one(&sg, &vi->ctrl_mq, sizeof(vi->ctrl_mq));
+ vi->ctrl->mq.virtqueue_pairs = cpu_to_virtio16(vi->vdev, queue_pairs);
+ sg_init_one(&sg, &vi->ctrl->mq, sizeof(vi->ctrl->mq));
if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MQ,
VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET, &sg)) {
@@ -1653,22 +1657,22 @@ static void virtnet_set_rx_mode(struct net_device *dev)
if (!virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_RX))
return;
- vi->ctrl_promisc = ((dev->flags & IFF_PROMISC) != 0);
- vi->ctrl_allmulti = ((dev->flags & IFF_ALLMULTI) != 0);
+ vi->ctrl->promisc = ((dev->flags & IFF_PROMISC) != 0);
+ vi->ctrl->allmulti = ((dev->flags & IFF_ALLMULTI) != 0);
- sg_init_one(sg, &vi->ctrl_promisc, sizeof(vi->ctrl_promisc));
+ sg_init_one(sg, &vi->ctrl->promisc, sizeof(vi->ctrl->promisc));
if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_RX,
VIRTIO_NET_CTRL_RX_PROMISC, sg))
dev_warn(&dev->dev, "Failed to %sable promisc mode.\n",
- vi->ctrl_promisc ? "en" : "dis");
+ vi->ctrl->promisc ? "en" : "dis");
- sg_init_one(sg, &vi->ctrl_allmulti, sizeof(vi->ctrl_allmulti));
+ sg_init_one(sg, &vi->ctrl->allmulti, sizeof(vi->ctrl->allmulti));
if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_RX,
VIRTIO_NET_CTRL_RX_ALLMULTI, sg))
dev_warn(&dev->dev, "Failed to %sable allmulti mode.\n",
- vi->ctrl_allmulti ? "en" : "dis");
+ vi->ctrl->allmulti ? "en" : "dis");
uc_count = netdev_uc_count(dev);
mc_count = netdev_mc_count(dev);
@@ -1714,8 +1718,8 @@ static int virtnet_vlan_rx_add_vid(struct net_device *dev,
struct virtnet_info *vi = netdev_priv(dev);
struct scatterlist sg;
- vi->ctrl_vid = vid;
- sg_init_one(&sg, &vi->ctrl_vid, sizeof(vi->ctrl_vid));
+ vi->ctrl->vid = vid;
+ sg_init_one(&sg, &vi->ctrl->vid, sizeof(vi->ctrl->vid));
if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_VLAN,
VIRTIO_NET_CTRL_VLAN_ADD, &sg))
@@ -1729,8 +1733,8 @@ static int virtnet_vlan_rx_kill_vid(struct net_device *dev,
struct virtnet_info *vi = netdev_priv(dev);
struct scatterlist sg;
- vi->ctrl_vid = vid;
- sg_init_one(&sg, &vi->ctrl_vid, sizeof(vi->ctrl_vid));
+ vi->ctrl->vid = vid;
+ sg_init_one(&sg, &vi->ctrl->vid, sizeof(vi->ctrl->vid));
if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_VLAN,
VIRTIO_NET_CTRL_VLAN_DEL, &sg))
@@ -2126,9 +2130,9 @@ static int virtnet_restore_up(struct virtio_device *vdev)
static int virtnet_set_guest_offloads(struct virtnet_info *vi, u64 offloads)
{
struct scatterlist sg;
- vi->ctrl_offloads = cpu_to_virtio64(vi->vdev, offloads);
+ vi->ctrl->offloads = cpu_to_virtio64(vi->vdev, offloads);
- sg_init_one(&sg, &vi->ctrl_offloads, sizeof(vi->ctrl_offloads));
+ sg_init_one(&sg, &vi->ctrl->offloads, sizeof(vi->ctrl->offloads));
if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_GUEST_OFFLOADS,
VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET, &sg)) {
@@ -2351,6 +2355,7 @@ static void virtnet_free_queues(struct virtnet_info *vi)
kfree(vi->rq);
kfree(vi->sq);
+ kfree(vi->ctrl);
}
static void _free_receive_bufs(struct virtnet_info *vi)
@@ -2543,6 +2548,9 @@ static int virtnet_alloc_queues(struct virtnet_info *vi)
{
int i;
+ vi->ctrl = kzalloc(sizeof(*vi->ctrl), GFP_KERNEL);
+ if (!vi->ctrl)
+ goto err_ctrl;
vi->sq = kzalloc(sizeof(*vi->sq) * vi->max_queue_pairs, GFP_KERNEL);
if (!vi->sq)
goto err_sq;
@@ -2571,6 +2579,8 @@ static int virtnet_alloc_queues(struct virtnet_info *vi)
err_rq:
kfree(vi->sq);
err_sq:
+ kfree(vi->ctrl);
+err_ctrl:
return -ENOMEM;
}
--
MST
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 net 1/3] virtio_net: split out ctrl buffer
@ 2018-04-19 5:30 ` Michael S. Tsirkin
0 siblings, 0 replies; 20+ messages in thread
From: Michael S. Tsirkin @ 2018-04-19 5:30 UTC (permalink / raw)
To: linux-kernel
Cc: Thomas Huth, Eric Dumazet, netdev, Cornelia Huck, virtualization,
Mikulas Patocka, David Miller
When sending control commands, virtio net sets up several buffers for
DMA. The buffers are all part of the net device which means it's
actually allocated by kvmalloc so it's in theory (on extreme memory
pressure) possible to get a vmalloc'ed buffer which on some platforms
means we can't DMA there.
Fix up by moving the DMA buffers into a separate structure.
Reported-by: Mikulas Patocka <mpatocka@redhat.com>
Suggested-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
Changes from v1:
build fix
drivers/net/virtio_net.c | 68 +++++++++++++++++++++++++++---------------------
1 file changed, 39 insertions(+), 29 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 7b187ec..3d0eff53 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -147,6 +147,17 @@ struct receive_queue {
struct xdp_rxq_info xdp_rxq;
};
+/* Control VQ buffers: protected by the rtnl lock */
+struct control_buf {
+ struct virtio_net_ctrl_hdr hdr;
+ virtio_net_ctrl_ack status;
+ struct virtio_net_ctrl_mq mq;
+ u8 promisc;
+ u8 allmulti;
+ u16 vid;
+ u64 offloads;
+};
+
struct virtnet_info {
struct virtio_device *vdev;
struct virtqueue *cvq;
@@ -192,14 +203,7 @@ struct virtnet_info {
struct hlist_node node;
struct hlist_node node_dead;
- /* Control VQ buffers: protected by the rtnl lock */
- struct virtio_net_ctrl_hdr ctrl_hdr;
- virtio_net_ctrl_ack ctrl_status;
- struct virtio_net_ctrl_mq ctrl_mq;
- u8 ctrl_promisc;
- u8 ctrl_allmulti;
- u16 ctrl_vid;
- u64 ctrl_offloads;
+ struct control_buf *ctrl;
/* Ethtool settings */
u8 duplex;
@@ -1454,25 +1458,25 @@ static bool virtnet_send_command(struct virtnet_info *vi, u8 class, u8 cmd,
/* Caller should know better */
BUG_ON(!virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ));
- vi->ctrl_status = ~0;
- vi->ctrl_hdr.class = class;
- vi->ctrl_hdr.cmd = cmd;
+ vi->ctrl->status = ~0;
+ vi->ctrl->hdr.class = class;
+ vi->ctrl->hdr.cmd = cmd;
/* Add header */
- sg_init_one(&hdr, &vi->ctrl_hdr, sizeof(vi->ctrl_hdr));
+ sg_init_one(&hdr, &vi->ctrl->hdr, sizeof(vi->ctrl->hdr));
sgs[out_num++] = &hdr;
if (out)
sgs[out_num++] = out;
/* Add return status. */
- sg_init_one(&stat, &vi->ctrl_status, sizeof(vi->ctrl_status));
+ sg_init_one(&stat, &vi->ctrl->status, sizeof(vi->ctrl->status));
sgs[out_num] = &stat;
BUG_ON(out_num + 1 > ARRAY_SIZE(sgs));
virtqueue_add_sgs(vi->cvq, sgs, out_num, 1, vi, GFP_ATOMIC);
if (unlikely(!virtqueue_kick(vi->cvq)))
- return vi->ctrl_status == VIRTIO_NET_OK;
+ return vi->ctrl->status == VIRTIO_NET_OK;
/* Spin for a response, the kick causes an ioport write, trapping
* into the hypervisor, so the request should be handled immediately.
@@ -1481,7 +1485,7 @@ static bool virtnet_send_command(struct virtnet_info *vi, u8 class, u8 cmd,
!virtqueue_is_broken(vi->cvq))
cpu_relax();
- return vi->ctrl_status == VIRTIO_NET_OK;
+ return vi->ctrl->status == VIRTIO_NET_OK;
}
static int virtnet_set_mac_address(struct net_device *dev, void *p)
@@ -1593,8 +1597,8 @@ static int _virtnet_set_queues(struct virtnet_info *vi, u16 queue_pairs)
if (!vi->has_cvq || !virtio_has_feature(vi->vdev, VIRTIO_NET_F_MQ))
return 0;
- vi->ctrl_mq.virtqueue_pairs = cpu_to_virtio16(vi->vdev, queue_pairs);
- sg_init_one(&sg, &vi->ctrl_mq, sizeof(vi->ctrl_mq));
+ vi->ctrl->mq.virtqueue_pairs = cpu_to_virtio16(vi->vdev, queue_pairs);
+ sg_init_one(&sg, &vi->ctrl->mq, sizeof(vi->ctrl->mq));
if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MQ,
VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET, &sg)) {
@@ -1653,22 +1657,22 @@ static void virtnet_set_rx_mode(struct net_device *dev)
if (!virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_RX))
return;
- vi->ctrl_promisc = ((dev->flags & IFF_PROMISC) != 0);
- vi->ctrl_allmulti = ((dev->flags & IFF_ALLMULTI) != 0);
+ vi->ctrl->promisc = ((dev->flags & IFF_PROMISC) != 0);
+ vi->ctrl->allmulti = ((dev->flags & IFF_ALLMULTI) != 0);
- sg_init_one(sg, &vi->ctrl_promisc, sizeof(vi->ctrl_promisc));
+ sg_init_one(sg, &vi->ctrl->promisc, sizeof(vi->ctrl->promisc));
if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_RX,
VIRTIO_NET_CTRL_RX_PROMISC, sg))
dev_warn(&dev->dev, "Failed to %sable promisc mode.\n",
- vi->ctrl_promisc ? "en" : "dis");
+ vi->ctrl->promisc ? "en" : "dis");
- sg_init_one(sg, &vi->ctrl_allmulti, sizeof(vi->ctrl_allmulti));
+ sg_init_one(sg, &vi->ctrl->allmulti, sizeof(vi->ctrl->allmulti));
if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_RX,
VIRTIO_NET_CTRL_RX_ALLMULTI, sg))
dev_warn(&dev->dev, "Failed to %sable allmulti mode.\n",
- vi->ctrl_allmulti ? "en" : "dis");
+ vi->ctrl->allmulti ? "en" : "dis");
uc_count = netdev_uc_count(dev);
mc_count = netdev_mc_count(dev);
@@ -1714,8 +1718,8 @@ static int virtnet_vlan_rx_add_vid(struct net_device *dev,
struct virtnet_info *vi = netdev_priv(dev);
struct scatterlist sg;
- vi->ctrl_vid = vid;
- sg_init_one(&sg, &vi->ctrl_vid, sizeof(vi->ctrl_vid));
+ vi->ctrl->vid = vid;
+ sg_init_one(&sg, &vi->ctrl->vid, sizeof(vi->ctrl->vid));
if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_VLAN,
VIRTIO_NET_CTRL_VLAN_ADD, &sg))
@@ -1729,8 +1733,8 @@ static int virtnet_vlan_rx_kill_vid(struct net_device *dev,
struct virtnet_info *vi = netdev_priv(dev);
struct scatterlist sg;
- vi->ctrl_vid = vid;
- sg_init_one(&sg, &vi->ctrl_vid, sizeof(vi->ctrl_vid));
+ vi->ctrl->vid = vid;
+ sg_init_one(&sg, &vi->ctrl->vid, sizeof(vi->ctrl->vid));
if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_VLAN,
VIRTIO_NET_CTRL_VLAN_DEL, &sg))
@@ -2126,9 +2130,9 @@ static int virtnet_restore_up(struct virtio_device *vdev)
static int virtnet_set_guest_offloads(struct virtnet_info *vi, u64 offloads)
{
struct scatterlist sg;
- vi->ctrl_offloads = cpu_to_virtio64(vi->vdev, offloads);
+ vi->ctrl->offloads = cpu_to_virtio64(vi->vdev, offloads);
- sg_init_one(&sg, &vi->ctrl_offloads, sizeof(vi->ctrl_offloads));
+ sg_init_one(&sg, &vi->ctrl->offloads, sizeof(vi->ctrl->offloads));
if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_GUEST_OFFLOADS,
VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET, &sg)) {
@@ -2351,6 +2355,7 @@ static void virtnet_free_queues(struct virtnet_info *vi)
kfree(vi->rq);
kfree(vi->sq);
+ kfree(vi->ctrl);
}
static void _free_receive_bufs(struct virtnet_info *vi)
@@ -2543,6 +2548,9 @@ static int virtnet_alloc_queues(struct virtnet_info *vi)
{
int i;
+ vi->ctrl = kzalloc(sizeof(*vi->ctrl), GFP_KERNEL);
+ if (!vi->ctrl)
+ goto err_ctrl;
vi->sq = kzalloc(sizeof(*vi->sq) * vi->max_queue_pairs, GFP_KERNEL);
if (!vi->sq)
goto err_sq;
@@ -2571,6 +2579,8 @@ static int virtnet_alloc_queues(struct virtnet_info *vi)
err_rq:
kfree(vi->sq);
err_sq:
+ kfree(vi->ctrl);
+err_ctrl:
return -ENOMEM;
}
--
MST
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 net 2/3] virtio_net: fix adding vids on big-endian
2018-04-19 5:30 [PATCH v2 net 0/3] virtio: ctrl buffer fixes Michael S. Tsirkin
2018-04-19 5:30 ` Michael S. Tsirkin
@ 2018-04-19 5:30 ` Michael S. Tsirkin
2018-04-19 12:26 ` Jason Wang
` (3 more replies)
2018-04-19 5:30 ` Michael S. Tsirkin
` (3 subsequent siblings)
5 siblings, 4 replies; 20+ messages in thread
From: Michael S. Tsirkin @ 2018-04-19 5:30 UTC (permalink / raw)
To: linux-kernel
Cc: Mikulas Patocka, Eric Dumazet, David Miller, Thomas Huth,
Cornelia Huck, Jason Wang, virtualization, netdev
Programming vids (adding or removing them) still passes
guest-endian values in the DMA buffer. That's wrong
if guest is big-endian and when virtio 1 is enabled.
Note: this is on top of a previous patch:
virtio_net: split out ctrl buffer
Fixes: 9465a7a6f ("virtio_net: enable v1.0 support")
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/net/virtio_net.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 3d0eff53..f84fe04 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -154,7 +154,7 @@ struct control_buf {
struct virtio_net_ctrl_mq mq;
u8 promisc;
u8 allmulti;
- u16 vid;
+ __virtio16 vid;
u64 offloads;
};
@@ -1718,7 +1718,7 @@ static int virtnet_vlan_rx_add_vid(struct net_device *dev,
struct virtnet_info *vi = netdev_priv(dev);
struct scatterlist sg;
- vi->ctrl->vid = vid;
+ vi->ctrl->vid = cpu_to_virtio16(vi->vdev, vid);
sg_init_one(&sg, &vi->ctrl->vid, sizeof(vi->ctrl->vid));
if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_VLAN,
@@ -1733,7 +1733,7 @@ static int virtnet_vlan_rx_kill_vid(struct net_device *dev,
struct virtnet_info *vi = netdev_priv(dev);
struct scatterlist sg;
- vi->ctrl->vid = vid;
+ vi->ctrl->vid = cpu_to_virtio16(vi->vdev, vid);
sg_init_one(&sg, &vi->ctrl->vid, sizeof(vi->ctrl->vid));
if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_VLAN,
--
MST
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 net 2/3] virtio_net: fix adding vids on big-endian
2018-04-19 5:30 [PATCH v2 net 0/3] virtio: ctrl buffer fixes Michael S. Tsirkin
2018-04-19 5:30 ` Michael S. Tsirkin
2018-04-19 5:30 ` [PATCH v2 net 2/3] virtio_net: fix adding vids on big-endian Michael S. Tsirkin
@ 2018-04-19 5:30 ` Michael S. Tsirkin
2018-04-19 5:30 ` [PATCH v2 net 3/3] virtio_net: sparse annotation fix Michael S. Tsirkin
` (2 subsequent siblings)
5 siblings, 0 replies; 20+ messages in thread
From: Michael S. Tsirkin @ 2018-04-19 5:30 UTC (permalink / raw)
To: linux-kernel
Cc: Thomas Huth, Eric Dumazet, netdev, Cornelia Huck, virtualization,
Mikulas Patocka, David Miller
Programming vids (adding or removing them) still passes
guest-endian values in the DMA buffer. That's wrong
if guest is big-endian and when virtio 1 is enabled.
Note: this is on top of a previous patch:
virtio_net: split out ctrl buffer
Fixes: 9465a7a6f ("virtio_net: enable v1.0 support")
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/net/virtio_net.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 3d0eff53..f84fe04 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -154,7 +154,7 @@ struct control_buf {
struct virtio_net_ctrl_mq mq;
u8 promisc;
u8 allmulti;
- u16 vid;
+ __virtio16 vid;
u64 offloads;
};
@@ -1718,7 +1718,7 @@ static int virtnet_vlan_rx_add_vid(struct net_device *dev,
struct virtnet_info *vi = netdev_priv(dev);
struct scatterlist sg;
- vi->ctrl->vid = vid;
+ vi->ctrl->vid = cpu_to_virtio16(vi->vdev, vid);
sg_init_one(&sg, &vi->ctrl->vid, sizeof(vi->ctrl->vid));
if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_VLAN,
@@ -1733,7 +1733,7 @@ static int virtnet_vlan_rx_kill_vid(struct net_device *dev,
struct virtnet_info *vi = netdev_priv(dev);
struct scatterlist sg;
- vi->ctrl->vid = vid;
+ vi->ctrl->vid = cpu_to_virtio16(vi->vdev, vid);
sg_init_one(&sg, &vi->ctrl->vid, sizeof(vi->ctrl->vid));
if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_VLAN,
--
MST
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 net 3/3] virtio_net: sparse annotation fix
2018-04-19 5:30 [PATCH v2 net 0/3] virtio: ctrl buffer fixes Michael S. Tsirkin
` (3 preceding siblings ...)
2018-04-19 5:30 ` [PATCH v2 net 3/3] virtio_net: sparse annotation fix Michael S. Tsirkin
@ 2018-04-19 5:30 ` Michael S. Tsirkin
2018-04-19 12:27 ` Jason Wang
2018-04-19 12:27 ` Jason Wang
2018-04-19 20:34 ` [PATCH v2 net 0/3] virtio: ctrl buffer fixes David Miller
5 siblings, 2 replies; 20+ messages in thread
From: Michael S. Tsirkin @ 2018-04-19 5:30 UTC (permalink / raw)
To: linux-kernel
Cc: Mikulas Patocka, Eric Dumazet, David Miller, Thomas Huth,
Cornelia Huck, Jason Wang, virtualization, netdev
offloads is a buffer in virtio format, should use
the __virtio64 tag.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/net/virtio_net.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index f84fe04..c5b11f2 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -155,7 +155,7 @@ struct control_buf {
u8 promisc;
u8 allmulti;
__virtio16 vid;
- u64 offloads;
+ __virtio64 offloads;
};
struct virtnet_info {
--
MST
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 net 3/3] virtio_net: sparse annotation fix
2018-04-19 5:30 [PATCH v2 net 0/3] virtio: ctrl buffer fixes Michael S. Tsirkin
` (2 preceding siblings ...)
2018-04-19 5:30 ` Michael S. Tsirkin
@ 2018-04-19 5:30 ` Michael S. Tsirkin
2018-04-19 5:30 ` Michael S. Tsirkin
2018-04-19 20:34 ` [PATCH v2 net 0/3] virtio: ctrl buffer fixes David Miller
5 siblings, 0 replies; 20+ messages in thread
From: Michael S. Tsirkin @ 2018-04-19 5:30 UTC (permalink / raw)
To: linux-kernel
Cc: Thomas Huth, Eric Dumazet, netdev, Cornelia Huck, virtualization,
Mikulas Patocka, David Miller
offloads is a buffer in virtio format, should use
the __virtio64 tag.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/net/virtio_net.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index f84fe04..c5b11f2 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -155,7 +155,7 @@ struct control_buf {
u8 promisc;
u8 allmulti;
__virtio16 vid;
- u64 offloads;
+ __virtio64 offloads;
};
struct virtnet_info {
--
MST
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v2 net 1/3] virtio_net: split out ctrl buffer
2018-04-19 5:30 ` Michael S. Tsirkin
@ 2018-04-19 12:26 ` Jason Wang
-1 siblings, 0 replies; 20+ messages in thread
From: Jason Wang @ 2018-04-19 12:26 UTC (permalink / raw)
To: Michael S. Tsirkin, linux-kernel
Cc: Mikulas Patocka, Eric Dumazet, David Miller, Thomas Huth,
Cornelia Huck, virtualization, netdev
On 2018年04月19日 13:30, Michael S. Tsirkin wrote:
> When sending control commands, virtio net sets up several buffers for
> DMA. The buffers are all part of the net device which means it's
> actually allocated by kvmalloc so it's in theory (on extreme memory
> pressure) possible to get a vmalloc'ed buffer which on some platforms
> means we can't DMA there.
>
> Fix up by moving the DMA buffers into a separate structure.
>
> Reported-by: Mikulas Patocka <mpatocka@redhat.com>
> Suggested-by: Eric Dumazet <eric.dumazet@gmail.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>
> Changes from v1:
> build fix
>
> drivers/net/virtio_net.c | 68 +++++++++++++++++++++++++++---------------------
> 1 file changed, 39 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 7b187ec..3d0eff53 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -147,6 +147,17 @@ struct receive_queue {
> struct xdp_rxq_info xdp_rxq;
> };
>
> +/* Control VQ buffers: protected by the rtnl lock */
> +struct control_buf {
> + struct virtio_net_ctrl_hdr hdr;
> + virtio_net_ctrl_ack status;
> + struct virtio_net_ctrl_mq mq;
> + u8 promisc;
> + u8 allmulti;
> + u16 vid;
> + u64 offloads;
> +};
> +
> struct virtnet_info {
> struct virtio_device *vdev;
> struct virtqueue *cvq;
> @@ -192,14 +203,7 @@ struct virtnet_info {
> struct hlist_node node;
> struct hlist_node node_dead;
>
> - /* Control VQ buffers: protected by the rtnl lock */
> - struct virtio_net_ctrl_hdr ctrl_hdr;
> - virtio_net_ctrl_ack ctrl_status;
> - struct virtio_net_ctrl_mq ctrl_mq;
> - u8 ctrl_promisc;
> - u8 ctrl_allmulti;
> - u16 ctrl_vid;
> - u64 ctrl_offloads;
> + struct control_buf *ctrl;
>
> /* Ethtool settings */
> u8 duplex;
> @@ -1454,25 +1458,25 @@ static bool virtnet_send_command(struct virtnet_info *vi, u8 class, u8 cmd,
> /* Caller should know better */
> BUG_ON(!virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ));
>
> - vi->ctrl_status = ~0;
> - vi->ctrl_hdr.class = class;
> - vi->ctrl_hdr.cmd = cmd;
> + vi->ctrl->status = ~0;
> + vi->ctrl->hdr.class = class;
> + vi->ctrl->hdr.cmd = cmd;
> /* Add header */
> - sg_init_one(&hdr, &vi->ctrl_hdr, sizeof(vi->ctrl_hdr));
> + sg_init_one(&hdr, &vi->ctrl->hdr, sizeof(vi->ctrl->hdr));
> sgs[out_num++] = &hdr;
>
> if (out)
> sgs[out_num++] = out;
>
> /* Add return status. */
> - sg_init_one(&stat, &vi->ctrl_status, sizeof(vi->ctrl_status));
> + sg_init_one(&stat, &vi->ctrl->status, sizeof(vi->ctrl->status));
> sgs[out_num] = &stat;
>
> BUG_ON(out_num + 1 > ARRAY_SIZE(sgs));
> virtqueue_add_sgs(vi->cvq, sgs, out_num, 1, vi, GFP_ATOMIC);
>
> if (unlikely(!virtqueue_kick(vi->cvq)))
> - return vi->ctrl_status == VIRTIO_NET_OK;
> + return vi->ctrl->status == VIRTIO_NET_OK;
>
> /* Spin for a response, the kick causes an ioport write, trapping
> * into the hypervisor, so the request should be handled immediately.
> @@ -1481,7 +1485,7 @@ static bool virtnet_send_command(struct virtnet_info *vi, u8 class, u8 cmd,
> !virtqueue_is_broken(vi->cvq))
> cpu_relax();
>
> - return vi->ctrl_status == VIRTIO_NET_OK;
> + return vi->ctrl->status == VIRTIO_NET_OK;
> }
>
> static int virtnet_set_mac_address(struct net_device *dev, void *p)
> @@ -1593,8 +1597,8 @@ static int _virtnet_set_queues(struct virtnet_info *vi, u16 queue_pairs)
> if (!vi->has_cvq || !virtio_has_feature(vi->vdev, VIRTIO_NET_F_MQ))
> return 0;
>
> - vi->ctrl_mq.virtqueue_pairs = cpu_to_virtio16(vi->vdev, queue_pairs);
> - sg_init_one(&sg, &vi->ctrl_mq, sizeof(vi->ctrl_mq));
> + vi->ctrl->mq.virtqueue_pairs = cpu_to_virtio16(vi->vdev, queue_pairs);
> + sg_init_one(&sg, &vi->ctrl->mq, sizeof(vi->ctrl->mq));
>
> if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MQ,
> VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET, &sg)) {
> @@ -1653,22 +1657,22 @@ static void virtnet_set_rx_mode(struct net_device *dev)
> if (!virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_RX))
> return;
>
> - vi->ctrl_promisc = ((dev->flags & IFF_PROMISC) != 0);
> - vi->ctrl_allmulti = ((dev->flags & IFF_ALLMULTI) != 0);
> + vi->ctrl->promisc = ((dev->flags & IFF_PROMISC) != 0);
> + vi->ctrl->allmulti = ((dev->flags & IFF_ALLMULTI) != 0);
>
> - sg_init_one(sg, &vi->ctrl_promisc, sizeof(vi->ctrl_promisc));
> + sg_init_one(sg, &vi->ctrl->promisc, sizeof(vi->ctrl->promisc));
>
> if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_RX,
> VIRTIO_NET_CTRL_RX_PROMISC, sg))
> dev_warn(&dev->dev, "Failed to %sable promisc mode.\n",
> - vi->ctrl_promisc ? "en" : "dis");
> + vi->ctrl->promisc ? "en" : "dis");
>
> - sg_init_one(sg, &vi->ctrl_allmulti, sizeof(vi->ctrl_allmulti));
> + sg_init_one(sg, &vi->ctrl->allmulti, sizeof(vi->ctrl->allmulti));
>
> if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_RX,
> VIRTIO_NET_CTRL_RX_ALLMULTI, sg))
> dev_warn(&dev->dev, "Failed to %sable allmulti mode.\n",
> - vi->ctrl_allmulti ? "en" : "dis");
> + vi->ctrl->allmulti ? "en" : "dis");
>
> uc_count = netdev_uc_count(dev);
> mc_count = netdev_mc_count(dev);
> @@ -1714,8 +1718,8 @@ static int virtnet_vlan_rx_add_vid(struct net_device *dev,
> struct virtnet_info *vi = netdev_priv(dev);
> struct scatterlist sg;
>
> - vi->ctrl_vid = vid;
> - sg_init_one(&sg, &vi->ctrl_vid, sizeof(vi->ctrl_vid));
> + vi->ctrl->vid = vid;
> + sg_init_one(&sg, &vi->ctrl->vid, sizeof(vi->ctrl->vid));
>
> if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_VLAN,
> VIRTIO_NET_CTRL_VLAN_ADD, &sg))
> @@ -1729,8 +1733,8 @@ static int virtnet_vlan_rx_kill_vid(struct net_device *dev,
> struct virtnet_info *vi = netdev_priv(dev);
> struct scatterlist sg;
>
> - vi->ctrl_vid = vid;
> - sg_init_one(&sg, &vi->ctrl_vid, sizeof(vi->ctrl_vid));
> + vi->ctrl->vid = vid;
> + sg_init_one(&sg, &vi->ctrl->vid, sizeof(vi->ctrl->vid));
>
> if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_VLAN,
> VIRTIO_NET_CTRL_VLAN_DEL, &sg))
> @@ -2126,9 +2130,9 @@ static int virtnet_restore_up(struct virtio_device *vdev)
> static int virtnet_set_guest_offloads(struct virtnet_info *vi, u64 offloads)
> {
> struct scatterlist sg;
> - vi->ctrl_offloads = cpu_to_virtio64(vi->vdev, offloads);
> + vi->ctrl->offloads = cpu_to_virtio64(vi->vdev, offloads);
>
> - sg_init_one(&sg, &vi->ctrl_offloads, sizeof(vi->ctrl_offloads));
> + sg_init_one(&sg, &vi->ctrl->offloads, sizeof(vi->ctrl->offloads));
>
> if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_GUEST_OFFLOADS,
> VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET, &sg)) {
> @@ -2351,6 +2355,7 @@ static void virtnet_free_queues(struct virtnet_info *vi)
>
> kfree(vi->rq);
> kfree(vi->sq);
> + kfree(vi->ctrl);
> }
>
> static void _free_receive_bufs(struct virtnet_info *vi)
> @@ -2543,6 +2548,9 @@ static int virtnet_alloc_queues(struct virtnet_info *vi)
> {
> int i;
>
> + vi->ctrl = kzalloc(sizeof(*vi->ctrl), GFP_KERNEL);
> + if (!vi->ctrl)
> + goto err_ctrl;
> vi->sq = kzalloc(sizeof(*vi->sq) * vi->max_queue_pairs, GFP_KERNEL);
> if (!vi->sq)
> goto err_sq;
> @@ -2571,6 +2579,8 @@ static int virtnet_alloc_queues(struct virtnet_info *vi)
> err_rq:
> kfree(vi->sq);
> err_sq:
> + kfree(vi->ctrl);
> +err_ctrl:
> return -ENOMEM;
> }
>
Acked-by: Jason Wang <jasowang@redhat.com>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 net 1/3] virtio_net: split out ctrl buffer
@ 2018-04-19 12:26 ` Jason Wang
0 siblings, 0 replies; 20+ messages in thread
From: Jason Wang @ 2018-04-19 12:26 UTC (permalink / raw)
To: Michael S. Tsirkin, linux-kernel
Cc: Thomas Huth, Eric Dumazet, netdev, Cornelia Huck, virtualization,
Mikulas Patocka, David Miller
On 2018年04月19日 13:30, Michael S. Tsirkin wrote:
> When sending control commands, virtio net sets up several buffers for
> DMA. The buffers are all part of the net device which means it's
> actually allocated by kvmalloc so it's in theory (on extreme memory
> pressure) possible to get a vmalloc'ed buffer which on some platforms
> means we can't DMA there.
>
> Fix up by moving the DMA buffers into a separate structure.
>
> Reported-by: Mikulas Patocka <mpatocka@redhat.com>
> Suggested-by: Eric Dumazet <eric.dumazet@gmail.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>
> Changes from v1:
> build fix
>
> drivers/net/virtio_net.c | 68 +++++++++++++++++++++++++++---------------------
> 1 file changed, 39 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 7b187ec..3d0eff53 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -147,6 +147,17 @@ struct receive_queue {
> struct xdp_rxq_info xdp_rxq;
> };
>
> +/* Control VQ buffers: protected by the rtnl lock */
> +struct control_buf {
> + struct virtio_net_ctrl_hdr hdr;
> + virtio_net_ctrl_ack status;
> + struct virtio_net_ctrl_mq mq;
> + u8 promisc;
> + u8 allmulti;
> + u16 vid;
> + u64 offloads;
> +};
> +
> struct virtnet_info {
> struct virtio_device *vdev;
> struct virtqueue *cvq;
> @@ -192,14 +203,7 @@ struct virtnet_info {
> struct hlist_node node;
> struct hlist_node node_dead;
>
> - /* Control VQ buffers: protected by the rtnl lock */
> - struct virtio_net_ctrl_hdr ctrl_hdr;
> - virtio_net_ctrl_ack ctrl_status;
> - struct virtio_net_ctrl_mq ctrl_mq;
> - u8 ctrl_promisc;
> - u8 ctrl_allmulti;
> - u16 ctrl_vid;
> - u64 ctrl_offloads;
> + struct control_buf *ctrl;
>
> /* Ethtool settings */
> u8 duplex;
> @@ -1454,25 +1458,25 @@ static bool virtnet_send_command(struct virtnet_info *vi, u8 class, u8 cmd,
> /* Caller should know better */
> BUG_ON(!virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ));
>
> - vi->ctrl_status = ~0;
> - vi->ctrl_hdr.class = class;
> - vi->ctrl_hdr.cmd = cmd;
> + vi->ctrl->status = ~0;
> + vi->ctrl->hdr.class = class;
> + vi->ctrl->hdr.cmd = cmd;
> /* Add header */
> - sg_init_one(&hdr, &vi->ctrl_hdr, sizeof(vi->ctrl_hdr));
> + sg_init_one(&hdr, &vi->ctrl->hdr, sizeof(vi->ctrl->hdr));
> sgs[out_num++] = &hdr;
>
> if (out)
> sgs[out_num++] = out;
>
> /* Add return status. */
> - sg_init_one(&stat, &vi->ctrl_status, sizeof(vi->ctrl_status));
> + sg_init_one(&stat, &vi->ctrl->status, sizeof(vi->ctrl->status));
> sgs[out_num] = &stat;
>
> BUG_ON(out_num + 1 > ARRAY_SIZE(sgs));
> virtqueue_add_sgs(vi->cvq, sgs, out_num, 1, vi, GFP_ATOMIC);
>
> if (unlikely(!virtqueue_kick(vi->cvq)))
> - return vi->ctrl_status == VIRTIO_NET_OK;
> + return vi->ctrl->status == VIRTIO_NET_OK;
>
> /* Spin for a response, the kick causes an ioport write, trapping
> * into the hypervisor, so the request should be handled immediately.
> @@ -1481,7 +1485,7 @@ static bool virtnet_send_command(struct virtnet_info *vi, u8 class, u8 cmd,
> !virtqueue_is_broken(vi->cvq))
> cpu_relax();
>
> - return vi->ctrl_status == VIRTIO_NET_OK;
> + return vi->ctrl->status == VIRTIO_NET_OK;
> }
>
> static int virtnet_set_mac_address(struct net_device *dev, void *p)
> @@ -1593,8 +1597,8 @@ static int _virtnet_set_queues(struct virtnet_info *vi, u16 queue_pairs)
> if (!vi->has_cvq || !virtio_has_feature(vi->vdev, VIRTIO_NET_F_MQ))
> return 0;
>
> - vi->ctrl_mq.virtqueue_pairs = cpu_to_virtio16(vi->vdev, queue_pairs);
> - sg_init_one(&sg, &vi->ctrl_mq, sizeof(vi->ctrl_mq));
> + vi->ctrl->mq.virtqueue_pairs = cpu_to_virtio16(vi->vdev, queue_pairs);
> + sg_init_one(&sg, &vi->ctrl->mq, sizeof(vi->ctrl->mq));
>
> if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MQ,
> VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET, &sg)) {
> @@ -1653,22 +1657,22 @@ static void virtnet_set_rx_mode(struct net_device *dev)
> if (!virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_RX))
> return;
>
> - vi->ctrl_promisc = ((dev->flags & IFF_PROMISC) != 0);
> - vi->ctrl_allmulti = ((dev->flags & IFF_ALLMULTI) != 0);
> + vi->ctrl->promisc = ((dev->flags & IFF_PROMISC) != 0);
> + vi->ctrl->allmulti = ((dev->flags & IFF_ALLMULTI) != 0);
>
> - sg_init_one(sg, &vi->ctrl_promisc, sizeof(vi->ctrl_promisc));
> + sg_init_one(sg, &vi->ctrl->promisc, sizeof(vi->ctrl->promisc));
>
> if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_RX,
> VIRTIO_NET_CTRL_RX_PROMISC, sg))
> dev_warn(&dev->dev, "Failed to %sable promisc mode.\n",
> - vi->ctrl_promisc ? "en" : "dis");
> + vi->ctrl->promisc ? "en" : "dis");
>
> - sg_init_one(sg, &vi->ctrl_allmulti, sizeof(vi->ctrl_allmulti));
> + sg_init_one(sg, &vi->ctrl->allmulti, sizeof(vi->ctrl->allmulti));
>
> if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_RX,
> VIRTIO_NET_CTRL_RX_ALLMULTI, sg))
> dev_warn(&dev->dev, "Failed to %sable allmulti mode.\n",
> - vi->ctrl_allmulti ? "en" : "dis");
> + vi->ctrl->allmulti ? "en" : "dis");
>
> uc_count = netdev_uc_count(dev);
> mc_count = netdev_mc_count(dev);
> @@ -1714,8 +1718,8 @@ static int virtnet_vlan_rx_add_vid(struct net_device *dev,
> struct virtnet_info *vi = netdev_priv(dev);
> struct scatterlist sg;
>
> - vi->ctrl_vid = vid;
> - sg_init_one(&sg, &vi->ctrl_vid, sizeof(vi->ctrl_vid));
> + vi->ctrl->vid = vid;
> + sg_init_one(&sg, &vi->ctrl->vid, sizeof(vi->ctrl->vid));
>
> if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_VLAN,
> VIRTIO_NET_CTRL_VLAN_ADD, &sg))
> @@ -1729,8 +1733,8 @@ static int virtnet_vlan_rx_kill_vid(struct net_device *dev,
> struct virtnet_info *vi = netdev_priv(dev);
> struct scatterlist sg;
>
> - vi->ctrl_vid = vid;
> - sg_init_one(&sg, &vi->ctrl_vid, sizeof(vi->ctrl_vid));
> + vi->ctrl->vid = vid;
> + sg_init_one(&sg, &vi->ctrl->vid, sizeof(vi->ctrl->vid));
>
> if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_VLAN,
> VIRTIO_NET_CTRL_VLAN_DEL, &sg))
> @@ -2126,9 +2130,9 @@ static int virtnet_restore_up(struct virtio_device *vdev)
> static int virtnet_set_guest_offloads(struct virtnet_info *vi, u64 offloads)
> {
> struct scatterlist sg;
> - vi->ctrl_offloads = cpu_to_virtio64(vi->vdev, offloads);
> + vi->ctrl->offloads = cpu_to_virtio64(vi->vdev, offloads);
>
> - sg_init_one(&sg, &vi->ctrl_offloads, sizeof(vi->ctrl_offloads));
> + sg_init_one(&sg, &vi->ctrl->offloads, sizeof(vi->ctrl->offloads));
>
> if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_GUEST_OFFLOADS,
> VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET, &sg)) {
> @@ -2351,6 +2355,7 @@ static void virtnet_free_queues(struct virtnet_info *vi)
>
> kfree(vi->rq);
> kfree(vi->sq);
> + kfree(vi->ctrl);
> }
>
> static void _free_receive_bufs(struct virtnet_info *vi)
> @@ -2543,6 +2548,9 @@ static int virtnet_alloc_queues(struct virtnet_info *vi)
> {
> int i;
>
> + vi->ctrl = kzalloc(sizeof(*vi->ctrl), GFP_KERNEL);
> + if (!vi->ctrl)
> + goto err_ctrl;
> vi->sq = kzalloc(sizeof(*vi->sq) * vi->max_queue_pairs, GFP_KERNEL);
> if (!vi->sq)
> goto err_sq;
> @@ -2571,6 +2579,8 @@ static int virtnet_alloc_queues(struct virtnet_info *vi)
> err_rq:
> kfree(vi->sq);
> err_sq:
> + kfree(vi->ctrl);
> +err_ctrl:
> return -ENOMEM;
> }
>
Acked-by: Jason Wang <jasowang@redhat.com>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 net 2/3] virtio_net: fix adding vids on big-endian
2018-04-19 5:30 ` [PATCH v2 net 2/3] virtio_net: fix adding vids on big-endian Michael S. Tsirkin
2018-04-19 12:26 ` Jason Wang
@ 2018-04-19 12:26 ` Jason Wang
2018-04-19 13:26 ` Cornelia Huck
2018-04-19 13:26 ` Cornelia Huck
3 siblings, 0 replies; 20+ messages in thread
From: Jason Wang @ 2018-04-19 12:26 UTC (permalink / raw)
To: Michael S. Tsirkin, linux-kernel
Cc: Mikulas Patocka, Eric Dumazet, David Miller, Thomas Huth,
Cornelia Huck, virtualization, netdev
On 2018年04月19日 13:30, Michael S. Tsirkin wrote:
> Programming vids (adding or removing them) still passes
> guest-endian values in the DMA buffer. That's wrong
> if guest is big-endian and when virtio 1 is enabled.
>
> Note: this is on top of a previous patch:
> virtio_net: split out ctrl buffer
>
> Fixes: 9465a7a6f ("virtio_net: enable v1.0 support")
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> drivers/net/virtio_net.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 3d0eff53..f84fe04 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -154,7 +154,7 @@ struct control_buf {
> struct virtio_net_ctrl_mq mq;
> u8 promisc;
> u8 allmulti;
> - u16 vid;
> + __virtio16 vid;
> u64 offloads;
> };
>
> @@ -1718,7 +1718,7 @@ static int virtnet_vlan_rx_add_vid(struct net_device *dev,
> struct virtnet_info *vi = netdev_priv(dev);
> struct scatterlist sg;
>
> - vi->ctrl->vid = vid;
> + vi->ctrl->vid = cpu_to_virtio16(vi->vdev, vid);
> sg_init_one(&sg, &vi->ctrl->vid, sizeof(vi->ctrl->vid));
>
> if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_VLAN,
> @@ -1733,7 +1733,7 @@ static int virtnet_vlan_rx_kill_vid(struct net_device *dev,
> struct virtnet_info *vi = netdev_priv(dev);
> struct scatterlist sg;
>
> - vi->ctrl->vid = vid;
> + vi->ctrl->vid = cpu_to_virtio16(vi->vdev, vid);
> sg_init_one(&sg, &vi->ctrl->vid, sizeof(vi->ctrl->vid));
>
> if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_VLAN,
Acked-by: Jason Wang <jasowang@redhat.com>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 net 2/3] virtio_net: fix adding vids on big-endian
2018-04-19 5:30 ` [PATCH v2 net 2/3] virtio_net: fix adding vids on big-endian Michael S. Tsirkin
@ 2018-04-19 12:26 ` Jason Wang
2018-04-19 12:26 ` Jason Wang
` (2 subsequent siblings)
3 siblings, 0 replies; 20+ messages in thread
From: Jason Wang @ 2018-04-19 12:26 UTC (permalink / raw)
To: Michael S. Tsirkin, linux-kernel
Cc: Thomas Huth, Eric Dumazet, netdev, Cornelia Huck, virtualization,
Mikulas Patocka, David Miller
On 2018年04月19日 13:30, Michael S. Tsirkin wrote:
> Programming vids (adding or removing them) still passes
> guest-endian values in the DMA buffer. That's wrong
> if guest is big-endian and when virtio 1 is enabled.
>
> Note: this is on top of a previous patch:
> virtio_net: split out ctrl buffer
>
> Fixes: 9465a7a6f ("virtio_net: enable v1.0 support")
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> drivers/net/virtio_net.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 3d0eff53..f84fe04 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -154,7 +154,7 @@ struct control_buf {
> struct virtio_net_ctrl_mq mq;
> u8 promisc;
> u8 allmulti;
> - u16 vid;
> + __virtio16 vid;
> u64 offloads;
> };
>
> @@ -1718,7 +1718,7 @@ static int virtnet_vlan_rx_add_vid(struct net_device *dev,
> struct virtnet_info *vi = netdev_priv(dev);
> struct scatterlist sg;
>
> - vi->ctrl->vid = vid;
> + vi->ctrl->vid = cpu_to_virtio16(vi->vdev, vid);
> sg_init_one(&sg, &vi->ctrl->vid, sizeof(vi->ctrl->vid));
>
> if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_VLAN,
> @@ -1733,7 +1733,7 @@ static int virtnet_vlan_rx_kill_vid(struct net_device *dev,
> struct virtnet_info *vi = netdev_priv(dev);
> struct scatterlist sg;
>
> - vi->ctrl->vid = vid;
> + vi->ctrl->vid = cpu_to_virtio16(vi->vdev, vid);
> sg_init_one(&sg, &vi->ctrl->vid, sizeof(vi->ctrl->vid));
>
> if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_VLAN,
Acked-by: Jason Wang <jasowang@redhat.com>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 net 3/3] virtio_net: sparse annotation fix
2018-04-19 5:30 ` Michael S. Tsirkin
@ 2018-04-19 12:27 ` Jason Wang
2018-04-19 12:27 ` Jason Wang
1 sibling, 0 replies; 20+ messages in thread
From: Jason Wang @ 2018-04-19 12:27 UTC (permalink / raw)
To: Michael S. Tsirkin, linux-kernel
Cc: Mikulas Patocka, Eric Dumazet, David Miller, Thomas Huth,
Cornelia Huck, virtualization, netdev
On 2018年04月19日 13:30, Michael S. Tsirkin wrote:
> offloads is a buffer in virtio format, should use
> the __virtio64 tag.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> drivers/net/virtio_net.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index f84fe04..c5b11f2 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -155,7 +155,7 @@ struct control_buf {
> u8 promisc;
> u8 allmulti;
> __virtio16 vid;
> - u64 offloads;
> + __virtio64 offloads;
> };
>
> struct virtnet_info {
Acked-by: Jason Wang <jasowang@redhat.com>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 net 3/3] virtio_net: sparse annotation fix
2018-04-19 5:30 ` Michael S. Tsirkin
2018-04-19 12:27 ` Jason Wang
@ 2018-04-19 12:27 ` Jason Wang
1 sibling, 0 replies; 20+ messages in thread
From: Jason Wang @ 2018-04-19 12:27 UTC (permalink / raw)
To: Michael S. Tsirkin, linux-kernel
Cc: Thomas Huth, Eric Dumazet, netdev, Cornelia Huck, virtualization,
Mikulas Patocka, David Miller
On 2018年04月19日 13:30, Michael S. Tsirkin wrote:
> offloads is a buffer in virtio format, should use
> the __virtio64 tag.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> drivers/net/virtio_net.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index f84fe04..c5b11f2 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -155,7 +155,7 @@ struct control_buf {
> u8 promisc;
> u8 allmulti;
> __virtio16 vid;
> - u64 offloads;
> + __virtio64 offloads;
> };
>
> struct virtnet_info {
Acked-by: Jason Wang <jasowang@redhat.com>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 net 2/3] virtio_net: fix adding vids on big-endian
2018-04-19 5:30 ` [PATCH v2 net 2/3] virtio_net: fix adding vids on big-endian Michael S. Tsirkin
2018-04-19 12:26 ` Jason Wang
2018-04-19 12:26 ` Jason Wang
@ 2018-04-19 13:26 ` Cornelia Huck
2018-04-19 14:56 ` Michael S. Tsirkin
2018-04-19 14:56 ` Michael S. Tsirkin
2018-04-19 13:26 ` Cornelia Huck
3 siblings, 2 replies; 20+ messages in thread
From: Cornelia Huck @ 2018-04-19 13:26 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, Mikulas Patocka, Eric Dumazet, David Miller,
Thomas Huth, Jason Wang, virtualization, netdev
On Thu, 19 Apr 2018 08:30:49 +0300
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> Programming vids (adding or removing them) still passes
> guest-endian values in the DMA buffer. That's wrong
> if guest is big-endian and when virtio 1 is enabled.
>
> Note: this is on top of a previous patch:
> virtio_net: split out ctrl buffer
>
> Fixes: 9465a7a6f ("virtio_net: enable v1.0 support")
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> drivers/net/virtio_net.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
Ouch. Have you seen any bug reports for that?
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 net 2/3] virtio_net: fix adding vids on big-endian
2018-04-19 5:30 ` [PATCH v2 net 2/3] virtio_net: fix adding vids on big-endian Michael S. Tsirkin
` (2 preceding siblings ...)
2018-04-19 13:26 ` Cornelia Huck
@ 2018-04-19 13:26 ` Cornelia Huck
3 siblings, 0 replies; 20+ messages in thread
From: Cornelia Huck @ 2018-04-19 13:26 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Thomas Huth, Eric Dumazet, netdev, linux-kernel, virtualization,
Mikulas Patocka, David Miller
On Thu, 19 Apr 2018 08:30:49 +0300
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> Programming vids (adding or removing them) still passes
> guest-endian values in the DMA buffer. That's wrong
> if guest is big-endian and when virtio 1 is enabled.
>
> Note: this is on top of a previous patch:
> virtio_net: split out ctrl buffer
>
> Fixes: 9465a7a6f ("virtio_net: enable v1.0 support")
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> drivers/net/virtio_net.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
Ouch. Have you seen any bug reports for that?
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 net 2/3] virtio_net: fix adding vids on big-endian
2018-04-19 13:26 ` Cornelia Huck
2018-04-19 14:56 ` Michael S. Tsirkin
@ 2018-04-19 14:56 ` Michael S. Tsirkin
1 sibling, 0 replies; 20+ messages in thread
From: Michael S. Tsirkin @ 2018-04-19 14:56 UTC (permalink / raw)
To: Cornelia Huck
Cc: linux-kernel, Mikulas Patocka, Eric Dumazet, David Miller,
Thomas Huth, Jason Wang, virtualization, netdev
On Thu, Apr 19, 2018 at 03:26:41PM +0200, Cornelia Huck wrote:
> On Thu, 19 Apr 2018 08:30:49 +0300
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>
> > Programming vids (adding or removing them) still passes
> > guest-endian values in the DMA buffer. That's wrong
> > if guest is big-endian and when virtio 1 is enabled.
> >
> > Note: this is on top of a previous patch:
> > virtio_net: split out ctrl buffer
> >
> > Fixes: 9465a7a6f ("virtio_net: enable v1.0 support")
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> > drivers/net/virtio_net.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
>
> Ouch. Have you seen any bug reports for that?
No, but then vlans within VMs aren't used too often
(as opposed to attaching vlans by the HV).
--
MST
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 net 2/3] virtio_net: fix adding vids on big-endian
2018-04-19 13:26 ` Cornelia Huck
@ 2018-04-19 14:56 ` Michael S. Tsirkin
2018-04-19 14:56 ` Michael S. Tsirkin
1 sibling, 0 replies; 20+ messages in thread
From: Michael S. Tsirkin @ 2018-04-19 14:56 UTC (permalink / raw)
To: Cornelia Huck
Cc: Thomas Huth, Eric Dumazet, netdev, linux-kernel, virtualization,
Mikulas Patocka, David Miller
On Thu, Apr 19, 2018 at 03:26:41PM +0200, Cornelia Huck wrote:
> On Thu, 19 Apr 2018 08:30:49 +0300
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>
> > Programming vids (adding or removing them) still passes
> > guest-endian values in the DMA buffer. That's wrong
> > if guest is big-endian and when virtio 1 is enabled.
> >
> > Note: this is on top of a previous patch:
> > virtio_net: split out ctrl buffer
> >
> > Fixes: 9465a7a6f ("virtio_net: enable v1.0 support")
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> > drivers/net/virtio_net.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
>
> Ouch. Have you seen any bug reports for that?
No, but then vlans within VMs aren't used too often
(as opposed to attaching vlans by the HV).
--
MST
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 net 0/3] virtio: ctrl buffer fixes
2018-04-19 5:30 [PATCH v2 net 0/3] virtio: ctrl buffer fixes Michael S. Tsirkin
` (4 preceding siblings ...)
2018-04-19 5:30 ` Michael S. Tsirkin
@ 2018-04-19 20:34 ` David Miller
2018-04-20 0:49 ` Michael S. Tsirkin
5 siblings, 1 reply; 20+ messages in thread
From: David Miller @ 2018-04-19 20:34 UTC (permalink / raw)
To: mst; +Cc: linux-kernel, mpatocka, eric.dumazet, thuth, cohuck
From: "Michael S. Tsirkin" <mst@redhat.com>
Date: Thu, 19 Apr 2018 08:30:47 +0300
> Here are a couple of fixes related to the virtio control buffer.
> Lightly tested on x86 only.
Thanks for taking care of the control buffer DMA'ability issue.
Want any of these queued up for -stable?
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 net 0/3] virtio: ctrl buffer fixes
2018-04-19 20:34 ` [PATCH v2 net 0/3] virtio: ctrl buffer fixes David Miller
@ 2018-04-20 0:49 ` Michael S. Tsirkin
2018-04-20 1:10 ` David Miller
0 siblings, 1 reply; 20+ messages in thread
From: Michael S. Tsirkin @ 2018-04-20 0:49 UTC (permalink / raw)
To: David Miller; +Cc: linux-kernel, mpatocka, eric.dumazet, thuth, cohuck
On Thu, Apr 19, 2018 at 04:34:22PM -0400, David Miller wrote:
> From: "Michael S. Tsirkin" <mst@redhat.com>
> Date: Thu, 19 Apr 2018 08:30:47 +0300
>
> > Here are a couple of fixes related to the virtio control buffer.
> > Lightly tested on x86 only.
>
> Thanks for taking care of the control buffer DMA'ability issue.
>
> Want any of these queued up for -stable?
Good point. Patches 1-2 for sure.
Thanks!
--
MST
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 net 0/3] virtio: ctrl buffer fixes
2018-04-20 0:49 ` Michael S. Tsirkin
@ 2018-04-20 1:10 ` David Miller
0 siblings, 0 replies; 20+ messages in thread
From: David Miller @ 2018-04-20 1:10 UTC (permalink / raw)
To: mst; +Cc: linux-kernel, mpatocka, eric.dumazet, thuth, cohuck
From: "Michael S. Tsirkin" <mst@redhat.com>
Date: Fri, 20 Apr 2018 03:49:19 +0300
> On Thu, Apr 19, 2018 at 04:34:22PM -0400, David Miller wrote:
>> From: "Michael S. Tsirkin" <mst@redhat.com>
>> Date: Thu, 19 Apr 2018 08:30:47 +0300
>>
>> > Here are a couple of fixes related to the virtio control buffer.
>> > Lightly tested on x86 only.
>>
>> Thanks for taking care of the control buffer DMA'ability issue.
>>
>> Want any of these queued up for -stable?
>
> Good point. Patches 1-2 for sure.
Ok, queued up 1 and 2.
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2018-04-20 1:10 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-19 5:30 [PATCH v2 net 0/3] virtio: ctrl buffer fixes Michael S. Tsirkin
2018-04-19 5:30 ` [PATCH v2 net 1/3] virtio_net: split out ctrl buffer Michael S. Tsirkin
2018-04-19 5:30 ` Michael S. Tsirkin
2018-04-19 12:26 ` Jason Wang
2018-04-19 12:26 ` Jason Wang
2018-04-19 5:30 ` [PATCH v2 net 2/3] virtio_net: fix adding vids on big-endian Michael S. Tsirkin
2018-04-19 12:26 ` Jason Wang
2018-04-19 12:26 ` Jason Wang
2018-04-19 13:26 ` Cornelia Huck
2018-04-19 14:56 ` Michael S. Tsirkin
2018-04-19 14:56 ` Michael S. Tsirkin
2018-04-19 13:26 ` Cornelia Huck
2018-04-19 5:30 ` Michael S. Tsirkin
2018-04-19 5:30 ` [PATCH v2 net 3/3] virtio_net: sparse annotation fix Michael S. Tsirkin
2018-04-19 5:30 ` Michael S. Tsirkin
2018-04-19 12:27 ` Jason Wang
2018-04-19 12:27 ` Jason Wang
2018-04-19 20:34 ` [PATCH v2 net 0/3] virtio: ctrl buffer fixes David Miller
2018-04-20 0:49 ` Michael S. Tsirkin
2018-04-20 1:10 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.