DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 1/3] ethdev: add max burst size to device info
@ 2017-12-12 10:05 Nikhil Agarwal
  2017-12-12 10:05 ` [dpdk-dev] [PATCH 2/3] net/dpaa: implement max burst size in dev info Nikhil Agarwal
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Nikhil Agarwal @ 2017-12-12 10:05 UTC (permalink / raw)
  To: dev; +Cc: david.hunt, nikhil.agarwal, hemant.agrawal, ferruh.yigit

Currently, if the  rte_eth_rx_burst() function returns a value less than
*nb_pkts*, the application will assume that no more packets are present.

Some of the hw queue based hardware can only support smaller burst for RX
and TX and thus break the expectation of the rx_burst API.

This patch adds support to provide the maximum burst size that can be
supported by a given PMD. The dev_info is being memset to '0' in
rte_ethdev library. The value of '0' indicates that any value for burst
size can be supported i.e. no change for existing PMDs.

The application can now use the lowest available max_burst_size value
for rte_eth_rx_burst.

Signed-off-by: Nikhil Agarwal <nikhil.agarwal@linaro.org>
---
 lib/librte_ether/rte_ethdev.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 341c2d6..3ab6f02 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -1047,6 +1047,7 @@ struct rte_eth_dev_info {
 	/** Configured number of rx/tx queues */
 	uint16_t nb_rx_queues; /**< Number of RX queues. */
 	uint16_t nb_tx_queues; /**< Number of TX queues. */
+	uint16_t max_burst_size; /**< MAX burst size, 0 for no limit. */
 };
 
 /**
-- 
2.7.4

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

* [dpdk-dev] [PATCH 2/3] net/dpaa: implement max burst size in dev info
  2017-12-12 10:05 [dpdk-dev] [PATCH 1/3] ethdev: add max burst size to device info Nikhil Agarwal
@ 2017-12-12 10:05 ` Nikhil Agarwal
  2017-12-12 10:05 ` [dpdk-dev] [PATCH 3/3] examples/l3fwd-power: use device max burst size Nikhil Agarwal
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 13+ messages in thread
From: Nikhil Agarwal @ 2017-12-12 10:05 UTC (permalink / raw)
  To: dev; +Cc: david.hunt, nikhil.agarwal, hemant.agrawal, ferruh.yigit

Signed-off-by: Nikhil Agarwal <nikhil.agarwal@linaro.org>
---
 drivers/net/dpaa/dpaa_ethdev.c | 1 +
 drivers/net/dpaa/dpaa_ethdev.h | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/net/dpaa/dpaa_ethdev.c b/drivers/net/dpaa/dpaa_ethdev.c
index cf5a2ec..79c6cc4 100644
--- a/drivers/net/dpaa/dpaa_ethdev.c
+++ b/drivers/net/dpaa/dpaa_ethdev.c
@@ -242,6 +242,7 @@ static void dpaa_eth_dev_info(struct rte_eth_dev *dev,
 	dev_info->max_tx_queues = dpaa_intf->nb_tx_queues;
 	dev_info->min_rx_bufsize = DPAA_MIN_RX_BUF_SIZE;
 	dev_info->max_rx_pktlen = DPAA_MAX_RX_PKT_LEN;
+	dev_info->max_burst_size = DPAA_MAX_BURST_SIZE;
 	dev_info->max_mac_addrs = DPAA_MAX_MAC_FILTER;
 	dev_info->max_hash_mac_addrs = 0;
 	dev_info->max_vfs = 0;
diff --git a/drivers/net/dpaa/dpaa_ethdev.h b/drivers/net/dpaa/dpaa_ethdev.h
index 5457d61..0400dce 100644
--- a/drivers/net/dpaa/dpaa_ethdev.h
+++ b/drivers/net/dpaa/dpaa_ethdev.h
@@ -64,6 +64,7 @@
 
 #define DPAA_MIN_RX_BUF_SIZE 512
 #define DPAA_MAX_RX_PKT_LEN  10240
+#define DPAA_MAX_BURST_SIZE			16
 
 /* RX queue tail drop threshold
  * currently considering 32 KB packets.
-- 
2.7.4

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

* [dpdk-dev] [PATCH 3/3] examples/l3fwd-power: use device max burst size
  2017-12-12 10:05 [dpdk-dev] [PATCH 1/3] ethdev: add max burst size to device info Nikhil Agarwal
  2017-12-12 10:05 ` [dpdk-dev] [PATCH 2/3] net/dpaa: implement max burst size in dev info Nikhil Agarwal
