LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Jason Wang <jasowang@redhat.com>
To: "Michael S. Tsirkin" <mst@redhat.com>, linux-kernel@vger.kernel.org
Cc: Mikulas Patocka <mpatocka@redhat.com>,
	Eric Dumazet <eric.dumazet@gmail.com>,
	David Miller <davem@davemloft.net>,
	Thomas Huth <thuth@redhat.com>, Cornelia Huck <cohuck@redhat.com>,
	virtualization@lists.linux-foundation.org,
	netdev@vger.kernel.org
Subject: Re: [PATCH v2 net 1/3] virtio_net: split out ctrl buffer
Date: Thu, 19 Apr 2018 20:26:33 +0800	[thread overview]
Message-ID: <43c20511-6c4a-5b8d-37f0-49e220bf5596@redhat.com> (raw)
In-Reply-To: <1524115776-334953-2-git-send-email-mst@redhat.com>



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>

  reply	other threads:[~2018-04-19 12:26 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 12:26   ` Jason Wang [this message]
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 13:26   ` Cornelia Huck
2018-04-19 14:56     ` Michael S. Tsirkin
2018-04-19  5:30 ` [PATCH v2 net 3/3] virtio_net: sparse annotation fix Michael S. Tsirkin
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=43c20511-6c4a-5b8d-37f0-49e220bf5596@redhat.com \
    --to=jasowang@redhat.com \
    --cc=cohuck@redhat.com \
    --cc=davem@davemloft.net \
    --cc=eric.dumazet@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mpatocka@redhat.com \
    --cc=mst@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=thuth@redhat.com \
    --cc=virtualization@lists.linux-foundation.org \
    --subject='Re: [PATCH v2 net 1/3] virtio_net: split out ctrl buffer' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).