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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240701073550.GK17134@kernel.org>
Date: Mon, 1 Jul 2024 08:35:50 +0100
From: Simon Horman <horms@...nel.org>
To: Roger Quadros <rogerq@...nel.org>
Cc: "David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Siddharth Vadapalli <s-vadapalli@...com>,
	Julien Panis <jpanis@...libre.com>, Andrew Lunn <andrew@...n.ch>,
	srk@...com, vigneshr@...com, danishanwar@...com,
	pekka Varis <p-varis@...com>, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-omap@...r.kernel.org
Subject: Re: [PATCH net-next v2 5/7] net: ethernet: ti: cpsw_ale: add
 policer/classifier helpers

On Fri, Jun 28, 2024 at 03:01:54PM +0300, Roger Quadros wrote:
> The Policer registers in the ALE register space are just shadow registers
> and use an index field in the policer table control register to read/write
> to the actual Polier registers.
> Add helper functions to Read and Write to Policer registers.
> 
> Also add a helper function to set the thread value to classifier/policer
> mapping. Any packet that first matches the classifier will be sent to the
> thread (flow) that is set in the classifer to thread mapping table.

nit: classifier

     Flagged by checkpatch.pl --codespell

> If not set then it goes to the default flow.
> 
> Signed-off-by: Roger Quadros <rogerq@...nel.org>
> ---
>  drivers/net/ethernet/ti/cpsw_ale.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/drivers/net/ethernet/ti/cpsw_ale.c b/drivers/net/ethernet/ti/cpsw_ale.c
> index 7bd0dc20f894..75a17184d34c 100644
> --- a/drivers/net/ethernet/ti/cpsw_ale.c
> +++ b/drivers/net/ethernet/ti/cpsw_ale.c
> @@ -1626,3 +1626,27 @@ u32 cpsw_ale_get_num_entries(struct cpsw_ale *ale)
>  {
>  	return ale ? ale->params.ale_entries : 0;
>  }
> +
> +/* Reads the specified policer index into ALE POLICER registers */
> +static void cpsw_ale_policer_read_idx(struct cpsw_ale *ale, u32 idx)
> +{
> +	idx &= ALE_POLICER_TBL_INDEX_MASK;
> +	writel_relaxed(idx, ale->params.ale_regs + ALE_POLICER_TBL_CTL);
> +}
> +
> +/* Writes the ALE POLICER registers into the specified policer index */
> +static void cpsw_ale_policer_write_idx(struct cpsw_ale *ale, u32 idx)
> +{
> +	idx &= ALE_POLICER_TBL_INDEX_MASK;
> +	idx |= ALE_POLICER_TBL_WRITE_ENABLE;
> +	writel_relaxed(idx, ale->params.ale_regs + ALE_POLICER_TBL_CTL);
> +}
> +
> +/* enables/disables the custom thread value for the specified policer index */
> +static void cpsw_ale_policer_thread_idx_enable(struct cpsw_ale *ale, u32 idx,
> +					       u32 thread_id, bool enable)
> +{
> +	regmap_field_write(ale->fields[ALE_THREAD_CLASS_INDEX], idx);
> +	regmap_field_write(ale->fields[ALE_THREAD_VALUE], thread_id);
> +	regmap_field_write(ale->fields[ALE_THREAD_ENABLE], enable ? 1 : 0);
> +}
> 

I like that this patch-set is broken out into nice discrete patches,
including this one. So I'm in two minds about the comment I'm about to
make, but here goes.

As these helpers are unused this raises Warnings with W=1 builds on
gcc-13 and clang-18, which is generally undesirable for networking patches.

I can think of a few options here;
* Ignore the warnings
* Squash this patch into the following one
* Add some annotations, e.g. __maybe_unused or __always_unused.
  Likely dropped in the next patch.

I think I lean towards the last option.
But I won't push the point any further regardless.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