@ 2017-12-12 10:05 ` Nikhil Agarwal
  2017-12-12 10:45 ` [dpdk-dev] [PATCH 1/3] ethdev: add max burst size to device info Matan Azrad
  2018-05-22 22:17 ` Thomas Monjalon
  3 siblings, 0 replies; 13+ messages in thread
From: Nikhil Agarwal @ 2017-12-12 10:05 UTC (permalink / raw)
  To: dev; +Cc: david.hunt, nikhil.agarwal, hemant.agrawal, ferruh.yigit

On some of the hardware e.g. DPAA, rx burst can only return
upto 16 packets, which causes the application to assume that
no more packets are present.

This patch modifies the application to use device published
packet burst size.

Signed-off-by: Nikhil Agarwal <nikhil.agarwal@linaro.org>
---
 examples/l3fwd-power/main.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/examples/l3fwd-power/main.c b/examples/l3fwd-power/main.c
index d80f663..000ac53 100644
--- a/examples/l3fwd-power/main.c
+++ b/examples/l3fwd-power/main.c
@@ -145,6 +145,7 @@
 #define RTE_TEST_TX_DESC_DEFAULT 512
 static uint16_t nb_rxd = RTE_TEST_RX_DESC_DEFAULT;
 static uint16_t nb_txd = RTE_TEST_TX_DESC_DEFAULT;
+static uint16_t max_pkt_burst = MAX_PKT_BURST;
 
 /* ethernet addresses of ports */
 static struct ether_addr ports_eth_addr[RTE_MAX_ETHPORTS];
@@ -421,7 +422,7 @@ power_timer_cb(__attribute__((unused)) struct rte_timer *tim,
 			rte_power_freq_down(lcore_id);
 	}
 	else if ( (unsigned)(stats[lcore_id].nb_rx_processed /
-		stats[lcore_id].nb_iteration_looped) < MAX_PKT_BURST) {
+		stats[lcore_id].nb_iteration_looped) < max_pkt_burst) {
 		/**
 		 * scale down a step if average packet per iteration less
 		 * than expectation.
@@ -759,9 +760,9 @@ power_freq_scaleup_heuristic(unsigned lcore_id,
  * HW Rx queue size is 128 by default, Rx burst read at maximum 32 entries
  * per iteration
  */
-#define FREQ_GEAR1_RX_PACKET_THRESHOLD             MAX_PKT_BURST
-#define FREQ_GEAR2_RX_PACKET_THRESHOLD             (MAX_PKT_BURST*2)
-#define FREQ_GEAR3_RX_PACKET_THRESHOLD             (MAX_PKT_BURST*3)
+#define FREQ_GEAR1_RX_PACKET_THRESHOLD             max_pkt_burst
+#define FREQ_GEAR2_RX_PACKET_THRESHOLD             (max_pkt_burst*2)
+#define FREQ_GEAR3_RX_PACKET_THRESHOLD             (max_pkt_burst*3)
 #define FREQ_UP_TREND1_ACC   1
 #define FREQ_UP_TREND2_ACC   100
 #define FREQ_UP_THRESHOLD    10000
@@ -950,7 +951,7 @@ main_loop(__attribute__((unused)) void *dummy)
 			queueid = rx_queue->queue_id;
 
 			nb_rx = rte_eth_rx_burst(portid, queueid, pkts_burst,
-								MAX_PKT_BURST);
+								max_pkt_burst);
 
 			stats[lcore_id].nb_rx_processed += nb_rx;
 			if (unlikely(nb_rx == 0)) {
@@ -1703,6 +1704,9 @@ main(int argc, char **argv)
 		dev_rxq_num = dev_info.max_rx_queues;
 		dev_txq_num = dev_info.max_tx_queues;
 
+		if (dev_info.max_burst_size &&
+				dev_info.max_burst_size < max_pkt_burst)
+			max_pkt_burst = dev_info.max_burst_size;
 		nb_rx_queue = get_port_n_rx_queues(portid);
 		if (nb_rx_queue > dev_rxq_num)
 			rte_exit(EXIT_FAILURE,
-- 
2.7.4

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

* Re: [dpdk-dev] [PATCH 1/3] ethdev: add max burst size to device info
  2017-12-12 10:05 [dpdk-dev] [PATCH 1/3] ethdev: add max burst size to device info Nikhil Agarwal
  2017-12-12 10:05 ` [dpdk-dev] [PATCH 2/3] net/dpaa: implement max burst size in dev info Nikhil Agarwal
  2017-12-12 10:05 ` [dpdk-dev] [PATCH 3/3] examples/l3fwd-power: use device max burst size Nikhil Agarwal
@ 2017-12-12 10:45 ` Matan Azrad
  2017-12-12 11:03   ` Ananyev, Konstantin
  2018-05-22 22:17 ` Thomas Monjalon
  3 siblings, 1 reply; 13+ messages in thread
From: Matan Azrad @ 2017-12-12 10:45 UTC (permalink / raw)
  To: Nikhil Agarwal, dev
  Cc: david.hunt, nikhil.agarwal, hemant.agrawal, ferruh.yigit

Hi Nikhil

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Nikhil Agarwal
> Sent: Tuesday, December 12, 2017 12:05 PM
> To: dev@dpdk.org
> Cc: david.hunt@intel.com; nikhil.agarwal@nxp.com;
> hemant.agrawal@nxp.com; ferruh.yigit@intel.com
> Subject: [dpdk-dev] [PATCH 1/3] ethdev: add max burst size to device info
> 
> Currently, if the  rte_eth_rx_burst() function returns a value less than
> *nb_pkts*, the application will assume that no more packets are present.
> 
> Some of the hw queue based hardware can only support smaller burst for RX
> and TX and thus break the expectation of the rx_burst API.
>

Doesn't such like devices PMDs should try to retrieve multiple HW burst to adjust the asked received  packet number?
 
