DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev]  [RFC] ethdev: add a field for rte_eth_rxq_info
@ 2020-06-18 12:35 Chengchang Tang
  2020-08-26  1:57 ` [dpdk-dev] [PATCH 0/3] add RX buffer size " Chengchang Tang
                   ` (4 more replies)
  0 siblings, 5 replies; 59+ messages in thread
From: Chengchang Tang @ 2020-06-18 12:35 UTC (permalink / raw)
  To: dev; +Cc: linuxarm, thomas, ferruh.yigit, arybchenko

In common practice, PMD configure the rx_buf_size according to the data room
size of the object in mempool. But in fact the final value is related to the
specifications of hw, and its values will affect the number of fragments in
recieving pkts.

At present, we seem to have no way to espose relevant information to upper
layer users.

Add a field named rx_bufsize in rte_eth_rxq_info to indicate the buffer size
used in recieving pkts for hw.

Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
---
 lib/librte_ethdev/rte_ethdev.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index 0f6d053..82b7e98 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -1306,6 +1306,7 @@ struct rte_eth_rxq_info {
 	struct rte_eth_rxconf conf; /**< queue config parameters. */
 	uint8_t scattered_rx;       /**< scattered packets RX supported. */
 	uint16_t nb_desc;           /**< configured number of RXDs. */
+	uint16_t rx_bufsize;        /**< size of RX buffer. */
 } __rte_cache_min_aligned;

 /**
--
2.7.4


^ permalink raw reply	[flat|nested] 59+ messages in thread

* [dpdk-dev]  [PATCH 0/3] add RX buffer size for rte_eth_rxq_info
  2020-06-18 12:35 [dpdk-dev] [RFC] ethdev: add a field for rte_eth_rxq_info Chengchang Tang
@ 2020-08-26  1:57 ` Chengchang Tang
  2020-08-26  1:57   ` [dpdk-dev] [PATCH 1/3] ethdev: add a field " Chengchang Tang
                     ` (2 more replies)
  2020-08-26  7:12 ` [dpdk-dev] [PATCH v2 0/3] add Rx buffer size for rxq info structure Chengchang Tang
                   ` (3 subsequent siblings)
  4 siblings, 3 replies; 59+ messages in thread
From: Chengchang Tang @ 2020-08-26  1:57 UTC (permalink / raw)
  To: dev; +Cc: linuxarm, thomas, arybchenko, ferruh.yigit

In common practice, PMD configure the RX buffer size which indicate the
buffer length could be used for HW in receiving packets according to the
data room size of the object in mempool. But in fact, the final value is
related to the specifications of HW, and its values will affect the number
of fragments in receiving packets when scatter is enabled. By the way,
some PMDs may force to enable scatter when the MTU is bigger than the HW
RX buffer size.

At present, we have no way to expose relevant information to upper layer
users. So, add a field named rx_buf_size in rte_eth_rxq_info to indicate
the buffer size used in receiving packets for HW. And this field is
optional, so there is no need to forcibly update all PMDs.

This patchset also add hns3 PMD implementation and update the testpmd.

Chengchang Tang (3):
  ethdev: add a field for rte_eth_rxq_info
  app/testpmd: Add RX buffer size display in queue info querry
  net/hns3: add rx buffer size to rx qinfo querry

 app/test-pmd/config.c          | 1 +
 drivers/net/hns3/hns3_rxtx.c   | 2 ++
 lib/librte_ethdev/rte_ethdev.h | 2 ++
 3 files changed, 5 insertions(+)

--
2.7.4


^ permalink raw reply	[flat|nested] 59+ messages in thread

* [dpdk-dev]  [PATCH 1/3] ethdev: add a field for rte_eth_rxq_info
  2020-08-26  1:57 ` [dpdk-dev] [PATCH 0/3] add RX buffer size " Chengchang Tang
@ 2020-08-26  1:57   ` Chengchang Tang
  2020-08-26  1:57   ` [dpdk-dev] [PATCH 2/3] app/testpmd: Add RX buffer size display in queue info querry Chengchang Tang
  2020-08-26  1:57   ` [dpdk-dev] [PATCH 3/3] net/hns3: add RX buffer size to rx qinfo querry Chengchang Tang
  2 siblings, 0 replies; 59+ messages in thread
From: Chengchang Tang @ 2020-08-26  1:57 UTC (permalink / raw)
  To: dev; +Cc: linuxarm, thomas, arybchenko, ferruh.yigit

Add a field named rx_buf_size in rte_eth_rxq_info to indicate the buffer
size used in receiving packets for HW.

In this way, upper-layer users can get this information by calling
rte_eth_rx_queue_info_get.

Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 lib/librte_ethdev/rte_ethdev.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index 70295d7..9fed5cb 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -1420,6 +1420,8 @@ struct rte_eth_rxq_info {
 	struct rte_eth_rxconf conf; /**< queue config parameters. */
 	uint8_t scattered_rx;       /**< scattered packets RX supported. */
 	uint16_t nb_desc;           /**< configured number of RXDs. */
+	/**< buffer size used for hardware when receive packets. */
+	uint16_t rx_buf_size;
 } __rte_cache_min_aligned;

 /**
--
2.7.4


^ permalink raw reply	[flat|nested] 59+ messages in thread

* [dpdk-dev] [PATCH 2/3] app/testpmd: Add RX buffer size display in queue info querry
  2020-08-26  1:57 ` [dpdk-dev] [PATCH 0/3] add RX buffer size " Chengchang Tang
  2020-08-26  1:57   ` [dpdk-dev] [PATCH 1/3] ethdev: add a field " Chengchang Tang
@ 2020-08-26  1:57   ` Chengchang Tang
  2020-08-26  1:57   ` [dpdk-dev] [PATCH 3/3] net/hns3: add RX buffer size to rx qinfo querry Chengchang Tang
  2 siblings, 0 replies; 59+ messages in thread
From: Chengchang Tang @ 2020-08-26  1:57 UTC (permalink / raw)
  To: dev; +Cc: linuxarm, thomas, arybchenko, ferruh.yigit

Add RX buffer size to queue info querry cmd so that the user can get the
buffer length used by HW queue for receiving packets.

Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
---
 app/test-pmd/config.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 30bee33..b432ac6 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -452,6 +452,7 @@ rx_queue_infos_display(portid_t port_id, uint16_t queue_id)
 		(qinfo.conf.rx_deferred_start != 0) ? "on" : "off");
 	printf("\nRX scattered packets: %s",
 		(qinfo.scattered_rx != 0) ? "on" : "off");
+	printf("\nRX buffer size: %hu", qinfo.rx_buf_size);
 	printf("\nNumber of RXDs: %hu", qinfo.nb_desc);

 	if (rte_eth_rx_burst_mode_get(port_id, queue_id, &mode) == 0)
--
2.7.4


^ permalink raw reply	[flat|nested] 59+ messages in thread

* [dpdk-dev] [PATCH 3/3] net/hns3: add RX buffer size to rx qinfo querry
  2020-08-26  1:57 ` [dpdk-dev] [PATCH 0/3] add RX buffer size " Chengchang Tang
  2020-08-26  1:57   ` [dpdk-dev] [PATCH 1/3] ethdev: add a field " Chengchang Tang
  2020-08-26  1:57   ` [dpdk-dev] [PATCH 2/3] app/testpmd: Add RX buffer size display in queue info querry Chengchang Tang
@ 2020-08-26  1:57   ` Chengchang Tang
  2 siblings, 0 replies; 59+ messages in thread
From: Chengchang Tang @ 2020-08-26  1:57 UTC (permalink / raw)
  To: dev; +Cc: linuxarm, thomas, arybchenko, ferruh.yigit

Report hns3 PMD configured RX buffer size in RX qinfo querry.

Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
---
 drivers/net/hns3/hns3_rxtx.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/hns3/hns3_rxtx.c b/drivers/net/hns3/hns3_rxtx.c
index fc1a256..50881d4 100644
--- a/drivers/net/hns3/hns3_rxtx.c
+++ b/drivers/net/hns3/hns3_rxtx.c
@@ -2830,6 +2830,8 @@ hns3_rxq_info_get(struct rte_eth_dev *dev, uint16_t queue_id,
 	qinfo->mp = rxq->mb_pool;
 	qinfo->nb_desc = rxq->nb_rx_desc;
 	qinfo->scattered_rx = dev->data->scattered_rx;
+	/* Report the HW RX buffer length to user */
+	qinfo->rx_buf_size = rxq->rx_buf_len;

 	/*
 	 * If there are no available Rx buffer descriptors, incoming packets
--
2.7.4


^ permalink raw reply	[flat|nested] 59+ messages in thread

* [dpdk-dev] [PATCH v2 0/3] add Rx buffer size for rxq info structure
  2020-06-18 12:35 [dpdk-dev] [RFC] ethdev: add a field for rte_eth_rxq_info Chengchang Tang
  2020-08-26  1:57 ` [dpdk-dev] [PATCH 0/3] add RX buffer size " Chengchang Tang
@ 2020-08-26  7:12 ` Chengchang Tang
  2020-08-26  7:12   ` [dpdk-dev] [PATCH v2 1/3] ethdev: add a field " Chengchang Tang
                     ` (2 more replies)
  2020-08-29  7:13 ` [dpdk-dev] [PATCH v3 0/4] add Rx buffer size for rxq info structure Chengchang Tang
                   ` (2 subsequent siblings)
  4 siblings, 3 replies; 59+ messages in thread
From: Chengchang Tang @ 2020-08-26  7:12 UTC (permalink / raw)
  To: dev; +Cc: linuxarm, thomas, arybchenko, ferruh.yigit

In common practice, PMD configure the Rx buffer size which indicate the
buffer length could be used for HW in receiving packets according to the
data room size of the object in mempool. But in fact, the final value is
related to the specifications of HW, and its values will affect the number
of fragments in receiving packets when scatter is enabled. By the way,
some PMDs may force to enable scatter when the MTU is bigger than the HW
Rx buffer size.

At present, we have no way to expose relevant information to upper layer
users. So, add a field named rx_buf_size in rte_eth_rxq_info to indicate
the buffer size used in receiving packets for HW. And this field is
optional, so there is no need to forcibly update all PMDs.

This patchset also add hns3 PMD implementation and update the testpmd.

Chengchang Tang (3):
  ethdev: add a field for rxq info structure
  app/testpmd: add Rx buffer size display in queue info query
  net/hns3: add Rx buffer size to Rx qinfo query

 app/test-pmd/config.c          | 1 +
 drivers/net/hns3/hns3_rxtx.c   | 2 ++
 lib/librte_ethdev/rte_ethdev.h | 2 ++
 3 files changed, 5 insertions(+)

--
2.7.4


^ permalink raw reply	[flat|nested] 59+ messages in thread

* [dpdk-dev] [PATCH v2 1/3] ethdev: add a field for rxq info structure
  2020-08-26  7:12 ` [dpdk-dev] [PATCH v2 0/3] add Rx buffer size for rxq info structure Chengchang Tang
@ 2020-08-26  7:12   ` Chengchang Tang
  2020-08-26  7:29     ` Wei Hu (Xavier)
  2020-08-26  7:12   ` [dpdk-dev] [PATCH v2 2/3] app/testpmd: add Rx buffer size display in queue info query Chengchang Tang
  2020-08-26  7:12   ` [dpdk-dev] [PATCH v2 3/3] net/hns3: add Rx buffer size to Rx qinfo query Chengchang Tang
  2 siblings, 1 reply; 59+ messages in thread
From: Chengchang Tang @ 2020-08-26  7:12 UTC (permalink / raw)
  To: dev; +Cc: linuxarm, thomas, arybchenko, ferruh.yigit

Add a field named rx_buf_size in rte_eth_rxq_info to indicate the buffer
size used in receiving packets for HW.

In this way, upper-layer users can get this information by calling
rte_eth_rx_queue_info_get.

Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
v1 -> v2: fix wrong headline format
---
 lib/librte_ethdev/rte_ethdev.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index 70295d7..9fed5cb 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -1420,6 +1420,8 @@ struct rte_eth_rxq_info {
 	struct rte_eth_rxconf conf; /**< queue config parameters. */
 	uint8_t scattered_rx;       /**< scattered packets RX supported. */
 	uint16_t nb_desc;           /**< configured number of RXDs. */
