diff mbox

[dpdk-dev,4/6] net/e1000: implement descriptor status API (em)

Message ID 1488388752-1819-5-git-send-email-olivier.matz@6wind.com (mailing list archive)
State Superseded, archived
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Olivier Matz March 1, 2017, 5:19 p.m. UTC 
  Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 drivers/net/e1000/e1000_ethdev.h |  5 ++++
 drivers/net/e1000/em_ethdev.c    |  2 ++
 drivers/net/e1000/em_rxtx.c      | 49 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 56 insertions(+)

Comments

Wenzhuo Lu March 2, 2017, 1:22 a.m. UTC | #1 
Hi Oliver,

> -----Original Message-----
> From: Olivier Matz [mailto:olivier.matz@6wind.com]
> Sent: Thursday, March 2, 2017 1:19 AM
> To: dev@dpdk.org; thomas.monjalon@6wind.com; Ananyev, Konstantin; Lu,
> Wenzhuo; Zhang, Helin; Wu, Jingjing; adrien.mazarguil@6wind.com;
> nelio.laranjeiro@6wind.com
> Cc: Yigit, Ferruh; Richardson, Bruce
> Subject: [PATCH 4/6] net/e1000: implement descriptor status API (em)
> 
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> +int
> +eth_em_tx_descriptor_status(struct rte_eth_dev *dev, uint16_t tx_queue_id,
> +	uint16_t offset)
> +{
> +	volatile uint8_t *status;
> +	struct em_tx_queue *txq;
> +	uint32_t desc;
> +
> +	txq = dev->data->tx_queues[tx_queue_id];
> +	if (unlikely(offset >= txq->nb_tx_desc))
> +		return -EINVAL;
> +
> +	desc = txq->tx_tail + offset;
> +	/* go to next desc that has the RS bit */
> +	desc = ((desc + txq->tx_rs_thresh - 1) / txq->tx_rs_thresh) *
> +		txq->tx_rs_thresh;
The descriptor may be changed here. So the return value may not be for the offset one. Why?

> +	status = &txq->tx_ring[desc].upper.fields.status;
> +	if (*status & E1000_TXD_STAT_DD)
> +		return RTE_ETH_TX_DESC_DONE;
> +
> +	return RTE_ETH_TX_DESC_FULL;
> +}
> +
>  void
>  em_dev_clear_queues(struct rte_eth_dev *dev)  {
> --
> 2.8.1
Olivier Matz March 2, 2017, 2:46 p.m. UTC | #2 
Hi Wenzhuo,

On Thu, 2 Mar 2017 01:22:25 +0000, "Lu, Wenzhuo" <wenzhuo.lu@intel.com> wrote:
> Hi Oliver,
> 
> > -----Original Message-----
> > From: Olivier Matz [mailto:olivier.matz@6wind.com]
> > Sent: Thursday, March 2, 2017 1:19 AM
> > To: dev@dpdk.org; thomas.monjalon@6wind.com; Ananyev, Konstantin; Lu,
> > Wenzhuo; Zhang, Helin; Wu, Jingjing; adrien.mazarguil@6wind.com;
> > nelio.laranjeiro@6wind.com
> > Cc: Yigit, Ferruh; Richardson, Bruce
> > Subject: [PATCH 4/6] net/e1000: implement descriptor status API (em)
> > 
> > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> > +int
> > +eth_em_tx_descriptor_status(struct rte_eth_dev *dev, uint16_t tx_queue_id,
> > +	uint16_t offset)
> > +{
> > +	volatile uint8_t *status;
> > +	struct em_tx_queue *txq;
> > +	uint32_t desc;
> > +
> > +	txq = dev->data->tx_queues[tx_queue_id];
> > +	if (unlikely(offset >= txq->nb_tx_desc))
> > +		return -EINVAL;
> > +
> > +	desc = txq->tx_tail + offset;
> > +	/* go to next desc that has the RS bit */
> > +	desc = ((desc + txq->tx_rs_thresh - 1) / txq->tx_rs_thresh) *
> > +		txq->tx_rs_thresh;  
> The descriptor may be changed here. So the return value may not be for the offset one. Why?

Yes, desc index can change. From what I understand, the
"descriptor done" (DD) bit is only set on descriptors which are
flagged with the "report status" (RS) bit.

Here is an example from:
http://www.dpdk.org/ml/archives/dev/2016-November/050679.html

|----------------------------------------------------------------|
|               D       R       R       R                        |
|        ............xxxxxxxxxxxxxxxxxxxxxxxxx                   |
|        <descs sent><- descs not sent yet  ->                   |
|        ............xxxxxxxxxxxxxxxxxxxxxxxxx                   |
|----------------------------------------------------------------|
        ^last_desc_cleaned=8                    ^next_rs=47
                ^next_dd=15                   ^tail=45
                     ^hw_head=20

                     <----  nb_used  --------->

The hardware is currently processing the descriptor 20
'R' means the descriptor has the RS bit
'D' means the descriptor has the DD + RS bits
'x' are packets in txq (not sent)
'.' are packet already sent but not freed by sw


Regards,
Olivier
Wenzhuo Lu March 3, 2017, 1:15 a.m. UTC | #3 
Hi Olivier,

> -----Original Message-----
> From: Olivier Matz [mailto:olivier.matz@6wind.com]
> Sent: Thursday, March 2, 2017 10:47 PM
> To: Lu, Wenzhuo
> Cc: dev@dpdk.org; thomas.monjalon@6wind.com; Ananyev, Konstantin; Zhang,
> Helin; Wu, Jingjing; adrien.mazarguil@6wind.com; nelio.laranjeiro@6wind.com;
> Yigit, Ferruh; Richardson, Bruce
> Subject: Re: [PATCH 4/6] net/e1000: implement descriptor status API (em)
> 
> Hi Wenzhuo,
> 
> On Thu, 2 Mar 2017 01:22:25 +0000, "Lu, Wenzhuo" <wenzhuo.lu@intel.com>
> wrote:
> > Hi Oliver,
> >
> > > -----Original Message-----
> > > From: Olivier Matz [mailto:olivier.matz@6wind.com]
> > > Sent: Thursday, March 2, 2017 1:19 AM
> > > To: dev@dpdk.org; thomas.monjalon@6wind.com; Ananyev, Konstantin;
> > > Lu, Wenzhuo; Zhang, Helin; Wu, Jingjing; adrien.mazarguil@6wind.com;
> > > nelio.laranjeiro@6wind.com
> > > Cc: Yigit, Ferruh; Richardson, Bruce
> > > Subject: [PATCH 4/6] net/e1000: implement descriptor status API (em)
> > >
> > > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> > > +int
> > > +eth_em_tx_descriptor_status(struct rte_eth_dev *dev, uint16_t
> tx_queue_id,
> > > +	uint16_t offset)
> > > +{
> > > +	volatile uint8_t *status;
> > > +	struct em_tx_queue *txq;
> > > +	uint32_t desc;
> > > +
> > > +	txq = dev->data->tx_queues[tx_queue_id];
> > > +	if (unlikely(offset >= txq->nb_tx_desc))
> > > +		return -EINVAL;
> > > +
> > > +	desc = txq->tx_tail + offset;
> > > +	/* go to next desc that has the RS bit */
> > > +	desc = ((desc + txq->tx_rs_thresh - 1) / txq->tx_rs_thresh) *
> > > +		txq->tx_rs_thresh;
> > The descriptor may be changed here. So the return value may not be for the
> offset one. Why?
> 
> Yes, desc index can change. From what I understand, the "descriptor done" (DD)
> bit is only set on descriptors which are flagged with the "report status" (RS) bit.
Yes, you're right. Sorry for the noise :)

> 
> Here is an example from:
> http://www.dpdk.org/ml/archives/dev/2016-November/050679.html
> 
> |----------------------------------------------------------------|
> |               D       R       R       R                        |
> |        ............xxxxxxxxxxxxxxxxxxxxxxxxx                   |
> |        <descs sent><- descs not sent yet  ->                   |
> |        ............xxxxxxxxxxxxxxxxxxxxxxxxx                   |
> |----------------------------------------------------------------|
>         ^last_desc_cleaned=8                    ^next_rs=47
>                 ^next_dd=15                   ^tail=45
>                      ^hw_head=20
> 
>                      <----  nb_used  --------->
> 
> The hardware is currently processing the descriptor 20 'R' means the descriptor
> has the RS bit 'D' means the descriptor has the DD + RS bits 'x' are packets in txq
> (not sent) '.' are packet already sent but not freed by sw
> 
> 
> Regards,
> Olivier
diff mbox

Patch

diff --git a/drivers/net/e1000/e1000_ethdev.h b/drivers/net/e1000/e1000_ethdev.h
index 2b63b1a..e3fd7fc 100644
--- a/drivers/net/e1000/e1000_ethdev.h
+++ b/drivers/net/e1000/e1000_ethdev.h
@@ -380,6 +380,11 @@  uint32_t eth_em_rx_queue_count(struct rte_eth_dev *dev,
 
 int eth_em_rx_descriptor_done(void *rx_queue, uint16_t offset);
 
+int eth_em_rx_descriptor_status(struct rte_eth_dev *dev,
+	uint16_t rx_queue_id, uint16_t offset);
+int eth_em_tx_descriptor_status(struct rte_eth_dev *dev,
+	uint16_t tx_queue_id, uint16_t offset);
+
 int eth_em_tx_queue_setup(struct rte_eth_dev *dev, uint16_t tx_queue_id,
 		uint16_t nb_tx_desc, unsigned int socket_id,
 		const struct rte_eth_txconf *tx_conf);
diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
index 4066ef9..4f34c14 100644
--- a/drivers/net/e1000/em_ethdev.c
+++ b/drivers/net/e1000/em_ethdev.c
@@ -205,6 +205,8 @@  static const struct eth_dev_ops eth_em_ops = {
 	.rx_queue_release     = eth_em_rx_queue_release,
 	.rx_queue_count       = eth_em_rx_queue_count,
 	.rx_descriptor_done   = eth_em_rx_descriptor_done,
+	.rx_descriptor_status = eth_em_rx_descriptor_status,
+	.tx_descriptor_status = eth_em_tx_descriptor_status,
 	.tx_queue_setup       = eth_em_tx_queue_setup,
 	.tx_queue_release     = eth_em_tx_queue_release,
 	.rx_queue_intr_enable = eth_em_rx_queue_intr_enable,
diff --git a/drivers/net/e1000/em_rxtx.c b/drivers/net/e1000/em_rxtx.c
index d099d6a..1651454 100644
--- a/drivers/net/e1000/em_rxtx.c
+++ b/drivers/net/e1000/em_rxtx.c
@@ -1473,6 +1473,55 @@  eth_em_rx_descriptor_done(void *rx_queue, uint16_t offset)
 	return !!(rxdp->status & E1000_RXD_STAT_DD);
 }
 
+int
+eth_em_rx_descriptor_status(struct rte_eth_dev *dev, uint16_t rx_queue_id,
+	uint16_t offset)
+{
+	volatile uint8_t *status;
+	struct em_rx_queue *rxq;
+	uint32_t desc;
+
+	rxq = dev->data->rx_queues[rx_queue_id];
+	if (unlikely(offset >= rxq->nb_rx_desc))
+		return -EINVAL;
+
+	if (offset >= rxq->nb_rx_desc - rxq->nb_rx_hold)
+		return RTE_ETH_RX_DESC_USED;
+
+	desc = rxq->rx_tail + offset;
+	if (desc >= rxq->nb_rx_desc)
+		desc -= rxq->nb_rx_desc;
+
+	status = &rxq->rx_ring[desc].status;
+	if (*status & E1000_RXD_STAT_DD)
+		return RTE_ETH_RX_DESC_DONE;
+
+	return RTE_ETH_RX_DESC_AVAIL;
+}
+
+int
+eth_em_tx_descriptor_status(struct rte_eth_dev *dev, uint16_t tx_queue_id,
+	uint16_t offset)
+{
+	volatile uint8_t *status;
+	struct em_tx_queue *txq;
+	uint32_t desc;
+
+	txq = dev->data->tx_queues[tx_queue_id];
+	if (unlikely(offset >= txq->nb_tx_desc))
+		return -EINVAL;
+
+	desc = txq->tx_tail + offset;
+	/* go to next desc that has the RS bit */
+	desc = ((desc + txq->tx_rs_thresh - 1) / txq->tx_rs_thresh) *
+		txq->tx_rs_thresh;
+	status = &txq->tx_ring[desc].upper.fields.status;
+	if (*status & E1000_TXD_STAT_DD)
+		return RTE_ETH_TX_DESC_DONE;
+
+	return RTE_ETH_TX_DESC_FULL;
+}
+
 void
 em_dev_clear_queues(struct rte_eth_dev *dev)
 {