netdev - Re: [PATCH v2 3/5] VSOCK: support receive mergeable rx buffer in guest

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] [day] [month] [year] [list]

Message-ID: <5C136CC1.8090706@huawei.com>
Date:   Fri, 14 Dec 2018 16:41:37 +0800
From:   jiangyiwen <jiangyiwen@...wei.com>
To:     Stefan Hajnoczi <stefanha@...hat.com>
CC:     "Michael S. Tsirkin" <mst@...hat.com>,
        Jason Wang <jasowang@...hat.com>, <netdev@...r.kernel.org>,
        <kvm@...r.kernel.org>, <virtualization@...ts.linux-foundation.org>
Subject: Re: [PATCH v2 3/5] VSOCK: support receive mergeable rx buffer in
 guest

On 2018/12/14 0:20, Stefan Hajnoczi wrote:
> On Wed, Dec 12, 2018 at 05:31:39PM +0800, jiangyiwen wrote:
>> +static struct virtio_vsock_pkt *receive_mergeable(struct virtqueue *vq,
>> +		struct virtio_vsock *vsock, unsigned int *total_len)
>> +{
>> +	struct virtio_vsock_pkt *pkt;
>> +	u16 num_buf;
>> +	void *buf;
>> +	unsigned int len;
>> +	size_t vsock_hlen = sizeof(struct virtio_vsock_pkt);
>> +
>> +	buf = virtqueue_get_buf(vq, &len);
>> +	if (!buf)
>> +		return NULL;
>> +
>> +	*total_len = len;
>> +	vsock->rx_buf_nr--;
>> +
>> +	if (unlikely(len < vsock_hlen)) {
>> +		put_page(virt_to_head_page(buf));
>> +		return NULL;
>> +	}
>> +
>> +	pkt = buf;
>> +	num_buf = le16_to_cpu(pkt->mrg_rxbuf_hdr.num_buffers);
>> +	if (!num_buf || num_buf > VIRTIO_VSOCK_MAX_VEC_NUM) {
>> +		put_page(virt_to_head_page(buf));
>> +		return NULL;
>> +	}
>> +
>> +	/* Initialize pkt residual structure */
>> +	memset(&pkt->work, 0, vsock_hlen - sizeof(struct virtio_vsock_hdr) -
>> +			sizeof(struct virtio_vsock_mrg_rxbuf_hdr));
> 
> struct virtio_vsock_pkt is an internal driver state structure.  It must
> be able to change without breaking the host<->guest device interface.
> Exposing struct virtio_vsock_pkt across the device interface makes this
> impossible.
> 
> One way to solve this is by placing a header length field at the
> beginning of the mergeable buffer.  That way the device knows at which
> offset the payload should be placed and the driver can continue to use
> the allocation scheme you've chosen (where a single page contains the
> virtio_vsock_pkt and payload).
> 
> Another issue is that virtio requires exclusive read OR write
> descriptors.  You cannot have a descriptor that is both read and write.
> So there's not a huge advantage to combining the driver state struct and
> payload, except maybe the driver can save a memory allocation.
> 

Right, I will separate pkt and payload in different pages, as you mentioned
in another patch.

Thanks,
Yiwen.