+	/**< buffer size used for hardware when receive packets. */
+	uint16_t rx_buf_size;
 } __rte_cache_min_aligned;

 /**
--
2.7.4


^ permalink raw reply	[flat|nested] 59+ messages in thread

* [dpdk-dev] [PATCH v2 2/3] app/testpmd: add Rx buffer size display in queue info query
  2020-08-26  7:12 ` [dpdk-dev] [PATCH v2 0/3] add Rx buffer size for rxq info structure Chengchang Tang
  2020-08-26  7:12   ` [dpdk-dev] [PATCH v2 1/3] ethdev: add a field " Chengchang Tang
@ 2020-08-26  7:12   ` Chengchang Tang
  2020-08-26  7:28     ` Wei Hu (Xavier)
  2020-08-26 14:42     ` Stephen Hemminger
  2020-08-26  7:12   ` [dpdk-dev] [PATCH v2 3/3] net/hns3: add Rx buffer size to Rx qinfo query Chengchang Tang
  2 siblings, 2 replies; 59+ messages in thread
From: Chengchang Tang @ 2020-08-26  7:12 UTC (permalink / raw)
  To: dev; +Cc: linuxarm, thomas, arybchenko, ferruh.yigit

Add Rx buffer size to queue info querry cmd so that the user can get the
buffer length used by HW queue for receiving packets.

Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
---
v1 -> v2: fix some spelling mistake
---
 app/test-pmd/config.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 30bee33..b432ac6 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -452,6 +452,7 @@ rx_queue_infos_display(portid_t port_id, uint16_t queue_id)
 		(qinfo.conf.rx_deferred_start != 0) ? "on" : "off");
 	printf("\nRX scattered packets: %s",
 		(qinfo.scattered_rx != 0) ? "on" : "off");
+	printf("\nRX buffer size: %hu", qinfo.rx_buf_size);
 	printf("\nNumber of RXDs: %hu", qinfo.nb_desc);

 	if (rte_eth_rx_burst_mode_get(port_id, queue_id, &mode) == 0)
--
2.7.4


^ permalink raw reply	[flat|nested] 59+ messages in thread

* [dpdk-dev] [PATCH v2 3/3] net/hns3: add Rx buffer size to Rx qinfo query
  2020-08-26  7:12 ` [dpdk-dev] [PATCH v2 0/3] add Rx buffer size for rxq info structure Chengchang Tang
  2020-08-26  7:12   ` [dpdk-dev] [PATCH v2 1/3] ethdev: add a field " Chengchang Tang
  2020-08-26  7:12   ` [dpdk-dev] [PATCH v2 2/3] app/testpmd: add Rx buffer size display in queue info query Chengchang Tang
@ 2020-08-26  7:12   ` Chengchang Tang
  2020-08-26  7:20     ` Wei Hu (Xavier)
  2 siblings, 1 reply; 59+ messages in thread
From: Chengchang Tang @ 2020-08-26  7:12 UTC (permalink / raw)
  To: dev; +Cc: linuxarm, thomas, arybchenko, ferruh.yigit

Report hns3 PMD configured Rx buffer size in Rx queue information query.

Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
---
v1 -> v2: fix some spelling mistake.
---
 drivers/net/hns3/hns3_rxtx.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/hns3/hns3_rxtx.c b/drivers/net/hns3/hns3_rxtx.c
index fc1a256..50881d4 100644
--- a/drivers/net/hns3/hns3_rxtx.c
+++ b/drivers/net/hns3/hns3_rxtx.c
@@ -2830,6 +2830,8 @@ hns3_rxq_info_get(struct rte_eth_dev *dev, uint16_t queue_id,
 	qinfo->mp = rxq->mb_pool;
 	qinfo->nb_desc = rxq->nb_rx_desc;
 	qinfo->scattered_rx = dev->data->scattered_rx;
+	/* Report the HW Rx buffer length to user */
+	qinfo->rx_buf_size = rxq->rx_buf_len;

 	/*
 	 * If there are no available Rx buffer descriptors, incoming packets
--
2.7.4


^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [dpdk-dev] [PATCH v2 3/3] net/hns3: add Rx buffer size to Rx qinfo query
  2020-08-26  7:12   ` [dpdk-dev] [PATCH v2 3/3] net/hns3: add Rx buffer size to Rx qinfo query Chengchang Tang
@ 2020-08-26  7:20     ` Wei Hu (Xavier)
  0 siblings, 0 replies; 59+ messages in thread
From: Wei Hu (Xavier) @ 2020-08-26  7:20 UTC (permalink / raw)
  To: Chengchang Tang, dev; +Cc: linuxarm, thomas, arybchenko, ferruh.yigit


On 2020/8/26 15:12, Chengchang Tang wrote:
> Report hns3 PMD configured Rx buffer size in Rx queue information query.
>
> Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
Reviewed-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
> ---
> v1 -> v2: fix some spelling mistake.
> ---
>   drivers/net/hns3/hns3_rxtx.c | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/hns3/hns3_rxtx.c b/drivers/net/hns3/hns3_rxtx.c
> index fc1a256..50881d4 100644
> --- a/drivers/net/hns3/hns3_rxtx.c
> +++ b/drivers/net/hns3/hns3_rxtx.c
> @@ -2830,6 +2830,8 @@ hns3_rxq_info_get(struct rte_eth_dev *dev, uint16_t queue_id,
>   	qinfo->mp = rxq->mb_pool;
>   	qinfo->nb_desc = rxq->nb_rx_desc;
>   	qinfo->scattered_rx = dev->data->scattered_rx;
> +	/* Report the HW Rx buffer length to user */
> +	qinfo->rx_buf_size = rxq->rx_buf_len;
>
>   	/*
>   	 * If there are no available Rx buffer descriptors, incoming packets
> --
> 2.7.4
>

^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [dpdk-dev] [PATCH v2 2/3] app/testpmd: add Rx buffer size display in queue info query
  2020-08-26  7:12   ` [dpdk-dev] [PATCH v2 2/3] app/testpmd: add Rx buffer size display in queue info query Chengchang Tang
@ 2020-08-26  7:28     ` Wei Hu (Xavier)
  2020-08-26 14:42     ` Stephen Hemminger
  1 sibling, 0 replies; 59+ messages in thread
From: Wei Hu (Xavier) @ 2020-08-26  7:28 UTC (permalink / raw)
  To: Chengchang Tang, dev; +Cc: thomas, arybchenko, ferruh.yigit


On 2020/8/26 15:12, Chengchang Tang wrote:
> Add Rx buffer size to queue info querry cmd so that the user can get the
> buffer length used by HW queue for receiving packets.
>
> Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
Reviewed-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
> ---
> v1 -> v2: fix some spelling mistake
> ---
>   app/test-pmd/config.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index 30bee33..b432ac6 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -452,6 +452,7 @@ rx_queue_infos_display(portid_t port_id, uint16_t queue_id)
>   		(qinfo.conf.rx_deferred_start != 0) ? "on" : "off");
>   	printf("\nRX scattered packets: %s",
>   		(qinfo.scattered_rx != 0) ? "on" : "off");
> +	printf("\nRX buffer size: %hu", qinfo.rx_buf_size);
>   	printf("\nNumber of RXDs: %hu", qinfo.nb_desc);
>
>   	if (rte_eth_rx_burst_mode_get(port_id, queue_id, &mode) == 0)
> --
> 2.7.4
>

^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [dpdk-dev] [PATCH v2 1/3] ethdev: add a field for rxq info structure
  2020-08-26  7:12   ` [dpdk-dev] [PATCH v2 1/3] ethdev: add a field " Chengchang Tang
@ 2020-08-26  7:29     ` Wei Hu (Xavier)
  0 siblings, 0 replies; 59+ messages in thread
From: Wei Hu (Xavier) @ 2020-08-26  7:29 UTC (permalink / raw)
  To: Chengchang Tang, dev; +Cc: thomas, arybchenko, ferruh.yigit


On 2020/8/26 15:12, Chengchang Tang wrote:
> Add a field named rx_buf_size in rte_eth_rxq_info to indicate the buffer
> size used in receiving packets for HW.
>
> In this way, upper-layer users can get this information by calling
> rte_eth_rx_queue_info_get.
>
> Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
Reviewed-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
> ---
> v1 -> v2: fix wrong headline format
> ---
>   lib/librte_ethdev/rte_ethdev.h | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
> index 70295d7..9fed5cb 100644
> --- a/lib/librte_ethdev/rte_ethdev.h
> +++ b/lib/librte_ethdev/rte_ethdev.h
> @@ -1420,6 +1420,8 @@ struct rte_eth_rxq_info {
>   	struct rte_eth_rxconf conf; /**< queue config parameters. */
>   	uint8_t scattered_rx;       /**< scattered packets RX supported. */
>   	uint16_t nb_desc;           /**< configured number of RXDs. */
> +	/**< buffer size used for hardware when receive packets. */
> +	uint16_t rx_buf_size;
>   } __rte_cache_min_aligned;
>
>   /**
> --
> 2.7.4
>

^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [dpdk-dev] [PATCH v2 2/3] app/testpmd: add Rx buffer size display in queue info query
  2020-08-26  7:12   ` [dpdk-dev] [PATCH v2 2/3] app/testpmd: add Rx buffer size display in queue info query Chengchang Tang
  2020-08-26  7:28     ` Wei Hu (Xavier)
@ 2020-08-26 14:42     ` Stephen Hemminger
  2020-08-29  6:48       ` Chengchang Tang
  1 sibling, 1 reply; 59+ messages in thread
From: Stephen Hemminger @ 2020-08-26 14:42 UTC (permalink / raw)
  To: Chengchang Tang; +Cc: dev, linuxarm, thomas, arybchenko, ferruh.yigit

On Wed, 26 Aug 2020 15:12:22 +0800
Chengchang Tang <tangchengchang@huawei.com> wrote:

> Add Rx buffer size to queue info querry cmd so that the user can get the
> buffer length used by HW queue for receiving packets.
> 
> Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>

You might want to add this info to proc-info tool as well.

^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [dpdk-dev] [PATCH v2 2/3] app/testpmd: add Rx buffer size display in queue info query
  2020-08-26 14:42     ` Stephen Hemminger
@ 2020-08-29  6:48       ` Chengchang Tang
  0 siblings, 0 replies; 59+ messages in thread
From: Chengchang Tang @ 2020-08-29  6:48 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev, linuxarm, thomas, arybchenko, ferruh.yigit

Hi, Stephen Hemminger

On 2020/8/26 22:42, Stephen Hemminger wrote:
> On Wed, 26 Aug 2020 15:12:22 +0800
> Chengchang Tang <tangchengchang@huawei.com> wrote:
> 
>> Add Rx buffer size to queue info querry cmd so that the user can get the
>> buffer length used by HW queue for receiving packets.
>>
>> Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
> 
> You might want to add this info to proc-info tool as well.
> 
> .
> 
Thanks for your suggestion.

I will add this to proc-info tool in the next version.

Thanks, Chengchang


^ permalink raw reply	[flat|nested] 59+ messages in thread

* [dpdk-dev] [PATCH v3 0/4] add Rx buffer size for rxq info structure
  2020-06-18 12:35 [dpdk-dev] [RFC] ethdev: add a field for rte_eth_rxq_info Chengchang Tang
  2020-08-26  1:57 ` [dpdk-dev] [PATCH 0/3] add RX buffer size " Chengchang Tang
  2020-08-26  7:12 ` [dpdk-dev] [PATCH v2 0/3] add Rx buffer size for rxq info structure Chengchang Tang
@ 2020-08-29  7:13 ` Chengchang Tang
  2020-08-29  7:13   ` [dpdk-dev] [PATCH v3 1/4] ethdev: add a field " Chengchang Tang
                     ` (3 more replies)
  2020-09-05  9:07 ` [dpdk-dev] [PATCH v4 0/5] add Rx buffer size for rxq info structure Chengchang Tang
  2020-09-21 13:22 ` [dpdk-dev] [PATCH v5 0/2] add Rx buffer size for rxq info structure Chengchang Tang
  4 siblings, 4 replies; 59+ messages in thread
From: Chengchang Tang @ 2020-08-29  7:13 UTC (permalink / raw)
  To: dev; +Cc: linuxarm, thomas, arybchenko, ferruh.yigit, wenzhuo.lu, maryam.tahhan

In common practice, PMD configure the Rx buffer size which indicate the
buffer length could be used for HW in receiving packets according to the
data room size of the object in mempool. But in fact, the final value is
related to the specifications of HW, and its values will affect the number
of fragments in receiving packets when scatter is enabled. By the way,
some PMDs may force to enable scatter when the MTU is bigger than the HW
Rx buffer size.

At present, we have no way to expose relevant information to upper layer
users. So, add a field named rx_buf_size in rte_eth_rxq_info to indicate
the buffer size used in receiving packets for HW. And this field is
optional, so there is no need to forcibly update all PMDs.

This patchset also update testpmd, proc-info tools and add hns3 PMD
implementation.

Chengchang Tang (4):
  ethdev: add a field for rxq info structure
  app/testpmd: add Rx buffer size display in queue info query
  app/procinfo: add Rx buffer size to --show-port
  net/hns3: add Rx buffer size to Rx qinfo query

 app/proc-info/main.c           | 2 ++
 app/test-pmd/config.c          | 1 +
 drivers/net/hns3/hns3_rxtx.c   | 2 ++
 lib/librte_ethdev/rte_ethdev.h | 2 ++
 4 files changed, 7 insertions(+)

--
2.7.4


^ permalink raw reply	[flat|nested] 59+ messages in thread

* [dpdk-dev] [PATCH v3 1/4] ethdev: add a field for rxq info structure
  2020-08-29  7:13 ` [dpdk-dev] [PATCH v3 0/4] add Rx buffer size for rxq info structure Chengchang Tang
@ 2020-08-29  7:13   ` Chengchang Tang
  2020-09-01 15:33     ` Matan Azrad
                       ` (2 more replies)
  2020-08-29  7:13   ` [dpdk-dev] [PATCH v3 2/4] app/testpmd: add Rx buffer size display in queue info query Chengchang Tang
                     ` (2 subsequent siblings)
  3 siblings, 3 replies; 59+ messages in thread
From: Chengchang Tang @ 2020-08-29  7:13 UTC (permalink / raw)
  To: dev; +Cc: linuxarm, thomas, arybchenko, ferruh.yigit, wenzhuo.lu, maryam.tahhan

Add a field named rx_buf_size in rte_eth_rxq_info to indicate the buffer
size used in receiving packets for HW.

In this way, upper-layer users can get this information by calling
rte_eth_rx_queue_info_get.

Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
Reviewed-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 lib/librte_ethdev/rte_ethdev.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index 70295d7..9fed5cb 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -1420,6 +1420,8 @@ struct rte_eth_rxq_info {
 	struct rte_eth_rxconf conf; /**< queue config parameters. */
 	uint8_t scattered_rx;       /**< scattered packets RX supported. */
 	uint16_t nb_desc;           /**< configured number of RXDs. */
+	/**< buffer size used for hardware when receive packets. */
+	uint16_t rx_buf_size;
 } __rte_cache_min_aligned;

 /**
--
2.7.4


^ permalink raw reply	[flat|nested] 59+ messages in thread

* [dpdk-dev] [PATCH v3 2/4] app/testpmd: add Rx buffer size display in queue info query
  2020-08-29  7:13 ` [dpdk-dev] [PATCH v3 0/4] add Rx buffer size for rxq info structure Chengchang Tang
  2020-08-29  7:13   ` [dpdk-dev] [PATCH v3 1/4] ethdev: add a field " Chengchang Tang
@ 2020-08-29  7:13   ` Chengchang Tang
  2020-08-29  7:13   ` [dpdk-dev] [PATCH v3 3/4] app/procinfo: add Rx buffer size to --show-port Chengchang Tang
  2020-08-29  7:13   ` [dpdk-dev] [PATCH v3 4/4] net/hns3: add Rx buffer size to Rx qinfo query Chengchang Tang
  3 siblings, 0 replies; 59+ messages in thread
From: Chengchang Tang @ 2020-08-29  7:13 UTC (permalink / raw)
  To: dev; +Cc: linuxarm, thomas, arybchenko, ferruh.yigit, wenzhuo.lu, maryam.tahhan

Add Rx buffer size to queue info querry cmd so that the user can get the
buffer length used by HW queue for receiving packets.

Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
Reviewed-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
---
 app/test-pmd/config.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 30bee33..b432ac6 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -452,6 +452,7 @@ rx_queue_infos_display(portid_t port_id, uint16_t queue_id)
 		(qinfo.conf.rx_deferred_start != 0) ? "on" : "off");
 	printf("\nRX scattered packets: %s",
 		(qinfo.scattered_rx != 0) ? "on" : "off");
+	printf("\nRX buffer size: %hu", qinfo.rx_buf_size);
 	printf("\nNumber of RXDs: %hu", qinfo.nb_desc);

 	if (rte_eth_rx_burst_mode_get(port_id, queue_id, &mode) == 0)
--
2.7.4


^ permalink raw reply	[flat|nested] 59+ messages in thread

* [dpdk-dev] [PATCH v3 3/4] app/procinfo: add Rx buffer size to --show-port
  2020-08-29  7:13 ` [dpdk-dev] [PATCH v3 0/4] add Rx buffer size for rxq info structure Chengchang Tang
  2020-08-29  7:13   ` [dpdk-dev] [PATCH v3 1/4] ethdev: add a field " Chengchang Tang
  2020-08-29  7:13   ` [dpdk-dev] [PATCH v3 2/4] app/testpmd: add Rx buffer size display in queue info query Chengchang Tang
@ 2020-08-29  7:13   ` Chengchang Tang
  2020-08-29  7:13   ` [dpdk-dev] [PATCH v3 4/4] net/hns3: add Rx buffer size to Rx qinfo query Chengchang Tang
  3 siblings, 0 replies; 59+ messages in thread
From: Chengchang Tang @ 2020-08-29  7:13 UTC (permalink / raw)
  To: dev; +Cc: linuxarm, thomas, arybchenko, ferruh.yigit, wenzhuo.lu, maryam.tahhan

Add Rx buffer size to show_port function to display it in the port PMD
information so that the user can get the buffer length used by HW queue
of receiving packets.

Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
---
 app/proc-info/main.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/app/proc-info/main.c b/app/proc-info/main.c
index abeca4a..e840dea 100644
--- a/app/proc-info/main.c
+++ b/app/proc-info/main.c
@@ -711,11 +711,13 @@ show_port(void)
 			if (ret == 0) {
 				printf("\t  -- queue %d rx scatter %d"
 						" descriptors %d"
+						" rx buffer size %d"
 						" offloads 0x%"PRIx64
 						" mempool socket %d\n",
 						j,
 						queue_info.scattered_rx,
 						queue_info.nb_desc,
+						queue_info.rx_buf_size,
 						queue_info.conf.offloads,
 						queue_info.mp->socket_id);
 			}
--
2.7.4


^ permalink raw reply	[flat|nested] 59+ messages in thread

* [dpdk-dev] [PATCH v3 4/4] net/hns3: add Rx buffer size to Rx qinfo query
  2020-08-29  7:13 ` [dpdk-dev] [PATCH v3 0/4] add Rx buffer size for rxq info structure Chengchang Tang
                     ` (2 preceding siblings ...)
  2020-08-29  7:13   ` [dpdk-dev] [PATCH v3 3/4] app/procinfo: add Rx buffer size to --show-port Chengchang Tang
@ 2020-08-29  7:13   ` Chengchang Tang
  3 siblings, 0 replies; 59+ messages in thread
From: Chengchang Tang @ 2020-08-29  7:13 UTC (permalink / raw)
  To: dev; +Cc: linuxarm, thomas, arybchenko, ferruh.yigit, wenzhuo.lu, maryam.tahhan

Report hns3 PMD configured Rx buffer size in Rx queue information query.

Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
Reviewed-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
---
 drivers/net/hns3/hns3_rxtx.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/hns3/hns3_rxtx.c b/drivers/net/hns3/hns3_rxtx.c
index fc1a256..d67c5ad 100644
--- a/drivers/net/hns3/hns3_rxtx.c
+++ b/drivers/net/hns3/hns3_rxtx.c
@@ -2830,6 +2830,8 @@ hns3_rxq_info_get(struct rte_eth_dev *dev, uint16_t queue_id,
 	qinfo->mp = rxq->mb_pool;
 	qinfo->nb_desc = rxq->nb_rx_desc;
 	qinfo->scattered_rx = dev->data->scattered_rx;
+	/* Report the HW Rx buffer length to user */
+	qinfo->rx_buf_size = rxq->rx_buf_len;

 	/*
 	 * If there are no available Rx buffer descriptors, incoming packets
--
2.7.4


^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [dpdk-dev] [PATCH v3 1/4] ethdev: add a field for rxq info structure
  2020-08-29  7:13   ` [dpdk-dev] [PATCH v3 1/4] ethdev: add a field " Chengchang Tang
@ 2020-09-01 15:33     ` Matan Azrad
  2020-09-02  3:52       ` Chengchang Tang
  2020-09-03 15:01     ` Ferruh Yigit
  2020-09-03 15:35     ` Bruce Richardson
  2 siblings, 1 reply; 59+ messages in thread
From: Matan Azrad @ 2020-09-01 15:33 UTC (permalink / raw)
  To: Chengchang Tang, dev
  Cc: linuxarm, NBU-Contact-Thomas Monjalon, arybchenko, ferruh.yigit,
	wenzhuo.lu, maryam.tahhan


Hi Chengchang

Please see some question below.

From: Chengchang Tang
> Add a field named rx_buf_size in rte_eth_rxq_info to indicate the buffer size
> used in receiving packets for HW.
> 
> In this way, upper-layer users can get this information by calling
> rte_eth_rx_queue_info_get.
> 
> Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
> Reviewed-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
> ---
>  lib/librte_ethdev/rte_ethdev.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
> index 70295d7..9fed5cb 100644
> --- a/lib/librte_ethdev/rte_ethdev.h
> +++ b/lib/librte_ethdev/rte_ethdev.h
> @@ -1420,6 +1420,8 @@ struct rte_eth_rxq_info {
>         struct rte_eth_rxconf conf; /**< queue config parameters. */
>         uint8_t scattered_rx;       /**< scattered packets RX supported. */
>         uint16_t nb_desc;           /**< configured number of RXDs. */
> +       /**< buffer size used for hardware when receive packets. */
> +       uint16_t rx_buf_size;

Is it the maximum supported Rx buffer by the HW?
If yes, maybe max_rx_buf_size is better name?

Maybe document that 0 means - no limitation by HW?

Must application read it in order to know if its datapath should handle multi-segment buffers?

Maybe it will be good to force application to configure scatter when this field is valid and smaller than max_rx_pkt_len\max_lro.. (<= room size)...


>  } __rte_cache_min_aligned;
> 
>  /**
> --
> 2.7.4


^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [dpdk-dev] [PATCH v3 1/4] ethdev: add a field for rxq info structure
  2020-09-01 15:33     ` Matan Azrad
@ 2020-09-02  3:52       ` Chengchang Tang
  2020-09-02  7:19         ` Matan Azrad
  2020-09-03 14:55         ` Ferruh Yigit
  0 siblings, 2 replies; 59+ messages in thread
From: Chengchang Tang @ 2020-09-02  3:52 UTC (permalink / raw)
  To: Matan Azrad, dev
  Cc: maryam.tahhan, linuxarm, ferruh.yigit, wenzhuo.lu,
	NBU-Contact-Thomas Monjalon, arybchenko

Hi, Matan

On 2020/9/1 23:33, Matan Azrad wrote:
> 
> Hi Chengchang
> 
> Please see some question below.
> 
> From: Chengchang Tang
>> Add a field named rx_buf_size in rte_eth_rxq_info to indicate the buffer size
>> used in receiving packets for HW.
>>
>> In this way, upper-layer users can get this information by calling
>> rte_eth_rx_queue_info_get.
>>
>> Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
>> Reviewed-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
>> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
>> ---
>>  lib/librte_ethdev/rte_ethdev.h | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
>> index 70295d7..9fed5cb 100644
>> --- a/lib/librte_ethdev/rte_ethdev.h
>> +++ b/lib/librte_ethdev/rte_ethdev.h
>> @@ -1420,6 +1420,8 @@ struct rte_eth_rxq_info {
>>         struct rte_eth_rxconf conf; /**< queue config parameters. */
>>         uint8_t scattered_rx;       /**< scattered packets RX supported. */
>>         uint16_t nb_desc;           /**< configured number of RXDs. */
>> +       /**< buffer size used for hardware when receive packets. */
>> +       uint16_t rx_buf_size;
> 
> Is it the maximum supported Rx buffer by the HW?
> If yes, maybe max_rx_buf_size is better name?

No, it is the Rx buffer size currently used by HW.

IMHO, the structure rte_eth_rxq_info and associated query API are mainly used to query HW configurations
at runtime or after queue is configured/setup. Therefore, the content of this structure should be the
current HW configuration.

> Maybe document that 0 means - no limitation by HW?

Yes, there is no need to fill this filed for HW that has no restrictions on it.
I'll add it in v4.

> Must application read it in order to know if its datapath should handle multi-segment buffers?

I think it's more appropriate to use scattered_rx to determine if multi-segment buffers should be handled.