> This patch adds support to provide the maximum burst size that can be
> supported by a given PMD. The dev_info is being memset to '0' in
> rte_ethdev library. The value of '0' indicates that any value for burst size can
> be supported i.e. no change for existing PMDs.
> 
> The application can now use the lowest available max_burst_size value for
> rte_eth_rx_burst.
> 

If you are talking about performance, maybe the right field to expose is something like "perf_burst_size" or "preferred_burst_size".
I also suggest to expose different fields for RX and for TX.
Maybe the rte_eth_rx\tx_burst() descriptions should be updated. 

Thanks
Matan.

> Signed-off-by: Nikhil Agarwal <nikhil.agarwal@linaro.org>
> ---
>  lib/librte_ether/rte_ethdev.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> index 341c2d6..3ab6f02 100644
> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> @@ -1047,6 +1047,7 @@ struct rte_eth_dev_info {
>  	/** Configured number of rx/tx queues */
>  	uint16_t nb_rx_queues; /**< Number of RX queues. */
>  	uint16_t nb_tx_queues; /**< Number of TX queues. */
> +	uint16_t max_burst_size; /**< MAX burst size, 0 for no limit. */
>  };
> 
>  /**
> --
> 2.7.4

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

* Re: [dpdk-dev] [PATCH 1/3] ethdev: add max burst size to device info
  2017-12-12 10:45 ` [dpdk-dev] [PATCH 1/3] ethdev: add max burst size to device info Matan Azrad
@ 2017-12-12 11:03   ` Ananyev, Konstantin
  2017-12-12 13:43     ` Shreyansh Jain
  0 siblings, 1 reply; 13+ messages in thread
From: Ananyev, Konstantin @ 2017-12-12 11:03 UTC (permalink / raw)
  To: Matan Azrad, Nikhil Agarwal, dev
  Cc: Hunt, David, nikhil.agarwal, hemant.agrawal, Yigit, Ferruh



> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Matan Azrad
> Sent: Tuesday, December 12, 2017 10:46 AM
> To: Nikhil Agarwal <nikhil.agarwal@linaro.org>; dev@dpdk.org
> Cc: Hunt, David <david.hunt@intel.com>; nikhil.agarwal@nxp.com; hemant.agrawal@nxp.com; Yigit, Ferruh <ferruh.yigit@intel.com>
> Subject: Re: [dpdk-dev] [PATCH 1/3] ethdev: add max burst size to device info
> 
> Hi Nikhil
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Nikhil Agarwal
> > Sent: Tuesday, December 12, 2017 12:05 PM
> > To: dev@dpdk.org
> > Cc: david.hunt@intel.com; nikhil.agarwal@nxp.com;
> > hemant.agrawal@nxp.com; ferruh.yigit@intel.com
> > Subject: [dpdk-dev] [PATCH 1/3] ethdev: add max burst size to device info
> >
> > Currently, if the  rte_eth_rx_burst() function returns a value less than
> > *nb_pkts*, the application will assume that no more packets are present.
> >
> > Some of the hw queue based hardware can only support smaller burst for RX
> > and TX and thus break the expectation of the rx_burst API.
> >
> 
> Doesn't such like devices PMDs should try to retrieve multiple HW burst to adjust the asked received  packet number?

Same thought here...
Can't that limitation be hidden inside PMD by calling HW burst multiple times? 
Also if I am not mistaken - it would increase size of struct rte_eth_dev_info, right?
If so, then it means ABI breakage.
Konstantin

> 
> > This patch adds support to provide the maximum burst size that can be
> > supported by a given PMD. The dev_info is being memset to '0' in
> > rte_ethdev library. The value of '0' indicates that any value for burst size can
> > be supported i.e. no change for existing PMDs.
> >
> > The application can now use the lowest available max_burst_size value for
> > rte_eth_rx_burst.
> >
> 
> If you are talking about performance, maybe the right field to expose is something like "perf_burst_size" or "preferred_burst_size".
> I also suggest to expose different fields for RX and for TX.
> Maybe the rte_eth_rx\tx_burst() descriptions should be updated.
> 
> Thanks
> Matan.
> 
> > Signed-off-by: Nikhil Agarwal <nikhil.agarwal@linaro.org>
> > ---
> >  lib/librte_ether/rte_ethdev.h | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> > index 341c2d6..3ab6f02 100644
> > --- a/lib/librte_ether/rte_ethdev.h
> > +++ b/lib/librte_ether/rte_ethdev.h
> > @@ -1047,6 +1047,7 @@ struct rte_eth_dev_info {
> >  	/** Configured number of rx/tx queues */
> >  	uint16_t nb_rx_queues; /**< Number of RX queues. */
> >  	uint16_t nb_tx_queues; /**< Number of TX queues. */
> > +	uint16_t max_burst_size; /**< MAX burst size, 0 for no limit. */
> >  };
> >
> >  /**
> > --
> > 2.7.4

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

* Re: [dpdk-dev] [PATCH 1/3] ethdev: add max burst size to device info
  2017-12-12 11:03   ` Ananyev, Konstantin
@ 2017-12-12 13:43     ` Shreyansh Jain
  2017-12-13 12:52       ` Ananyev, Konstantin
  0 siblings, 1 reply; 13+ messages in thread
From: Shreyansh Jain @ 2017-12-12 13:43 UTC (permalink / raw)
  To: Ananyev, Konstantin, Matan Azrad, Nikhil Agarwal
  Cc: dev, Hunt, David, nikhil.agarwal, hemant.agrawal, Yigit, Ferruh

On Tuesday 12 December 2017 04:33 PM, Ananyev, Konstantin wrote:
> 
> 
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Matan Azrad
>> Sent: Tuesday, December 12, 2017 10:46 AM
>> To: Nikhil Agarwal <nikhil.agarwal@linaro.org>; dev@dpdk.org
>> Cc: Hunt, David <david.hunt@intel.com>; nikhil.agarwal@nxp.com; hemant.agrawal@nxp.com; Yigit, Ferruh <ferruh.yigit@intel.com>
>> Subject: Re: [dpdk-dev] [PATCH 1/3] ethdev: add max burst size to device info
>>
>> Hi Nikhil
>>
>>> -----Original Message-----
>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Nikhil Agarwal
>>> Sent: Tuesday, December 12, 2017 12:05 PM
>>> To: dev@dpdk.org
>>> Cc: david.hunt@intel.com; nikhil.agarwal@nxp.com;
>>> hemant.agrawal@nxp.com; ferruh.yigit@intel.com
>>> Subject: [dpdk-dev] [PATCH 1/3] ethdev: add max burst size to device info
>>>
>>> Currently, if the  rte_eth_rx_burst() function returns a value less than
>>> *nb_pkts*, the application will assume that no more packets are present..
>>>
>>> Some of the hw queue based hardware can only support smaller burst for RX
>>> and TX and thus break the expectation of the rx_burst API.
>>>
>>
>> Doesn't such like devices PMDs should try to retrieve multiple HW burst to adjust the asked received  packet number?
> 
> Same thought here...
> Can't that limitation be hidden inside PMD by calling HW burst multiple times?

This might be required in some cases for performance.
It is possible that for each request containing N buffers, if the PMD 
fetches all N (more than its preferred burst_size), cache misses reduce 
the performance - especially for SoC with limited cache size.

Also, a complete cycle of 
application->driver->hardware->driver->application can help driver 
prefetch buffers - which, in case of hw burst looping, might be too 
little to complete the prefetch cycle.

To summarize, indeed this is for performance specific cases and the idea 
that @Matan gave for renaming 'perf_buf_size' to highlight this, sounds 
logical.

> Also if I am not mistaken - it would increase size of struct rte_eth_dev_info, right?
> If so, then it means ABI breakage.

Yes, deprecation notice should have been sent - if we continue with the 
dev_info change. To me that looks as one of the option. Maybe, someone 
on the list has a better idea.

> Konstantin
> 
>>
>>> This patch adds support to provide the maximum burst size that can be
>>> supported by a given PMD. The dev_info is being memset to '0' in
>>> rte_ethdev library. The value of '0' indicates that any value for burst size can
>>> be supported i.e. no change for existing PMDs.
>>>
>>> The application can now use the lowest available max_burst_size value for
>>> rte_eth_rx_burst.
>>>
>>
>> If you are talking about performance, maybe the right field to expose is something like "perf_burst_size" or "preferred_burst_size".
>> I also suggest to expose different fields for RX and for TX.
>> Maybe the rte_eth_rx\tx_burst() descriptions should be updated.
>>
>> Thanks
>> Matan.
>>
>>> Signed-off-by: Nikhil Agarwal <nikhil.agarwal@linaro.org>
>>> ---
>>>   lib/librte_ether/rte_ethdev.h | 1 +
>>>   1 file changed, 1 insertion(+)
>>>
>>> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
>>> index 341c2d6..3ab6f02 100644
>>> --- a/lib/librte_ether/rte_ethdev.h
>>> +++ b/lib/librte_ether/rte_ethdev.h
>>> @@ -1047,6 +1047,7 @@ struct rte_eth_dev_info {
>>>   	/** Configured number of rx/tx queues */
>>>   	uint16_t nb_rx_queues; /**< Number of RX queues. */
>>>   	uint16_t nb_tx_queues; /**< Number of TX queues. */
>>> +	uint16_t max_burst_size; /**< MAX burst size, 0 for no limit. */
>>>   };
>>>
>>>   /**
>>> --
>>> 2.7.4
> 
> 

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

* Re: [dpdk-dev] [PATCH 1/3] ethdev: add max burst size to device info
  2017-12-12 13:43     ` Shreyansh Jain
@ 2017-12-13 12:52       ` Ananyev, Konstantin
  2017-12-13 15:22         ` Shreyansh Jain
  0 siblings, 1 reply; 13+ messages in thread
From: Ananyev, Konstantin @ 2017-12-13 12:52 UTC (permalink / raw)
  To: Shreyansh Jain, Matan Azrad, Nikhil Agarwal
  Cc: dev, Hunt, David, nikhil.agarwal, hemant.agrawal, Yigit, Ferruh



> -----Original Message-----
> From: Shreyansh Jain [mailto:shreyansh.jain@nxp.com]
> Sent: Tuesday, December 12, 2017 1:44 PM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Matan Azrad <matan@mellanox.com>; Nikhil Agarwal
> <nikhil.agarwal@linaro.org>
> Cc: dev@dpdk.org; Hunt, David <david.hunt@intel.com>; nikhil.agarwal@nxp.com; hemant.agrawal@nxp.com; Yigit, Ferruh
> <ferruh.yigit@intel.com>
> Subject: Re: [PATCH 1/3] ethdev: add max burst size to device info
> 
> On Tuesday 12 December 2017 04:33 PM, Ananyev, Konstantin wrote:
> >
> >
> >> -----Original Message-----
> >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Matan Azrad
> >> Sent: Tuesday, December 12, 2017 10:46 AM
> >> To: Nikhil Agarwal <nikhil.agarwal@linaro.org>; dev@dpdk.org
> >> Cc: Hunt, David <david.hunt@intel.com>; nikhil.agarwal@nxp.com; hemant.agrawal@nxp.com; Yigit, Ferruh <ferruh.yigit@intel.com>
> >> Subject: Re: [dpdk-dev] [PATCH 1/3] ethdev: add max burst size to device info
> >>
> >> Hi Nikhil
> >>
> >>> -----Original Message-----
> >>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Nikhil Agarwal
> >>> Sent: Tuesday, December 12, 2017 12:05 PM
> >>> To: dev@dpdk.org
> >>> Cc: david.hunt@intel.com; nikhil.agarwal@nxp.com;
> >>> hemant.agrawal@nxp.com; ferruh.yigit@intel.com
> >>> Subject: [dpdk-dev] [PATCH 1/3] ethdev: add max burst size to device info
> >>>
> >>> Currently, if the  rte_eth_rx_burst() function returns a value less than
> >>> *nb_pkts*, the application will assume that no more packets are present..
> >>>
> >>> Some of the hw queue based hardware can only support smaller burst for RX
> >>> and TX and thus break the expectation of the rx_burst API.
> >>>
> >>
> >> Doesn't such like devices PMDs should try to retrieve multiple HW burst to adjust the asked received  packet number?
> >
> > Same thought here...
> > Can't that limitation be hidden inside PMD by calling HW burst multiple times?
> 
> This might be required in some cases for performance.
> It is possible that for each request containing N buffers, if the PMD
> fetches all N (more than its preferred burst_size), cache misses reduce
> the performance - especially for SoC with limited cache size.
> 
> Also, a complete cycle of
> application->driver->hardware->driver->application can help driver
> prefetch buffers - which, in case of hw burst looping, might be too
> little to complete the prefetch cycle.
> 
> To summarize, indeed this is for performance specific cases and the idea
> that @Matan gave for renaming 'perf_buf_size' to highlight this, sounds
> logical.
> 
> > Also if I am not mistaken - it would increase size of struct rte_eth_dev_info, right?
> > If so, then it means ABI breakage.
> 
> Yes, deprecation notice should have been sent - if we continue with the
> dev_info change. To me that looks as one of the option. Maybe, someone
> on the list has a better idea.

In theory there is at least 2 free bytes at the end of struct rte_eth_desc_lim.
Might be it would help a bit.
Konstantin

> 
> > Konstantin
> >
> >>
> >>> This patch adds support to provide the maximum burst size that can be
> >>> supported by a given PMD. The dev_info is being memset to '0' in
> >>> rte_ethdev library. The value of '0' indicates that any value for burst size can
> >>> be supported i.e. no change for existing PMDs.
> >>>
> >>> The application can now use the lowest available max_burst_size value for
> >>> rte_eth_rx_burst.
> >>>
> >>
> >> If you are talking about performance, maybe the right field to expose is something like "perf_burst_size" or "preferred_burst_size".
> >> I also suggest to expose different fields for RX and for TX.
> >> Maybe the rte_eth_rx\tx_burst() descriptions should be updated.
> >>
> >> Thanks
> >> Matan.
> >>
> >>> Signed-off-by: Nikhil Agarwal <nikhil.agarwal@linaro.org>
> >>> ---
> >>>   lib/librte_ether/rte_ethdev.h | 1 +
> >>>   1 file changed, 1 insertion(+)
> >>>
> >>> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> >>> index 341c2d6..3ab6f02 100644
> >>> --- a/lib/librte_ether/rte_ethdev.h
> >>> +++ b/lib/librte_ether/rte_ethdev.h
> >>> @@ -1047,6 +1047,7 @@ struct rte_eth_dev_info {
> >>>   	/** Configured number of rx/tx queues */
> >>>   	uint16_t nb_rx_queues; /**< Number of RX queues. */
> >>>   	uint16_t nb_tx_queues; /**< Number of TX queues. */
> >>> +	uint16_t max_burst_size; /**< MAX burst size, 0 for no limit. */
> >>>   };
> >>>
> >>>   /**
> >>> --
> >>> 2.7.4
> >
> >


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

* Re: [dpdk-dev] [PATCH 1/3] ethdev: add max burst size to device info
  2017-12-13 12:52       ` Ananyev, Konstantin
@ 2017-12-13 15:22         ` Shreyansh Jain
  0 siblings, 0 replies; 13+ messages in thread
From: Shreyansh Jain @ 2017-12-13 15:22 UTC (permalink / raw)
  To: Ananyev, Konstantin, Matan Azrad, Nikhil Agarwal
  Cc: dev, Hunt, David, nikhil.agarwal, hemant.agrawal, Yigit, Ferruh

On Wednesday 13 December 2017 06:22 PM, Ananyev, Konstantin wrote:
> 
> 
>> -----Original Message-----
>> From: Shreyansh Jain [mailto:shreyansh.jain@nxp.com]
>> Sent: Tuesday, December 12, 2017 1:44 PM
>> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Matan Azrad <matan@mellanox.com>; Nikhil Agarwal
>> <nikhil.agarwal@linaro.org>
>> Cc: dev@dpdk.org; Hunt, David <david.hunt@intel.com>; nikhil.agarwal@nxp.com; hemant.agrawal@nxp.com; Yigit, Ferruh
>> <ferruh.yigit@intel.com>
>> Subject: Re: [PATCH 1/3] ethdev: add max burst size to device info
>>

[...]

>>>> Doesn't such like devices PMDs should try to retrieve multiple HW burst to adjust the asked received  packet number?
>>>
>>> Same thought here...
>>> Can't that limitation be hidden inside PMD by calling HW burst multiple times?
>>
>> This might be required in some cases for performance.
>> It is possible that for each request containing N buffers, if the PMD
>> fetches all N (more than its preferred burst_size), cache misses reduce
>> the performance - especially for SoC with limited cache size.
>>
>> Also, a complete cycle of
>> application->driver->hardware->driver->application can help driver
>> prefetch buffers - which, in case of hw burst looping, might be too
>> little to complete the prefetch cycle.
>>
>> To summarize, indeed this is for performance specific cases and the idea
>> that @Matan gave for renaming 'perf_buf_size' to highlight this, sounds
>> logical.
>>
>>> Also if I am not mistaken - it would increase size of struct rte_eth_dev_info, right?
>>> If so, then it means ABI breakage.
>>
>> Yes, deprecation notice should have been sent - if we continue with the
>> dev_info change. To me that looks as one of the option. Maybe, someone
>> on the list has a better idea.
> 
> In theory there is at least 2 free bytes at the end of struct rte_eth_desc_lim.
> Might be it would help a bit.

:) Thanks for highlighting.
So, you feel it is OK to reuse this assumption of hole in alignment? It 
certainly would bypass the ABI limitation but not sure which cases it 
would break (for example, same assumptions by others)

It would an issue in case of 32bit systems, though.

[...]

-
Shreyansh

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

* Re: [dpdk-dev] [PATCH 1/3] ethdev: add max burst size to device info
  2017-12-12 10:05 [dpdk-dev] [PATCH 1/3] ethdev: add max burst size to device info Nikhil Agarwal
                   ` (2 preceding siblings ...)
  2017-12-12 10:45 ` [dpdk-dev] [PATCH 1/3] ethdev: add max burst size to device info Matan Azrad
@ 2018-05-22 22:17 ` Thomas Monjalon
  2019-04-05 14:55   ` Ferruh Yigit
  3 siblings, 1 reply; 13+ messages in thread
From: Thomas Monjalon @ 2018-05-22 22:17 UTC (permalink / raw)
  To: Nikhil Agarwal
  Cc: dev, david.hunt, nikhil.agarwal, hemant.agrawal, ferruh.yigit,
	arybchenko, shahafs

12/12/2017 11:05, Nikhil Agarwal:
> Currently, if the  rte_eth_rx_burst() function returns a value less than
> *nb_pkts*, the application will assume that no more packets are present.
> 
> Some of the hw queue based hardware can only support smaller burst for RX
> and TX and thus break the expectation of the rx_burst API.
> 
> This patch adds support to provide the maximum burst size that can be
> supported by a given PMD. The dev_info is being memset to '0' in
> rte_ethdev library. The value of '0' indicates that any value for burst
> size can be supported i.e. no change for existing PMDs.
> 
> The application can now use the lowest available max_burst_size value
> for rte_eth_rx_burst.
> 
> Signed-off-by: Nikhil Agarwal <nikhil.agarwal@linaro.org>
> ---
> @@ -1047,6 +1047,7 @@ struct rte_eth_dev_info {
>  	/** Configured number of rx/tx queues */
>  	uint16_t nb_rx_queues; /**< Number of RX queues. */
>  	uint16_t nb_tx_queues; /**< Number of TX queues. */
> +	uint16_t max_burst_size; /**< MAX burst size, 0 for no limit. */
>  };

