diff mbox series

[v3,1/9] vhost: provide helper for host notifier ctrl

Message ID 20181213100910.13087-2-xiao.w.wang@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Maxime Coquelin
Headers
Series support SW assisted VDPA live migration |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/mellanox-Performance-Testing success Performance Testing PASS
ci/intel-Performance-Testing success Performance Testing PASS

Commit Message

Xiao Wang Dec. 13, 2018, 10:09 a.m. UTC 
  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(-)

Comments

Maxime Coquelin Dec. 14, 2018, 1:33 p.m. UTC | #1 
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;
>
Xiao Wang Dec. 14, 2018, 7:05 p.m. UTC | #2 
Hi,

> -----Original Message-----
> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]
> Sent: Friday, December 14, 2018 5:33 AM
> To: Wang, Xiao W <xiao.w.wang@intel.com>;
> alejandro.lucero@netronome.com; Bie, Tiwei <tiwei.bie@intel.com>
> Cc: dev@dpdk.org; Wang, Zhihong <zhihong.wang@intel.com>; Ye, Xiaolong
> <xiaolong.ye@intel.com>
> Subject: Re: [PATCH v3 1/9] vhost: provide helper for host notifier ctrl
> 
> 
> 
> 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?

Compared with the vhost_attach_vdpa_device, I think we should not just disable host notifier, but also destroy the vhost port. Also, this internal API is currently not used.
Yes, we need to mention this point in the commit message. BTW, I prefer to remove this unused internal API, by a separate patch.

BRs,
Xiao

> 
> 
> >   	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;
> >
diff mbox series

Patch

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);
 	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;