> 
> Maybe it will be good to force application to configure scatter when this field is valid and smaller than max_rx_pkt_len\max_lro.. (<= room size)...
> 
> 
>>  } __rte_cache_min_aligned;
>>
>>  /**



^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [dpdk-dev] [PATCH v3 1/4] ethdev: add a field for rxq info structure
  2020-09-02  3:52       ` Chengchang Tang
@ 2020-09-02  7:19         ` Matan Azrad
  2020-09-02 10:01           ` Chengchang Tang
  2020-09-03 15:00           ` Ferruh Yigit
  2020-09-03 14:55         ` Ferruh Yigit
  1 sibling, 2 replies; 59+ messages in thread
From: Matan Azrad @ 2020-09-02  7:19 UTC (permalink / raw)
  To: Chengchang Tang, dev
  Cc: maryam.tahhan, linuxarm, ferruh.yigit, wenzhuo.lu,
	NBU-Contact-Thomas Monjalon, arybchenko


Hi Chengchang

From: Chengchang Tang
> Hi, Matan
> 
> On 2020/9/1 23:33, Matan Azrad wrote:
> >
> > Hi Chengchang
> >
> > Please see some question below.
> >
> > From: Chengchang Tang
> >> Add a field named rx_buf_size in rte_eth_rxq_info to indicate the
> >> buffer size used in receiving packets for HW.
> >>
> >> In this way, upper-layer users can get this information by calling
> >> rte_eth_rx_queue_info_get.
> >>
> >> Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
> >> Reviewed-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
> >> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
> >> ---
> >>  lib/librte_ethdev/rte_ethdev.h | 2 ++
> >>  1 file changed, 2 insertions(+)
> >>
> >> diff --git a/lib/librte_ethdev/rte_ethdev.h
> >> b/lib/librte_ethdev/rte_ethdev.h index 70295d7..9fed5cb 100644
> >> --- a/lib/librte_ethdev/rte_ethdev.h
> >> +++ b/lib/librte_ethdev/rte_ethdev.h
> >> @@ -1420,6 +1420,8 @@ struct rte_eth_rxq_info {
> >>         struct rte_eth_rxconf conf; /**< queue config parameters. */
> >>         uint8_t scattered_rx;       /**< scattered packets RX supported. */
> >>         uint16_t nb_desc;           /**< configured number of RXDs. */
> >> +       /**< buffer size used for hardware when receive packets. */
> >> +       uint16_t rx_buf_size;
> >
> > Is it the maximum supported Rx buffer by the HW?
> > If yes, maybe max_rx_buf_size is better name?
> 
> No, it is the Rx buffer size currently used by HW.

Doesn't it defined by the user? Using Rx queue mem-pool mbuf room size?

And it may be different per Rx queue....

> IMHO, the structure rte_eth_rxq_info and associated query API are mainly
> used to query HW configurations at runtime or after queue is
> configured/setup. Therefore, the content of this structure should be the
> current HW configuration.

It looks me more like capabilities...
The one which define the current configuration is the user by the configuration APIs(after reading the capabilities).

I don't think we have here all the current configurations, so what is special in this one?


> > Maybe document that 0 means - no limitation by HW?
> 
> Yes, there is no need to fill this filed for HW that has no restrictions on it.
> I'll add it in v4.
> 
> > Must application read it in order to know if its datapath should handle
> multi-segment buffers?
> 
> I think it's more appropriate to use scattered_rx to determine if multi-
> segment buffers should be handled.
> 
> >
> > Maybe it will be good to force application to configure scatter when this
> field is valid and smaller than max_rx_pkt_len\max_lro.. (<= room size)...

Can you explain more what is the issue you came to solve?

> >
> >>  } __rte_cache_min_aligned;
> >>
> >>  /**
> 


^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [dpdk-dev] [PATCH v3 1/4] ethdev: add a field for rxq info structure
  2020-09-02  7:19         ` Matan Azrad
@ 2020-09-02 10:01           ` Chengchang Tang
  2020-09-02 10:30             ` Matan Azrad
  2020-09-03 15:00           ` Ferruh Yigit
  1 sibling, 1 reply; 59+ messages in thread
From: Chengchang Tang @ 2020-09-02 10:01 UTC (permalink / raw)
  To: Matan Azrad, dev
  Cc: maryam.tahhan, linuxarm, ferruh.yigit, wenzhuo.lu,
	NBU-Contact-Thomas Monjalon, arybchenko

Hi, Matan

On 2020/9/2 15:19, Matan Azrad wrote:
> 
> Hi Chengchang
> 
> From: Chengchang Tang
>> Hi, Matan
>>
>> On 2020/9/1 23:33, Matan Azrad wrote:
>>>
>>> Hi Chengchang
>>>
>>> Please see some question below.
>>>
>>> From: Chengchang Tang
>>>> Add a field named rx_buf_size in rte_eth_rxq_info to indicate the
>>>> buffer size used in receiving packets for HW.
>>>>
>>>> In this way, upper-layer users can get this information by calling
>>>> rte_eth_rx_queue_info_get.
>>>>
>>>> Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
>>>> Reviewed-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
>>>> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
>>>> ---
>>>>  lib/librte_ethdev/rte_ethdev.h | 2 ++
>>>>  1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/lib/librte_ethdev/rte_ethdev.h
>>>> b/lib/librte_ethdev/rte_ethdev.h index 70295d7..9fed5cb 100644
>>>> --- a/lib/librte_ethdev/rte_ethdev.h
>>>> +++ b/lib/librte_ethdev/rte_ethdev.h
>>>> @@ -1420,6 +1420,8 @@ struct rte_eth_rxq_info {
>>>>         struct rte_eth_rxconf conf; /**< queue config parameters. */
>>>>         uint8_t scattered_rx;       /**< scattered packets RX supported. */
>>>>         uint16_t nb_desc;           /**< configured number of RXDs. */
>>>> +       /**< buffer size used for hardware when receive packets. */
>>>> +       uint16_t rx_buf_size;
>>>
>>> Is it the maximum supported Rx buffer by the HW?
>>> If yes, maybe max_rx_buf_size is better name?
>>
>> No, it is the Rx buffer size currently used by HW.
> 
> Doesn't it defined by the user? Using Rx queue mem-pool mbuf room size?
>
> And it may be different per Rx queue....

Yes, it is defined by user using the Rx queue mem-pool mbuf room size.
When different queues are bound to different mempools, different queues may have different value.
> 
>> IMHO, the structure rte_eth_rxq_info and associated query API are mainly
>> used to query HW configurations at runtime or after queue is
>> configured/setup. Therefore, the content of this structure should be the
>> current HW configuration.
> 
> It looks me more like capabilities...
> The one which define the current configuration is the user by the configuration APIs(after reading the capabilities).

I prefer to consider the structure rte_eth_dev_info as the capabilities. Because rxq_info and
associated APIs are not available until the queue is configured. And the max rx_buf_size is
already exists in dev_info.
>
> I don't think we have here all the current configurations, so what is special in this one?

I think the structure is used to store the queue-related configuration, especially the final HW
configuration that may be different from user configuration and some configurations that are not
mandatory for the user(PMDs will use a default configuration). Such as the scatterred_rx and
rx_drop_en in rte_eth_rxconf, some PMDs will adjust it in some cases based on their HW constraints.

This configuration item meets the above criteria. The value range of rx_buf_size varies according
to HW. Some HW may require 1k-alignment, while others may require several fixed values. So, the PMDs
will configure it based on their HW constraints. This results in difference between the user
configuration and the actual configuration and this value affects the memory usage and performance.
I think there's a need for a way to expose that information.
> 
> 
>>> Maybe document that 0 means - no limitation by HW?
>>
>> Yes, there is no need to fill this filed for HW that has no restrictions on it.
>> I'll add it in v4.
>>
>>> Must application read it in order to know if its datapath should handle
>> multi-segment buffers?
>>
>> I think it's more appropriate to use scattered_rx to determine if multi-
>> segment buffers should be handled.
>>
>>>
>>> Maybe it will be good to force application to configure scatter when this
>> field is valid and smaller than max_rx_pkt_len\max_lro.. (<= room size)...
> 
> Can you explain more what is the issue you came to solve?

This HW information may be useful when there is some problems running a application. This structure
and related APIs can be used to expose it at run time.
> 
>>>
>>>>  } __rte_cache_min_aligned;
>>>>
>>>>  /**
>>
> .
> 


^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [dpdk-dev] [PATCH v3 1/4] ethdev: add a field for rxq info structure
  2020-09-02 10:01           ` Chengchang Tang
@ 2020-09-02 10:30             ` Matan Azrad
  2020-09-03  1:48               ` Chengchang Tang
  0 siblings, 1 reply; 59+ messages in thread
From: Matan Azrad @ 2020-09-02 10:30 UTC (permalink / raw)
  To: Chengchang Tang, dev
  Cc: maryam.tahhan, linuxarm, ferruh.yigit, wenzhuo.lu,
	NBU-Contact-Thomas Monjalon, arybchenko

Hi Chengchang

From: Chengchang Tang
> Hi, Matan
> 
> On 2020/9/2 15:19, Matan Azrad wrote:
> >
> > Hi Chengchang
> >
> > From: Chengchang Tang
> >> Hi, Matan
> >>
> >> On 2020/9/1 23:33, Matan Azrad wrote:
> >>>
> >>> Hi Chengchang
> >>>
> >>> Please see some question below.
> >>>
> >>> From: Chengchang Tang
> >>>> Add a field named rx_buf_size in rte_eth_rxq_info to indicate the
> >>>> buffer size used in receiving packets for HW.
> >>>>
> >>>> In this way, upper-layer users can get this information by calling
> >>>> rte_eth_rx_queue_info_get.
> >>>>
> >>>> Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
> >>>> Reviewed-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
> >>>> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
> >>>> ---
> >>>>  lib/librte_ethdev/rte_ethdev.h | 2 ++
> >>>>  1 file changed, 2 insertions(+)
> >>>>
> >>>> diff --git a/lib/librte_ethdev/rte_ethdev.h
> >>>> b/lib/librte_ethdev/rte_ethdev.h index 70295d7..9fed5cb 100644
> >>>> --- a/lib/librte_ethdev/rte_ethdev.h
> >>>> +++ b/lib/librte_ethdev/rte_ethdev.h
> >>>> @@ -1420,6 +1420,8 @@ struct rte_eth_rxq_info {
> >>>>         struct rte_eth_rxconf conf; /**< queue config parameters. */
> >>>>         uint8_t scattered_rx;       /**< scattered packets RX supported. */
> >>>>         uint16_t nb_desc;           /**< configured number of RXDs. */
> >>>> +       /**< buffer size used for hardware when receive packets. */
> >>>> +       uint16_t rx_buf_size;
> >>>
> >>> Is it the maximum supported Rx buffer by the HW?
> >>> If yes, maybe max_rx_buf_size is better name?
> >>
> >> No, it is the Rx buffer size currently used by HW.

> > Doesn't it defined by the user? Using Rx queue mem-pool mbuf room size?
> >
> > And it may be different per Rx queue....
> 
> Yes, it is defined by user using the Rx queue mem-pool mbuf room size.
> When different queues are bound to different mempools, different queues
> may have different value.
> >
> >> IMHO, the structure rte_eth_rxq_info and associated query API are
> >> mainly used to query HW configurations at runtime or after queue is
> >> configured/setup. Therefore, the content of this structure should be
> >> the current HW configuration.
> >
> > It looks me more like capabilities...
> > The one which define the current configuration is the user by the
> configuration APIs(after reading the capabilities).
> 
> I prefer to consider the structure rte_eth_dev_info as the capabilities.

Yes.

> Because rxq_info and associated APIs are not available until the queue is
> configured. And the max rx_buf_size is already exists in dev_info.
> >
> > I don't think we have here all the current configurations, so what is special
> in this one?
> 
> I think the structure is used to store the queue-related configuration,
> especially the final HW configuration that may be different from user
> configuration and some configurations that are not mandatory for the
> user(PMDs will use a default configuration). Such as the scatterred_rx and
> rx_drop_en in rte_eth_rxconf, some PMDs will adjust it in some cases based
> on their HW constraints.

Ok, this struct(struct rte_eth_rxq_info) is new for me.
Thanks for the explanation.
 
> This configuration item meets the above criteria. The value range of
> rx_buf_size varies according to HW. Some HW may require 1k-alignment,
> while others may require several fixed values. So, the PMDs will configure it
> based on their HW constraints. This results in difference between the user
> configuration and the actual configuration and this value affects the memory
> usage and performance.
> I think there's a need for a way to expose that information.

So the user can configure X and the driver will use Y!=X?

Should the application validate its own configurations after setting them successfully?

> >
> >>> Maybe document that 0 means - no limitation by HW?
> >>
> >> Yes, there is no need to fill this filed for HW that has no restrictions on it.
> >> I'll add it in v4.
> >>
> >>> Must application read it in order to know if its datapath should
> >>> handle
> >> multi-segment buffers?
> >>
> >> I think it's more appropriate to use scattered_rx to determine if
> >> multi- segment buffers should be handled.
> >>
> >>>
> >>> Maybe it will be good to force application to configure scatter when
> >>> this
> >> field is valid and smaller than max_rx_pkt_len\max_lro.. (<= room size)...
> >
> > Can you explain more what is the issue you came to solve?
> 
> This HW information may be useful when there is some problems running a
> application. This structure and related APIs can be used to expose it at run
> time.
> >
OK

^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [dpdk-dev] [PATCH v3 1/4] ethdev: add a field for rxq info structure
  2020-09-02 10:30             ` Matan Azrad
@ 2020-09-03  1:48               ` Chengchang Tang
  2020-09-06 13:45                 ` Matan Azrad
  0 siblings, 1 reply; 59+ messages in thread
From: Chengchang Tang @ 2020-09-03  1:48 UTC (permalink / raw)
  To: Matan Azrad, dev
  Cc: maryam.tahhan, linuxarm, ferruh.yigit, wenzhuo.lu,
	NBU-Contact-Thomas Monjalon, arybchenko

Hi, Matan

On 2020/9/2 18:30, Matan Azrad wrote:
> Hi Chengchang
> 
> From: Chengchang Tang
>> Hi, Matan
>>
>> On 2020/9/2 15:19, Matan Azrad wrote:
>>>
>>> Hi Chengchang
>>>
>>> From: Chengchang Tang
>>>> Hi, Matan
>>>>
>>>> On 2020/9/1 23:33, Matan Azrad wrote:
>>>>>
>>>>> Hi Chengchang
>>>>>
>>>>> Please see some question below.
>>>>>
>>>>> From: Chengchang Tang
>>>>>> Add a field named rx_buf_size in rte_eth_rxq_info to indicate the
>>>>>> buffer size used in receiving packets for HW.
>>>>>>
>>>>>> In this way, upper-layer users can get this information by calling
>>>>>> rte_eth_rx_queue_info_get.
>>>>>>
>>>>>> Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
>>>>>> Reviewed-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
>>>>>> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
>>>>>> ---
>>>>>>  lib/librte_ethdev/rte_ethdev.h | 2 ++
>>>>>>  1 file changed, 2 insertions(+)
>>>>>>
>>>>>> diff --git a/lib/librte_ethdev/rte_ethdev.h
>>>>>> b/lib/librte_ethdev/rte_ethdev.h index 70295d7..9fed5cb 100644
>>>>>> --- a/lib/librte_ethdev/rte_ethdev.h
>>>>>> +++ b/lib/librte_ethdev/rte_ethdev.h
>>>>>> @@ -1420,6 +1420,8 @@ struct rte_eth_rxq_info {
>>>>>>         struct rte_eth_rxconf conf; /**< queue config parameters. */
>>>>>>         uint8_t scattered_rx;       /**< scattered packets RX supported. */
>>>>>>         uint16_t nb_desc;           /**< configured number of RXDs. */
>>>>>> +       /**< buffer size used for hardware when receive packets. */
>>>>>> +       uint16_t rx_buf_size;
>>>>>
>>>>> Is it the maximum supported Rx buffer by the HW?
>>>>> If yes, maybe max_rx_buf_size is better name?
>>>>
>>>> No, it is the Rx buffer size currently used by HW.
> 
>>> Doesn't it defined by the user? Using Rx queue mem-pool mbuf room size?
>>>
>>> And it may be different per Rx queue....
>>
>> Yes, it is defined by user using the Rx queue mem-pool mbuf room size.
>> When different queues are bound to different mempools, different queues
>> may have different value.
>>>
>>>> IMHO, the structure rte_eth_rxq_info and associated query API are
>>>> mainly used to query HW configurations at runtime or after queue is
>>>> configured/setup. Therefore, the content of this structure should be
>>>> the current HW configuration.
>>>
>>> It looks me more like capabilities...
>>> The one which define the current configuration is the user by the
>> configuration APIs(after reading the capabilities).
>>
>> I prefer to consider the structure rte_eth_dev_info as the capabilities.
> 
> Yes.
> 
>> Because rxq_info and associated APIs are not available until the queue is
>> configured. And the max rx_buf_size is already exists in dev_info.
>>>
>>> I don't think we have here all the current configurations, so what is special
>> in this one?
>>
>> I think the structure is used to store the queue-related configuration,
>> especially the final HW configuration that may be different from user
>> configuration and some configurations that are not mandatory for the
>> user(PMDs will use a default configuration). Such as the scatterred_rx and
>> rx_drop_en in rte_eth_rxconf, some PMDs will adjust it in some cases based
>> on their HW constraints.
> 
> Ok, this struct(struct rte_eth_rxq_info) is new for me.
> Thanks for the explanation.
>  
>> This configuration item meets the above criteria. The value range of
>> rx_buf_size varies according to HW. Some HW may require 1k-alignment,
>> while others may require several fixed values. So, the PMDs will configure it
>> based on their HW constraints. This results in difference between the user
>> configuration and the actual configuration and this value affects the memory
>> usage and performance.
>> I think there's a need for a way to expose that information.
> 
> So the user can configure X and the driver will use Y!=X?

Yes, it depends on the HW. In the queue setup API, it just checks whether the input is greater
than the required minimum value. But HW usually has requirements for alignment and so on.
So when X does not meet these requirements, PMDs will calculate a new value Y that meets these
requirements to configure the hardware (Y <= X, to ensure no memory overflow occurs).
> 
> Should the application validate its own configurations after setting them successfully?

It depends on their own needs. The application should not be forced to verify it to avoid affecting
the ease of use of PMDs. For some applications, they don't care about this value.
> 
>>>
>>>>> Maybe document that 0 means - no limitation by HW?
>>>>
>>>> Yes, there is no need to fill this filed for HW that has no restrictions on it.
>>>> I'll add it in v4.
>>>>
>>>>> Must application read it in order to know if its datapath should
>>>>> handle
>>>> multi-segment buffers?
>>>>
>>>> I think it's more appropriate to use scattered_rx to determine if
>>>> multi- segment buffers should be handled.
>>>>
>>>>>
>>>>> Maybe it will be good to force application to configure scatter when
>>>>> this
>>>> field is valid and smaller than max_rx_pkt_len\max_lro.. (<= room size)...
>>>
>>> Can you explain more what is the issue you came to solve?
>>
>> This HW information may be useful when there is some problems running a
>> application. This structure and related APIs can be used to expose it at run
>> time.
>>>
> OK
> 
> .
> 


^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [dpdk-dev] [PATCH v3 1/4] ethdev: add a field for rxq info structure
  2020-09-02  3:52       ` Chengchang Tang
  2020-09-02  7:19         ` Matan Azrad
@ 2020-09-03 14:55         ` Ferruh Yigit
  1 sibling, 0 replies; 59+ messages in thread
