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
> -----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
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?