On Thu, Oct 21, 2021 at 9:04 AM Andrew Rybchenko
<andrew.rybche...@oktetlabs.ru> wrote:
>
> From: Viacheslav Galaktionov <viacheslav.galaktio...@oktetlabs.ru>
>
> MAE counters can be polled from a control thread if no service core is
> allocated for this.
>
> Signed-off-by: Viacheslav Galaktionov <viacheslav.galaktio...@oktetlabs.ru>
> Signed-off-by: Andrew Rybchenko <andrew.rybche...@oktetlabs.ru>
> Reviewed-by: Andy Moreton <amore...@xilinx.com>
> ---
> The problem to require service cores for HW offload was raised by
> David on review in 21.08 release cycle.
Thanks for following up!
>
> doc/guides/rel_notes/release_21_11.rst | 1 +
> drivers/net/sfc/sfc_mae.h | 26 +++++-
> drivers/net/sfc/sfc_mae_counter.c | 120 ++++++++++++++++++++-----
> 3 files changed, 123 insertions(+), 24 deletions(-)
>
> diff --git a/doc/guides/rel_notes/release_21_11.rst
> b/doc/guides/rel_notes/release_21_11.rst
> index 041383ee2a..9517e0fb0a 100644
> --- a/doc/guides/rel_notes/release_21_11.rst
> +++ b/doc/guides/rel_notes/release_21_11.rst
> @@ -158,6 +158,7 @@ New Features
>
> * Added port representors support on SN1000 SmartNICs
> * Added flow API transfer proxy support
> + * Added support for flow counters without service cores
>
> * **Updated Marvell cnxk crypto PMD.**
>
> diff --git a/drivers/net/sfc/sfc_mae.h b/drivers/net/sfc/sfc_mae.h
> index 23dcf1e482..45a2fdc3bb 100644
> --- a/drivers/net/sfc/sfc_mae.h
> +++ b/drivers/net/sfc/sfc_mae.h
> @@ -127,6 +127,13 @@ struct sfc_mae_counters {
> unsigned int n_mae_counters;
> };
>
> +/** Options for MAE counter polling mode */
> +enum sfc_mae_counter_polling_mode {
> + SFC_MAE_COUNTER_POLLING_OFF = 0,
> + SFC_MAE_COUNTER_POLLING_SERVICE,
> + SFC_MAE_COUNTER_POLLING_THREAD,
> +};
> +
> struct sfc_mae_counter_registry {
> /* Common counter information */
> /** Counters collection */
> @@ -143,10 +150,21 @@ struct sfc_mae_counter_registry {
> bool use_credits;
>
> /* Information used by configuration routines */
> - /** Counter service core ID */
> - uint32_t service_core_id;
> - /** Counter service ID */
> - uint32_t service_id;
> + enum sfc_mae_counter_polling_mode polling_mode;
> + union {
> + struct {
> + /** Counter service core ID */
> + uint32_t core_id;
> + /** Counter service ID */
> + uint32_t id;
> + } service;
> + struct {
> + /** Counter thread ID */
> + pthread_t id;
> + /** The thread should keep running */
> + volatile bool run;
volatile is probably unneeded.
> + } thread;
> + } polling;
> };
>
> /**
> diff --git a/drivers/net/sfc/sfc_mae_counter.c
> b/drivers/net/sfc/sfc_mae_counter.c
> index 418caffe59..5f2aea1bf4 100644
> --- a/drivers/net/sfc/sfc_mae_counter.c
> +++ b/drivers/net/sfc/sfc_mae_counter.c
> @@ -45,9 +45,6 @@ sfc_mae_counter_rxq_required(struct sfc_adapter *sa)
> if (encp->enc_mae_supported == B_FALSE)
> return false;
>
> - if (sfc_mae_counter_get_service_lcore(sa) == RTE_MAX_LCORE)
> - return false;
> -
> return true;
> }
>
> @@ -402,6 +399,23 @@ sfc_mae_counter_routine(void *arg)
> return 0;
> }
>
> +static void *
> +sfc_mae_counter_thread(void *data)
> +{
> + struct sfc_adapter *sa = data;
> + struct sfc_mae_counter_registry *counter_registry =
> + &sa->mae.counter_registry;
> +
> + /*
> + * Check run condition without atomic since it is not a problem
> + * if we run a bit more before we notice stop request
> + */
I find it clearer when we have clear pairs of atomic acquire
load/release store (maybe because I feel like I understand something
:-)).
So it may not be a problem, but is there a reason to avoid this
(acquire) atomic load?
Otherwise, patch lgtm.
Thanks.
--
David Marchand
