> -----Original Message-----
> From: Laurent Hardy <laurent.ha...@6wind.com>
> Sent: Monday, March 30, 2020 10:34 PM
> To: dev@dpdk.org; Ye, Xiaolong <xiaolong...@intel.com>; Zhang, Qi Z
> <qi.z.zh...@intel.com>
> Cc: olivier.m...@6wind.com; david.march...@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.m...@6wind.com>
> Signed-off-by: Laurent Hardy <laurent.ha...@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