What is the status of this proposal?

Recently, the preferred tuning have been added by
	"ethdev: support PMD-tuned Tx/Rx parameters"
	http://dpdk.org/commit/3be82f5cc5

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

* Re: [dpdk-dev] [PATCH 1/3] ethdev: add max burst size to device info
  2018-05-22 22:17 ` Thomas Monjalon
@ 2019-04-05 14:55   ` Ferruh Yigit
  2019-04-05 14:55     ` Ferruh Yigit
  2019-04-05 14:57     ` Yigit, Ferruh
  0 siblings, 2 replies; 13+ messages in thread
From: Ferruh Yigit @ 2019-04-05 14:55 UTC (permalink / raw)
  To: Nikhil Agarwal, nikhil.agarwal, hemant.agrawal
  Cc: Thomas Monjalon, dev, david.hunt, arybchenko, shahafs

On 5/22/2018 11:17 PM, Thomas Monjalon wrote:
> 12/12/2017 11:05, Nikhil Agarwal:
>> Currently, if the  rte_eth_rx_burst() function returns a value less than
>> *nb_pkts*, the application will assume that no more packets are present.
>>
>> Some of the hw queue based hardware can only support smaller burst for RX
>> and TX and thus break the expectation of the rx_burst API.
>>
>> This patch adds support to provide the maximum burst size that can be
>> supported by a given PMD. The dev_info is being memset to '0' in
>> rte_ethdev library. The value of '0' indicates that any value for burst
>> size can be supported i.e. no change for existing PMDs.
>>
>> The application can now use the lowest available max_burst_size value
>> for rte_eth_rx_burst.
>>
>> Signed-off-by: Nikhil Agarwal <nikhil.agarwal@linaro.org>
>> ---
>> @@ -1047,6 +1047,7 @@ struct rte_eth_dev_info {
>>  	/** Configured number of rx/tx queues */
>>  	uint16_t nb_rx_queues; /**< Number of RX queues. */
>>  	uint16_t nb_tx_queues; /**< Number of TX queues. */
>> +	uint16_t max_burst_size; /**< MAX burst size, 0 for no limit. */
>>  };
> 
> What is the status of this proposal?
> 
> Recently, the preferred tuning have been added by
> 	"ethdev: support PMD-tuned Tx/Rx parameters"
> 	http://dpdk.org/commit/3be82f5cc5

