[dpdk-dev] [PATCH 1/7] vhost: make capabilities configurable

[Wang, Zhihong]

Hi Maxime,

> -----Original Message-----
> From: Maxime Coquelin [mailto:maxime.coquelin at redhat.com]
> Sent: Tuesday, February 6, 2018 6:19 PM
> To: Wang, Zhihong <zhihong.wang at intel.com>; dev at dpdk.org
> Cc: Tan, Jianfeng <jianfeng.tan at intel.com>; Bie, Tiwei
> <tiwei.bie at intel.com>; yliu at fridaylinux.org; Liang, Cunming
> <cunming.liang at intel.com>; Wang, Xiao W <xiao.w.wang at intel.com>; Daly,
> Dan <dan.daly at intel.com>
> Subject: Re: [PATCH 1/7] vhost: make capabilities configurable
> 
> Hi Zhihong,
> 
...
> > +int rte_vhost_driver_set_queue_num(const char *path, uint16_t
> queue_num)
> > +{
> > +	struct vhost_user_socket *vsocket;
> > +
> > +	pthread_mutex_lock(&vhost_user.mutex);
> > +	vsocket = find_vhost_user_socket(path);
> > +	if (vsocket)
> > +		vsocket->queue_num = queue_num;
> 
> Shouldn't be MIN(queue_num, VHOST_MAX_QUEUE_PAIRS) to be sure you
> can
> switch from HW offload to SW processing?

Yes, the check is necessary.

> 
> > +	pthread_mutex_unlock(&vhost_user.mutex);
> > +
> > +	return vsocket ? 0 : -1;
> > +}
...
> > -static void
> > +static int
> >   vhost_user_set_protocol_features(struct virtio_net *dev,
> >   				 uint64_t protocol_features)
> >   {
> > -	if (protocol_features & ~VHOST_USER_PROTOCOL_FEATURES)
> 
> I think the above check is still necessary, or it should be checked
> in rte_vhost_driver_set_protocol_features().

Thanks. Yes I think all set capabilities should be contained in the
capabilities of the vhost-user lib.

Will update rte_vhost_driver_set_*().

-Zhihong

> 
> Indeed, the application shouldn't set a protocol feature bit that isn't
> supported by the libvhost-user library.
> 
> > -		return;
> > +	uint64_t vhost_protocol_features = 0;
> > +
> > +	rte_vhost_driver_get_protocol_features(dev->ifname,
> > +			&vhost_protocol_features);
> > +	if (protocol_features & ~vhost_protocol_features) {
> > +		RTE_LOG(ERR, VHOST_CONFIG,
> > +			"(%d) received invalid negotiated
> protocol_features.\n",
> > +			dev->vid);
> > +		return -1;
> > +	}
> >
> >   	dev->protocol_features = protocol_features;
> > +
> > +	return 0;
> >   }
> >
> >   static int
> > @@ -1391,7 +1416,8 @@ vhost_user_msg_handler(int vid, int fd)
> >   		break;
> >
> >   	case VHOST_USER_GET_PROTOCOL_FEATURES:
> > -		vhost_user_get_protocol_features(dev, &msg);
> > +		msg.payload.u64 = vhost_user_get_protocol_features(dev);
> > +		msg.size = sizeof(msg.payload.u64);
> >   		send_vhost_reply(fd, &msg);
> >   		break;
> >   	case VHOST_USER_SET_PROTOCOL_FEATURES:
> > @@ -1451,7 +1477,7 @@ vhost_user_msg_handler(int vid, int fd)
> >   		break;
> >
> >   	case VHOST_USER_GET_QUEUE_NUM:
> > -		msg.payload.u64 = VHOST_MAX_QUEUE_PAIRS;
> > +		msg.payload.u64 = vhost_user_get_queue_num(dev);
> >   		msg.size = sizeof(msg.payload.u64);
> >   		send_vhost_reply(fd, &msg);
> >   		break;
> >
> 
> Maxime