[RFC,v1,3/7] ethdev: introduce Rx queue based limit watermark
Checks
Commit Message
LWM(limit watermark) is a per Rx queue attribute that notifies dpdk
application event of RTE_ETH_EVENT_RXQ_LIMIT_REACHED when the Rx
queue's usable descriptor is under the watermark.
To simplify its configuration, LWM is a percentage of Rx queue
descriptor size with valid value of [0,99].
Setting LWM to 0 means disable it.
Add LWM's configuration handle in eth_dev_ops.
Signed-off-by: Spike Du <spiked@nvidia.com>
---
lib/ethdev/ethdev_driver.h | 7 +++++++
lib/ethdev/rte_ethdev.c | 28 ++++++++++++++++++++++++++++
lib/ethdev/rte_ethdev.h | 30 +++++++++++++++++++++++++++++-
lib/ethdev/version.map | 3 +++
4 files changed, 67 insertions(+), 1 deletion(-)
Comments
On 5/6/22 06:56, Spike Du wrote:
> LWM(limit watermark) is a per Rx queue attribute that notifies dpdk
dpdk is not necessary about.
I'm not sure that "attribute" can notify application. Please,
reword the description.
> application event of RTE_ETH_EVENT_RXQ_LIMIT_REACHED when the Rx
> queue's usable descriptor is under the watermark.
> To simplify its configuration, LWM is a percentage of Rx queue
> descriptor size with valid value of [0,99].
> Setting LWM to 0 means disable it.
... which is the default.
Can I request notification when no descriptors left?
1 seems to be close to the answer, but not in the case of big
Rx rings.
> Add LWM's configuration handle in eth_dev_ops.
handle sounds bad here. May be "driver callback" or "driver
method".
>
> Signed-off-by: Spike Du <spiked@nvidia.com>
> ---
> lib/ethdev/ethdev_driver.h | 7 +++++++
> lib/ethdev/rte_ethdev.c | 28 ++++++++++++++++++++++++++++
> lib/ethdev/rte_ethdev.h | 30 +++++++++++++++++++++++++++++-
> lib/ethdev/version.map | 3 +++
> 4 files changed, 67 insertions(+), 1 deletion(-)
>
> diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
> index 69d9dc2..1e9cdbf 100644
> --- a/lib/ethdev/ethdev_driver.h
> +++ b/lib/ethdev/ethdev_driver.h
> @@ -470,6 +470,10 @@ typedef int (*eth_rx_queue_setup_t)(struct rte_eth_dev *dev,
> const struct rte_eth_rxconf *rx_conf,
> struct rte_mempool *mb_pool);
>
> +typedef int (*eth_rx_queue_set_lwm_t)(struct rte_eth_dev *dev,
> + uint16_t rx_queue_id,
> + uint8_t lwm);
> +
Please, add full description including parameters and return
values.
> /** @internal Setup a transmit queue of an Ethernet device. */
> typedef int (*eth_tx_queue_setup_t)(struct rte_eth_dev *dev,
> uint16_t tx_queue_id,
> @@ -1283,6 +1287,9 @@ struct eth_dev_ops {
>
> /** Dump private info from device */
> eth_dev_priv_dump_t eth_dev_priv_dump;
> +
> + /** Set Rx queue limit watermark */
> + eth_rx_queue_set_lwm_t rx_queue_set_lwm;
> };
>
> /**
> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
> index 29a3d80..1e4fc6a 100644
> --- a/lib/ethdev/rte_ethdev.c
> +++ b/lib/ethdev/rte_ethdev.c
> @@ -4414,6 +4414,34 @@ int rte_eth_set_queue_rate_limit(uint16_t port_id, uint16_t queue_idx,
> queue_idx, tx_rate));
> }
>
> +int rte_eth_rx_queue_set_lwm(uint16_t port_id, uint16_t queue_idx,
> + uint8_t lwm)
> +{
> + struct rte_eth_dev *dev;
> + struct rte_eth_dev_info dev_info;
> + int ret;
> +
> + RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> + dev = &rte_eth_devices[port_id];
> +
> + ret = rte_eth_dev_info_get(port_id, &dev_info);
> + if (ret != 0)
> + return ret;
> +
> + if (queue_idx > dev_info.max_rx_queues) {
It should be >=
> + RTE_ETHDEV_LOG(ERR,
> + "Set queue rate limit:port %u: invalid queue ID=%u\n",
> + port_id, queue_idx);
> + return -EINVAL;
> + }
> +
> + if (lwm > 99)
> + return -EINVAL;
> + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->rx_queue_set_lwm, -ENOTSUP);
> + return eth_err(port_id, (*dev->dev_ops->rx_queue_set_lwm)(dev,
> + queue_idx, lwm));
> +}
> +
> RTE_INIT(eth_dev_init_fp_ops)
> {
> uint32_t i;
> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> index 04cff8e..f29e53b 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -1249,8 +1249,12 @@ struct rte_eth_rxconf {
> */
> union rte_eth_rxseg *rx_seg;
>
> - uint64_t reserved_64s[2]; /**< Reserved for future fields */
> + uint64_t reserved_64s;
> + uint32_t reserved_32s;
> + uint32_t lwm:8;
> + uint32_t reserved_bits:24;
I strong dislike bit fields for such purpose. It should
be uint8_t field.
Since we break ABI below anyway, we can break it here as well.
> void *reserved_ptrs[2]; /**< Reserved for future fields */
> +
No unrelated changes, please.
> };
>
> /**
> @@ -3668,6 +3672,29 @@ int rte_eth_dev_set_vlan_ether_type(uint16_t port_id,
> */
> int rte_eth_dev_set_vlan_pvid(uint16_t port_id, uint16_t pvid, int on);
>
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice.
> + *
> + * Set Rx queue based limit watermark.
> + *
> + * @param port_id
> + * The port identifier of the Ethernet device.
> + * @param queue_idx
> + * The index of the receive queue
> + * @param lwm
> + * The limit watermark percentage of Rx queue descriptor size.
> + * The valid range is [0,99].
> + * Setting 0 means disable limit watermark.
> + *
> + * @return
> + * - (0) if successful.
> + * - negative if failed.
Please, be precise with negative return values specification
and its meaning.
> + */
> +__rte_experimental
> +int rte_eth_rx_queue_set_lwm(uint16_t port_id, uint16_t queue_idx,
> + uint8_t lwm);
> +
> typedef void (*buffer_tx_error_fn)(struct rte_mbuf **unsent, uint16_t count,
> void *userdata);
>
> @@ -3873,6 +3900,7 @@ enum rte_eth_event_type {
> RTE_ETH_EVENT_DESTROY, /**< port is released */
> RTE_ETH_EVENT_IPSEC, /**< IPsec offload related event */
> RTE_ETH_EVENT_FLOW_AGED,/**< New aged-out flows is detected */
> + RTE_ETH_EVENT_RXQ_LIMIT_REACHED,/**< RX queue limit reached */
RX -> Rx
as I understand it is an ABI breakage.
> RTE_ETH_EVENT_MAX /**< max value of this enum */
> };
>
> diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
> index 20391ab..8b85ad8 100644
> --- a/lib/ethdev/version.map
> +++ b/lib/ethdev/version.map
> @@ -279,6 +279,9 @@ EXPERIMENTAL {
> rte_flow_async_action_handle_create;
> rte_flow_async_action_handle_destroy;
> rte_flow_async_action_handle_update;
> +
> + # added in 22.07
> + rte_eth_rx_queue_set_lwm;
> };
>
> INTERNAL {
@@ -470,6 +470,10 @@ typedef int (*eth_rx_queue_setup_t)(struct rte_eth_dev *dev,
const struct rte_eth_rxconf *rx_conf,
struct rte_mempool *mb_pool);
+typedef int (*eth_rx_queue_set_lwm_t)(struct rte_eth_dev *dev,
+ uint16_t rx_queue_id,
+ uint8_t lwm);
+
/** @internal Setup a transmit queue of an Ethernet device. */
typedef int (*eth_tx_queue_setup_t)(struct rte_eth_dev *dev,
uint16_t tx_queue_id,
@@ -1283,6 +1287,9 @@ struct eth_dev_ops {
/** Dump private info from device */
eth_dev_priv_dump_t eth_dev_priv_dump;
+
+ /** Set Rx queue limit watermark */
+ eth_rx_queue_set_lwm_t rx_queue_set_lwm;
};
/**
@@ -4414,6 +4414,34 @@ int rte_eth_set_queue_rate_limit(uint16_t port_id, uint16_t queue_idx,
queue_idx, tx_rate));
}
+int rte_eth_rx_queue_set_lwm(uint16_t port_id, uint16_t queue_idx,
+ uint8_t lwm)
+{
+ struct rte_eth_dev *dev;
+ struct rte_eth_dev_info dev_info;
+ int ret;
+
+ RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+ dev = &rte_eth_devices[port_id];
+
+ ret = rte_eth_dev_info_get(port_id, &dev_info);
+ if (ret != 0)
+ return ret;
+
+ if (queue_idx > dev_info.max_rx_queues) {
+ RTE_ETHDEV_LOG(ERR,
+ "Set queue rate limit:port %u: invalid queue ID=%u\n",
+ port_id, queue_idx);
+ return -EINVAL;
+ }
+
+ if (lwm > 99)
+ return -EINVAL;
+ RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->rx_queue_set_lwm, -ENOTSUP);
+ return eth_err(port_id, (*dev->dev_ops->rx_queue_set_lwm)(dev,
+ queue_idx, lwm));
+}
+
RTE_INIT(eth_dev_init_fp_ops)
{
uint32_t i;
@@ -1249,8 +1249,12 @@ struct rte_eth_rxconf {
*/
union rte_eth_rxseg *rx_seg;
- uint64_t reserved_64s[2]; /**< Reserved for future fields */
+ uint64_t reserved_64s;
+ uint32_t reserved_32s;
+ uint32_t lwm:8;
+ uint32_t reserved_bits:24;
void *reserved_ptrs[2]; /**< Reserved for future fields */
+
};
/**
@@ -3668,6 +3672,29 @@ int rte_eth_dev_set_vlan_ether_type(uint16_t port_id,
*/
int rte_eth_dev_set_vlan_pvid(uint16_t port_id, uint16_t pvid, int on);
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Set Rx queue based limit watermark.
+ *
+ * @param port_id
+ * The port identifier of the Ethernet device.
+ * @param queue_idx
+ * The index of the receive queue
+ * @param lwm
+ * The limit watermark percentage of Rx queue descriptor size.
+ * The valid range is [0,99].
+ * Setting 0 means disable limit watermark.
+ *
+ * @return
+ * - (0) if successful.
+ * - negative if failed.
+ */
+__rte_experimental
+int rte_eth_rx_queue_set_lwm(uint16_t port_id, uint16_t queue_idx,
+ uint8_t lwm);
+
typedef void (*buffer_tx_error_fn)(struct rte_mbuf **unsent, uint16_t count,
void *userdata);
@@ -3873,6 +3900,7 @@ enum rte_eth_event_type {
RTE_ETH_EVENT_DESTROY, /**< port is released */
RTE_ETH_EVENT_IPSEC, /**< IPsec offload related event */
RTE_ETH_EVENT_FLOW_AGED,/**< New aged-out flows is detected */
+ RTE_ETH_EVENT_RXQ_LIMIT_REACHED,/**< RX queue limit reached */
RTE_ETH_EVENT_MAX /**< max value of this enum */
};
@@ -279,6 +279,9 @@ EXPERIMENTAL {
rte_flow_async_action_handle_create;
rte_flow_async_action_handle_destroy;
rte_flow_async_action_handle_update;
+
+ # added in 22.07
+ rte_eth_rx_queue_set_lwm;
};
INTERNAL {