[dpdk-dev,PATCHv5,1/8] ethdev: add new API to retrieve RX/TX queue information
Commit Message
From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
Add the ability for the upper layer to query RX/TX queue information.
Add into rte_eth_dev_info new fields to represent information about
RX/TX descriptors min/max/alig nnumbers per queue for the device.
Add new structures:
struct rte_eth_rxq_info
struct rte_eth_txq_info
new functions:
rte_eth_rx_queue_info_get
rte_eth_tx_queue_info_get
into rte_etdev API.
Left extra free space in the queue info structures,
so extra fields could be added later without ABI breakage.
Add new fields:
rx_desc_lim
tx_desc_lim
into rte_eth_dev_info.
v2 changes:
- Add formal check for the qinfo input parameter.
- As suggested rename 'rx_qinfo/tx_qinfo' to 'rxq_info/txq_info'
v3 changes:
- Updated rte_ether_version.map
- Merged with latest changes
v4 changes:
- rte_ether_version.map: move new functions into DPDK_2.1 sub-space.
v5 changes:
- adressed previous code-review comments
- rte_ether_version.map: move new functions into DPDK_2.2 sub-space.
- added new fields into rte_eth_dev_info
Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
---
lib/librte_ether/rte_ethdev.c | 68 +++++++++++++++++++++++++++
lib/librte_ether/rte_ethdev.h | 85 +++++++++++++++++++++++++++++++++-
lib/librte_ether/rte_ether_version.map | 8 ++++
3 files changed, 159 insertions(+), 2 deletions(-)
Comments
Hi Konstantin
> +/**
> + * Ethernet device RX queue information structure.
> + * Used to retieve information about configured queue.
> + */
> +struct rte_eth_rxq_info {
> + struct rte_mempool *mp; /**< mempool used by that queue. */
> + struct rte_eth_rxconf conf; /**< queue config parameters. */
> + uint8_t scattered_rx; /**< scattered packets RX supported. */
> + uint16_t nb_desc; /**< configured number of RXDs. */
Here i need two more fields in this struct :
uint16_t free_desc : for free queue descriptors
uint16_t used_desc : for used queue descriptors
> +} __rte_cache_aligned;
> +
> +/**
> + * Ethernet device TX queue information structure.
> + * Used to retieve information about configured queue.
> + */
> +struct rte_eth_txq_info {
> + struct rte_eth_txconf conf; /**< queue config parameters. */
> + uint16_t nb_desc; /**< configured number of TXDs. */
And also here.
> +} __rte_cache_aligned;
> +
> struct rte_eth_dev;
How to add them without breaking API ? I would prefer to see them now, so
I'll send an update on your patch series that i'll use this 2 more
fields. The purpose will be to provide analysis of the usage of the RX
and TX queues.
Hi Amine,
> -----Original Message-----
> From: Amine Kherbouche [mailto:amine.kherbouche@6wind.com]
> Sent: Wednesday, October 14, 2015 12:40 PM
> To: Ananyev, Konstantin; dev@dpdk.org
> Subject: Re: [dpdk-dev, PATCHv5, 1/8] ethdev: add new API to retrieve RX/TX queue information
>
>
>
> Hi Konstantin
> > +/**
> > + * Ethernet device RX queue information structure.
> > + * Used to retieve information about configured queue.
> > + */
> > +struct rte_eth_rxq_info {
> > + struct rte_mempool *mp; /**< mempool used by that queue. */
> > + struct rte_eth_rxconf conf; /**< queue config parameters. */
> > + uint8_t scattered_rx; /**< scattered packets RX supported. */
> > + uint16_t nb_desc; /**< configured number of RXDs. */
> Here i need two more fields in this struct :
> uint16_t free_desc : for free queue descriptors
> uint16_t used_desc : for used queue descriptors
> > +} __rte_cache_aligned;
Yep, that’s good thing to have.
But to fill them properly extra work is required, so my thought was to left it for future expansion.
> > +
> > +/**
> > + * Ethernet device TX queue information structure.
> > + * Used to retieve information about configured queue.
> > + */
> > +struct rte_eth_txq_info {
> > + struct rte_eth_txconf conf; /**< queue config parameters. */
> > + uint16_t nb_desc; /**< configured number of TXDs. */
> And also here.
> > +} __rte_cache_aligned;
> > +
> > struct rte_eth_dev;
> How to add them without breaking API ?
As I said in the patch comments:
"left extra free space in the queue info structures,
so extra fields could be added later without ABI breakage."
As you can see both queue info structures have size 64B each.
So there is plenty of space for expansion without ABI breakage.
I would prefer to see them now, so
> I'll send an update on your patch series that i'll use this 2 more
> fields. The purpose will be to provide analysis of the usage of the RX
> and TX queues.
Well, in general I am not opposed to add these fields to the structures.
In fact, I think it is a good thing to have.
But if we'll add them without making queue_info_get() feeling them properly -
it might create a lot of controversy.
So unless you prepared to make changes in all queue_info_get() too,
my opinion - let's leave it as it is for 2.2, and add new fields in later releases.
As I said above - ABI wouldn't be affected.
Konstantin
Hi Konstantin
> Well, in general I am not opposed to add these fields to the structures.
> In fact, I think it is a good thing to have.
> But if we'll add them without making queue_info_get() feeling them properly -
> it might create a lot of controversy.
> So unless you prepared to make changes in all queue_info_get() too,
> my opinion - let's leave it as it is for 2.2, and add new fields in later releases.
Thanks for your answer, I'm almost done with all changes in
queue_info_get(), so i have just to
test and send a series of v6 for your series.
Best,
Amine Kherbouche
> -----Original Message-----
> From: Amine Kherbouche [mailto:amine.kherbouche@6wind.com]
> Sent: Wednesday, October 14, 2015 1:22 PM
> To: Ananyev, Konstantin; dev@dpdk.org
> Subject: Re: [dpdk-dev, PATCHv5, 1/8] ethdev: add new API to retrieve RX/TX queue information
>
>
> Hi Konstantin
>
> > Well, in general I am not opposed to add these fields to the structures.
> > In fact, I think it is a good thing to have.
> > But if we'll add them without making queue_info_get() feeling them properly -
> > it might create a lot of controversy.
> > So unless you prepared to make changes in all queue_info_get() too,
> > my opinion - let's leave it as it is for 2.2, and add new fields in later releases.
> Thanks for your answer, I'm almost done with all changes in
> queue_info_get(), so i have just to
> test and send a series of v6 for your series.
So your v6 will make all implemented queue_info_get()s to fill these two new fields correctly, right?
Konstantin
>
> Best,
> Amine Kherbouche
On 01/10/2015 20:54, Konstantin Ananyev wrote:
> Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
Acked-by: Remy Horton <remy.horton@intel.com>
>
> So your v6 will make all implemented queue_info_get()s to fill these two new fields correctly, right?
> Konstantin
Yes.
Amine Kherbouche
> -----Original Message-----
> From: Amine Kherbouche [mailto:amine.kherbouche@6wind.com]
> Sent: Wednesday, October 14, 2015 1:47 PM
> To: Ananyev, Konstantin; dev@dpdk.org
> Subject: Re: [dpdk-dev, PATCHv5, 1/8] ethdev: add new API to retrieve RX/TX queue information
>
>
>
> >
> > So your v6 will make all implemented queue_info_get()s to fill these two new fields correctly, right?
> > Konstantin
> Yes.
Ok great, will wait for v6 then.
Konstantin
>
> Amine Kherbouche
On Thu, 1 Oct 2015 20:54:46 +0100
Konstantin Ananyev <konstantin.ananyev@intel.com> wrote:
> + if (nb_rx_desc > dev_info.rx_desc_lim.nb_max ||
> + nb_rx_desc < dev_info.rx_desc_lim.nb_min ||
> + nb_rx_desc % dev_info.rx_desc_lim.nb_align != 0) {
> +
Preferred indentation style is to line up the conditional.
if (nb_rx_desc > dev_info.rx_desc_lim.nb_max ||
nb_rx_desc < dev_info.rx_desc_lim.nb_min ||
nb_rx_desc % dev_info.rx_desc_lim.nb_align != 0) {
> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Wednesday, October 14, 2015 5:09 PM
> To: Ananyev, Konstantin
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCHv5 1/8] ethdev: add new API to retrieve RX/TX queue information
>
> On Thu, 1 Oct 2015 20:54:46 +0100
> Konstantin Ananyev <konstantin.ananyev@intel.com> wrote:
>
> > + if (nb_rx_desc > dev_info.rx_desc_lim.nb_max ||
> > + nb_rx_desc < dev_info.rx_desc_lim.nb_min ||
> > + nb_rx_desc % dev_info.rx_desc_lim.nb_align != 0) {
> > +
>
> Preferred indentation style is to line up the conditional.
> if (nb_rx_desc > dev_info.rx_desc_lim.nb_max ||
> nb_rx_desc < dev_info.rx_desc_lim.nb_min ||
> nb_rx_desc % dev_info.rx_desc_lim.nb_align != 0) {
As I remember within DPDK we use an extra tab as preferred indentation style for several years.
Why it suddenly changed now?
Konstantin
On Wed, Oct 14, 2015 at 06:44:38PM +0000, Ananyev, Konstantin wrote:
>
>
> > -----Original Message-----
> > From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> > Sent: Wednesday, October 14, 2015 5:09 PM
> > To: Ananyev, Konstantin
> > Cc: dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCHv5 1/8] ethdev: add new API to retrieve RX/TX queue information
> >
> > On Thu, 1 Oct 2015 20:54:46 +0100
> > Konstantin Ananyev <konstantin.ananyev@intel.com> wrote:
> >
> > > + if (nb_rx_desc > dev_info.rx_desc_lim.nb_max ||
> > > + nb_rx_desc < dev_info.rx_desc_lim.nb_min ||
> > > + nb_rx_desc % dev_info.rx_desc_lim.nb_align != 0) {
> > > +
> >
> > Preferred indentation style is to line up the conditional.
> > if (nb_rx_desc > dev_info.rx_desc_lim.nb_max ||
> > nb_rx_desc < dev_info.rx_desc_lim.nb_min ||
> > nb_rx_desc % dev_info.rx_desc_lim.nb_align != 0) {
>
>
> As I remember within DPDK we use an extra tab as preferred indentation style for several years.
> Why it suddenly changed now?
> Konstantin
>
We actually have a mix of styles, some using double-indent on continuation, and
some using aligning as above. I prefer the double indent, as I don't like mixing
tabs and spaces for indentation and the resulting line-up of lines for those poor
souls using 4-char tabs, but overall, the global rule is to try and copy the
existing style in the file being modified. There are already instances of
this style of indent in ethdev.c [I checked, which is why I didn't object myself
to Stephen's comment :-)]
/Bruce
This v6 series is an enhancement to queue information API v5.
Amine Kherbouche (6):
ethdev: enhance rte_eth_(tx|rx)q_info struct
testpmd: enhance the command to display RX/TX queue information
virtio: add support for eth_(rxq|txq)_info_get
e1000: enhance eth_(rxq|txq)_info_get to retrieve more queue
information
i40e: enhance eth_(rxq|txq)_info_get to retrieve more queue
information
ixgbe: enhance eth_(rxq|txq)_info_get to retrieve more queue
information
app/test-pmd/config.c | 8 ++++++--
drivers/net/e1000/em_rxtx.c | 4 ++++
drivers/net/e1000/igb_rxtx.c | 4 ++++
drivers/net/i40e/i40e_rxtx.c | 4 ++++
drivers/net/ixgbe/ixgbe_rxtx.c | 4 ++++
drivers/net/virtio/virtio_ethdev.c | 28 ++++++++++++++++++++++++++++
drivers/net/virtio/virtio_ethdev.h | 4 ++++
lib/librte_ether/rte_ethdev.h | 4 ++++
8 files changed, 58 insertions(+), 2 deletions(-)
On 2015/10/14 19:50, Ananyev, Konstantin wrote:
> Hi Amine,
>
>> -----Original Message-----
>> From: Amine Kherbouche [mailto:amine.kherbouche@6wind.com]
>> Sent: Wednesday, October 14, 2015 12:40 PM
>> To: Ananyev, Konstantin; dev@dpdk.org
>> Subject: Re: [dpdk-dev, PATCHv5, 1/8] ethdev: add new API to retrieve RX/TX queue information
>>
>>
>>
>> Hi Konstantin
>>> +/**
>>> + * Ethernet device RX queue information structure.
>>> + * Used to retieve information about configured queue.
>>> + */
>>> +struct rte_eth_rxq_info {
>>> + struct rte_mempool *mp; /**< mempool used by that queue. */
>>> + struct rte_eth_rxconf conf; /**< queue config parameters. */
>>> + uint8_t scattered_rx; /**< scattered packets RX supported. */
>>> + uint16_t nb_desc; /**< configured number of RXDs. */
>> Here i need two more fields in this struct :
>> uint16_t free_desc : for free queue descriptors
>> uint16_t used_desc : for used queue descriptors
But as I know it is different all the time, am I right?
If yes, I don't know what's the value of this field.
Thanks,
Michael
>>> +} __rte_cache_aligned;
>
On 20/10/2015 09:53, Qiu, Michael wrote:
> But as I know it is different all the time, am I right?
> If yes, I don't know what's the value of this field.
It can be used to get some snapshot/instant view informations while we
have to monitor and debug.
On 2015/10/20 16:09, Vincent JARDIN wrote:
> On 20/10/2015 09:53, Qiu, Michael wrote:
>> But as I know it is different all the time, am I right?
>> If yes, I don't know what's the value of this field.
> It can be used to get some snapshot/instant view informations while we
> have to monitor and debug.
So this field is mainly for debug?
Thanks,
Michael
@@ -1447,6 +1447,19 @@ rte_eth_rx_queue_setup(uint8_t port_id, uint16_t rx_queue_id,
return -EINVAL;
}
+ if (nb_rx_desc > dev_info.rx_desc_lim.nb_max ||
+ nb_rx_desc < dev_info.rx_desc_lim.nb_min ||
+ nb_rx_desc % dev_info.rx_desc_lim.nb_align != 0) {
+
+ PMD_DEBUG_TRACE("Invalid value for nb_rx_desc(=%hu), "
+ "should be: <= %hu, = %hu, and a product of %hu\n",
+ nb_rx_desc,
+ dev_info.rx_desc_lim.nb_max,
+ dev_info.rx_desc_lim.nb_min,
+ dev_info.rx_desc_lim.nb_align);
+ return -EINVAL;
+ }
+
if (rx_conf == NULL)
rx_conf = &dev_info.default_rxconf;
@@ -1786,11 +1799,18 @@ void
rte_eth_dev_info_get(uint8_t port_id, struct rte_eth_dev_info *dev_info)
{
struct rte_eth_dev *dev;
+ const struct rte_eth_desc_lim lim = {
+ .nb_max = UINT16_MAX,
+ .nb_min = 0,
+ .nb_align = 1,
+ };
VALID_PORTID_OR_RET(port_id);
dev = &rte_eth_devices[port_id];
memset(dev_info, 0, sizeof(struct rte_eth_dev_info));
+ dev_info->rx_desc_lim = lim;
+ dev_info->tx_desc_lim = lim;
FUNC_PTR_OR_RET(*dev->dev_ops->dev_infos_get);
(*dev->dev_ops->dev_infos_get)(dev, dev_info);
@@ -3449,6 +3469,54 @@ rte_eth_remove_tx_callback(uint8_t port_id, uint16_t queue_id,
}
int
+rte_eth_rx_queue_info_get(uint8_t port_id, uint16_t queue_id,
+ struct rte_eth_rxq_info *qinfo)
+{
+ struct rte_eth_dev *dev;
+
+ VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+
+ if (qinfo == NULL)
+ return -EINVAL;
+
+ dev = &rte_eth_devices[port_id];
+ if (queue_id >= dev->data->nb_rx_queues) {
+ PMD_DEBUG_TRACE("Invalid RX queue_id=%d\n", queue_id);
+ return -EINVAL;
+ }
+
+ FUNC_PTR_OR_ERR_RET(*dev->dev_ops->rxq_info_get, -ENOTSUP);
+
+ memset(qinfo, 0, sizeof(*qinfo));
+ dev->dev_ops->rxq_info_get(dev, queue_id, qinfo);
+ return 0;
+}
+
+int
+rte_eth_tx_queue_info_get(uint8_t port_id, uint16_t queue_id,
+ struct rte_eth_txq_info *qinfo)
+{
+ struct rte_eth_dev *dev;
+
+ VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+
+ if (qinfo == NULL)
+ return -EINVAL;
+
+ dev = &rte_eth_devices[port_id];
+ if (queue_id >= dev->data->nb_tx_queues) {
+ PMD_DEBUG_TRACE("Invalid TX queue_id=%d\n", queue_id);
+ return -EINVAL;
+ }
+
+ FUNC_PTR_OR_ERR_RET(*dev->dev_ops->txq_info_get, -ENOTSUP);
+
+ memset(qinfo, 0, sizeof(*qinfo));
+ dev->dev_ops->txq_info_get(dev, queue_id, qinfo);
+ return 0;
+}
+
+int
rte_eth_dev_set_mc_addr_list(uint8_t port_id,
struct ether_addr *mc_addr_set,
uint32_t nb_mc_addr)
@@ -653,6 +653,15 @@ struct rte_eth_txconf {
};
/**
+ * A structure contains information about HW descriptor ring limitations.
+ */
+struct rte_eth_desc_lim {
+ uint16_t nb_max; /**< Max allowed number of descriptors. */
+ uint16_t nb_min; /**< Min allowed number of descriptors. */
+ uint16_t nb_align; /**< Number of descriptors should be aligned to. */
+};
+
+/**
* This enum indicates the flow control mode
*/
enum rte_eth_fc_mode {
@@ -945,6 +954,8 @@ struct rte_eth_dev_info {
uint16_t vmdq_queue_base; /**< First queue ID for VMDQ pools. */
uint16_t vmdq_queue_num; /**< Queue number for VMDQ pools. */
uint16_t vmdq_pool_base; /**< First ID of VMDQ pools. */
+ struct rte_eth_desc_lim rx_desc_lim; /**< RX descriptors limits */
+ struct rte_eth_desc_lim tx_desc_lim; /**< TX descriptors limits */
};
/** Maximum name length for extended statistics counters */
@@ -962,6 +973,26 @@ struct rte_eth_xstats {
uint64_t value;
};
+/**
+ * Ethernet device RX queue information structure.
+ * Used to retieve information about configured queue.
+ */
+struct rte_eth_rxq_info {
+ struct rte_mempool *mp; /**< mempool used by that queue. */
+ struct rte_eth_rxconf conf; /**< queue config parameters. */
+ uint8_t scattered_rx; /**< scattered packets RX supported. */
+ uint16_t nb_desc; /**< configured number of RXDs. */
+} __rte_cache_aligned;
+
+/**
+ * Ethernet device TX queue information structure.
+ * Used to retieve information about configured queue.
+ */
+struct rte_eth_txq_info {
+ struct rte_eth_txconf conf; /**< queue config parameters. */
+ uint16_t nb_desc; /**< configured number of TXDs. */
+} __rte_cache_aligned;
+
struct rte_eth_dev;
struct rte_eth_dev_callback;
@@ -1073,6 +1104,12 @@ typedef uint32_t (*eth_rx_queue_count_t)(struct rte_eth_dev *dev,
typedef int (*eth_rx_descriptor_done_t)(void *rxq, uint16_t offset);
/**< @internal Check DD bit of specific RX descriptor */
+typedef void (*eth_rxq_info_get_t)(struct rte_eth_dev *dev,
+ uint16_t rx_queue_id, struct rte_eth_rxq_info *qinfo);
+
+typedef void (*eth_txq_info_get_t)(struct rte_eth_dev *dev,
+ uint16_t tx_queue_id, struct rte_eth_txq_info *qinfo);
+
typedef int (*mtu_set_t)(struct rte_eth_dev *dev, uint16_t mtu);
/**< @internal Set MTU. */
@@ -1465,9 +1502,13 @@ struct eth_dev_ops {
rss_hash_update_t rss_hash_update;
/** Get current RSS hash configuration. */
rss_hash_conf_get_t rss_hash_conf_get;
- eth_filter_ctrl_t filter_ctrl; /**< common filter control*/
+ eth_filter_ctrl_t filter_ctrl;
+ /**< common filter control. */
eth_set_mc_addr_list_t set_mc_addr_list; /**< set list of mcast addrs */
-
+ eth_rxq_info_get_t rxq_info_get;
+ /**< retrieve RX queue information. */
+ eth_txq_info_get_t txq_info_get;
+ /**< retrieve TX queue information. */
/** Turn IEEE1588/802.1AS timestamping on. */
eth_timesync_enable_t timesync_enable;
/** Turn IEEE1588/802.1AS timestamping off. */
@@ -3821,6 +3862,46 @@ int rte_eth_remove_tx_callback(uint8_t port_id, uint16_t queue_id,
struct rte_eth_rxtx_callback *user_cb);
/**
+ * Retrieve information about given port's RX queue.
+ *
+ * @param port_id
+ * The port identifier of the Ethernet device.
+ * @param queue_id
+ * The RX queue on the Ethernet device for which information
+ * will be retrieved.
+ * @param qinfo
+ * A pointer to a structure of type *rte_eth_rxq_info_info* to be filled with
+ * the information of the Ethernet device.
+ *
+ * @return
+ * - 0: Success
+ * - -ENOTSUP: routine is not supported by the device PMD.
+ * - -EINVAL: The port_id or the queue_id is out of range.
+ */
+int rte_eth_rx_queue_info_get(uint8_t port_id, uint16_t queue_id,
+ struct rte_eth_rxq_info *qinfo);
+
+/**
+ * Retrieve information about given port's TX queue.
+ *
+ * @param port_id
+ * The port identifier of the Ethernet device.
+ * @param queue_id
+ * The TX queue on the Ethernet device for which information
+ * will be retrieved.
+ * @param qinfo
+ * A pointer to a structure of type *rte_eth_txq_info_info* to be filled with
+ * the information of the Ethernet device.
+ *
+ * @return
+ * - 0: Success
+ * - -ENOTSUP: routine is not supported by the device PMD.
+ * - -EINVAL: The port_id or the queue_id is out of range.
+ */
+int rte_eth_tx_queue_info_get(uint8_t port_id, uint16_t queue_id,
+ struct rte_eth_txq_info *qinfo);
+
+/*
* Retrieve number of available registers for access
*
* @param port_id
@@ -127,3 +127,11 @@ DPDK_2.1 {
rte_eth_timesync_read_tx_timestamp;
} DPDK_2.0;
+
+DPDK_2.2 {
+ global:
+
+ rte_eth_rx_queue_info_get;
+ rte_eth_tx_queue_info_get;
+
+} DPDK_2.1;