diff mbox series

[DPDK] net/virtio: packed ring notification data feature support

Message ID 20191204150312.32697-1-Cheng1.jiang@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Maxime Coquelin
Headers
Series [DPDK] net/virtio: packed ring notification data feature support |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-intel-Performance success Performance Testing PASS
ci/Intel-compilation success Compilation OK
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-compilation fail Compilie Testing issues
ci/travis-robot warning Travis build: failed
ci/iol-testing success Testing PASS

Commit Message

Jiang, Cheng1 Dec. 4, 2019, 3:03 p.m. UTC 
  This patch supports the feature that the driver passes extra data
(besides identifying the virtqueue) in its device notifications.

Signed-off-by: Cheng Jiang <Cheng1.jiang@intel.com>
---
 drivers/net/virtio/virtio_ethdev.h |  3 ++-
 drivers/net/virtio/virtio_pci.c    | 15 ++++++++++++++-
 drivers/net/virtio/virtio_pci.h    |  6 ++++++
 3 files changed, 22 insertions(+), 2 deletions(-)

Comments

Marvin Liu Dec. 9, 2019, 1:59 a.m. UTC | #1 
> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Cheng Jiang
> Sent: Wednesday, December 04, 2019 11:03 PM
> To: dev@dpdk.org
> Cc: maxime.coquelin@redhat.com; Bie, Tiwei <tiwei.bie@intel.com>; Wang,
> Zhihong <zhihong.wang@intel.com>; Jiang, Cheng1 <cheng1.jiang@intel.com>
> Subject: [dpdk-dev] [DPDK] net/virtio: packed ring notification data
> feature support
> 
> This patch supports the feature that the driver passes extra data
> (besides identifying the virtqueue) in its device notifications.
> 
> Signed-off-by: Cheng Jiang <Cheng1.jiang@intel.com>
> ---
>  drivers/net/virtio/virtio_ethdev.h |  3 ++-
>  drivers/net/virtio/virtio_pci.c    | 15 ++++++++++++++-
>  drivers/net/virtio/virtio_pci.h    |  6 ++++++
>  3 files changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/virtio/virtio_ethdev.h
> b/drivers/net/virtio/virtio_ethdev.h
> index a10111758..cd8947656 100644
> --- a/drivers/net/virtio/virtio_ethdev.h
> +++ b/drivers/net/virtio/virtio_ethdev.h
> @@ -36,7 +36,8 @@
>  	 1ULL << VIRTIO_F_IN_ORDER        |	\
>  	 1ULL << VIRTIO_F_RING_PACKED	  |	\
>  	 1ULL << VIRTIO_F_IOMMU_PLATFORM  |	\
> -	 1ULL << VIRTIO_F_ORDER_PLATFORM)
> +	 1ULL << VIRTIO_F_ORDER_PLATFORM  |	\
> +	 1ULL << VIRTIO_F_NOTIFICATION_DATA)
> 
>  #define VIRTIO_PMD_SUPPORTED_GUEST_FEATURES	\
>  	(VIRTIO_PMD_DEFAULT_GUEST_FEATURES |	\
> diff --git a/drivers/net/virtio/virtio_pci.c
> b/drivers/net/virtio/virtio_pci.c
> index 4468e89cb..2462a7dab 100644
> --- a/drivers/net/virtio/virtio_pci.c
> +++ b/drivers/net/virtio/virtio_pci.c
> @@ -418,7 +418,20 @@ modern_del_queue(struct virtio_hw *hw, struct
> virtqueue *vq)
>  static void
>  modern_notify_queue(struct virtio_hw *hw __rte_unused, struct virtqueue
> *vq)
>  {

Hi Cheng,
hw pointer will be used in notify function, please remove rte_unused attribute. 

Thanks,
Marvin

> -	rte_write16(vq->vq_queue_index, vq->notify_addr);
> +	uint32_t notify_data;
> +
> +	if (!vtpci_with_feature(hw, VIRTIO_F_NOTIFICATION_DATA)) {
> +		rte_write16(vq->vq_queue_index, vq->notify_addr);
> +		return;
> +	}
> +
> +	if (vtpci_with_feature(hw, VIRTIO_F_RING_PACKED))
> +		notify_data = ((((uint32_t)vq->vq_packed.used_wrap_counter <<
> 15) |
> +				vq->vq_avail_idx) << 16) | vq->vq_queue_index;
> +	else
> +		notify_data = ((uint32_t)vq->vq_avail_idx << 16) |
> +				vq->vq_queue_index;
> +	rte_write32(notify_data, vq->notify_addr);
>  }
> 
>  const struct virtio_pci_ops modern_ops = {
> diff --git a/drivers/net/virtio/virtio_pci.h
> b/drivers/net/virtio/virtio_pci.h
> index a38cb45ad..7433d2f08 100644
> --- a/drivers/net/virtio/virtio_pci.h
> +++ b/drivers/net/virtio/virtio_pci.h
> @@ -135,6 +135,12 @@ struct virtnet_ctl;
>   */
>  #define VIRTIO_F_ORDER_PLATFORM 36
> 
> +/*
> + * This feature indicates that the driver passes extra data (besides
> + * identifying the virtqueue) in its device notifications.
> + */
> +#define VIRTIO_F_NOTIFICATION_DATA 38
> +
>  /* The Guest publishes the used index for which it expects an interrupt
>   * at the end of the avail ring. Host should ignore the avail->flags
> field. */
>  /* The Host publishes the avail index for which it expects a kick
> --
> 2.17.1
Jiang, Cheng1 Dec. 12, 2019, 9:08 a.m. UTC | #2 
> -----Original Message-----
> From: Liu, Yong
> Sent: Monday, December 9, 2019 10:00 AM
> To: Jiang, Cheng1 <cheng1.jiang@intel.com>; dev@dpdk.org
> Cc: maxime.coquelin@redhat.com; Bie, Tiwei <tiwei.bie@intel.com>; Wang,
> Zhihong <zhihong.wang@intel.com>; Jiang, Cheng1
> <cheng1.jiang@intel.com>
> Subject: RE: [dpdk-dev] [DPDK] net/virtio: packed ring notification data
> feature support
> 
> 
> 
> > -----Original Message-----
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Cheng Jiang
> > Sent: Wednesday, December 04, 2019 11:03 PM
> > To: dev@dpdk.org
> > Cc: maxime.coquelin@redhat.com; Bie, Tiwei <tiwei.bie@intel.com>;
> > Wang, Zhihong <zhihong.wang@intel.com>; Jiang, Cheng1
> > <cheng1.jiang@intel.com>
> > Subject: [dpdk-dev] [DPDK] net/virtio: packed ring notification data
> > feature support
> >
> > This patch supports the feature that the driver passes extra data
> > (besides identifying the virtqueue) in its device notifications.
> >
> > Signed-off-by: Cheng Jiang <Cheng1.jiang@intel.com>
> > ---
> >  drivers/net/virtio/virtio_ethdev.h |  3 ++-
> >  drivers/net/virtio/virtio_pci.c    | 15 ++++++++++++++-
> >  drivers/net/virtio/virtio_pci.h    |  6 ++++++
> >  3 files changed, 22 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/virtio/virtio_ethdev.h
> > b/drivers/net/virtio/virtio_ethdev.h
> > index a10111758..cd8947656 100644
> > --- a/drivers/net/virtio/virtio_ethdev.h
> > +++ b/drivers/net/virtio/virtio_ethdev.h
> > @@ -36,7 +36,8 @@
> >  	 1ULL << VIRTIO_F_IN_ORDER        |	\
> >  	 1ULL << VIRTIO_F_RING_PACKED	  |	\
> >  	 1ULL << VIRTIO_F_IOMMU_PLATFORM  |	\
> > -	 1ULL << VIRTIO_F_ORDER_PLATFORM)
> > +	 1ULL << VIRTIO_F_ORDER_PLATFORM  |	\
> > +	 1ULL << VIRTIO_F_NOTIFICATION_DATA)
> >
> >  #define VIRTIO_PMD_SUPPORTED_GUEST_FEATURES	\
> >  	(VIRTIO_PMD_DEFAULT_GUEST_FEATURES |	\
> > diff --git a/drivers/net/virtio/virtio_pci.c
> > b/drivers/net/virtio/virtio_pci.c index 4468e89cb..2462a7dab 100644
> > --- a/drivers/net/virtio/virtio_pci.c
> > +++ b/drivers/net/virtio/virtio_pci.c
> > @@ -418,7 +418,20 @@ modern_del_queue(struct virtio_hw *hw, struct
> > virtqueue *vq)  static void  modern_notify_queue(struct virtio_hw *hw
> > __rte_unused, struct virtqueue
> > *vq)
> >  {
> 
> Hi Cheng,
> hw pointer will be used in notify function, please remove rte_unused
> attribute.
> 
> Thanks,
> Marvin
> 

Sure, I'll remove it in the next version.

Thanks very much!
Cheng

> > -	rte_write16(vq->vq_queue_index, vq->notify_addr);
> > +	uint32_t notify_data;
> > +
> > +	if (!vtpci_with_feature(hw, VIRTIO_F_NOTIFICATION_DATA)) {
> > +		rte_write16(vq->vq_queue_index, vq->notify_addr);
> > +		return;
> > +	}
> > +
> > +	if (vtpci_with_feature(hw, VIRTIO_F_RING_PACKED))
> > +		notify_data = ((((uint32_t)vq-
> >vq_packed.used_wrap_counter <<
> > 15) |
> > +				vq->vq_avail_idx) << 16) | vq-
> >vq_queue_index;
> > +	else
> > +		notify_data = ((uint32_t)vq->vq_avail_idx << 16) |
> > +				vq->vq_queue_index;
> > +	rte_write32(notify_data, vq->notify_addr);
> >  }
> >
> >  const struct virtio_pci_ops modern_ops = { diff --git
> > a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
> > index a38cb45ad..7433d2f08 100644
> > --- a/drivers/net/virtio/virtio_pci.h
> > +++ b/drivers/net/virtio/virtio_pci.h
> > @@ -135,6 +135,12 @@ struct virtnet_ctl;
> >   */
> >  #define VIRTIO_F_ORDER_PLATFORM 36
> >
> > +/*
> > + * This feature indicates that the driver passes extra data (besides
> > + * identifying the virtqueue) in its device notifications.
> > + */
> > +#define VIRTIO_F_NOTIFICATION_DATA 38
> > +
> >  /* The Guest publishes the used index for which it expects an interrupt
> >   * at the end of the avail ring. Host should ignore the avail->flags
> > field. */
> >  /* The Host publishes the avail index for which it expects a kick
> > --
> > 2.17.1
Marvin Liu Jan. 9, 2020, 7:29 a.m. UTC | #3 
> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Cheng Jiang
> Sent: Wednesday, December 04, 2019 11:03 PM
> To: dev@dpdk.org
> Cc: maxime.coquelin@redhat.com; Bie, Tiwei <tiwei.bie@intel.com>; Wang,
> Zhihong <zhihong.wang@intel.com>; Jiang, Cheng1 <cheng1.jiang@intel.com>
> Subject: [dpdk-dev] [DPDK] net/virtio: packed ring notification data
> feature support
> 
> This patch supports the feature that the driver passes extra data
> (besides identifying the virtqueue) in its device notifications.
> 
> Signed-off-by: Cheng Jiang <Cheng1.jiang@intel.com>
> ---
>  drivers/net/virtio/virtio_ethdev.h |  3 ++-
>  drivers/net/virtio/virtio_pci.c    | 15 ++++++++++++++-
>  drivers/net/virtio/virtio_pci.h    |  6 ++++++
>  3 files changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/virtio/virtio_ethdev.h
> b/drivers/net/virtio/virtio_ethdev.h
> index a10111758..cd8947656 100644
> --- a/drivers/net/virtio/virtio_ethdev.h
> +++ b/drivers/net/virtio/virtio_ethdev.h
> +	if (vtpci_with_feature(hw, VIRTIO_F_RING_PACKED))
> +		notify_data = ((((uint32_t)vq->vq_packed.used_wrap_counter <<
> 15) |
> +				vq->vq_avail_idx) << 16) | vq->vq_queue_index;
> +	else
> +		notify_data = ((uint32_t)vq->vq_avail_idx << 16) |
> +				vq->vq_queue_index;

Hi Cheng,
According to virtio1.1 spec, wrap counter should refer to next available descriptor.
So used_wrap_counter should be replaced with avail_wrap_counter. Sorry for late noticing. 

Thanks,
Marvin

> +	rte_write32(notify_data, vq->notify_addr);
>  }
>
Jiang, Cheng1 Jan. 13, 2020, 2:57 a.m. UTC | #4 
Hi Marvin,

> -----Original Message-----
> From: Liu, Yong
> Sent: Thursday, January 9, 2020 3:29 PM
> To: Jiang, Cheng1 <cheng1.jiang@intel.com>; dev@dpdk.org
> Cc: maxime.coquelin@redhat.com; Bie, Tiwei <tiwei.bie@intel.com>; Wang,
> Zhihong <zhihong.wang@intel.com>; Jiang, Cheng1
> <cheng1.jiang@intel.com>
> Subject: RE: [dpdk-dev] [DPDK] net/virtio: packed ring notification data
> feature support
> 
> 
> 
> > -----Original Message-----
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Cheng Jiang
> > Sent: Wednesday, December 04, 2019 11:03 PM
> > To: dev@dpdk.org
> > Cc: maxime.coquelin@redhat.com; Bie, Tiwei <tiwei.bie@intel.com>;
> > Wang, Zhihong <zhihong.wang@intel.com>; Jiang, Cheng1
> > <cheng1.jiang@intel.com>
> > Subject: [dpdk-dev] [DPDK] net/virtio: packed ring notification data
> > feature support
> >
> > This patch supports the feature that the driver passes extra data
> > (besides identifying the virtqueue) in its device notifications.
> >
> > Signed-off-by: Cheng Jiang <Cheng1.jiang@intel.com>
> > ---
> >  drivers/net/virtio/virtio_ethdev.h |  3 ++-
> >  drivers/net/virtio/virtio_pci.c    | 15 ++++++++++++++-
> >  drivers/net/virtio/virtio_pci.h    |  6 ++++++
> >  3 files changed, 22 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/virtio/virtio_ethdev.h
> > b/drivers/net/virtio/virtio_ethdev.h
> > index a10111758..cd8947656 100644
> > --- a/drivers/net/virtio/virtio_ethdev.h
> > +++ b/drivers/net/virtio/virtio_ethdev.h
> > +	if (vtpci_with_feature(hw, VIRTIO_F_RING_PACKED))
> > +		notify_data = ((((uint32_t)vq-
> >vq_packed.used_wrap_counter <<
> > 15) |
> > +				vq->vq_avail_idx) << 16) | vq-
> >vq_queue_index;
> > +	else
> > +		notify_data = ((uint32_t)vq->vq_avail_idx << 16) |
> > +				vq->vq_queue_index;
> 
> Hi Cheng,
> According to virtio1.1 spec, wrap counter should refer to next available
> descriptor.
> So used_wrap_counter should be replaced with avail_wrap_counter. Sorry
> for late noticing.
> 
> Thanks,
> Marvin
> 

Sure, I'm going to modify it in the next version.
Thanks very much.

Cheng

> > +	rte_write32(notify_data, vq->notify_addr);
> >  }
> >
diff mbox series

Patch

diff --git a/drivers/net/virtio/virtio_ethdev.h b/drivers/net/virtio/virtio_ethdev.h
index a10111758..cd8947656 100644
--- a/drivers/net/virtio/virtio_ethdev.h
+++ b/drivers/net/virtio/virtio_ethdev.h
@@ -36,7 +36,8 @@ 
 	 1ULL << VIRTIO_F_IN_ORDER        |	\
 	 1ULL << VIRTIO_F_RING_PACKED	  |	\
 	 1ULL << VIRTIO_F_IOMMU_PLATFORM  |	\
-	 1ULL << VIRTIO_F_ORDER_PLATFORM)
+	 1ULL << VIRTIO_F_ORDER_PLATFORM  |	\
+	 1ULL << VIRTIO_F_NOTIFICATION_DATA)
 
 #define VIRTIO_PMD_SUPPORTED_GUEST_FEATURES	\
 	(VIRTIO_PMD_DEFAULT_GUEST_FEATURES |	\
diff --git a/drivers/net/virtio/virtio_pci.c b/drivers/net/virtio/virtio_pci.c
index 4468e89cb..2462a7dab 100644
--- a/drivers/net/virtio/virtio_pci.c
+++ b/drivers/net/virtio/virtio_pci.c
@@ -418,7 +418,20 @@  modern_del_queue(struct virtio_hw *hw, struct virtqueue *vq)
 static void
 modern_notify_queue(struct virtio_hw *hw __rte_unused, struct virtqueue *vq)
 {
-	rte_write16(vq->vq_queue_index, vq->notify_addr);
+	uint32_t notify_data;
+
+	if (!vtpci_with_feature(hw, VIRTIO_F_NOTIFICATION_DATA)) {
+		rte_write16(vq->vq_queue_index, vq->notify_addr);
+		return;
+	}
+
+	if (vtpci_with_feature(hw, VIRTIO_F_RING_PACKED))
+		notify_data = ((((uint32_t)vq->vq_packed.used_wrap_counter << 15) |
+				vq->vq_avail_idx) << 16) | vq->vq_queue_index;
+	else
+		notify_data = ((uint32_t)vq->vq_avail_idx << 16) |
+				vq->vq_queue_index;
+	rte_write32(notify_data, vq->notify_addr);
 }
 
 const struct virtio_pci_ops modern_ops = {
diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
index a38cb45ad..7433d2f08 100644
--- a/drivers/net/virtio/virtio_pci.h
+++ b/drivers/net/virtio/virtio_pci.h
@@ -135,6 +135,12 @@  struct virtnet_ctl;
  */
 #define VIRTIO_F_ORDER_PLATFORM 36
 
+/*
+ * This feature indicates that the driver passes extra data (besides
+ * identifying the virtqueue) in its device notifications.
+ */
+#define VIRTIO_F_NOTIFICATION_DATA 38
+
 /* The Guest publishes the used index for which it expects an interrupt
  * at the end of the avail ring. Host should ignore the avail->flags field. */
 /* The Host publishes the avail index for which it expects a kick