| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <e7769bd7-e4fa-412a-8ad1-c0e5b8655a52@molgen.mpg.de>
Date: Tue, 25 Jun 2024 10:31:49 +0200
From: Paul Menzel <pmenzel@...gen.mpg.de>
To: Marcin Szycik <marcin.szycik@...ux.intel.com>
Cc: intel-wired-lan@...ts.osuosl.org, netdev@...r.kernel.org,
aleksander.lobakin@...el.com, przemyslaw.kitszel@...el.com,
michal.swiatkowski@...ux.intel.com
Subject: Re: [Intel-wired-lan] [PATCH iwl-next v2 7/7] ice: Add tracepoint for
adding and removing switch rules
Dear Marcin,
Thank you for your patch.
Am 24.06.24 um 16:45 schrieb Marcin Szycik:
> Track the number of rules and recipes added to switch. Add a tracepoint to
> ice_aq_sw_rules(), which shows both rule and recipe count. This information
> can be helpful when designing a set of rules to program to the hardware, as
> it shows where the practical limit is. Actual limits are known (64 recipes,
> 32k rules), but it's hard to translate these values to how many rules the
> *user* can actually create, because of extra metadata being implicitly
> added, and recipe/rule chaining. Chaining combines several recipes/rules to
> create a larger recipe/rule, so one large rule added by the user might
> actually consume multiple rules from hardware perspective.
>
> Rule counter is simply incremented/decremented in ice_aq_sw_rules(), since
> all rules are added or removed via it.
>
> Counting recipes is harder, as recipes can't be removed (only overwritten).
> Recipes added via ice_aq_add_recipe() could end up being unused, when
> there is an error in later stages of rule creation. Instead, track the
> allocation and freeing of recipes, which should reflect the actual usage of
> recipes (if something fails after recipe(s) were created, caller should
> free them). Also, a number of recipes are loaded from NVM by default -
> initialize the recipe counter with the number of these recipes on switch
> initialization.
Can you please add an example how to use the tracepoint?
> Reviewed-by: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
> Signed-off-by: Marcin Szycik <marcin.szycik@...ux.intel.com>
> ---
> drivers/net/ethernet/intel/ice/ice_common.c | 3 +++
> drivers/net/ethernet/intel/ice/ice_switch.c | 22 +++++++++++++++++++--
> drivers/net/ethernet/intel/ice/ice_trace.h | 18 +++++++++++++++++
> drivers/net/ethernet/intel/ice/ice_type.h | 2 ++
> 4 files changed, 43 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_common.c b/drivers/net/ethernet/intel/ice/ice_common.c
> index 6abd1b3796ab..009716a12a26 100644
> --- a/drivers/net/ethernet/intel/ice/ice_common.c
> +++ b/drivers/net/ethernet/intel/ice/ice_common.c
> @@ -934,6 +934,9 @@ static int ice_init_fltr_mgmt_struct(struct ice_hw *hw)
> INIT_LIST_HEAD(&sw->vsi_list_map_head);
> sw->prof_res_bm_init = 0;
>
> + /* Initialize recipe count with default recipes read from NVM */
> + sw->recp_cnt = ICE_SW_LKUP_LAST;
> +
> status = ice_init_def_sw_recp(hw);
> if (status) {
> devm_kfree(ice_hw_to_dev(hw), hw->switch_info);
> diff --git a/drivers/net/ethernet/intel/ice/ice_switch.c b/drivers/net/ethernet/intel/ice/ice_switch.c
> index 27828cdfe085..3caafcdc301f 100644
> --- a/drivers/net/ethernet/intel/ice/ice_switch.c
> +++ b/drivers/net/ethernet/intel/ice/ice_switch.c
> @@ -3,6 +3,7 @@
>
> #include "ice_lib.h"
> #include "ice_switch.h"
> +#include "ice_trace.h"
>
> #define ICE_ETH_DA_OFFSET 0
> #define ICE_ETH_ETHTYPE_OFFSET 12
> @@ -1961,6 +1962,15 @@ ice_aq_sw_rules(struct ice_hw *hw, void *rule_list, u16 rule_list_sz,
> hw->adminq.sq_last_status == ICE_AQ_RC_ENOENT)
> status = -ENOENT;
>
> + if (!status) {
> + if (opc == ice_aqc_opc_add_sw_rules)
> + hw->switch_info->rule_cnt += num_rules;
> + else if (opc == ice_aqc_opc_remove_sw_rules)
> + hw->switch_info->rule_cnt -= num_rules;
> + }
> +
> + trace_ice_aq_sw_rules(hw->switch_info);
> +
> return status;
> }
>
> @@ -2181,8 +2191,10 @@ int ice_alloc_recipe(struct ice_hw *hw, u16 *rid)
> sw_buf->res_type = cpu_to_le16(res_type);
> status = ice_aq_alloc_free_res(hw, sw_buf, buf_len,
> ice_aqc_opc_alloc_res);
> - if (!status)
> + if (!status) {
> *rid = le16_to_cpu(sw_buf->elem[0].e.sw_resp);
> + hw->switch_info->recp_cnt++;
> + }
>
> return status;
> }
> @@ -2196,7 +2208,13 @@ int ice_alloc_recipe(struct ice_hw *hw, u16 *rid)
> */
> static int ice_free_recipe_res(struct ice_hw *hw, u16 rid)
> {
> - return ice_free_hw_res(hw, ICE_AQC_RES_TYPE_RECIPE, 1, &rid);
> + int status;
> +
> + status = ice_free_hw_res(hw, ICE_AQC_RES_TYPE_RECIPE, 1, &rid);
> + if (!status)
> + hw->switch_info->recp_cnt--;
> +
> + return status;
> }
>
> /**
> diff --git a/drivers/net/ethernet/intel/ice/ice_trace.h b/drivers/net/ethernet/intel/ice/ice_trace.h
> index 244cddd2a9ea..07aab6e130cd 100644
> --- a/drivers/net/ethernet/intel/ice/ice_trace.h
> +++ b/drivers/net/ethernet/intel/ice/ice_trace.h
> @@ -330,6 +330,24 @@ DEFINE_EVENT(ice_esw_br_port_template,
> TP_ARGS(port)
> );
>
> +DECLARE_EVENT_CLASS(ice_switch_stats_template,
> + TP_PROTO(struct ice_switch_info *sw_info),
> + TP_ARGS(sw_info),
> + TP_STRUCT__entry(__field(u16, rule_cnt)
> + __field(u8, recp_cnt)),
> + TP_fast_assign(__entry->rule_cnt = sw_info->rule_cnt;
> + __entry->recp_cnt = sw_info->recp_cnt;),
> + TP_printk("rules=%u recipes=%u",
> + __entry->rule_cnt,
> + __entry->recp_cnt)
> +);
> +
> +DEFINE_EVENT(ice_switch_stats_template,
> + ice_aq_sw_rules,
> + TP_PROTO(struct ice_switch_info *sw_info),
> + TP_ARGS(sw_info)
> +);
> +
> /* End tracepoints */
>
> #endif /* _ICE_TRACE_H_ */
> diff --git a/drivers/net/ethernet/intel/ice/ice_type.h b/drivers/net/ethernet/intel/ice/ice_type.h
> index c330a436d11a..b6bc2de53b0a 100644
> --- a/drivers/net/ethernet/intel/ice/ice_type.h
> +++ b/drivers/net/ethernet/intel/ice/ice_type.h
> @@ -764,6 +764,8 @@ struct ice_switch_info {
> struct ice_sw_recipe *recp_list;
> u16 prof_res_bm_init;
> u16 max_used_prof_index;
> + u16 rule_cnt;
> + u8 recp_cnt;
>
> DECLARE_BITMAP(prof_res_bm[ICE_MAX_NUM_PROFILES], ICE_MAX_FV_WORDS);
> };
Kind regards,
Paul
Powered by blists - more mailing lists