* [dpdk-dev] [PATCH] i40evf: use non spinning delay when issuing AQ request to PF
@ 2020-03-30 14:33 Laurent Hardy
2020-04-04 1:51 ` Zhang, Qi Z
2020-04-04 21:51 ` Stephen Hemminger
0 siblings, 2 replies; 3+ messages in thread
From: Laurent Hardy @ 2020-03-30 14:33 UTC (permalink / raw)
To: dev, xiaolong.ye, qi.z.zhang; +Cc: olivier.matz, david.marchand
When a port is handled by the i40evf dpdk pmd we could observe a cpu usage
around 70% in case of rte eth stats functions (rte_eth_stats_get and
rte_eth_xstats_get) called periodically via an application control thread.
This is due to the polling mechanism to handle communication between VF
and PF introduced for x710 (eg: VSI and virtual channel).
After issuing any request to the PF, the VF will wait in a blocking mode
until it gets a response from the PF or until timeout (2sec).
Instead, uses rte_delay_us_sleep to sleep for ASQ_DELAY_MS, which will
use system sleep and will not block the CPU core.
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
Signed-off-by: Laurent Hardy <laurent.hardy@6wind.com>
---
Hi all,
Some question coming along with this patch:
1)Is there any downside to use the rte_eal_delay_us_sleep which will
put the thread to sleep instead of the rte_eal_delay_ms (for both
control plane and dataplane threads) ?
2)Another alternative to this patch could be to modify at librte_eal
layer the function rte_eal_delay to put the thread to sleep in case of
application control plane thread (by looking at the coreid).
--- a/lib/librte_eal/common/include/generic/rte_cycles.h
+++ b/lib/librte_eal/common/include/generic/rte_cycles.h
@@ -13,9 +13,11 @@
*/
#include <stdint.h>
+#include <unistd.h>
#include <rte_compat.h>
#include <rte_debug.h>
#include <rte_atomic.h>
+#include <rte_lcore.h>
#define MS_PER_S 1000
#define US_PER_S 1000000
@@ -147,7 +149,10 @@ extern void
static inline void
rte_delay_ms(unsigned ms)
{
- rte_delay_us(ms * 1000);
+ if (rte_lcore_id() == LCORE_ID_ANY)
+ usleep(ms * 1000);
+ else
+ rte_delay_us(ms * 1000);
}
regards,
Laurent
---
drivers/net/i40e/i40e_ethdev_vf.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index 244397e0e..c700c66fd 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -315,6 +315,7 @@ _atomic_set_cmd(struct i40e_vf *vf, enum virtchnl_ops ops)
#define MAX_TRY_TIMES 200
#define ASQ_DELAY_MS 10
+#define DELAY_MS(x) DELAY(x * 1000)
static int
i40evf_execute_vf_cmd(struct rte_eth_dev *dev, struct vf_cmd_info *args)
@@ -358,7 +359,7 @@ i40evf_execute_vf_cmd(struct rte_eth_dev *dev, struct vf_cmd_info *args)
break;
} else if (ret == I40EVF_MSG_ERR)
break;
- rte_delay_ms(ASQ_DELAY_MS);
+ DELAY_MS(ASQ_DELAY_MS);
/* If don't read msg or read sys event, continue */
} while (i++ < MAX_TRY_TIMES);
_clear_cmd(vf);
@@ -380,7 +381,7 @@ i40evf_execute_vf_cmd(struct rte_eth_dev *dev, struct vf_cmd_info *args)
ret == I40EVF_MSG_CMD) {
break;
}
- rte_delay_ms(ASQ_DELAY_MS);
+ DELAY_MS(ASQ_DELAY_MS);
/* If don't read msg or read sys event, continue */
} while (i++ < MAX_TRY_TIMES);
_clear_cmd(vf);
@@ -394,7 +395,7 @@ i40evf_execute_vf_cmd(struct rte_eth_dev *dev, struct vf_cmd_info *args)
err = 0;
break;
}
- rte_delay_ms(ASQ_DELAY_MS);
+ DELAY_MS(ASQ_DELAY_MS);
/* If don't read msg or read sys event, continue */
} while (i++ < MAX_TRY_TIMES);
/* If there's no response is received, clear command */
--
2.24.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [dpdk-dev] [PATCH] i40evf: use non spinning delay when issuing AQ request to PF
2020-03-30 14:33 [dpdk-dev] [PATCH] i40evf: use non spinning delay when issuing AQ request to PF Laurent Hardy
@ 2020-04-04 1:51 ` Zhang, Qi Z
2020-04-04 21:51 ` Stephen Hemminger
1 sibling, 0 replies; 3+ messages in thread
From: Zhang, Qi Z @ 2020-04-04 1:51 UTC (permalink / raw)
To: Laurent Hardy, dev, Ye, Xiaolong; +Cc: olivier.matz, david.marchand
> -----Original Message-----
> From: Laurent Hardy <laurent.hardy@6wind.com>
> Sent: Monday, March 30, 2020 10:34 PM
> To: dev@dpdk.org; Ye, Xiaolong <xiaolong.ye@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>
> Cc: olivier.matz@6wind.com; david.marchand@redhat.com
> Subject: [PATCH] i40evf: use non spinning delay when issuing AQ request to
> PF
>
> When a port is handled by the i40evf dpdk pmd we could observe a cpu
> usage around 70% in case of rte eth stats functions (rte_eth_stats_get and
> rte_eth_xstats_get) called periodically via an application control thread.
>
> This is due to the polling mechanism to handle communication between VF
> and PF introduced for x710 (eg: VSI and virtual channel).
>
> After issuing any request to the PF, the VF will wait in a blocking mode until
> it gets a response from the PF or until timeout (2sec).
> Instead, uses rte_delay_us_sleep to sleep for ASQ_DELAY_MS, which will use
> system sleep and will not block the CPU core.
>
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> Signed-off-by: Laurent Hardy <laurent.hardy@6wind.com>
>
> ---
> Hi all,
> Some question coming along with this patch:
> 1)Is there any downside to use the rte_eal_delay_us_sleep which will put the
> thread to sleep instead of the rte_eal_delay_ms (for both control plane and
> dataplane threads) ?
Do we already have API rte_delay_us_callback_register, so user can decide if block or non-block delay they want?
>
> 2)Another alternative to this patch could be to modify at librte_eal layer the
> function rte_eal_delay to put the thread to sleep in case of application
> control plane thread (by looking at the coreid).
>
> --- a/lib/librte_eal/common/include/generic/rte_cycles.h
> +++ b/lib/librte_eal/common/include/generic/rte_cycles.h
> @@ -13,9 +13,11 @@
> */
>
> #include <stdint.h>
> +#include <unistd.h>
> #include <rte_compat.h>
> #include <rte_debug.h>
> #include <rte_atomic.h>
> +#include <rte_lcore.h>
>
> #define MS_PER_S 1000
> #define US_PER_S 1000000
> @@ -147,7 +149,10 @@ extern void
> static inline void
> rte_delay_ms(unsigned ms)
> {
> - rte_delay_us(ms * 1000);
> + if (rte_lcore_id() == LCORE_ID_ANY)
> + usleep(ms * 1000);
> + else
> + rte_delay_us(ms * 1000);
> }
>
> regards,
> Laurent
> ---
> drivers/net/i40e/i40e_ethdev_vf.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/i40e/i40e_ethdev_vf.c
> b/drivers/net/i40e/i40e_ethdev_vf.c
> index 244397e0e..c700c66fd 100644
> --- a/drivers/net/i40e/i40e_ethdev_vf.c
> +++ b/drivers/net/i40e/i40e_ethdev_vf.c
> @@ -315,6 +315,7 @@ _atomic_set_cmd(struct i40e_vf *vf, enum
> virtchnl_ops ops)
>
> #define MAX_TRY_TIMES 200
> #define ASQ_DELAY_MS 10
> +#define DELAY_MS(x) DELAY(x * 1000)
Ideally the macros in base/i40e_osdep.h should only be consumed by base code itself.
The pmd driver that adapt to ethdev should take rte_xxx APIs.
>
> static int
> i40evf_execute_vf_cmd(struct rte_eth_dev *dev, struct vf_cmd_info *args)
> @@ -358,7 +359,7 @@ i40evf_execute_vf_cmd(struct rte_eth_dev *dev,
> struct vf_cmd_info *args)
> break;
> } else if (ret == I40EVF_MSG_ERR)
> break;
> - rte_delay_ms(ASQ_DELAY_MS);
> + DELAY_MS(ASQ_DELAY_MS);
> /* If don't read msg or read sys event, continue */
> } while (i++ < MAX_TRY_TIMES);
> _clear_cmd(vf);
> @@ -380,7 +381,7 @@ i40evf_execute_vf_cmd(struct rte_eth_dev *dev,
> struct vf_cmd_info *args)
> ret == I40EVF_MSG_CMD) {
> break;
> }
> - rte_delay_ms(ASQ_DELAY_MS);
> + DELAY_MS(ASQ_DELAY_MS);
> /* If don't read msg or read sys event, continue */
> } while (i++ < MAX_TRY_TIMES);
> _clear_cmd(vf);
> @@ -394,7 +395,7 @@ i40evf_execute_vf_cmd(struct rte_eth_dev *dev,
> struct vf_cmd_info *args)
> err = 0;
> break;
> }
> - rte_delay_ms(ASQ_DELAY_MS);
> + DELAY_MS(ASQ_DELAY_MS);
> /* If don't read msg or read sys event, continue */
> } while (i++ < MAX_TRY_TIMES);
> /* If there's no response is received, clear command */
> --
> 2.24.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [dpdk-dev] [PATCH] i40evf: use non spinning delay when issuing AQ request to PF
2020-03-30 14:33 [dpdk-dev] [PATCH] i40evf: use non spinning delay when issuing AQ request to PF Laurent Hardy
2020-04-04 1:51 ` Zhang, Qi Z
@ 2020-04-04 21:51 ` Stephen Hemminger
1 sibling, 0 replies; 3+ messages in thread
From: Stephen Hemminger @ 2020-04-04 21:51 UTC (permalink / raw)
To: Laurent Hardy; +Cc: dev, xiaolong.ye, qi.z.zhang, olivier.matz, david.marchand
On Mon, 30 Mar 2020 16:33:30 +0200
Laurent Hardy <laurent.hardy@6wind.com> wrote:
> When a port is handled by the i40evf dpdk pmd we could observe a cpu usage
> around 70% in case of rte eth stats functions (rte_eth_stats_get and
> rte_eth_xstats_get) called periodically via an application control thread.
>
> This is due to the polling mechanism to handle communication between VF
> and PF introduced for x710 (eg: VSI and virtual channel).
>
> After issuing any request to the PF, the VF will wait in a blocking mode
> until it gets a response from the PF or until timeout (2sec).
> Instead, uses rte_delay_us_sleep to sleep for ASQ_DELAY_MS, which will
> use system sleep and will not block the CPU core.
>
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> Signed-off-by: Laurent Hardy <laurent.hardy@6wind.com>
Any thing sleeping for that long (1ms) should never spin.
Is there anyway to use something like a file descriptor/interrupt
for something this long?
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-04-04 21:51 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-30 14:33 [dpdk-dev] [PATCH] i40evf: use non spinning delay when issuing AQ request to PF Laurent Hardy
2020-04-04 1:51 ` Zhang, Qi Z
2020-04-04 21:51 ` Stephen Hemminger
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).