netdev - Re: [PATCH 1/3] vhost-net: check the support of mergeable buffer outside the receive loop

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20110117084644.GB23479@redhat.com>
Date:	Mon, 17 Jan 2011 10:46:44 +0200
From:	"Michael S. Tsirkin" <mst@...hat.com>
To:	Jason Wang <jasowang@...hat.com>
Cc:	virtualization@...ts.osdl.org, netdev@...r.kernel.org,
	kvm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/3] vhost-net: check the support of mergeable buffer
 outside the receive loop

On Mon, Jan 17, 2011 at 04:10:59PM +0800, Jason Wang wrote:
> No need to check the support of mergeable buffer inside the recevie
> loop as the whole handle_rx()_xx is in the read critical region.  So
> this patch move it ahead of the receiving loop.
> 
> Signed-off-by: Jason Wang <jasowang@...hat.com>

Well feature check is mostly just features & bit
so why would it be slower? Because of the rcu
adding memory barriers? Do you observe a
measureable speedup with this patch?

Apropos, I noticed that the check in vhost_has_feature
is wrong: it uses the same kind of RCU as the
private pointer. So we'll have to fix that properly
by adding more lockdep classes, but for now
we'll need to make
the check 1 || lockdep_is_held(&dev->mutex);
and add a TODO.

> ---
>  drivers/vhost/net.c |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 14fc189..95e49de 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -411,7 +411,7 @@ static void handle_rx_mergeable(struct vhost_net *net)
>  	};
>  
>  	size_t total_len = 0;
> -	int err, headcount;
> +	int err, headcount, mergeable;
>  	size_t vhost_hlen, sock_hlen;
>  	size_t vhost_len, sock_len;
>  	struct socket *sock = rcu_dereference(vq->private_data);
> @@ -425,6 +425,7 @@ static void handle_rx_mergeable(struct vhost_net *net)
>  
>  	vq_log = unlikely(vhost_has_feature(&net->dev, VHOST_F_LOG_ALL)) ?
>  		vq->log : NULL;
> +	mergeable = vhost_has_feature(&net->dev, VIRTIO_NET_F_MRG_RXBUF);
>  
>  	while ((sock_len = peek_head_len(sock->sk))) {
>  		sock_len += sock_hlen;
> @@ -474,7 +475,7 @@ static void handle_rx_mergeable(struct vhost_net *net)
>  			break;
>  		}
>  		/* TODO: Should check and handle checksum. */
> -		if (vhost_has_feature(&net->dev, VIRTIO_NET_F_MRG_RXBUF) &&
> +		if (likely(mergeable) &&
>  		    memcpy_toiovecend(vq->hdr, (unsigned char *)&headcount,
>  				      offsetof(typeof(hdr), num_buffers),
>  				      sizeof hdr.num_buffers)) {
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html