Re: [dpdk-dev] [PATCH v3 1/9] vhost: provide helper for host notifier ctrl

[Maxime Coquelin <maxime.coquelin@redhat.com>]

On 12/13/18 11:09 AM, Xiao Wang wrote:
> VDPA driver can decide if it needs to enable/disable the host notifier
> mapping, so exposing a API can allow flexibility. A later patch will
> base on this.
> 
> Signed-off-by: Xiao Wang <xiao.w.wang@intel.com>
> ---
> v2:
> * Reword the vdpa host notifier control API comment.
> ---
>   drivers/net/ifc/ifcvf_vdpa.c           |  3 +++
>   lib/librte_vhost/rte_vdpa.h            | 18 ++++++++++++++++++
>   lib/librte_vhost/rte_vhost_version.map |  1 +
>   lib/librte_vhost/vhost.c               |  3 +--
>   lib/librte_vhost/vhost_user.c          |  7 +------
>   5 files changed, 24 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/ifc/ifcvf_vdpa.c b/drivers/net/ifc/ifcvf_vdpa.c
> index 97a57f182..e844109f3 100644
> --- a/drivers/net/ifc/ifcvf_vdpa.c
> +++ b/drivers/net/ifc/ifcvf_vdpa.c
> @@ -556,6 +556,9 @@ ifcvf_dev_config(int vid)
>   	rte_atomic32_set(&internal->dev_attached, 1);
>   	update_datapath(internal);
>   
> +	if (rte_vhost_host_notifier_ctrl(vid, true) != 0)
> +		DRV_LOG(NOTICE, "vDPA (%d): software relay is used.", did);
> +
>   	return 0;
>   }
>   
> diff --git a/lib/librte_vhost/rte_vdpa.h b/lib/librte_vhost/rte_vdpa.h
> index a418da47c..fff657391 100644
> --- a/lib/librte_vhost/rte_vdpa.h
> +++ b/lib/librte_vhost/rte_vdpa.h
> @@ -11,6 +11,8 @@
>    * Device specific vhost lib
>    */
>   
> +#include <stdbool.h>
> +
>   #include <rte_pci.h>
>   #include "rte_vhost.h"
>   
> @@ -155,4 +157,20 @@ rte_vdpa_get_device(int did);
>    */
>   int __rte_experimental
>   rte_vdpa_get_device_num(void);
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> + * Enable/Disable host notifier mapping for a vdpa port.
> + *
> + * @param vid
> + *  vhost device id
> + * @enable
> + *  true for host notifier map, false for host notifier unmap
> + * @return
> + *  0 on success, -1 on failure
> + */
> +int __rte_experimental
> +rte_vhost_host_notifier_ctrl(int vid, bool enable);
>   #endif /* _RTE_VDPA_H_ */
> diff --git a/lib/librte_vhost/rte_vhost_version.map b/lib/librte_vhost/rte_vhost_version.map
> index ae39b6e21..22302e972 100644
> --- a/lib/librte_vhost/rte_vhost_version.map
> +++ b/lib/librte_vhost/rte_vhost_version.map
> @@ -83,4 +83,5 @@ EXPERIMENTAL {
>   	rte_vhost_crypto_finalize_requests;
>   	rte_vhost_crypto_set_zero_copy;
>   	rte_vhost_va_from_guest_pa;
> +	rte_vhost_host_notifier_ctrl;
>   };
> diff --git a/lib/librte_vhost/vhost.c b/lib/librte_vhost/vhost.c
> index 70ac6bc9c..e7a60e0b4 100644
> --- a/lib/librte_vhost/vhost.c
> +++ b/lib/librte_vhost/vhost.c
> @@ -408,8 +408,7 @@ vhost_detach_vdpa_device(int vid)
>   	if (dev == NULL)
>   		return;
>   
> -	vhost_user_host_notifier_ctrl(vid, false);
> -
> +	vhost_destroy_device_notify(dev);
It seems that is addition is not mentioned in the commit message.
Why is it needed now?


>   	dev->vdpa_dev_id = -1;
>   }
>   
> diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
> index 3ea64eba6..5e0da0589 100644
> --- a/lib/librte_vhost/vhost_user.c
> +++ b/lib/librte_vhost/vhost_user.c
> @@ -2045,11 +2045,6 @@ vhost_user_msg_handler(int vid, int fd)
>   		if (vdpa_dev->ops->dev_conf)
>   			vdpa_dev->ops->dev_conf(dev->vid);
>   		dev->flags |= VIRTIO_DEV_VDPA_CONFIGURED;
> -		if (vhost_user_host_notifier_ctrl(dev->vid, true) != 0) {
> -			RTE_LOG(INFO, VHOST_CONFIG,
> -				"(%d) software relay is used for vDPA, performance may be low.\n",
> -				dev->vid);
> -		}
>   	}
>   
>   	return 0;
> @@ -2144,7 +2139,7 @@ static int vhost_user_slave_set_vring_host_notifier(struct virtio_net *dev,
>   	return process_slave_message_reply(dev, &msg);
>   }
>   
> -int vhost_user_host_notifier_ctrl(int vid, bool enable)
> +int rte_vhost_host_notifier_ctrl(int vid, bool enable)
>   {
>   	struct virtio_net *dev;
>   	struct rte_vdpa_device *vdpa_dev;
>