Re: [PATCH RFC v2 08/12] lib/librte_vhost: vhost-user support

["Xie, Huawei" <huawei.xie-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>]

> -----Original Message-----
> From: Linhaifeng [mailto:haifeng.lin-hv44wF8Li93QT0dZR+AlfA@public.gmane.org]
> Sent: Thursday, December 11, 2014 2:04 PM
> To: Xie, Huawei; dev-VfR2kkLFssw@public.gmane.org
> Cc: haifeng.lin-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org
> Subject: Re: [dpdk-dev] [PATCH RFC v2 08/12] lib/librte_vhost: vhost-user
> support
> 
> 
> 
> On 2014/12/11 5:37, Huawei Xie wrote:
> > vhost-user support
> >
> >
> > Signed-off-by: Huawei Xie <huawei.xie-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > ---
> >  lib/librte_vhost/Makefile                     |   5 +-
> >  lib/librte_vhost/vhost-net.h                  |   4 +
> >  lib/librte_vhost/vhost_cuse/virtio-net-cdev.c |   9 +
> >  lib/librte_vhost/vhost_user/vhost-net-user.c  | 422
> ++++++++++++++++++++++++++
> > +
> > +static struct vhost_server *g_vhost_server;
> 
> Only support one vhost-user port ?

The newer patch supports multiple vhost-user port.
Will send next week.
> 
> > +
> > +static const char *vhost_message_str[VHOST_USER_MAX] = {

> 
> why not use vserver_new_vq_conn(int fd, void* dat) ?
> 
>
Since currently we only pass memory address and 32bit number, will change to void *.
 > > +{
> > +	struct vhost_server *vserver = (void *)(uintptr_t)dat;
> regions[VHOST_MEMORY_MAX_NREGIONS];
> > +	uint64_t mapped_address, base_address = 0;
> > +
> > +	for (idx = 0; idx < memory.nregions; idx++) {
> > +		if (memory.regions[idx].guest_phys_addr == 0)
> > +			base_address = memory.regions[idx].userspace_addr;
> > +	}
> > +	if (base_address == 0) {
> 
> when will base_address is 0? how to test to run this code?

base_address with 0 is the requirement for vhost-cuse implementation.
Normally it is the region [0, 0xA0000).
In this version, we would keep this and not fix this. In future, will remove this.

> 
> > +		RTE_LOG(ERR, VHOST_CONFIG,
> > +			"couldn't find the mem region whose GPA is 0.\n");
> > +		return -1;
> > +	}
> > +
> > +	for (idx = 0; idx < memory.nregions; idx++) {
> > +		regions[idx].guest_phys_address =
> > +			memory.regions[idx].guest_phys_addr;
> > +		regions[idx].guest_phys_address_end =
> > +			memory.regions[idx].guest_phys_addr +
> > +			memory.regions[idx].memory_size;
> > +		regions[idx].memory_size = memory.regions[idx].memory_size;
> > +		regions[idx].userspace_address =
> > +			memory.regions[idx].userspace_addr;
> > +
> > +		/* This is ugly */
> > +		mapped_address = (uint64_t)(uintptr_t)mmap(NULL,
> > +			regions[idx].memory_size +
> > +				memory.regions[idx].mmap_offset,
> > +			PROT_READ | PROT_WRITE, MAP_SHARED,
> > +			pmsg->fds[idx],
> > +			0);
> 
> Can you mmap the region if gpa is 0? When i run VM with two numa node (qemu
> will create two hugepage file) found that always failed to mmap with the region
> which gpa is 0.
> 
> BTW can we ensure the memory regions cover with all the memory of hugepage
> for VM?
> 
We had discussed this. Seemed a qemu bug.
Do you have update on this?

> > +		RTE_LOG(INFO, VHOST_CONFIG,
> > +			"mapped region %d to %p\n",
> > +struct virtio_net_device_ops const *notify_ops;
> > +struct virtio_net * get_device(struct vhost_device_ctx ctx);
> > +
> > +#endif
> >
> 
> --
> Regards,
> Haifeng

Thanks Haifeng for your comments.
Next time, could you help cut the code not commented?
They are too long. It is really hard for me to find your comment.