Hi Nikhil, Hemant,

PMD returning preferred 'burst_size' support already added, I guess this
patchset is no more valid. I am updating this as rejected.

If something is missing or there are still some relevant pieces in this patch,
please send as a new version on top of latest head.

Thanks,
ferruh

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

* Re: [dpdk-dev] [PATCH 1/3] ethdev: add max burst size to device info
  2019-04-05 14:55   ` Ferruh Yigit
@ 2019-04-05 14:55     ` Ferruh Yigit
  2019-04-05 14:57     ` Yigit, Ferruh
  1 sibling, 0 replies; 13+ messages in thread
From: Ferruh Yigit @ 2019-04-05 14:55 UTC (permalink / raw)
  To: Nikhil Agarwal, nikhil.agarwal, hemant.agrawal
  Cc: Thomas Monjalon, dev, david.hunt, arybchenko, shahafs

On 5/22/2018 11:17 PM, Thomas Monjalon wrote:
> 12/12/2017 11:05, Nikhil Agarwal:
>> Currently, if the  rte_eth_rx_burst() function returns a value less than
>> *nb_pkts*, the application will assume that no more packets are present.
>>
>> Some of the hw queue based hardware can only support smaller burst for RX
>> and TX and thus break the expectation of the rx_burst API.
>>
>> This patch adds support to provide the maximum burst size that can be
>> supported by a given PMD. The dev_info is being memset to '0' in
>> rte_ethdev library. The value of '0' indicates that any value for burst
>> size can be supported i.e. no change for existing PMDs.
>>
>> The application can now use the lowest available max_burst_size value
>> for rte_eth_rx_burst.
>>
>> Signed-off-by: Nikhil Agarwal <nikhil.agarwal@linaro.org>
>> ---
>> @@ -1047,6 +1047,7 @@ struct rte_eth_dev_info {
>>  	/** Configured number of rx/tx queues */
>>  	uint16_t nb_rx_queues; /**< Number of RX queues. */
>>  	uint16_t nb_tx_queues; /**< Number of TX queues. */
>> +	uint16_t max_burst_size; /**< MAX burst size, 0 for no limit. */
>>  };
> 
> What is the status of this proposal?
> 
> Recently, the preferred tuning have been added by
> 	"ethdev: support PMD-tuned Tx/Rx parameters"
> 	http://dpdk.org/commit/3be82f5cc5

Hi Nikhil, Hemant,

PMD returning preferred 'burst_size' support already added, I guess this
patchset is no more valid. I am updating this as rejected.

If something is missing or there are still some relevant pieces in this patch,
please send as a new version on top of latest head.

Thanks,
ferruh

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

* Re: [dpdk-dev] [PATCH 1/3] ethdev: add max burst size to device info
  2019-04-05 14:55   ` Ferruh Yigit
  2019-04-05 14:55     ` Ferruh Yigit
@ 2019-04-05 14:57     ` Yigit, Ferruh
  2019-04-05 14:57       ` Yigit, Ferruh
  1 sibling, 1 reply; 13+ messages in thread
From: Yigit, Ferruh @ 2019-04-05 14:57 UTC (permalink / raw)
  To: Ferruh Yigit, Nikhil Agarwal, nikhil.agarwal, hemant.agrawal
  Cc: Thomas Monjalon, dev, david.hunt, arybchenko, shahafs

On 4/5/2019 3:55 PM, Ferruh Yigit wrote:
> On 5/22/2018 11:17 PM, Thomas Monjalon wrote:
>> 12/12/2017 11:05, Nikhil Agarwal:
>>> Currently, if the  rte_eth_rx_burst() function returns a value less than
>>> *nb_pkts*, the application will assume that no more packets are present.
>>>
>>> Some of the hw queue based hardware can only support smaller burst for RX
>>> and TX and thus break the expectation of the rx_burst API.
>>>
>>> This patch adds support to provide the maximum burst size that can be
>>> supported by a given PMD. The dev_info is being memset to '0' in
>>> rte_ethdev library. The value of '0' indicates that any value for burst
>>> size can be supported i.e. no change for existing PMDs.
>>>
>>> The application can now use the lowest available max_burst_size value
>>> for rte_eth_rx_burst.
>>>
>>> Signed-off-by: Nikhil Agarwal <nikhil.agarwal@linaro.org>
>>> ---
>>> @@ -1047,6 +1047,7 @@ struct rte_eth_dev_info {
>>>  	/** Configured number of rx/tx queues */
>>>  	uint16_t nb_rx_queues; /**< Number of RX queues. */
>>>  	uint16_t nb_tx_queues; /**< Number of TX queues. */
>>> +	uint16_t max_burst_size; /**< MAX burst size, 0 for no limit. */
>>>  };
>>
>> What is the status of this proposal?
>>
>> Recently, the preferred tuning have been added by
>> 	"ethdev: support PMD-tuned Tx/Rx parameters"
>> 	http://dpdk.org/commit/3be82f5cc5
> 
> Hi Nikhil, Hemant,
> 
> PMD returning preferred 'burst_size' support already added, I guess this
> patchset is no more valid. I am updating this as rejected.
> 
> If something is missing or there are still some relevant pieces in this patch,
> please send as a new version on top of latest head.

As reference, mentioned patches:
https://patches.dpdk.org/patch/32112/
https://patches.dpdk.org/patch/32113/
https://patches.dpdk.org/patch/32114/

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

* Re: [dpdk-dev] [PATCH 1/3] ethdev: add max burst size to device info
  2019-04-05 14:57     ` Yigit, Ferruh
