linux-kernel - Re: [PATCH net-next] net: microchip: vcap: Change how the rule id is generated

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: <20221128144042.2097491-1-alexandr.lobakin@intel.com>
Date:   Mon, 28 Nov 2022 15:40:42 +0100
From:   Alexander Lobakin <alexandr.lobakin@...el.com>
To:     Horatiu Vultur <horatiu.vultur@...rochip.com>
Cc:     Alexander Lobakin <alexandr.lobakin@...el.com>,
        linux-arm-kernel@...ts.infradead.org, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org, davem@...emloft.net,
        edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
        lars.povlsen@...rochip.com, Steen.Hegelund@...rochip.com,
        daniel.machon@...rochip.com, UNGLinuxDriver@...rochip.com,
        casper.casan@...il.com
Subject: Re: [PATCH net-next] net: microchip: vcap: Change how the rule id is generated

From: Horatiu Vultur <horatiu.vultur@...rochip.com>
Date: Mon, 28 Nov 2022 15:29:59 +0100

> Currently whenever a new rule id is generated, it picks up the next
> number bigger than previous id. So it would always be 1, 2, 3, etc.
> When the rule with id 1 will be deleted and a new rule will be added,
> it will have the id 4 and not id 1.
> In theory this can be a problem if at some point a rule will be added
> and removed ~0 times. Then no more rules can be added because there
> are no more ids.
> 
> Change this such that when a new rule is added, search for an empty
> rule id starting with value of 1 as value 0 is reserved.
> 
> Fixes: c9da1ac1c212 ("net: microchip: sparx5: Adding initial tc flower support for VCAP API")
> Signed-off-by: Horatiu Vultur <horatiu.vultur@...rochip.com>
> ---
>  drivers/net/ethernet/microchip/vcap/vcap_api.c | 7 +------
>  drivers/net/ethernet/microchip/vcap/vcap_api.h | 1 -
>  2 files changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/drivers/net/ethernet/microchip/vcap/vcap_api.c b/drivers/net/ethernet/microchip/vcap/vcap_api.c
> index b50d002b646dc..b65819f3a927f 100644
> --- a/drivers/net/ethernet/microchip/vcap/vcap_api.c
> +++ b/drivers/net/ethernet/microchip/vcap/vcap_api.c
> @@ -974,17 +974,12 @@ static u32 vcap_next_rule_addr(u32 addr, struct vcap_rule_internal *ri)
>  /* Assign a unique rule id and autogenerate one if id == 0 */
>  static u32 vcap_set_rule_id(struct vcap_rule_internal *ri)
>  {
> -	u32 next_id;
> -
>  	if (ri->data.id != 0)
>  		return ri->data.id;
>  
> -	next_id = ri->vctrl->rule_id + 1;
> -
> -	for (next_id = ri->vctrl->rule_id + 1; next_id < ~0; ++next_id) {
> +	for (u32 next_id = 1; next_id < ~0; ++next_id) {
>  		if (!vcap_lookup_rule(ri->vctrl, next_id)) {

Or you can simply use IDA/IDR/XArray which takes care of all this :)


>  			ri->data.id = next_id;
> -			ri->vctrl->rule_id = next_id;
>  			break;
>  		}
>  	}
> diff --git a/drivers/net/ethernet/microchip/vcap/vcap_api.h b/drivers/net/ethernet/microchip/vcap/vcap_api.h
> index ca4499838306f..689c7270f2a89 100644
> --- a/drivers/net/ethernet/microchip/vcap/vcap_api.h
> +++ b/drivers/net/ethernet/microchip/vcap/vcap_api.h
> @@ -268,7 +268,6 @@ struct vcap_operations {
>  
>  /* VCAP API Client control interface */
>  struct vcap_control {
> -	u32 rule_id; /* last used rule id (unique across VCAP instances) */
>  	struct vcap_operations *ops;  /* client supplied operations */
>  	const struct vcap_info *vcaps; /* client supplied vcap models */
>  	const struct vcap_statistics *stats; /* client supplied vcap stats */
> -- 
> 2.38.0

Thanks,
Olek