Received-SPF: pass (google.com: domain of linux-wireless+bounces-6255-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Message-ID: <893ca115e668c6778344bcbd30168fe37bd5c80d.camel@sipsolutions.net>
Subject: Re: [PATCH] wifi: mac80211: split mesh fast tx cache into
 local/proxied/forwarded
From: Johannes Berg <johannes@sipsolutions.net>
To: Felix Fietkau <nbd@nbd.name>, linux-wireless@vger.kernel.org
Date: Fri, 12 Apr 2024 14:55:25 +0200
In-Reply-To: <20240412120634.88972-1-nbd@nbd.name>
References: <20240412120634.88972-1-nbd@nbd.name>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.50.4 (3.50.4-1.fc39) 
Precedence: bulk
MIME-Version: 1.0

On Fri, 2024-04-12 at 14:06 +0200, Felix Fietkau wrote:
>=20
> +struct ieee80211_mesh_fast_tx_key {
> +	u8 addr[ETH_ALEN] __aligned(2);
> +	enum ieee80211_mesh_fast_tx_type type;
> +};

Does it make sense to hash a bunch of zeroes there if the compiler
allocates more than a single byte for "type"? It's uglier, but maybe
makes more sense to not do that? There's also padding in there in that
case.

> @@ -646,12 +653,18 @@ void mesh_fast_tx_flush_addr(struct ieee80211_sub_i=
f_data *sdata,
>  			     const u8 *addr)
>  {
>  	struct mesh_tx_cache *cache =3D &sdata->u.mesh.tx_cache;
> +	struct ieee80211_mesh_fast_tx_key key =3D {};
>  	struct ieee80211_mesh_fast_tx *entry;
> +	int i;
> =20
> +	ether_addr_copy(key.addr, addr);
>  	spin_lock_bh(&cache->walk_lock);
> -	entry =3D rhashtable_lookup_fast(&cache->rht, addr, fast_tx_rht_params)=
;
> -	if (entry)
> -		mesh_fast_tx_entry_free(cache, entry);
> +	for (i =3D MESH_FAST_TX_TYPE_LOCAL; i < MESH_FAST_TX_TYPE_FORWARDED; i+=
+) {

Seems that should be "i <=3D ...FORWARDED".

Maybe better then to add a NUM_MESH_FAST_TX_TYPES or so to the enum and
then use "i < NUM_MESH_FAST_TX_TYPES", so new additions won't have to
update this, if they ever happen.

johannes