@ 2019-04-05 14:57       ` Yigit, Ferruh
  0 siblings, 0 replies; 13+ messages in thread
From: Yigit, Ferruh @ 2019-04-05 14:57 UTC (permalink / raw)
  To: Ferruh Yigit, Nikhil Agarwal, nikhil.agarwal, hemant.agrawal
  Cc: Thomas Monjalon, dev, david.hunt, arybchenko, shahafs

On 4/5/2019 3:55 PM, Ferruh Yigit wrote:
> On 5/22/2018 11:17 PM, Thomas Monjalon wrote:
>> 12/12/2017 11:05, Nikhil Agarwal:
>>> Currently, if the  rte_eth_rx_burst() function returns a value less than
>>> *nb_pkts*, the application will assume that no more packets are present.
>>>
>>> Some of the hw queue based hardware can only support smaller burst for RX
>>> and TX and thus break the expectation of the rx_burst API.
>>>
>>> This patch adds support to provide the maximum burst size that can be
>>> supported by a given PMD. The dev_info is being memset to '0' in
>>> rte_ethdev library. The value of '0' indicates that any value for burst
>>> size can be supported i.e. no change for existing PMDs.
>>>
>>> The application can now use the lowest available max_burst_size value
>>> for rte_eth_rx_burst.
>>>
>>> Signed-off-by: Nikhil Agarwal <nikhil.agarwal@linaro.org>
>>> ---
>>> @@ -1047,6 +1047,7 @@ struct rte_eth_dev_info {
>>>  	/** Configured number of rx/tx queues */
>>>  	uint16_t nb_rx_queues; /**< Number of RX queues. */
>>>  	uint16_t nb_tx_queues; /**< Number of TX queues. */
>>> +	uint16_t max_burst_size; /**< MAX burst size, 0 for no limit. */
>>>  };
>>
>> What is the status of this proposal?
>>
>> Recently, the preferred tuning have been added by
>> 	"ethdev: support PMD-tuned Tx/Rx parameters"
>> 	http://dpdk.org/commit/3be82f5cc5
> 
> Hi Nikhil, Hemant,
> 
> PMD returning preferred 'burst_size' support already added, I guess this
> patchset is no more valid. I am updating this as rejected.
> 
> If something is missing or there are still some relevant pieces in this patch,
> please send as a new version on top of latest head.

As reference, mentioned patches:
https://patches.dpdk.org/patch/32112/
https://patches.dpdk.org/patch/32113/
https://patches.dpdk.org/patch/32114/

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

end of thread, other threads:[~2019-04-05 14:57 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-12 10:05 [dpdk-dev] [PATCH 1/3] ethdev: add max burst size to device info Nikhil Agarwal
2017-12-12 10:05 ` [dpdk-dev] [PATCH 2/3] net/dpaa: implement max burst size in dev info Nikhil Agarwal
2017-12-12 10:05 ` [dpdk-dev] [PATCH 3/3] examples/l3fwd-power: use device max burst size Nikhil Agarwal
2017-12-12 10:45 ` [dpdk-dev] [PATCH 1/3] ethdev: add max burst size to device info Matan Azrad
2017-12-12 11:03   ` Ananyev, Konstantin
2017-12-12 13:43     ` Shreyansh Jain
2017-12-13 12:52       ` Ananyev, Konstantin
2017-12-13 15:22         ` Shreyansh Jain
2018-05-22 22:17 ` Thomas Monjalon
2019-04-05 14:55   ` Ferruh Yigit
2019-04-05 14:55     ` Ferruh Yigit
2019-04-05 14:57     ` Yigit, Ferruh
2019-04-05 14:57       ` Yigit, Ferruh

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).