From: Ferruh Yigit @ 2020-09-03 14:55 UTC (permalink / raw)
  To: Chengchang Tang, Matan Azrad, dev
  Cc: maryam.tahhan, linuxarm, wenzhuo.lu, NBU-Contact-Thomas Monjalon,
	arybchenko

On 9/2/2020 4:52 AM, Chengchang Tang wrote:
> Hi, Matan
> 
> On 2020/9/1 23:33, Matan Azrad wrote:
>>
>> Hi Chengchang
>>
>> Please see some question below.
>>
>> From: Chengchang Tang
>>> Add a field named rx_buf_size in rte_eth_rxq_info to indicate the buffer size
>>> used in receiving packets for HW.
>>>
>>> In this way, upper-layer users can get this information by calling
>>> rte_eth_rx_queue_info_get.
>>>
>>> Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
>>> Reviewed-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
>>> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
>>> ---
>>>  lib/librte_ethdev/rte_ethdev.h | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
>>> index 70295d7..9fed5cb 100644
>>> --- a/lib/librte_ethdev/rte_ethdev.h
>>> +++ b/lib/librte_ethdev/rte_ethdev.h
>>> @@ -1420,6 +1420,8 @@ struct rte_eth_rxq_info {
>>>         struct rte_eth_rxconf conf; /**< queue config parameters. */
>>>         uint8_t scattered_rx;       /**< scattered packets RX supported. */
>>>         uint16_t nb_desc;           /**< configured number of RXDs. */
>>> +       /**< buffer size used for hardware when receive packets. */
>>> +       uint16_t rx_buf_size;
>>
>> Is it the maximum supported Rx buffer by the HW?
>> If yes, maybe max_rx_buf_size is better name?
> 
> No, it is the Rx buffer size currently used by HW.
> 
> IMHO, the structure rte_eth_rxq_info and associated query API are mainly used to query HW configurations
> at runtime or after queue is configured/setup. Therefore, the content of this structure should be the
> current HW configuration.
> 
>> Maybe document that 0 means - no limitation by HW?
> 
> Yes, there is no need to fill this filed for HW that has no restrictions on it.
> I'll add it in v4.

This field in not for max Rx buffer size, so HW restriction doesn't really apply
here.
But as far as I remember it has been decided that the field will be optional for
PMDs, agree to document that part.

> 
>> Must application read it in order to know if its datapath should handle multi-segment buffers?
> 
> I think it's more appropriate to use scattered_rx to determine if multi-segment buffers should be handled.
> 
>>
>> Maybe it will be good to force application to configure scatter when this field is valid and smaller than max_rx_pkt_len\max_lro.. (<= room size)...
>>
>>
>>>  } __rte_cache_min_aligned;
>>>
>>>  /**
> 
> 


^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [dpdk-dev] [PATCH v3 1/4] ethdev: add a field for rxq info structure
  2020-09-02  7:19         ` Matan Azrad
  2020-09-02 10:01           ` Chengchang Tang
@ 2020-09-03 15:00           ` Ferruh Yigit
  1 sibling, 0 replies; 59+ messages in thread
From: Ferruh Yigit @ 2020-09-03 15:00 UTC (permalink / raw)
  To: Matan Azrad, Chengchang Tang, dev
  Cc: maryam.tahhan, linuxarm, wenzhuo.lu, NBU-Contact-Thomas Monjalon,
	arybchenko

On 9/2/2020 8:19 AM, Matan Azrad wrote:
> 
> Hi Chengchang
> 
> From: Chengchang Tang
>> Hi, Matan
>>
>> On 2020/9/1 23:33, Matan Azrad wrote:
>>>
>>> Hi Chengchang
>>>
>>> Please see some question below.
>>>
>>> From: Chengchang Tang
>>>> Add a field named rx_buf_size in rte_eth_rxq_info to indicate the
>>>> buffer size used in receiving packets for HW.
>>>>
>>>> In this way, upper-layer users can get this information by calling
>>>> rte_eth_rx_queue_info_get.
>>>>
>>>> Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
>>>> Reviewed-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
>>>> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
>>>> ---
>>>>  lib/librte_ethdev/rte_ethdev.h | 2 ++
>>>>  1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/lib/librte_ethdev/rte_ethdev.h
>>>> b/lib/librte_ethdev/rte_ethdev.h index 70295d7..9fed5cb 100644
>>>> --- a/lib/librte_ethdev/rte_ethdev.h
>>>> +++ b/lib/librte_ethdev/rte_ethdev.h
>>>> @@ -1420,6 +1420,8 @@ struct rte_eth_rxq_info {
>>>>         struct rte_eth_rxconf conf; /**< queue config parameters. */
>>>>         uint8_t scattered_rx;       /**< scattered packets RX supported. */
>>>>         uint16_t nb_desc;           /**< configured number of RXDs. */
>>>> +       /**< buffer size used for hardware when receive packets. */
>>>> +       uint16_t rx_buf_size;
>>>
>>> Is it the maximum supported Rx buffer by the HW?
>>> If yes, maybe max_rx_buf_size is better name?
>>
>> No, it is the Rx buffer size currently used by HW.
> 
> Doesn't it defined by the user? Using Rx queue mem-pool mbuf room size?
> 
> And it may be different per Rx queue....

There is no explicit configuration for Rx buffer size, PMDs does as you said
above and set mbuf data size as Rx buffer size, but this is not a defined rule,
technically PMD is free to select any size smaller than mbuf data size as their
Rx buffer size.
And this new field is to feed this configured Rx buffer size information back to
application.

> 
>> IMHO, the structure rte_eth_rxq_info and associated query API are mainly
>> used to query HW configurations at runtime or after queue is
>> configured/setup. Therefore, the content of this structure should be the
>> current HW configuration.
> 
> It looks me more like capabilities...
> The one which define the current configuration is the user by the configuration APIs(after reading the capabilities).
> 
> I don't think we have here all the current configurations, so what is special in this one?
> 
> 
>>> Maybe document that 0 means - no limitation by HW?
>>
>> Yes, there is no need to fill this filed for HW that has no restrictions on it.
>> I'll add it in v4.
>>
>>> Must application read it in order to know if its datapath should handle
>> multi-segment buffers?
>>
>> I think it's more appropriate to use scattered_rx to determine if multi-
>> segment buffers should be handled.
>>
>>>
>>> Maybe it will be good to force application to configure scatter when this
>> field is valid and smaller than max_rx_pkt_len\max_lro.. (<= room size)...
> 
> Can you explain more what is the issue you came to solve?
> 
>>>
>>>>  } __rte_cache_min_aligned;
>>>>
>>>>  /**
>>
> 


^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [dpdk-dev] [PATCH v3 1/4] ethdev: add a field for rxq info structure
  2020-08-29  7:13   ` [dpdk-dev] [PATCH v3 1/4] ethdev: add a field " Chengchang Tang
  2020-09-01 15:33     ` Matan Azrad
@ 2020-09-03 15:01     ` Ferruh Yigit
  2020-09-04  1:43       ` Chengchang Tang
  2020-09-03 15:35     ` Bruce Richardson
  2 siblings, 1 reply; 59+ messages in thread
From: Ferruh Yigit @ 2020-09-03 15:01 UTC (permalink / raw)
  To: Chengchang Tang, dev
  Cc: linuxarm, thomas, arybchenko, wenzhuo.lu, maryam.tahhan

On 8/29/2020 8:13 AM, Chengchang Tang wrote:
> Add a field named rx_buf_size in rte_eth_rxq_info to indicate the buffer
> size used in receiving packets for HW.
> 
> In this way, upper-layer users can get this information by calling
> rte_eth_rx_queue_info_get.
> 
> Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
> Reviewed-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
> ---
>  lib/librte_ethdev/rte_ethdev.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
> index 70295d7..9fed5cb 100644
> --- a/lib/librte_ethdev/rte_ethdev.h
> +++ b/lib/librte_ethdev/rte_ethdev.h
> @@ -1420,6 +1420,8 @@ struct rte_eth_rxq_info {
>  	struct rte_eth_rxconf conf; /**< queue config parameters. */
>  	uint8_t scattered_rx;       /**< scattered packets RX supported. */
>  	uint16_t nb_desc;           /**< configured number of RXDs. */
> +	/**< buffer size used for hardware when receive packets. */
> +	uint16_t rx_buf_size;
>  } __rte_cache_min_aligned;
> 
>  /**
> --
> 2.7.4
> 

Can you please removed the relevant deprecation notice in this patch?

^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [dpdk-dev] [PATCH v3 1/4] ethdev: add a field for rxq info structure
  2020-08-29  7:13   ` [dpdk-dev] [PATCH v3 1/4] ethdev: add a field " Chengchang Tang
  2020-09-01 15:33     ` Matan Azrad
  2020-09-03 15:01     ` Ferruh Yigit
@ 2020-09-03 15:35     ` Bruce Richardson
  2020-09-04 14:25       ` Ferruh Yigit
  2 siblings, 1 reply; 59+ messages in thread
From: Bruce Richardson @ 2020-09-03 15:35 UTC (permalink / raw)
  To: Chengchang Tang
  Cc: dev, linuxarm, thomas, arybchenko, ferruh.yigit, wenzhuo.lu,
	maryam.tahhan

On Sat, Aug 29, 2020 at 03:13:16PM +0800, Chengchang Tang wrote:
> Add a field named rx_buf_size in rte_eth_rxq_info to indicate the buffer
> size used in receiving packets for HW.
> 
> In this way, upper-layer users can get this information by calling
> rte_eth_rx_queue_info_get.
> 
> Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
> Reviewed-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
> ---
>  lib/librte_ethdev/rte_ethdev.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
> index 70295d7..9fed5cb 100644
> --- a/lib/librte_ethdev/rte_ethdev.h
> +++ b/lib/librte_ethdev/rte_ethdev.h
> @@ -1420,6 +1420,8 @@ struct rte_eth_rxq_info {
>  	struct rte_eth_rxconf conf; /**< queue config parameters. */
>  	uint8_t scattered_rx;       /**< scattered packets RX supported. */
>  	uint16_t nb_desc;           /**< configured number of RXDs. */
> +	/**< buffer size used for hardware when receive packets. */
> +	uint16_t rx_buf_size;
>  } __rte_cache_min_aligned;
> 
Since this is breaking the ABI, this looks like the perfect opportunity to
add in a qinfo_size parameter to rte_eth_rx_queue_info_get() call which
allows ABI sanity-checking. Also, if passed through to the individual
drivers, allows them to make ABI determinations since driver functions
cannot be versioned, i.e. the driver info function cannot know whether it
has been called by queue_info_v21 or queue_info_v22.

/Bruce

^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [dpdk-dev] [PATCH v3 1/4] ethdev: add a field for rxq info structure
  2020-09-03 15:01     ` Ferruh Yigit
@ 2020-09-04  1:43       ` Chengchang Tang
  0 siblings, 0 replies; 59+ messages in thread
From: Chengchang Tang @ 2020-09-04  1:43 UTC (permalink / raw)
  To: Ferruh Yigit, dev; +Cc: linuxarm, thomas, arybchenko, wenzhuo.lu, maryam.tahhan



On 2020/9/3 23:01, Ferruh Yigit wrote:
> On 8/29/2020 8:13 AM, Chengchang Tang wrote:
>> Add a field named rx_buf_size in rte_eth_rxq_info to indicate the buffer
>> size used in receiving packets for HW.
>>
>> In this way, upper-layer users can get this information by calling
>> rte_eth_rx_queue_info_get.
>>
>> Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
>> Reviewed-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
>> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
>> ---
>>  lib/librte_ethdev/rte_ethdev.h | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
>> index 70295d7..9fed5cb 100644
>> --- a/lib/librte_ethdev/rte_ethdev.h
>> +++ b/lib/librte_ethdev/rte_ethdev.h
>> @@ -1420,6 +1420,8 @@ struct rte_eth_rxq_info {
>>  	struct rte_eth_rxconf conf; /**< queue config parameters. */
>>  	uint8_t scattered_rx;       /**< scattered packets RX supported. */
>>  	uint16_t nb_desc;           /**< configured number of RXDs. */
>> +	/**< buffer size used for hardware when receive packets. */
>> +	uint16_t rx_buf_size;
>>  } __rte_cache_min_aligned;
>>
>>  /**
>> --
>> 2.7.4
>>
> 
> Can you please removed the relevant deprecation notice in this patch?

OK, I will remove it in v4
> .
> 


^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [dpdk-dev] [PATCH v3 1/4] ethdev: add a field for rxq info structure
  2020-09-03 15:35     ` Bruce Richardson
@ 2020-09-04 14:25       ` Ferruh Yigit
  2020-09-04 15:14         ` Bruce Richardson
  2020-09-04 15:30         ` Bruce Richardson
  0 siblings, 2 replies; 59+ messages in thread
From: Ferruh Yigit @ 2020-09-04 14:25 UTC (permalink / raw)
  To: Bruce Richardson, Chengchang Tang
  Cc: dev, linuxarm, thomas, arybchenko, wenzhuo.lu, maryam.tahhan

On 9/3/2020 4:35 PM, Bruce Richardson wrote:
> On Sat, Aug 29, 2020 at 03:13:16PM +0800, Chengchang Tang wrote:
>> Add a field named rx_buf_size in rte_eth_rxq_info to indicate the buffer
>> size used in receiving packets for HW.
>>
>> In this way, upper-layer users can get this information by calling
>> rte_eth_rx_queue_info_get.
>>
>> Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
>> Reviewed-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
>> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
>> ---
>>  lib/librte_ethdev/rte_ethdev.h | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
>> index 70295d7..9fed5cb 100644
>> --- a/lib/librte_ethdev/rte_ethdev.h
>> +++ b/lib/librte_ethdev/rte_ethdev.h
>> @@ -1420,6 +1420,8 @@ struct rte_eth_rxq_info {
>>  	struct rte_eth_rxconf conf; /**< queue config parameters. */
>>  	uint8_t scattered_rx;       /**< scattered packets RX supported. */
>>  	uint16_t nb_desc;           /**< configured number of RXDs. */
>> +	/**< buffer size used for hardware when receive packets. */
>> +	uint16_t rx_buf_size;
>>  } __rte_cache_min_aligned;
>>
> Since this is breaking the ABI, this looks like the perfect opportunity to
> add in a qinfo_size parameter to rte_eth_rx_queue_info_get() call which
> allows ABI sanity-checking. Also, if passed through to the individual
> drivers, allows them to make ABI determinations since driver functions
> cannot be versioned, i.e. the driver info function cannot know whether it
> has been called by queue_info_v21 or queue_info_v22.
> 

Current approach we have is to detect the ABI breakage before release and either
block it or use ABI versioning.
What you suggest to add runtime checks assumes application can call old and new
APIs, which is not our case and not sure what this runtime check adds.

The option to use runtime check between library and driver can be needed if
library and driver from different DPDK versions can be used together, I think
enabling this can bring more complexity and better to use all components of a
DPDK release together.

And I think this kind of runtime checks should be discussed for all or none
APIs, instead of having them only for some APIs.

^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [dpdk-dev] [PATCH v3 1/4] ethdev: add a field for rxq info structure
  2020-09-04 14:25       ` Ferruh Yigit
@ 2020-09-04 15:14         ` Bruce Richardson
  2020-09-04 15:30         ` Bruce Richardson
  1 sibling, 0 replies; 59+ messages in thread
From: Bruce Richardson @ 2020-09-04 15:14 UTC (permalink / raw)
  To: Ferruh Yigit
  Cc: Chengchang Tang, dev, linuxarm, thomas, arybchenko, wenzhuo.lu,
	maryam.tahhan

On Fri, Sep 04, 2020 at 03:25:26PM +0100, Ferruh Yigit wrote:
> On 9/3/2020 4:35 PM, Bruce Richardson wrote:
> > On Sat, Aug 29, 2020 at 03:13:16PM +0800, Chengchang Tang wrote:
> >> Add a field named rx_buf_size in rte_eth_rxq_info to indicate the buffer
> >> size used in receiving packets for HW.
> >>
> >> In this way, upper-layer users can get this information by calling
> >> rte_eth_rx_queue_info_get.
> >>
> >> Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
> >> Reviewed-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
> >> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
> >> ---
> >>  lib/librte_ethdev/rte_ethdev.h | 2 ++
> >>  1 file changed, 2 insertions(+)
> >>
> >> diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
> >> index 70295d7..9fed5cb 100644
> >> --- a/lib/librte_ethdev/rte_ethdev.h
> >> +++ b/lib/librte_ethdev/rte_ethdev.h
> >> @@ -1420,6 +1420,8 @@ struct rte_eth_rxq_info {
> >>  	struct rte_eth_rxconf conf; /**< queue config parameters. */
> >>  	uint8_t scattered_rx;       /**< scattered packets RX supported. */
> >>  	uint16_t nb_desc;           /**< configured number of RXDs. */
> >> +	/**< buffer size used for hardware when receive packets. */
> >> +	uint16_t rx_buf_size;
> >>  } __rte_cache_min_aligned;
> >>
> > Since this is breaking the ABI, this looks like the perfect opportunity to
> > add in a qinfo_size parameter to rte_eth_rx_queue_info_get() call which
> > allows ABI sanity-checking. Also, if passed through to the individual
> > drivers, allows them to make ABI determinations since driver functions
> > cannot be versioned, i.e. the driver info function cannot know whether it
> > has been called by queue_info_v21 or queue_info_v22.
> > 
> 
> Current approach we have is to detect the ABI breakage before release and either
> block it or use ABI versioning.
> What you suggest to add runtime checks assumes application can call old and new
> APIs, which is not our case and not sure what this runtime check adds.
> 
> The option to use runtime check between library and driver can be needed if
> library and driver from different DPDK versions can be used together, I think
> enabling this can bring more complexity and better to use all components of a
> DPDK release together.
> 
> And I think this kind of runtime checks should be discussed for all or none
> APIs, instead of having them only for some APIs.

No the main concern here is to avoid the kind of ABI issues that hit the
crypto tree in a recent release. Passing the length of the array around
would actually make the regular versioning simpler (as I think we wouldn't
need to use the fancy linker versioning), but the main benefit is if that
info can be passed through to the driver layer, since driver functions
called via ethdev cannot be versioned.

/Bruce

^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [dpdk-dev] [PATCH v3 1/4] ethdev: add a field for rxq info structure
  2020-09-04 14:25       ` Ferruh Yigit
  2020-09-04 15:14         ` Bruce Richardson
@ 2020-09-04 15:30         ` Bruce Richardson
  1 sibling, 0 replies; 59+ messages in thread
