[v7,2/4] ethdev: support mulitiple mbuf pools per Rx queue
Checks
Commit Message
From: Hanumanth Pothula <hpothula@marvell.com>
Some of the HW has support for choosing memory pools based on the
packet's size. The capability allows to choose a memory pool based
on the packet's length.
This is often useful for saving the memory where the application
can create a different pool to steer the specific size of the
packet, thus enabling more efficient usage of memory.
For example, let's say HW has a capability of three pools,
- pool-1 size is 2K
- pool-2 size is > 2K and < 4K
- pool-3 size is > 4K
Here,
pool-1 can accommodate packets with sizes < 2K
pool-2 can accommodate packets with sizes > 2K and < 4K
pool-3 can accommodate packets with sizes > 4K
With multiple mempool capability enabled in SW, an application may
create three pools of different sizes and send them to PMD. Allowing
PMD to program HW based on the packet lengths. So that packets with
less than 2K are received on pool-1, packets with lengths between 2K
and 4K are received on pool-2 and finally packets greater than 4K
are received on pool-3.
Signed-off-by: Hanumanth Pothula <hpothula@marvell.com>
Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
---
doc/guides/rel_notes/release_22_11.rst | 6 ++
lib/ethdev/rte_ethdev.c | 81 ++++++++++++++++++++++----
lib/ethdev/rte_ethdev.h | 25 ++++++++
3 files changed, 101 insertions(+), 11 deletions(-)
Comments
07/10/2022 16:37, Andrew Rybchenko:
> From: Hanumanth Pothula <hpothula@marvell.com>
>
> Some of the HW has support for choosing memory pools based on the
> packet's size. The capability allows to choose a memory pool based
> on the packet's length.
The second sentence is redundant.
> This is often useful for saving the memory where the application
> can create a different pool to steer the specific size of the
> packet, thus enabling more efficient usage of memory.
[...]
> +* **Added support for mulitiple mbuf pools per ethdev Rx queue.**
mulitiple -> multiple
> +
> + * Added support for multiple mbuf pools per Rx queue. The capability allows
No need to repeat the title.
> + application to provide many mempools of different size and PMD to choose
> + a memory pool based on the packet's length and/or Rx buffers availability.
[...]
> + /* Ensure that we have one and only one source of Rx buffers */
> + if ((mp != NULL) +
+ operator?
Are we sure a boolean is always translated as 1?
> + (rx_conf != NULL && rx_conf->rx_nseg > 0) +
> + (rx_conf != NULL && rx_conf->rx_nmempool > 0) != 1) {
> + RTE_ETHDEV_LOG(ERR,
> + "Ambiguous Rx mempools configuration\n");
> + return -EINVAL;
> + }
[...]
> @@ -1067,6 +1067,24 @@ struct rte_eth_rxconf {
> */
> union rte_eth_rxseg *rx_seg;
>
> + /**
> + * Array of mempools to allocate Rx buffers from.
> + *
> + * This provides support for multiple mbuf pools per Rx queue.
> + * The capability is reported in device info via positive
> + * max_rx_mempools.
> + *
> + * It could be useful for more efficient usage of memory when an
> + * application creates different mempools to steer the specific
> + * size of the packet.
> + *
> + * Note that if Rx scatter is enabled, a packet may be delivered using
> + * a chain of mbufs obtained from single mempool or multiple mempools
> + * based on the NIC implementation.
> + */
> + struct rte_mempool **rx_mempools;
> + uint16_t rx_nmempool; /** < Number of Rx mempools */
The commit message suggests a configuration per packet size.
I guess it is not configurable in ethdev API?
If it is hard-configured in the HW or the driver only,
it should be specified here.
[...]
> + /**
> + * Maximum number of Rx mempools supported per Rx queue.
> + *
> + * Value greater than 0 means that the driver supports Rx queue
> + * mempools specification via rx_conf->rx_mempools.
> + */
> + uint16_t max_rx_mempools;
On Fri, 07 Oct 2022 18:08:57 +0200
Thomas Monjalon <thomas@monjalon.net> wrote:
> > + /* Ensure that we have one and only one source of Rx buffers */
> > + if ((mp != NULL) +
>
> + operator?
> Are we sure a boolean is always translated as 1?
Yes, it is likely part of C standard.
On Fri, 7 Oct 2022 09:18:14 -0700
Stephen Hemminger <stephen@networkplumber.org> wrote:
> On Fri, 07 Oct 2022 18:08:57 +0200
> Thomas Monjalon <thomas@monjalon.net> wrote:
>
> > > + /* Ensure that we have one and only one source of Rx buffers */
> > > + if ((mp != NULL) +
> >
> > + operator?
> > Are we sure a boolean is always translated as 1?
>
> Yes, it is likely part of C standard.
Found it: https://en.cppreference.com/w/c/language/operator_comparison
The type of any equality operator expression is int, and its value (which is not an lvalue) is 1
when the specified relationship holds true and 0 when the specified relationship does not hold.
On 10/7/22 19:20, Stephen Hemminger wrote:
> On Fri, 7 Oct 2022 09:18:14 -0700
> Stephen Hemminger <stephen@networkplumber.org> wrote:
>
>> On Fri, 07 Oct 2022 18:08:57 +0200
>> Thomas Monjalon <thomas@monjalon.net> wrote:
>>
>>>> + /* Ensure that we have one and only one source of Rx buffers */
>>>> + if ((mp != NULL) +
>>>
>>> + operator?
>>> Are we sure a boolean is always translated as 1?
>>
>> Yes, it is likely part of C standard.
>
>
> Found it: https://en.cppreference.com/w/c/language/operator_comparison
> The type of any equality operator expression is int, and its value (which is not an lvalue) is 1
> when the specified relationship holds true and 0 when the specified relationship does not hold.
Many thanks, Stephen.
On 10/7/22 19:08, Thomas Monjalon wrote:
> 07/10/2022 16:37, Andrew Rybchenko:
>> @@ -1067,6 +1067,24 @@ struct rte_eth_rxconf {
>> */
>> union rte_eth_rxseg *rx_seg;
>>
>> + /**
>> + * Array of mempools to allocate Rx buffers from.
>> + *
>> + * This provides support for multiple mbuf pools per Rx queue.
>> + * The capability is reported in device info via positive
>> + * max_rx_mempools.
>> + *
>> + * It could be useful for more efficient usage of memory when an
>> + * application creates different mempools to steer the specific
>> + * size of the packet.
>> + *
>> + * Note that if Rx scatter is enabled, a packet may be delivered using
>> + * a chain of mbufs obtained from single mempool or multiple mempools
>> + * based on the NIC implementation.
>> + */
>> + struct rte_mempool **rx_mempools;
>> + uint16_t rx_nmempool; /** < Number of Rx mempools */
>
> The commit message suggests a configuration per packet size.
> I guess it is not configurable in ethdev API?
> If it is hard-configured in the HW or the driver only,
> it should be specified here.
See v8
>
> [...]
>> + /**
>> + * Maximum number of Rx mempools supported per Rx queue.
>> + *
>> + * Value greater than 0 means that the driver supports Rx queue
>> + * mempools specification via rx_conf->rx_mempools.
>> + */
>> + uint16_t max_rx_mempools;
>
>
>
@@ -92,6 +92,12 @@ New Features
``rte_eth_cman_config_set()``, ``rte_eth_cman_info_get()``
to support congestion management.
+* **Added support for mulitiple mbuf pools per ethdev Rx queue.**
+
+ * Added support for multiple mbuf pools per Rx queue. The capability allows
+ application to provide many mempools of different size and PMD to choose
+ a memory pool based on the packet's length and/or Rx buffers availability.
+
* **Updated Intel iavf driver.**
* Added flow subscription support.
@@ -1739,6 +1739,41 @@ rte_eth_rx_queue_check_split(const struct rte_eth_rxseg_split *rx_seg,
return 0;
}
+static int
+rte_eth_rx_queue_check_mempools(struct rte_mempool **rx_mempools,
+ uint16_t n_mempools, uint32_t *min_buf_size,
+ const struct rte_eth_dev_info *dev_info)
+{
+ uint16_t pool_idx;
+ int ret;
+
+ if (n_mempools > dev_info->max_rx_mempools) {
+ RTE_ETHDEV_LOG(ERR,
+ "Too many Rx mempools %u vs maximum %u\n",
+ n_mempools, dev_info->max_rx_mempools);
+ return -EINVAL;
+ }
+
+ for (pool_idx = 0; pool_idx < n_mempools; pool_idx++) {
+ struct rte_mempool *mp = rx_mempools[pool_idx];
+
+ if (mp == NULL) {
+ RTE_ETHDEV_LOG(ERR, "null Rx mempool pointer\n");
+ return -EINVAL;
+ }
+
+ ret = rte_eth_check_rx_mempool(mp, RTE_PKTMBUF_HEADROOM,
+ dev_info->min_rx_bufsize);
+ if (ret != 0)
+ return ret;
+
+ *min_buf_size = RTE_MIN(*min_buf_size,
+ rte_pktmbuf_data_room_size(mp));
+ }
+
+ return 0;
+}
+
int
rte_eth_rx_queue_setup(uint16_t port_id, uint16_t rx_queue_id,
uint16_t nb_rx_desc, unsigned int socket_id,
@@ -1746,7 +1781,8 @@ rte_eth_rx_queue_setup(uint16_t port_id, uint16_t rx_queue_id,
struct rte_mempool *mp)
{
int ret;
- uint32_t mbp_buf_size;
+ uint64_t rx_offloads;
+ uint32_t mbp_buf_size = UINT32_MAX;
struct rte_eth_dev *dev;
struct rte_eth_dev_info dev_info;
struct rte_eth_rxconf local_conf;
@@ -1766,35 +1802,42 @@ rte_eth_rx_queue_setup(uint16_t port_id, uint16_t rx_queue_id,
if (ret != 0)
return ret;
+ rx_offloads = dev->data->dev_conf.rxmode.offloads;
+ if (rx_conf != NULL)
+ rx_offloads |= rx_conf->offloads;
+
+ /* Ensure that we have one and only one source of Rx buffers */
+ if ((mp != NULL) +
+ (rx_conf != NULL && rx_conf->rx_nseg > 0) +
+ (rx_conf != NULL && rx_conf->rx_nmempool > 0) != 1) {
+ RTE_ETHDEV_LOG(ERR,
+ "Ambiguous Rx mempools configuration\n");
+ return -EINVAL;
+ }
+
if (mp != NULL) {
/* Single pool configuration check. */
- if (rx_conf != NULL && rx_conf->rx_nseg != 0) {
- RTE_ETHDEV_LOG(ERR,
- "Ambiguous segment configuration\n");
- return -EINVAL;
- }
-
ret = rte_eth_check_rx_mempool(mp, RTE_PKTMBUF_HEADROOM,
dev_info.min_rx_bufsize);
if (ret != 0)
return ret;
mbp_buf_size = rte_pktmbuf_data_room_size(mp);
- } else {
+ } else if (rx_conf == NULL || rx_conf->rx_nseg > 0) {
const struct rte_eth_rxseg_split *rx_seg;
uint16_t n_seg;
/* Extended multi-segment configuration check. */
- if (rx_conf == NULL || rx_conf->rx_seg == NULL || rx_conf->rx_nseg == 0) {
+ if (rx_conf->rx_seg == NULL) {
RTE_ETHDEV_LOG(ERR,
- "Memory pool is null and no extended configuration provided\n");
+ "Memory pool is null and no multi-segment configuration provided\n");
return -EINVAL;
}
rx_seg = (const struct rte_eth_rxseg_split *)rx_conf->rx_seg;
n_seg = rx_conf->rx_nseg;
- if (rx_conf->offloads & RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT) {
+ if (rx_offloads & RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT) {
ret = rte_eth_rx_queue_check_split(rx_seg, n_seg,
&mbp_buf_size,
&dev_info);
@@ -1804,6 +1847,22 @@ rte_eth_rx_queue_setup(uint16_t port_id, uint16_t rx_queue_id,
RTE_ETHDEV_LOG(ERR, "No Rx segmentation offload configured\n");
return -EINVAL;
}
+ } else if (rx_conf == NULL || rx_conf->rx_nmempool > 0) {
+ /* Extended multi-pool configuration check. */
+ if (rx_conf->rx_mempools == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Memory pools array is null\n");
+ return -EINVAL;
+ }
+
+ ret = rte_eth_rx_queue_check_mempools(rx_conf->rx_mempools,
+ rx_conf->rx_nmempool,
+ &mbp_buf_size,
+ &dev_info);
+ if (ret != 0)
+ return ret;
+ } else {
+ RTE_ETHDEV_LOG(ERR, "Missing Rx mempool configuration\n");
+ return -EINVAL;
}
/* Use default specified by driver, if nb_rx_desc is zero */
@@ -1067,6 +1067,24 @@ struct rte_eth_rxconf {
*/
union rte_eth_rxseg *rx_seg;
+ /**
+ * Array of mempools to allocate Rx buffers from.
+ *
+ * This provides support for multiple mbuf pools per Rx queue.
+ * The capability is reported in device info via positive
+ * max_rx_mempools.
+ *
+ * It could be useful for more efficient usage of memory when an
+ * application creates different mempools to steer the specific
+ * size of the packet.
+ *
+ * Note that if Rx scatter is enabled, a packet may be delivered using
+ * a chain of mbufs obtained from single mempool or multiple mempools
+ * based on the NIC implementation.
+ */
+ struct rte_mempool **rx_mempools;
+ uint16_t rx_nmempool; /** < Number of Rx mempools */
+
uint64_t reserved_64s[2]; /**< Reserved for future fields */
void *reserved_ptrs[2]; /**< Reserved for future fields */
};
@@ -1614,6 +1632,13 @@ struct rte_eth_dev_info {
/** Configured number of Rx/Tx queues */
uint16_t nb_rx_queues; /**< Number of Rx queues. */
uint16_t nb_tx_queues; /**< Number of Tx queues. */
+ /**
+ * Maximum number of Rx mempools supported per Rx queue.
+ *
+ * Value greater than 0 means that the driver supports Rx queue
+ * mempools specification via rx_conf->rx_mempools.
+ */
+ uint16_t max_rx_mempools;
/** Rx parameter recommendations */
struct rte_eth_dev_portconf default_rxportconf;
/** Tx parameter recommendations */