From: Bruce Richardson @ 2020-09-04 15:30 UTC (permalink / raw)
  To: Ferruh Yigit
  Cc: Chengchang Tang, dev, linuxarm, thomas, arybchenko, wenzhuo.lu,
	maryam.tahhan

On Fri, Sep 04, 2020 at 03:25:26PM +0100, Ferruh Yigit wrote:
> On 9/3/2020 4:35 PM, Bruce Richardson wrote:
> > On Sat, Aug 29, 2020 at 03:13:16PM +0800, Chengchang Tang wrote:
> >> Add a field named rx_buf_size in rte_eth_rxq_info to indicate the buffer
> >> size used in receiving packets for HW.
> >>
> >> In this way, upper-layer users can get this information by calling
> >> rte_eth_rx_queue_info_get.
> >>
> >> Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
> >> Reviewed-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
> >> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
> >> ---
> >>  lib/librte_ethdev/rte_ethdev.h | 2 ++
> >>  1 file changed, 2 insertions(+)
> >>
> >> diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
> >> index 70295d7..9fed5cb 100644
> >> --- a/lib/librte_ethdev/rte_ethdev.h
> >> +++ b/lib/librte_ethdev/rte_ethdev.h
> >> @@ -1420,6 +1420,8 @@ struct rte_eth_rxq_info {
> >>  	struct rte_eth_rxconf conf; /**< queue config parameters. */
> >>  	uint8_t scattered_rx;       /**< scattered packets RX supported. */
> >>  	uint16_t nb_desc;           /**< configured number of RXDs. */
> >> +	/**< buffer size used for hardware when receive packets. */
> >> +	uint16_t rx_buf_size;
> >>  } __rte_cache_min_aligned;
> >>
> > Since this is breaking the ABI, this looks like the perfect opportunity to
> > add in a qinfo_size parameter to rte_eth_rx_queue_info_get() call which
> > allows ABI sanity-checking. Also, if passed through to the individual
> > drivers, allows them to make ABI determinations since driver functions
> > cannot be versioned, i.e. the driver info function cannot know whether it
> > has been called by queue_info_v21 or queue_info_v22.
> > 
> 
> Current approach we have is to detect the ABI breakage before release and either
> block it or use ABI versioning.
> What you suggest to add runtime checks assumes application can call old and new
> APIs, which is not our case and not sure what this runtime check adds.
> 
> The option to use runtime check between library and driver can be needed if
> library and driver from different DPDK versions can be used together, I think
> enabling this can bring more complexity and better to use all components of a
> DPDK release together.

Allow me to explain a bit further. For example, we add a new field to the
end of the configuration struct or the info struct, which changes the size
of the structure. We use function versioning to allow old and new ethdev
config APIs. The new API function just works passing the structure through
to the driver API. However, the old API implementation has a problem,
because it has just been passed a structure that is too small for use by
the driver functions, so it can't just pass it along. Therefore, the code
has two choices (that I am aware of):

1. allocate a new structure of the correct size, and call the driver API
with that. Once it returns, copy the fields over to the old structure to
return to the user.
2. create a new function pointer in the ops array for legacy compatibility
and have each driver implement a function to work with the old structure.

The alternative scheme is just to pass around the structure size. The
configure ethdev API takes the pointer and size, and does not need changing
if it does not use any new fields, just passes them through to the driver
configure API. Similarly there, any drivers that don't access the new
fields don't need changing, while those that do access them just need to
check the struct size before doing so to see what version of the structure
has been received.

> 
> And I think this kind of runtime checks should be discussed for all or none
> APIs, instead of having them only for some APIs.

Yes, I agree with that.
Incidentally, this same idea was covered in the context of system calls at
the linux plumbers conf:
https://linuxplumbersconf.org/event/7/contributions/657/attachments/639/1159/extensible_syscalls.pdf

Regards,
/Bruce

^ permalink raw reply	[flat|nested] 59+ messages in thread

* [dpdk-dev] [PATCH v4 0/5] add Rx buffer size for rxq info structure
  2020-06-18 12:35 [dpdk-dev] [RFC] ethdev: add a field for rte_eth_rxq_info Chengchang Tang
                   ` (2 preceding siblings ...)
  2020-08-29  7:13 ` [dpdk-dev] [PATCH v3 0/4] add Rx buffer size for rxq info structure Chengchang Tang
@ 2020-09-05  9:07 ` Chengchang Tang
  2020-09-05  9:07   ` [dpdk-dev] [PATCH v4 1/5] ethdev: add a field " Chengchang Tang
                     ` (4 more replies)
  2020-09-21 13:22 ` [dpdk-dev] [PATCH v5 0/2] add Rx buffer size for rxq info structure Chengchang Tang
  4 siblings, 5 replies; 59+ messages in thread
From: Chengchang Tang @ 2020-09-05  9:07 UTC (permalink / raw)
  To: dev; +Cc: linuxarm, thomas, arybchenko, ferruh.yigit, wenzhuo.lu, maryam.tahhan

In common practice, PMD configure the Rx buffer size which indicate the
buffer length could be used for HW in receiving packets according to the
data room size of the object in mempool. But in fact, the final value is
related to the specifications of HW, and its values will affect the number
of fragments in receiving packets when scatter is enabled. By the way,
some PMDs may force to enable scatter when the MTU is bigger than the HW
Rx buffer size.

At present, we have no way to expose relevant information to upper layer
users. So, add a field named rx_buf_size in rte_eth_rxq_info to indicate
the buffer size used in receiving packets for HW. And this field is
optional, so there is no need to forcibly update all PMDs.

This patchset also update testpmd, proc-info tools and add hns3 PMD
implementation.

v4:
  - remove deprecation notice once changes applied

Chengchang Tang (5):
  ethdev: add a field for rxq info structure
  app/testpmd: add Rx buffer size display in queue info query
  app/procinfo: add Rx buffer size to --show-port
  net/hns3: add Rx buffer size to Rx qinfo query
  doc: remove rxq info structure deprecation notice

 app/proc-info/main.c                 | 2 ++
 app/test-pmd/config.c                | 1 +
 doc/guides/rel_notes/deprecation.rst | 5 -----
 drivers/net/hns3/hns3_rxtx.c         | 2 ++
 lib/librte_ethdev/rte_ethdev.h       | 2 ++
 5 files changed, 7 insertions(+), 5 deletions(-)

--
2.7.4


^ permalink raw reply	[flat|nested] 59+ messages in thread

* [dpdk-dev] [PATCH v4 1/5] ethdev: add a field for rxq info structure
  2020-09-05  9:07 ` [dpdk-dev] [PATCH v4 0/5] add Rx buffer size for rxq info structure Chengchang Tang
@ 2020-09-05  9:07   ` Chengchang Tang
  2020-09-05 16:50     ` Stephen Hemminger
  2020-09-05  9:07   ` [dpdk-dev] [PATCH v4 2/5] app/testpmd: add Rx buffer size display in queue info query Chengchang Tang
                     ` (3 subsequent siblings)
  4 siblings, 1 reply; 59+ messages in thread
From: Chengchang Tang @ 2020-09-05  9:07 UTC (permalink / raw)
  To: dev; +Cc: linuxarm, thomas, arybchenko, ferruh.yigit, wenzhuo.lu, maryam.tahhan

Add a field named rx_buf_size in rte_eth_rxq_info to indicate the buffer
size used in receiving packets for HW.

In this way, upper-layer users can get this information by calling
rte_eth_rx_queue_info_get.

Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
Reviewed-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 lib/librte_ethdev/rte_ethdev.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index 70295d7..859111a 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -1420,6 +1420,8 @@ struct rte_eth_rxq_info {
 	struct rte_eth_rxconf conf; /**< queue config parameters. */
 	uint8_t scattered_rx;       /**< scattered packets RX supported. */
 	uint16_t nb_desc;           /**< configured number of RXDs. */
+	/**< Buffer size used for HW when receive packets. */
+	uint16_t rx_buf_size;
 } __rte_cache_min_aligned;

 /**
--
2.7.4


^ permalink raw reply	[flat|nested] 59+ messages in thread

* [dpdk-dev] [PATCH v4 2/5] app/testpmd: add Rx buffer size display in queue info query
  2020-09-05  9:07 ` [dpdk-dev] [PATCH v4 0/5] add Rx buffer size for rxq info structure Chengchang Tang
  2020-09-05  9:07   ` [dpdk-dev] [PATCH v4 1/5] ethdev: add a field " Chengchang Tang
@ 2020-09-05  9:07   ` Chengchang Tang
  2020-09-18  8:54     ` Ferruh Yigit
  2020-09-05  9:07   ` [dpdk-dev] [PATCH v4 3/5] app/procinfo: add Rx buffer size to --show-port Chengchang Tang
                     ` (2 subsequent siblings)
  4 siblings, 1 reply; 59+ messages in thread
From: Chengchang Tang @ 2020-09-05  9:07 UTC (permalink / raw)
  To: dev; +Cc: linuxarm, thomas, arybchenko, ferruh.yigit, wenzhuo.lu, maryam.tahhan

Add Rx buffer size to queue info querry cmd so that the user can get the
buffer length used by HW queue for receiving packets.

Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
Reviewed-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
---
 app/test-pmd/config.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 30bee33..b432ac6 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -452,6 +452,7 @@ rx_queue_infos_display(portid_t port_id, uint16_t queue_id)
 		(qinfo.conf.rx_deferred_start != 0) ? "on" : "off");
 	printf("\nRX scattered packets: %s",
 		(qinfo.scattered_rx != 0) ? "on" : "off");
+	printf("\nRX buffer size: %hu", qinfo.rx_buf_size);
 	printf("\nNumber of RXDs: %hu", qinfo.nb_desc);

 	if (rte_eth_rx_burst_mode_get(port_id, queue_id, &mode) == 0)
--
2.7.4


^ permalink raw reply	[flat|nested] 59+ messages in thread

* [dpdk-dev] [PATCH v4 3/5] app/procinfo: add Rx buffer size to --show-port
  2020-09-05  9:07 ` [dpdk-dev] [PATCH v4 0/5] add Rx buffer size for rxq info structure Chengchang Tang
  2020-09-05  9:07   ` [dpdk-dev] [PATCH v4 1/5] ethdev: add a field " Chengchang Tang
  2020-09-05  9:07   ` [dpdk-dev] [PATCH v4 2/5] app/testpmd: add Rx buffer size display in queue info query Chengchang Tang
@ 2020-09-05  9:07   ` Chengchang Tang
  2020-09-05 16:59     ` Stephen Hemminger
  2020-09-05  9:07   ` [dpdk-dev] [PATCH v4 4/5] net/hns3: add Rx buffer size to Rx qinfo query Chengchang Tang
  2020-09-05  9:07   ` [dpdk-dev] [PATCH v4 5/5] doc: remove rxq info structure deprecation notice Chengchang Tang
  4 siblings, 1 reply; 59+ messages in thread
From: Chengchang Tang @ 2020-09-05  9:07 UTC (permalink / raw)
  To: dev; +Cc: linuxarm, thomas, arybchenko, ferruh.yigit, wenzhuo.lu, maryam.tahhan

Add Rx buffer size to show_port function to display it in the port PMD
information so that the user can get the buffer length used by HW queue
of receiving packets.

Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
---
 app/proc-info/main.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/app/proc-info/main.c b/app/proc-info/main.c
index abeca4a..e840dea 100644
--- a/app/proc-info/main.c
+++ b/app/proc-info/main.c
@@ -711,11 +711,13 @@ show_port(void)
 			if (ret == 0) {
 				printf("\t  -- queue %d rx scatter %d"
 						" descriptors %d"
+						" rx buffer size %d"
 						" offloads 0x%"PRIx64
 						" mempool socket %d\n",
 						j,
 						queue_info.scattered_rx,
 						queue_info.nb_desc,
+						queue_info.rx_buf_size,
 						queue_info.conf.offloads,
 						queue_info.mp->socket_id);
 			}
--
2.7.4


^ permalink raw reply	[flat|nested] 59+ messages in thread

* [dpdk-dev] [PATCH v4 4/5] net/hns3: add Rx buffer size to Rx qinfo query
  2020-09-05  9:07 ` [dpdk-dev] [PATCH v4 0/5] add Rx buffer size for rxq info structure Chengchang Tang
                     ` (2 preceding siblings ...)
  2020-09-05  9:07   ` [dpdk-dev] [PATCH v4 3/5] app/procinfo: add Rx buffer size to --show-port Chengchang Tang
@ 2020-09-05  9:07   ` Chengchang Tang
  2020-09-05  9:07   ` [dpdk-dev] [PATCH v4 5/5] doc: remove rxq info structure deprecation notice Chengchang Tang
  4 siblings, 0 replies; 59+ messages in thread
From: Chengchang Tang @ 2020-09-05  9:07 UTC (permalink / raw)
  To: dev; +Cc: linuxarm, thomas, arybchenko, ferruh.yigit, wenzhuo.lu, maryam.tahhan

Report hns3 PMD configured Rx buffer size in Rx queue information query.

Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
Reviewed-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
---
 drivers/net/hns3/hns3_rxtx.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/hns3/hns3_rxtx.c b/drivers/net/hns3/hns3_rxtx.c
index fc1a256..d67c5ad 100644
--- a/drivers/net/hns3/hns3_rxtx.c
+++ b/drivers/net/hns3/hns3_rxtx.c
@@ -2830,6 +2830,8 @@ hns3_rxq_info_get(struct rte_eth_dev *dev, uint16_t queue_id,
 	qinfo->mp = rxq->mb_pool;
 	qinfo->nb_desc = rxq->nb_rx_desc;
 	qinfo->scattered_rx = dev->data->scattered_rx;
+	/* Report the HW Rx buffer length to user */
+	qinfo->rx_buf_size = rxq->rx_buf_len;

 	/*
 	 * If there are no available Rx buffer descriptors, incoming packets
--
2.7.4


^ permalink raw reply	[flat|nested] 59+ messages in thread

* [dpdk-dev] [PATCH v4 5/5] doc: remove rxq info structure deprecation notice
  2020-09-05  9:07 ` [dpdk-dev] [PATCH v4 0/5] add Rx buffer size for rxq info structure Chengchang Tang
                     ` (3 preceding siblings ...)
  2020-09-05  9:07   ` [dpdk-dev] [PATCH v4 4/5] net/hns3: add Rx buffer size to Rx qinfo query Chengchang Tang
@ 2020-09-05  9:07   ` Chengchang Tang
  2020-09-05 16:33     ` Thomas Monjalon
  4 siblings, 1 reply; 59+ messages in thread
From: Chengchang Tang @ 2020-09-05  9:07 UTC (permalink / raw)
  To: dev; +Cc: linuxarm, thomas, arybchenko, ferruh.yigit, wenzhuo.lu, maryam.tahhan

The change has been applied, so remove the notice.

Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
---
 doc/guides/rel_notes/deprecation.rst | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 345c38d..b6d57b9 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -215,11 +215,6 @@ Deprecation Notices
   specified lengths into the buffers allocated from the specified
   memory pools. The backward compatibility to existing API is preserved.

-* ethdev: The ``struct rte_eth_rxq_info`` will be modified to include
-  a new optional field, indicating the buffer size used in receiving packets
-  for HW. This change is planned for 20.11. For more details:
-  https://mails.dpdk.org/archives/dev/2020-July/176135.html.
-
 * ethdev: ``rx_descriptor_done`` dev_ops and ``rte_eth_rx_descriptor_done``
   will be deprecated in 20.11 and will be removed in 21.11.
   Existing ``rte_eth_rx_descriptor_status`` and ``rte_eth_tx_descriptor_status``
--
2.7.4


^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [dpdk-dev] [PATCH v4 5/5] doc: remove rxq info structure deprecation notice
  2020-09-05  9:07   ` [dpdk-dev] [PATCH v4 5/5] doc: remove rxq info structure deprecation notice Chengchang Tang
@ 2020-09-05 16:33     ` Thomas Monjalon
  2020-09-07  9:12       ` Chengchang Tang
  0 siblings, 1 reply; 59+ messages in thread
From: Thomas Monjalon @ 2020-09-05 16:33 UTC (permalink / raw)
  To: Chengchang Tang
  Cc: dev, linuxarm, arybchenko, ferruh.yigit, wenzhuo.lu, maryam.tahhan

05/09/2020 11:07, Chengchang Tang:
> The change has been applied, so remove the notice.

It should be atomic with the patch doing the change,
i.e. you can squash.

I think the oneline patches in testpmd and procinfo
can probably be squashed with the ethdev patch as well.

Thanks



^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [dpdk-dev] [PATCH v4 1/5] ethdev: add a field for rxq info structure
  2020-09-05  9:07   ` [dpdk-dev] [PATCH v4 1/5] ethdev: add a field " Chengchang Tang
@ 2020-09-05 16:50     ` Stephen Hemminger
  0 siblings, 0 replies; 59+ messages in thread
From: Stephen Hemminger @ 2020-09-05 16:50 UTC (permalink / raw)
  To: Chengchang Tang
  Cc: dev, linuxarm, thomas, arybchenko, ferruh.yigit, wenzhuo.lu,
	maryam.tahhan

On Sat, 5 Sep 2020 17:07:30 +0800
Chengchang Tang <tangchengchang@huawei.com> wrote:

> diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
> index 70295d7..859111a 100644
> --- a/lib/librte_ethdev/rte_ethdev.h
> +++ b/lib/librte_ethdev/rte_ethdev.h
> @@ -1420,6 +1420,8 @@ struct rte_eth_rxq_info {
>  	struct rte_eth_rxconf conf; /**< queue config parameters. */
>  	uint8_t scattered_rx;       /**< scattered packets RX supported. */
>  	uint16_t nb_desc;           /**< configured number of RXDs. */
> +	/**< Buffer size used for HW when receive packets. */
> +	uint16_t rx_buf_size;

Minor suggestion.
If you shorten the comment (which is not that useful) and use the same
case, then it will fit on the same line and look like the rest of the code.

	uint16_t rx_buf_size;      /**< hardware receive buffer size */

^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [dpdk-dev] [PATCH v4 3/5] app/procinfo: add Rx buffer size to --show-port
  2020-09-05  9:07   ` [dpdk-dev] [PATCH v4 3/5] app/procinfo: add Rx buffer size to --show-port Chengchang Tang
@ 2020-09-05 16:59     ` Stephen Hemminger
  2020-09-07  9:14       ` Chengchang Tang
  0 siblings, 1 reply; 59+ messages in thread
From: Stephen Hemminger @ 2020-09-05 16:59 UTC (permalink / raw)
  To: Chengchang Tang
  Cc: dev, linuxarm, thomas, arybchenko, ferruh.yigit, wenzhuo.lu,
	maryam.tahhan

On Sat, 5 Sep 2020 17:07:32 +0800
Chengchang Tang <tangchengchang@huawei.com> wrote:

>  				printf("\t  -- queue %d rx scatter %d"
>  						" descriptors %d"
> +						" rx buffer size %d"
>  						" offloads 0x%"PRIx64
>  						" mempool socket %d\n",
>  						j,
>  						queue_info.scattered_rx,
>  						queue_info.nb_desc,
> +						queue_info.rx_buf_size,
>  						queue_info.conf.offloads,
>  						queue_info.mp->socket_id);
>  			}

These should be using %u and need space after " for PRIx64
Why not:
  				printf("\t  -- queue %u rx scatter %u"
  						" descriptors %u"
 						" rx buffer size %u"
  						" offloads %#" PRIx64
  						" mempool socket %d\n",

^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [dpdk-dev] [PATCH v3 1/4] ethdev: add a field for rxq info structure
  2020-09-03  1:48               ` Chengchang Tang
@ 2020-09-06 13:45                 ` Matan Azrad
  2020-09-07  7:47                   ` Chengchang Tang
  0 siblings, 1 reply; 59+ messages in thread
From: Matan Azrad @ 2020-09-06 13:45 UTC (permalink / raw)
  To: Chengchang Tang, dev
  Cc: maryam.tahhan, linuxarm, ferruh.yigit, wenzhuo.lu,
	NBU-Contact-Thomas Monjalon, arybchenko


Hi  Chengchang

From: Chengchang Tang:
> Hi, Matan
> 
> On 2020/9/2 18:30, Matan Azrad wrote:
> > Hi Chengchang
> >
> > From: Chengchang Tang
> >> Hi, Matan
> >>
> >> On 2020/9/2 15:19, Matan Azrad wrote:
> >>>
> >>> Hi Chengchang
> >>>
> >>> From: Chengchang Tang
> >>>> Hi, Matan
> >>>>
> >>>> On 2020/9/1 23:33, Matan Azrad wrote:
> >>>>>
> >>>>> Hi Chengchang
> >>>>>
> >>>>> Please see some question below.
> >>>>>
> >>>>> From: Chengchang Tang
> >>>>>> Add a field named rx_buf_size in rte_eth_rxq_info to indicate the
> >>>>>> buffer size used in receiving packets for HW.
> >>>>>>
> >>>>>> In this way, upper-layer users can get this information by
> >>>>>> calling rte_eth_rx_queue_info_get.
> >>>>>>
> >>>>>> Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
> >>>>>> Reviewed-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
> >>>>>> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
> >>>>>> ---
> >>>>>>  lib/librte_ethdev/rte_ethdev.h | 2 ++
> >>>>>>  1 file changed, 2 insertions(+)
> >>>>>>
> >>>>>> diff --git a/lib/librte_ethdev/rte_ethdev.h
> >>>>>> b/lib/librte_ethdev/rte_ethdev.h index 70295d7..9fed5cb 100644
> >>>>>> --- a/lib/librte_ethdev/rte_ethdev.h
> >>>>>> +++ b/lib/librte_ethdev/rte_ethdev.h
> >>>>>> @@ -1420,6 +1420,8 @@ struct rte_eth_rxq_info {
> >>>>>>         struct rte_eth_rxconf conf; /**< queue config parameters. */
> >>>>>>         uint8_t scattered_rx;       /**< scattered packets RX supported.
> */
> >>>>>>         uint16_t nb_desc;           /**< configured number of RXDs. */
> >>>>>> +       /**< buffer size used for hardware when receive packets. */
> >>>>>> +       uint16_t rx_buf_size;
> >>>>>
> >>>>> Is it the maximum supported Rx buffer by the HW?
> >>>>> If yes, maybe max_rx_buf_size is better name?
> >>>>
> >>>> No, it is the Rx buffer size currently used by HW.
> >
> >>> Doesn't it defined by the user? Using Rx queue mem-pool mbuf room
> size?
> >>>
> >>> And it may be different per Rx queue....
> >>
> >> Yes, it is defined by user using the Rx queue mem-pool mbuf room size.
> >> When different queues are bound to different mempools, different
> >> queues may have different value.
> >>>
> >>>> IMHO, the structure rte_eth_rxq_info and associated query API are
> >>>> mainly used to query HW configurations at runtime or after queue is
> >>>> configured/setup. Therefore, the content of this structure should
> >>>> be the current HW configuration.
> >>>
> >>> It looks me more like capabilities...
> >>> The one which define the current configuration is the user by the
> >> configuration APIs(after reading the capabilities).
> >>
> >> I prefer to consider the structure rte_eth_dev_info as the capabilities.
> >
> > Yes.
> >
> >> Because rxq_info and associated APIs are not available until the
> >> queue is configured. And the max rx_buf_size is already exists in
> dev_info.
> >>>
> >>> I don't think we have here all the current configurations, so what
> >>> is special
> >> in this one?
> >>
> >> I think the structure is used to store the queue-related
> >> configuration, especially the final HW configuration that may be
> >> different from user configuration and some configurations that are
> >> not mandatory for the user(PMDs will use a default configuration).
> >> Such as the scatterred_rx and rx_drop_en in rte_eth_rxconf, some PMDs
> >> will adjust it in some cases based on their HW constraints.
> >
> > Ok, this struct(struct rte_eth_rxq_info) is new for me.
> > Thanks for the explanation.
> >
> >> This configuration item meets the above criteria. The value range of
> >> rx_buf_size varies according to HW. Some HW may require 1k-alignment,
> >> while others may require several fixed values. So, the PMDs will
> >> configure it based on their HW constraints. This results in
> >> difference between the user configuration and the actual
> >> configuration and this value affects the memory usage and performance.
> >> I think there's a need for a way to expose that information.
> >
> > So the user can configure X and the driver will use Y!=X?
> 
> Yes, it depends on the HW. In the queue setup API, it just checks whether
> the input is greater than the required minimum value. But HW usually has
> requirements for alignment and so on.
> So when X does not meet these requirements, PMDs will calculate a new
> value Y that meets these requirements to configure the hardware (Y <= X, to
> ensure no memory overflow occurs).
> > Should the application validate its own configurations after setting them
> successfully?
> 
> It depends on their own needs. The application should not be forced to
> verify it to avoid affecting the ease of use of PMDs. For some applications,
> they don't care about this value.

I understand, 
It looks me like a bad ping-pong between app and PMD (for all the fields in the struct),
And we should avoid adding fields to this structure if we can.

What's about adding field in rte_eth_dev_info to expose the rx buffer alignment supported by the PMD?
Then, application has all the knowledge you want to expose before the configuration.

> >
> >>>
> >>>>> Maybe document that 0 means - no limitation by HW?
> >>>>
> >>>> Yes, there is no need to fill this filed for HW that has no restrictions on
> it.
> >>>> I'll add it in v4.
> >>>>
> >>>>> Must application read it in order to know if its datapath should
> >>>>> handle
> >>>> multi-segment buffers?
> >>>>
> >>>> I think it's more appropriate to use scattered_rx to determine if
> >>>> multi- segment buffers should be handled.
> >>>>
> >>>>>
> >>>>> Maybe it will be good to force application to configure scatter
> >>>>> when this
> >>>> field is valid and smaller than max_rx_pkt_len\max_lro.. (<= room
> size)...
> >>>
> >>> Can you explain more what is the issue you came to solve?
> >>
> >> This HW information may be useful when there is some problems running
> >> a application. This structure and related APIs can be used to expose
> >> it at run time.
> >>>
> > OK
> >
> > .
> >


^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [dpdk-dev] [PATCH v3 1/4] ethdev: add a field for rxq info structure
  2020-09-06 13:45                 ` Matan Azrad
@ 2020-09-07  7:47                   ` Chengchang Tang
  2020-09-07  8:28                     ` Matan Azrad
  0 siblings, 1 reply; 59+ messages in thread
From: Chengchang Tang @ 2020-09-07  7:47 UTC (permalink / raw)
  To: Matan Azrad, dev
  Cc: maryam.tahhan, linuxarm, ferruh.yigit, wenzhuo.lu,
	NBU-Contact-Thomas Monjalon, arybchenko

Hi Matan

On 2020/9/6 21:45, Matan Azrad wrote:
> 
> Hi  Chengchang
> 
> From: Chengchang Tang:
>> Hi, Matan
>>
>> On 2020/9/2 18:30, Matan Azrad wrote:
>>> Hi Chengchang
>>>
>>> From: Chengchang Tang
>>>> Hi, Matan
>>>>
>>>> On 2020/9/2 15:19, Matan Azrad wrote:
>>>>>
>>>>> Hi Chengchang
>>>>>
>>>>> From: Chengchang Tang
>>>>>> Hi, Matan
>>>>>>
>>>>>> On 2020/9/1 23:33, Matan Azrad wrote:
>>>>>>>
>>>>>>> Hi Chengchang
>>>>>>>
>>>>>>> Please see some question below.
>>>>>>>
>>>>>>> From: Chengchang Tang
>>>>>>>> Add a field named rx_buf_size in rte_eth_rxq_info to indicate the
>>>>>>>> buffer size used in receiving packets for HW.
>>>>>>>>
>>>>>>>> In this way, upper-layer users can get this information by
>>>>>>>> calling rte_eth_rx_queue_info_get.
>>>>>>>>
>>>>>>>> Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
>>>>>>>> Reviewed-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
>>>>>>>> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
>>>>>>>> ---
>>>>>>>>  lib/librte_ethdev/rte_ethdev.h | 2 ++
>>>>>>>>  1 file changed, 2 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/lib/librte_ethdev/rte_ethdev.h
>>>>>>>> b/lib/librte_ethdev/rte_ethdev.h index 70295d7..9fed5cb 100644
>>>>>>>> --- a/lib/librte_ethdev/rte_ethdev.h
>>>>>>>> +++ b/lib/librte_ethdev/rte_ethdev.h
>>>>>>>> @@ -1420,6 +1420,8 @@ struct rte_eth_rxq_info {
>>>>>>>>         struct rte_eth_rxconf conf; /**< queue config parameters. */
>>>>>>>>         uint8_t scattered_rx;       /**< scattered packets RX supported.
>> */
>>>>>>>>         uint16_t nb_desc;           /**< configured number of RXDs. */
>>>>>>>> +       /**< buffer size used for hardware when receive packets. */
>>>>>>>> +       uint16_t rx_buf_size;
>>>>>>>
>>>>>>> Is it the maximum supported Rx buffer by the HW?
>>>>>>> If yes, maybe max_rx_buf_size is better name?
>>>>>>
>>>>>> No, it is the Rx buffer size currently used by HW.
>>>
>>>>> Doesn't it defined by the user? Using Rx queue mem-pool mbuf room
>> size?
>>>>>
>>>>> And it may be different per Rx queue....
>>>>
>>>> Yes, it is defined by user using the Rx queue mem-pool mbuf room size.
>>>> When different queues are bound to different mempools, different
>>>> queues may have different value.
>>>>>
>>>>>> IMHO, the structure rte_eth_rxq_info and associated query API are
>>>>>> mainly used to query HW configurations at runtime or after queue is
>>>>>> configured/setup. Therefore, the content of this structure should
>>>>>> be the current HW configuration.
>>>>>
>>>>> It looks me more like capabilities...
>>>>> The one which define the current configuration is the user by the
>>>> configuration APIs(after reading the capabilities).
>>>>
>>>> I prefer to consider the structure rte_eth_dev_info as the capabilities.
>>>
>>> Yes.
>>>
>>>> Because rxq_info and associated APIs are not available until the
>>>> queue is configured. And the max rx_buf_size is already exists in
>> dev_info.
>>>>>
>>>>> I don't think we have here all the current configurations, so what
>>>>> is special
>>>> in this one?
>>>>
>>>> I think the structure is used to store the queue-related
>>>> configuration, especially the final HW configuration that may be
>>>> different from user configuration and some configurations that are
>>>> not mandatory for the user(PMDs will use a default configuration).
>>>> Such as the scatterred_rx and rx_drop_en in rte_eth_rxconf, some PMDs
>>>> will adjust it in some cases based on their HW constraints.
>>>
>>> Ok, this struct(struct rte_eth_rxq_info) is new for me.
>>> Thanks for the explanation.
>>>
>>>> This configuration item meets the above criteria. The value range of
>>>> rx_buf_size varies according to HW. Some HW may require 1k-alignment,
>>>> while others may require several fixed values. So, the PMDs will
>>>> configure it based on their HW constraints. This results in
>>>> difference between the user configuration and the actual
>>>> configuration and this value affects the memory usage and performance.
>>>> I think there's a need for a way to expose that information.
>>>
>>> So the user can configure X and the driver will use Y!=X?
>>
>> Yes, it depends on the HW. In the queue setup API, it just checks whether
>> the input is greater than the required minimum value. But HW usually has
>> requirements for alignment and so on.
>> So when X does not meet these requirements, PMDs will calculate a new
>> value Y that meets these requirements to configure the hardware (Y <= X, to
>> ensure no memory overflow occurs).
>>> Should the application validate its own configurations after setting them
>> successfully?
>>
>> It depends on their own needs. The application should not be forced to
>> verify it to avoid affecting the ease of use of PMDs. For some applications,
>> they don't care about this value.
> 
> I understand, 
> It looks me like a bad ping-pong between app and PMD (for all the fields in the struct),
> And we should avoid adding fields to this structure if we can.
> 
> What's about adding field in rte_eth_dev_info to expose the rx buffer alignment supported by the PMD?
> Then, application has all the knowledge you want to expose before the configuration.

This may not work because there may be other restrictions besides alignment, which are
related to the hardware design. Therefore, it is difficult to describe all constraints
in a single field. Moreover, this approach seems to constrain the PMDs and HW to some
extent.
> 
>>>
>>>>>
>>>>>>> Maybe document that 0 means - no limitation by HW?
>>>>>>
>>>>>> Yes, there is no need to fill this filed for HW that has no restrictions on
>> it.
>>>>>> I'll add it in v4.
>>>>>>
>>>>>>> Must application read it in order to know if its datapath should
>>>>>>> handle
>>>>>> multi-segment buffers?
>>>>>>
>>>>>> I think it's more appropriate to use scattered_rx to determine if
>>>>>> multi- segment buffers should be handled.
>>>>>>
>>>>>>>
>>>>>>> Maybe it will be good to force application to configure scatter
>>>>>>> when this
>>>>>> field is valid and smaller than max_rx_pkt_len\max_lro.. (<= room
>> size)...
>>>>>
>>>>> Can you explain more what is the issue you came to solve?
>>>>
>>>> This HW information may be useful when there is some problems running
>>>> a application. This structure and related APIs can be used to expose
>>>> it at run time.
>>>>>
>>> OK
>>>
>>> .
>>>
> 
> 
> .
> 


^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [dpdk-dev] [PATCH v3 1/4] ethdev: add a field for rxq info structure
  2020-09-07  7:47                   ` Chengchang Tang
@ 2020-09-07  8:28                     ` Matan Azrad
  2020-09-07 12:06                       ` Chengchang Tang
  0 siblings, 1 reply; 59+ messages in thread
From: Matan Azrad @ 2020-09-07  8:28 UTC (permalink / raw)
  To: Chengchang Tang, dev
  Cc: maryam.tahhan, linuxarm, ferruh.yigit, wenzhuo.lu,
	NBU-Contact-Thomas Monjalon, arybchenko


Hi Chengchang

From: Chengchang Tang:
> Hi Matan
> 
> On 2020/9/6 21:45, Matan Azrad wrote:
> >
> > Hi  Chengchang
> >
> > From: Chengchang Tang:
> >> Hi, Matan
> >>
> >> On 2020/9/2 18:30, Matan Azrad wrote:
> >>> Hi Chengchang
> >>>
> >>> From: Chengchang Tang
> >>>> Hi, Matan
> >>>>
> >>>> On 2020/9/2 15:19, Matan Azrad wrote:
> >>>>>
> >>>>> Hi Chengchang
> >>>>>
> >>>>> From: Chengchang Tang
> >>>>>> Hi, Matan
> >>>>>>
> >>>>>> On 2020/9/1 23:33, Matan Azrad wrote:
> >>>>>>>
> >>>>>>> Hi Chengchang
> >>>>>>>
> >>>>>>> Please see some question below.
> >>>>>>>
> >>>>>>> From: Chengchang Tang
> >>>>>>>> Add a field named rx_buf_size in rte_eth_rxq_info to indicate
> >>>>>>>> the buffer size used in receiving packets for HW.
> >>>>>>>>
> >>>>>>>> In this way, upper-layer users can get this information by
> >>>>>>>> calling rte_eth_rx_queue_info_get.
> >>>>>>>>
> >>>>>>>> Signed-off-by: Chengchang Tang
> <tangchengchang@huawei.com>
> >>>>>>>> Reviewed-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
> >>>>>>>> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
> >>>>>>>> ---
> >>>>>>>>  lib/librte_ethdev/rte_ethdev.h | 2 ++
> >>>>>>>>  1 file changed, 2 insertions(+)
> >>>>>>>>
<snip>
> >>> So the user can configure X and the driver will use Y!=X?
> >>
> >> Yes, it depends on the HW. In the queue setup API, it just checks
> >> whether the input is greater than the required minimum value. But HW
> >> usually has requirements for alignment and so on.
> >> So when X does not meet these requirements, PMDs will calculate a new
> >> value Y that meets these requirements to configure the hardware (Y <=
> >> X, to ensure no memory overflow occurs).
> >>> Should the application validate its own configurations after setting
> >>> them
> >> successfully?
> >>
> >> It depends on their own needs. The application should not be forced
> >> to verify it to avoid affecting the ease of use of PMDs. For some
> >> applications, they don't care about this value.
> >
> > I understand,
> > It looks me like a bad ping-pong between app and PMD (for all the
> > fields in the struct), And we should avoid adding fields to this structure if
> we can.
> >
> > What's about adding field in rte_eth_dev_info to expose the rx buffer
> alignment supported by the PMD?
> > Then, application has all the knowledge you want to expose before the
> configuration.
> 
> This may not work because there may be other restrictions besides
> alignment, which are related to the hardware design. Therefore, it is difficult
> to describe all constraints in a single field.
> Moreover, this approach seems to
> constrain the PMDs and HW to some extent.

Ok, so maybe other ethdev capability API to get the Rx buffer size adjustment by the PMD?
Don't you think it is important information to the application in order to decide the mempool buffer size \ enabling scatter?

In any case, I think you should add documentation in the RX setup API that the HW buf size may be changed by the PMD.

<snip>

^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [dpdk-dev] [PATCH v4 5/5] doc: remove rxq info structure deprecation notice
  2020-09-05 16:33     ` Thomas Monjalon
@ 2020-09-07  9:12       ` Chengchang Tang
  0 siblings, 0 replies; 59+ messages in thread
From: Chengchang Tang @ 2020-09-07  9:12 UTC (permalink / raw)
  To: Thomas Monjalon
  Cc: dev, linuxarm, arybchenko, ferruh.yigit, wenzhuo.lu, maryam.tahhan



On 2020/9/6 0:33, Thomas Monjalon wrote:
> 05/09/2020 11:07, Chengchang Tang:
>> The change has been applied, so remove the notice.
> 
> It should be atomic with the patch doing the change,
> i.e. you can squash.
> 
> I think the oneline patches in testpmd and procinfo
> can probably be squashed with the ethdev patch as well.

OK, I will squash them in next version.

Thanks


^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [dpdk-dev] [PATCH v4 3/5] app/procinfo: add Rx buffer size to --show-port
  2020-09-05 16:59     ` Stephen Hemminger
@ 2020-09-07  9:14       ` Chengchang Tang
  2020-09-18 22:11         ` Stephen Hemminger
  0 siblings, 1 reply; 59+ messages in thread
From: Chengchang Tang @ 2020-09-07  9:14 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: dev, linuxarm, thomas, arybchenko, ferruh.yigit, wenzhuo.lu,
	maryam.tahhan



On 2020/9/6 0:59, Stephen Hemminger wrote:
> On Sat, 5 Sep 2020 17:07:32 +0800
> Chengchang Tang <tangchengchang@huawei.com> wrote:
> 
>>  				printf("\t  -- queue %d rx scatter %d"
>>  						" descriptors %d"
>> +						" rx buffer size %d"
>>  						" offloads 0x%"PRIx64
>>  						" mempool socket %d\n",
>>  						j,
>>  						queue_info.scattered_rx,
>>  						queue_info.nb_desc,
>> +						queue_info.rx_buf_size,
>>  						queue_info.conf.offloads,
>>  						queue_info.mp->socket_id);
>>  			}
> 
> These should be using %u and need space after " for PRIx64
> Why not:
>   				printf("\t  -- queue %u rx scatter %u"
>   						" descriptors %u"
>  						" rx buffer size %u"
>   						" offloads %#" PRIx64
>   						" mempool socket %d\n",
> 

OK, I will fix these in next version.

Thanks


^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [dpdk-dev] [PATCH v3 1/4] ethdev: add a field for rxq info structure
  2020-09-07  8:28                     ` Matan Azrad
@ 2020-09-07 12:06                       ` Chengchang Tang
  2020-09-07 13:02                         ` Matan Azrad
  0 siblings, 1 reply; 59+ messages in thread
From: Chengchang Tang @ 2020-09-07 12:06 UTC (permalink / raw)
  To: Matan Azrad, dev
  Cc: maryam.tahhan, linuxarm, ferruh.yigit, wenzhuo.lu,
	NBU-Contact-Thomas Monjalon, arybchenko

Hi Matan

On 2020/9/7 16:28, Matan Azrad wrote:
> 
> Hi Chengchang
> 
> From: Chengchang Tang:
>> Hi Matan
>>
>> On 2020/9/6 21:45, Matan Azrad wrote:
>>>
>>> Hi  Chengchang
>>>
>>> From: Chengchang Tang:
>>>> Hi, Matan
>>>>
>>>> On 2020/9/2 18:30, Matan Azrad wrote:
>>>>> Hi Chengchang
>>>>>
>>>>> From: Chengchang Tang
>>>>>> Hi, Matan
>>>>>>
>>>>>> On 2020/9/2 15:19, Matan Azrad wrote:
>>>>>>>
>>>>>>> Hi Chengchang
>>>>>>>
>>>>>>> From: Chengchang Tang
>>>>>>>> Hi, Matan
>>>>>>>>
>>>>>>>> On 2020/9/1 23:33, Matan Azrad wrote:
>>>>>>>>>
>>>>>>>>> Hi Chengchang
>>>>>>>>>
>>>>>>>>> Please see some question below.
>>>>>>>>>
>>>>>>>>> From: Chengchang Tang
>>>>>>>>>> Add a field named rx_buf_size in rte_eth_rxq_info to indicate
>>>>>>>>>> the buffer size used in receiving packets for HW.
>>>>>>>>>>
>>>>>>>>>> In this way, upper-layer users can get this information by
>>>>>>>>>> calling rte_eth_rx_queue_info_get.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Chengchang Tang
>> <tangchengchang@huawei.com>
>>>>>>>>>> Reviewed-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
>>>>>>>>>> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
>>>>>>>>>> ---
>>>>>>>>>>  lib/librte_ethdev/rte_ethdev.h | 2 ++
>>>>>>>>>>  1 file changed, 2 insertions(+)
>>>>>>>>>>
> <snip>
>>>>> So the user can configure X and the driver will use Y!=X?
>>>>
>>>> Yes, it depends on the HW. In the queue setup API, it just checks
>>>> whether the input is greater than the required minimum value. But HW
>>>> usually has requirements for alignment and so on.
>>>> So when X does not meet these requirements, PMDs will calculate a new
>>>> value Y that meets these requirements to configure the hardware (Y <=
>>>> X, to ensure no memory overflow occurs).
>>>>> Should the application validate its own configurations after setting
>>>>> them
>>>> successfully?
>>>>
>>>> It depends on their own needs. The application should not be forced
>>>> to verify it to avoid affecting the ease of use of PMDs. For some
>>>> applications, they don't care about this value.
>>>
>>> I understand,
>>> It looks me like a bad ping-pong between app and PMD (for all the
>>> fields in the struct), And we should avoid adding fields to this structure if
>> we can.
>>>
>>> What's about adding field in rte_eth_dev_info to expose the rx buffer
>> alignment supported by the PMD?
>>> Then, application has all the knowledge you want to expose before the
>> configuration.
>>
>> This may not work because there may be other restrictions besides
>> alignment, which are related to the hardware design. Therefore, it is difficult
>> to describe all constraints in a single field.
>> Moreover, this approach seems to
>> constrain the PMDs and HW to some extent.
> 
> Ok, so maybe other ethdev capability API to get the Rx buffer size adjustment by the PMD?
> Don't you think it is important information to the application in order to decide the mempool buffer size \ enabling scatter?

I guess what you mean is that it's more like a capability, so the application should query it through a capability API.
If i understand correctly, I agree with that. But I think it's okay to use this structure to export queue related information
at runtime. It focuses on querying the current queue configuration.

And there seems to be no suitable API for querying this capability. Maybe we need to introduce a new API to do this. But
I'm not sure if it's really necessary.

> 
> In any case, I think you should add documentation in the RX setup API that the HW buf size may be changed by the PMD.

There is not a defined rule of how to configure Rx buffer size. That is, there is no specific method to configure the Rx
buffer size for applications. However, most PMDs configure the Rx buffer size base on the data size of the mempool. So,
if the description is added to the setup API, the method for configuring the Rx buffer size is determined. I think this
issue should involve more people in the discussion, maybe we should send a separate patch.
> 
> <snip>
> 
> .
> 


^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [dpdk-dev] [PATCH v3 1/4] ethdev: add a field for rxq info structure
  2020-09-07 12:06                       ` Chengchang Tang
@ 2020-09-07 13:02                         ` Matan Azrad
  0 siblings, 0 replies; 59+ messages in thread
From: Matan Azrad @ 2020-09-07 13:02 UTC (permalink / raw)
  To: Chengchang Tang, dev
  Cc: maryam.tahhan, linuxarm, ferruh.yigit, wenzhuo.lu,
	NBU-Contact-Thomas Monjalon, arybchenko


Hi Chengchang
From: Chengchang Tang:
> Hi Matan
> 
> On 2020/9/7 16:28, Matan Azrad wrote:
> >
> > Hi Chengchang
> >
> > From: Chengchang Tang:
> >> Hi Matan
> >>
> >> On 2020/9/6 21:45, Matan Azrad wrote:
> >>>
> >>> Hi  Chengchang
> >>>
> >>> From: Chengchang Tang:
> >>>> Hi, Matan
> >>>>
> >>>> On 2020/9/2 18:30, Matan Azrad wrote:
> >>>>> Hi Chengchang
> >>>>>
> >>>>> From: Chengchang Tang
> >>>>>> Hi, Matan
> >>>>>>
> >>>>>> On 2020/9/2 15:19, Matan Azrad wrote:
> >>>>>>>
> >>>>>>> Hi Chengchang
> >>>>>>>
> >>>>>>> From: Chengchang Tang
> >>>>>>>> Hi, Matan
> >>>>>>>>
> >>>>>>>> On 2020/9/1 23:33, Matan Azrad wrote:
> >>>>>>>>>
> >>>>>>>>> Hi Chengchang
> >>>>>>>>>
> >>>>>>>>> Please see some question below.
> >>>>>>>>>
> >>>>>>>>> From: Chengchang Tang
> >>>>>>>>>> Add a field named rx_buf_size in rte_eth_rxq_info to indicate
> >>>>>>>>>> the buffer size used in receiving packets for HW.
> >>>>>>>>>>
> >>>>>>>>>> In this way, upper-layer users can get this information by
> >>>>>>>>>> calling rte_eth_rx_queue_info_get.
> >>>>>>>>>>
> >>>>>>>>>> Signed-off-by: Chengchang Tang
> >> <tangchengchang@huawei.com>
> >>>>>>>>>> Reviewed-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
> >>>>>>>>>> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
> >>>>>>>>>> ---
> >>>>>>>>>>  lib/librte_ethdev/rte_ethdev.h | 2 ++
> >>>>>>>>>>  1 file changed, 2 insertions(+)
> >>>>>>>>>>
> > <snip>
> >>>>> So the user can configure X and the driver will use Y!=X?
> >>>>
> >>>> Yes, it depends on the HW. In the queue setup API, it just checks
> >>>> whether the input is greater than the required minimum value. But
> >>>> HW usually has requirements for alignment and so on.
> >>>> So when X does not meet these requirements, PMDs will calculate a
> >>>> new value Y that meets these requirements to configure the hardware
> >>>> (Y <= X, to ensure no memory overflow occurs).
> >>>>> Should the application validate its own configurations after
> >>>>> setting them
> >>>> successfully?
> >>>>
> >>>> It depends on their own needs. The application should not be forced
> >>>> to verify it to avoid affecting the ease of use of PMDs. For some
> >>>> applications, they don't care about this value.
> >>>
> >>> I understand,
> >>> It looks me like a bad ping-pong between app and PMD (for all the
> >>> fields in the struct), And we should avoid adding fields to this
> >>> structure if
> >> we can.
> >>>
> >>> What's about adding field in rte_eth_dev_info to expose the rx
> >>> buffer
> >> alignment supported by the PMD?
> >>> Then, application has all the knowledge you want to expose before
> >>> the
> >> configuration.
> >>
> >> This may not work because there may be other restrictions besides
> >> alignment, which are related to the hardware design. Therefore, it is
> >> difficult to describe all constraints in a single field.
> >> Moreover, this approach seems to
> >> constrain the PMDs and HW to some extent.
> >
> > Ok, so maybe other ethdev capability API to get the Rx buffer size
> adjustment by the PMD?
> > Don't you think it is important information to the application in order to
> decide the mempool buffer size \ enabling scatter?
> 
> I guess what you mean is that it's more like a capability, so the application
> should query it through a capability API.

Yes.

> If i understand correctly, I agree with that. But I think it's okay to use this
> structure to export queue related information at runtime. It focuses on
> querying the current queue configuration.

Why do we need it If the capability(suggested above) say it already? 

> And there seems to be no suitable API for querying this capability. Maybe we
> need to introduce a new API to do this. But I'm not sure if it's really
> necessary.
 
For example if the user use room of size X in the RX mempool and the PMD configures X/2 because of HW limitation:
Don't you think it will affect the user configuration of mempool and scatter mode?

> > In any case, I think you should add documentation in the RX setup API that
> the HW buf size may be changed by the PMD.
> 
> There is not a defined rule of how to configure Rx buffer size. That is, there is
> no specific method to configure the Rx buffer size for applications. However,
> most PMDs configure the Rx buffer size base on the data size of the
> mempool. So, if the description is added to the setup API, the method for
> configuring the Rx buffer size is determined. I think this issue should involve
> more people in the discussion, maybe we should send a separate patch.

The mempool parameter of RX say it exactly, no?


> > <snip>
> >
> > .
> >


^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [dpdk-dev] [PATCH v4 2/5] app/testpmd: add Rx buffer size display in queue info query
  2020-09-05  9:07   ` [dpdk-dev] [PATCH v4 2/5] app/testpmd: add Rx buffer size display in queue info query Chengchang Tang
@ 2020-09-18  8:54     ` Ferruh Yigit
  2020-09-20  8:47       ` Chengchang Tang
  0 siblings, 1 reply; 59+ messages in thread
From: Ferruh Yigit @ 2020-09-18  8:54 UTC (permalink / raw)
  To: Chengchang Tang, dev
  Cc: linuxarm, thomas, arybchenko, wenzhuo.lu, maryam.tahhan

On 9/5/2020 10:07 AM, Chengchang Tang wrote:
> Add Rx buffer size to queue info querry cmd so that the user can get the
> buffer length used by HW queue for receiving packets.
> 
> Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
> Reviewed-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
> ---
>   app/test-pmd/config.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index 30bee33..b432ac6 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -452,6 +452,7 @@ rx_queue_infos_display(portid_t port_id, uint16_t queue_id)
>   		(qinfo.conf.rx_deferred_start != 0) ? "on" : "off");
>   	printf("\nRX scattered packets: %s",
>   		(qinfo.scattered_rx != 0) ? "on" : "off");
> +	printf("\nRX buffer size: %hu", qinfo.rx_buf_size);

Since this field is optional for PMD to fill, it may be confusing to 
display buffer size as "0".
What do you think print this value when "qinfo.rx_buf_size != 0"?

^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [dpdk-dev] [PATCH v4 3/5] app/procinfo: add Rx buffer size to --show-port
  2020-09-07  9:14       ` Chengchang Tang
@ 2020-09-18 22:11         ` Stephen Hemminger
  2020-09-21  2:06           ` Chengchang Tang
  0 siblings, 1 reply; 59+ messages in thread
From: Stephen Hemminger @ 2020-09-18 22:11 UTC (permalink / raw)
  To: Chengchang Tang
  Cc: dev, linuxarm, thomas, arybchenko, ferruh.yigit, wenzhuo.lu,
	maryam.tahhan

On Mon, 7 Sep 2020 17:14:48 +0800
Chengchang Tang <tangchengchang@huawei.com> wrote:

> On 2020/9/6 0:59, Stephen Hemminger wrote:
> > On Sat, 5 Sep 2020 17:07:32 +0800
> > Chengchang Tang <tangchengchang@huawei.com> wrote:
> >   
> >>  				printf("\t  -- queue %d rx scatter %d"
> >>  						" descriptors %d"
> >> +						" rx buffer size %d"
> >>  						" offloads 0x%"PRIx64
> >>  						" mempool socket %d\n",
> >>  						j,
> >>  						queue_info.scattered_rx,
> >>  						queue_info.nb_desc,
> >> +						queue_info.rx_buf_size,
> >>  						queue_info.conf.offloads,
> >>  						queue_info.mp->socket_id);
> >>  			}  
> > 
> > These should be using %u and need space after " for PRIx64
> > Why not:
> >   				printf("\t  -- queue %u rx scatter %u"
> >   						" descriptors %u"
> >  						" rx buffer size %u"
> >   						" offloads %#" PRIx64
> >   						" mempool socket %d\n",
> >   
> 
> OK, I will fix these in next version.
> 
> Thanks

NAK, these is superseded by.

Please look at the new improved procinfo still stuck in patchwork
https://patchwork.dpdk.org/patch/74960/

Let's put the buffer size there.

^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [dpdk-dev] [PATCH v4 2/5] app/testpmd: add Rx buffer size display in queue info query
  2020-09-18  8:54     ` Ferruh Yigit
@ 2020-09-20  8:47       ` Chengchang Tang
  0 siblings, 0 replies; 59+ messages in thread
From: Chengchang Tang @ 2020-09-20  8:47 UTC (permalink / raw)
  To: Ferruh Yigit, dev; +Cc: linuxarm, thomas, arybchenko, wenzhuo.lu, maryam.tahhan



On 2020/9/18 16:54, Ferruh Yigit wrote:
> On 9/5/2020 10:07 AM, Chengchang Tang wrote:
>> Add Rx buffer size to queue info querry cmd so that the user can get the
>> buffer length used by HW queue for receiving packets.
>>
>> Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
>> Reviewed-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
>> ---
>>   app/test-pmd/config.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
>> index 30bee33..b432ac6 100644
>> --- a/app/test-pmd/config.c
>> +++ b/app/test-pmd/config.c
>> @@ -452,6 +452,7 @@ rx_queue_infos_display(portid_t port_id, uint16_t queue_id)
>>           (qinfo.conf.rx_deferred_start != 0) ? "on" : "off");
>>       printf("\nRX scattered packets: %s",
>>           (qinfo.scattered_rx != 0) ? "on" : "off");
>> +    printf("\nRX buffer size: %hu", qinfo.rx_buf_size);
> 
> Since this field is optional for PMD to fill, it may be confusing to display buffer size as "0".
> What do you think print this value when "qinfo.rx_buf_size != 0"?

Agree, it will be modified in the next version.
> 
> .
> 


^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [dpdk-dev] [PATCH v4 3/5] app/procinfo: add Rx buffer size to --show-port
  2020-09-18 22:11         ` Stephen Hemminger
@ 2020-09-21  2:06           ` Chengchang Tang
  2020-09-21 11:26             ` Ferruh Yigit
  0 siblings, 1 reply; 59+ messages in thread
From: Chengchang Tang @ 2020-09-21  2:06 UTC (permalink / raw)
  To: Stephen Hemminger, thomas, Ferruh Yigit; +Cc: dev, linuxarm

Hi, All
These Patches improved procinfo was stuck in patchwork:
https://patchwork.dpdk.org/patch/74960/

These patches look good for me. Will these be applied?

I'm not sure what to do next. If these will be applied, I need modified
the code.

On 2020/9/19 6:11, Stephen Hemminger wrote:
> On Mon, 7 Sep 2020 17:14:48 +0800
> Chengchang Tang <tangchengchang@huawei.com> wrote:
> 
>> On 2020/9/6 0:59, Stephen Hemminger wrote:
>>> On Sat, 5 Sep 2020 17:07:32 +0800
>>> Chengchang Tang <tangchengchang@huawei.com> wrote:
>>>   
>>>>  				printf("\t  -- queue %d rx scatter %d"
>>>>  						" descriptors %d"
>>>> +						" rx buffer size %d"
>>>>  						" offloads 0x%"PRIx64
>>>>  						" mempool socket %d\n",
>>>>  						j,
>>>>  						queue_info.scattered_rx,
>>>>  						queue_info.nb_desc,
>>>> +						queue_info.rx_buf_size,
>>>>  						queue_info.conf.offloads,
>>>>  						queue_info.mp->socket_id);
>>>>  			}  
>>>
>>> These should be using %u and need space after " for PRIx64
>>> Why not:
>>>   				printf("\t  -- queue %u rx scatter %u"
>>>   						" descriptors %u"
>>>  						" rx buffer size %u"
>>>   						" offloads %#" PRIx64
>>>   						" mempool socket %d\n",
>>>   
>>
>> OK, I will fix these in next version.
>>
>> Thanks
> 
> NAK, these is superseded by.
> 
> Please look at the new improved procinfo still stuck in patchwork
> https://patchwork.dpdk.org/patch/74960/
> 
> Let's put the buffer size there.
> 
> .
> 


^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [dpdk-dev] [PATCH v4 3/5] app/procinfo: add Rx buffer size to --show-port
  2020-09-21  2:06           ` Chengchang Tang
@ 2020-09-21 11:26             ` Ferruh Yigit
  0 siblings, 0 replies; 59+ messages in thread
From: Ferruh Yigit @ 2020-09-21 11:26 UTC (permalink / raw)
  To: Chengchang Tang, Stephen Hemminger, thomas; +Cc: dev, linuxarm

On 9/21/2020 3:06 AM, Chengchang Tang wrote:
> Hi, All
> These Patches improved procinfo was stuck in patchwork:
> https://patchwork.dpdk.org/patch/74960/
> 
> These patches look good for me. Will these be applied?
> 
> I'm not sure what to do next. If these will be applied, I need modified
> the code.

Hi Chengchang,

I suggest don't hold your patches and send your next version on top of 
latest head of git repo, based on which patch applied first we can 
resolve the conflict accordingly.

^ permalink raw reply	[flat|nested] 59+ messages in thread

* [dpdk-dev] [PATCH v5 0/2] add Rx buffer size for rxq info structure
  2020-06-18 12:35 [dpdk-dev] [RFC] ethdev: add a field for rte_eth_rxq_info Chengchang Tang
                   ` (3 preceding siblings ...)
  2020-09-05  9:07 ` [dpdk-dev] [PATCH v4 0/5] add Rx buffer size for rxq info structure Chengchang Tang
@ 2020-09-21 13:22 ` Chengchang Tang
  2020-09-21 13:22   ` [dpdk-dev] [PATCH v5 1/2] ethdev: add a field " Chengchang Tang
                     ` (2 more replies)
  4 siblings, 3 replies; 59+ messages in thread
From: Chengchang Tang @ 2020-09-21 13:22 UTC (permalink / raw)
  To: dev; +Cc: linuxarm, thomas, arybchenko, ferruh.yigit, wenzhuo.lu, maryam.tahhan

In common practice, PMD configure the Rx buffer size which indicate the
buffer length could be used for HW in receiving packets according to the
data room size of the object in mempool. But in fact, the final value is
related to the specifications of HW, and its values will affect the number
of fragments in receiving packets when scatter is enabled. By the way,
some PMDs may force to enable scatter when the MTU is bigger than the HW
Rx buffer size.

At present, we have no way to expose relevant information to upper layer
users. So, add a field named rx_buf_size in rte_eth_rxq_info to indicate
the buffer size used in receiving packets for HW. And this field is
optional, so there is no need to forcibly update all PMDs.

This patchset also update testpmd, proc-info tools and add hns3 PMD
implementation.

v5:
  - squash oneline app patches and deprecation remove with ethdev patch

Chengchang Tang (2):
  ethdev: add a field for rxq info structure
  net/hns3: add Rx buffer size to Rx qinfo query

 app/proc-info/main.c                 | 13 +++++++++----
 app/test-pmd/config.c                |  2 ++
 doc/guides/rel_notes/deprecation.rst |  5 -----
 drivers/net/hns3/hns3_rxtx.c         |  2 ++
 lib/librte_ethdev/rte_ethdev.h       |  1 +
 5 files changed, 14 insertions(+), 9 deletions(-)

--
2.7.4


^ permalink raw reply	[flat|nested] 59+ messages in thread

* [dpdk-dev] [PATCH v5 1/2] ethdev: add a field for rxq info structure
  2020-09-21 13:22 ` [dpdk-dev] [PATCH v5 0/2] add Rx buffer size for rxq info structure Chengchang Tang
@ 2020-09-21 13:22   ` Chengchang Tang
  2020-09-21 14:18     ` Ferruh Yigit
  2020-09-21 13:22   ` [dpdk-dev] [PATCH v5 2/2] net/hns3: add Rx buffer size to Rx qinfo query Chengchang Tang
  2020-09-21 14:19   ` [dpdk-dev] [PATCH v5 0/2] add Rx buffer size for rxq info structure Ferruh Yigit
  2 siblings, 1 reply; 59+ messages in thread
From: Chengchang Tang @ 2020-09-21 13:22 UTC (permalink / raw)
  To: dev; +Cc: linuxarm, thomas, arybchenko, ferruh.yigit, wenzhuo.lu, maryam.tahhan

Add a field named rx_buf_size in rte_eth_rxq_info to indicate the buffer
size used in receiving packets for HW.

In this way, upper-layer users can get this information by calling
rte_eth_rx_queue_info_get.

Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
Reviewed-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 app/proc-info/main.c                 | 13 +++++++++----
 app/test-pmd/config.c                |  2 ++
 doc/guides/rel_notes/deprecation.rst |  5 -----
 lib/librte_ethdev/rte_ethdev.h       |  1 +
 4 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/app/proc-info/main.c b/app/proc-info/main.c
index 0b030d3..64fb83b 100644
--- a/app/proc-info/main.c
+++ b/app/proc-info/main.c
@@ -708,15 +708,20 @@ show_port(void)
 		for (j = 0; j < dev_info.nb_rx_queues; j++) {
 			ret = rte_eth_rx_queue_info_get(i, j, &queue_info);
 			if (ret == 0) {
-				printf("\t  -- queue %d rx scatter %d"
-						" descriptors %d"
-						" offloads 0x%"PRIx64
-						" mempool socket %d\n",
+				printf("\t  -- queue %u rx scatter %u"
+						" descriptors %u"
+						" offloads 0x%" PRIx64
+						" mempool socket %d",
 						j,
 						queue_info.scattered_rx,
 						queue_info.nb_desc,
 						queue_info.conf.offloads,
 						queue_info.mp->socket_id);
+
+				if (queue_info.rx_buf_size != 0)
+					printf(" rx buffer size %u",
+						queue_info.rx_buf_size);
+				printf("\n");
 			}
 		}

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index b6eb2a5..2d9a456 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -452,6 +452,8 @@ rx_queue_infos_display(portid_t port_id, uint16_t queue_id)
 		(qinfo.conf.rx_deferred_start != 0) ? "on" : "off");
 	printf("\nRX scattered packets: %s",
 		(qinfo.scattered_rx != 0) ? "on" : "off");
+	if (qinfo.rx_buf_size != 0)
+		printf("\nRX buffer size: %hu", qinfo.rx_buf_size);
 	printf("\nNumber of RXDs: %hu", qinfo.nb_desc);

 	if (rte_eth_rx_burst_mode_get(port_id, queue_id, &mode) == 0)
diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index b136576..1beb76d 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -189,11 +189,6 @@ Deprecation Notices
   specified lengths into the buffers allocated from the specified
   memory pools. The backward compatibility to existing API is preserved.

-* ethdev: The ``struct rte_eth_rxq_info`` will be modified to include
-  a new optional field, indicating the buffer size used in receiving packets
-  for HW. This change is planned for 20.11. For more details:
-  https://mails.dpdk.org/archives/dev/2020-July/176135.html.
-
 * ethdev: ``rx_descriptor_done`` dev_ops and ``rte_eth_rx_descriptor_done``
   will be removed in 21.11.
   Existing ``rte_eth_rx_descriptor_status`` and ``rte_eth_tx_descriptor_status``
diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index c3ab541..eeef52e 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -1449,6 +1449,7 @@ struct rte_eth_rxq_info {
 	struct rte_eth_rxconf conf; /**< queue config parameters. */
 	uint8_t scattered_rx;       /**< scattered packets RX supported. */
 	uint16_t nb_desc;           /**< configured number of RXDs. */
+	uint16_t rx_buf_size;       /**< hardware receive buffer size. */
 } __rte_cache_min_aligned;

 /**
--
2.7.4


^ permalink raw reply	[flat|nested] 59+ messages in thread

* [dpdk-dev] [PATCH v5 2/2] net/hns3: add Rx buffer size to Rx qinfo query
  2020-09-21 13:22 ` [dpdk-dev] [PATCH v5 0/2] add Rx buffer size for rxq info structure Chengchang Tang
  2020-09-21 13:22   ` [dpdk-dev] [PATCH v5 1/2] ethdev: add a field " Chengchang Tang
@ 2020-09-21 13:22   ` Chengchang Tang
  2020-09-21 14:19   ` [dpdk-dev] [PATCH v5 0/2] add Rx buffer size for rxq info structure Ferruh Yigit
  2 siblings, 0 replies; 59+ messages in thread
From: Chengchang Tang @ 2020-09-21 13:22 UTC (permalink / raw)
  To: dev; +Cc: linuxarm, thomas, arybchenko, ferruh.yigit, wenzhuo.lu, maryam.tahhan

Report hns3 PMD configured Rx buffer size in Rx queue information query.

Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
Reviewed-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
---
 drivers/net/hns3/hns3_rxtx.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/hns3/hns3_rxtx.c b/drivers/net/hns3/hns3_rxtx.c
index 308d0a6..c13931d 100644
--- a/drivers/net/hns3/hns3_rxtx.c
+++ b/drivers/net/hns3/hns3_rxtx.c
@@ -2851,6 +2851,8 @@ hns3_rxq_info_get(struct rte_eth_dev *dev, uint16_t queue_id,
 	qinfo->mp = rxq->mb_pool;
 	qinfo->nb_desc = rxq->nb_rx_desc;
 	qinfo->scattered_rx = dev->data->scattered_rx;
+	/* Report the HW Rx buffer length to user */
+	qinfo->rx_buf_size = rxq->rx_buf_len;

 	/*
 	 * If there are no available Rx buffer descriptors, incoming packets
--
2.7.4


^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [dpdk-dev] [PATCH v5 1/2] ethdev: add a field for rxq info structure
  2020-09-21 13:22   ` [dpdk-dev] [PATCH v5 1/2] ethdev: add a field " Chengchang Tang
@ 2020-09-21 14:18     ` Ferruh Yigit
  0 siblings, 0 replies; 59+ messages in thread
From: Ferruh Yigit @ 2020-09-21 14:18 UTC (permalink / raw)
  To: Chengchang Tang, dev
  Cc: linuxarm, thomas, arybchenko, wenzhuo.lu, maryam.tahhan

On 9/21/2020 2:22 PM, Chengchang Tang wrote:
> Add a field named rx_buf_size in rte_eth_rxq_info to indicate the buffer
> size used in receiving packets for HW.
> 
> In this way, upper-layer users can get this information by calling
> rte_eth_rx_queue_info_get.
> 
> Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
> Reviewed-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>

Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>


^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [dpdk-dev] [PATCH v5 0/2] add Rx buffer size for rxq info structure
  2020-09-21 13:22 ` [dpdk-dev] [PATCH v5 0/2] add Rx buffer size for rxq info structure Chengchang Tang
  2020-09-21 13:22   ` [dpdk-dev] [PATCH v5 1/2] ethdev: add a field " Chengchang Tang
  2020-09-21 13:22   ` [dpdk-dev] [PATCH v5 2/2] net/hns3: add Rx buffer size to Rx qinfo query Chengchang Tang
@ 2020-09-21 14:19   ` Ferruh Yigit
  2 siblings, 0 replies; 59+ messages in thread
From: Ferruh Yigit @ 2020-09-21 14:19 UTC (permalink / raw)
  To: Chengchang Tang, dev
  Cc: linuxarm, thomas, arybchenko, wenzhuo.lu, maryam.tahhan

On 9/21/2020 2:22 PM, Chengchang Tang wrote:
> In common practice, PMD configure the Rx buffer size which indicate the
> buffer length could be used for HW in receiving packets according to the
> data room size of the object in mempool. But in fact, the final value is
> related to the specifications of HW, and its values will affect the number
> of fragments in receiving packets when scatter is enabled. By the way,
> some PMDs may force to enable scatter when the MTU is bigger than the HW
> Rx buffer size.
> 
> At present, we have no way to expose relevant information to upper layer
> users. So, add a field named rx_buf_size in rte_eth_rxq_info to indicate
> the buffer size used in receiving packets for HW. And this field is
> optional, so there is no need to forcibly update all PMDs.
> 
> This patchset also update testpmd, proc-info tools and add hns3 PMD
> implementation.
> 
> v5:
>    - squash oneline app patches and deprecation remove with ethdev patch
> 
> Chengchang Tang (2):
>    ethdev: add a field for rxq info structure
>    net/hns3: add Rx buffer size to Rx qinfo query
> 

Series applied to dpdk-next-net/main, thanks.

^ permalink raw reply	[flat|nested] 59+ messages in thread

end of thread, other threads:[~2020-09-21 14:19 UTC | newest]

Thread overview: 59+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-18 12:35 [dpdk-dev] [RFC] ethdev: add a field for rte_eth_rxq_info Chengchang Tang
2020-08-26  1:57 ` [dpdk-dev] [PATCH 0/3] add RX buffer size " Chengchang Tang
2020-08-26  1:57   ` [dpdk-dev] [PATCH 1/3] ethdev: add a field " Chengchang Tang
2020-08-26  1:57   ` [dpdk-dev] [PATCH 2/3] app/testpmd: Add RX buffer size display in queue info querry Chengchang Tang
2020-08-26  1:57   ` [dpdk-dev] [PATCH 3/3] net/hns3: add RX buffer size to rx qinfo querry Chengchang Tang
2020-08-26  7:12 ` [dpdk-dev] [PATCH v2 0/3] add Rx buffer size for rxq info structure Chengchang Tang
2020-08-26  7:12   ` [dpdk-dev] [PATCH v2 1/3] ethdev: add a field " Chengchang Tang
2020-08-26  7:29     ` Wei Hu (Xavier)
2020-08-26  7:12   ` [dpdk-dev] [PATCH v2 2/3] app/testpmd: add Rx buffer size display in queue info query Chengchang Tang
2020-08-26  7:28     ` Wei Hu (Xavier)
2020-08-26 14:42     ` Stephen Hemminger
2020-08-29  6:48       ` Chengchang Tang
2020-08-26  7:12   ` [dpdk-dev] [PATCH v2 3/3] net/hns3: add Rx buffer size to Rx qinfo query Chengchang Tang
2020-08-26  7:20     ` Wei Hu (Xavier)
2020-08-29  7:13 ` [dpdk-dev] [PATCH v3 0/4] add Rx buffer size for rxq info structure Chengchang Tang
2020-08-29  7:13   ` [dpdk-dev] [PATCH v3 1/4] ethdev: add a field " Chengchang Tang
2020-09-01 15:33     ` Matan Azrad
2020-09-02  3:52       ` Chengchang Tang
2020-09-02  7:19         ` Matan Azrad
2020-09-02 10:01           ` Chengchang Tang
2020-09-02 10:30             ` Matan Azrad
2020-09-03  1:48               ` Chengchang Tang
2020-09-06 13:45                 ` Matan Azrad
2020-09-07  7:47                   ` Chengchang Tang
2020-09-07  8:28                     ` Matan Azrad
2020-09-07 12:06                       ` Chengchang Tang
2020-09-07 13:02                         ` Matan Azrad
2020-09-03 15:00           ` Ferruh Yigit
2020-09-03 14:55         ` Ferruh Yigit
2020-09-03 15:01     ` Ferruh Yigit
2020-09-04  1:43       ` Chengchang Tang
2020-09-03 15:35     ` Bruce Richardson
2020-09-04 14:25       ` Ferruh Yigit
2020-09-04 15:14         ` Bruce Richardson
2020-09-04 15:30         ` Bruce Richardson
2020-08-29  7:13   ` [dpdk-dev] [PATCH v3 2/4] app/testpmd: add Rx buffer size display in queue info query Chengchang Tang
2020-08-29  7:13   ` [dpdk-dev] [PATCH v3 3/4] app/procinfo: add Rx buffer size to --show-port Chengchang Tang
2020-08-29  7:13   ` [dpdk-dev] [PATCH v3 4/4] net/hns3: add Rx buffer size to Rx qinfo query Chengchang Tang
2020-09-05  9:07 ` [dpdk-dev] [PATCH v4 0/5] add Rx buffer size for rxq info structure Chengchang Tang
2020-09-05  9:07   ` [dpdk-dev] [PATCH v4 1/5] ethdev: add a field " Chengchang Tang
2020-09-05 16:50     ` Stephen Hemminger
2020-09-05  9:07   ` [dpdk-dev] [PATCH v4 2/5] app/testpmd: add Rx buffer size display in queue info query Chengchang Tang
2020-09-18  8:54     ` Ferruh Yigit
2020-09-20  8:47       ` Chengchang Tang
2020-09-05  9:07   ` [dpdk-dev] [PATCH v4 3/5] app/procinfo: add Rx buffer size to --show-port Chengchang Tang
2020-09-05 16:59     ` Stephen Hemminger
2020-09-07  9:14       ` Chengchang Tang
2020-09-18 22:11         ` Stephen Hemminger
2020-09-21  2:06           ` Chengchang Tang
2020-09-21 11:26             ` Ferruh Yigit
2020-09-05  9:07   ` [dpdk-dev] [PATCH v4 4/5] net/hns3: add Rx buffer size to Rx qinfo query Chengchang Tang
2020-09-05  9:07   ` [dpdk-dev] [PATCH v4 5/5] doc: remove rxq info structure deprecation notice Chengchang Tang
2020-09-05 16:33     ` Thomas Monjalon
2020-09-07  9:12       ` Chengchang Tang
2020-09-21 13:22 ` [dpdk-dev] [PATCH v5 0/2] add Rx buffer size for rxq info structure Chengchang Tang
2020-09-21 13:22   ` [dpdk-dev] [PATCH v5 1/2] ethdev: add a field " Chengchang Tang
2020-09-21 14:18     ` Ferruh Yigit
2020-09-21 13:22   ` [dpdk-dev] [PATCH v5 2/2] net/hns3: add Rx buffer size to Rx qinfo query Chengchang Tang
2020-09-21 14:19   ` [dpdk-dev] [PATCH v5 0/2] add Rx buffer size for rxq info structure Ferruh Yigit

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).