All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Luka Gejak" <luka.gejak@linux.dev>
To: "Johannes Berg" <johannes@sipsolutions.net>,
	<linux-wireless@vger.kernel.org>
Subject: Re: [PATCH 10/11] wifi: mac80211: avoid tailroom accounting on new key installation
Date: Wed, 14 Oct 2026 10:27:31 +0000	[thread overview]
Message-ID: <DM4HN6UYFJPO.2KDDCT5UHA52W@linux.dev> (raw)
In-Reply-To: <feaf62f6762bac17bc6e1edce2fc1d4d50adc3b4.camel@sipsolutions.net>

Hi Johannes,

On Tue Oct 13, 2026 at 6:22 PM UTC, Johannes Berg wrote:
> This is well and all, but I'm starting to think we should just not do
> this. Given that SW crypto really should be the slow-path, I think we
> can instead simply remove the RCU sync *completely*, and accept that in
> the race case, the (SW) crypto path may have to expand tailroom (and
> might drop a frame if allocation fails).
>
> That's far simpler, no need to add the RX_ONLY flag I did in patch 11
> etc., ultimately instead of adding code it ends up removing some, since
> we can also get rid of the dec_tailroom_needed_wk and all that.
>
> Thoughts?

Agreed, the counter only exists to avoid synchronize_net(). Two things I
would want in the same series though, because "expand, or maybe drop a
frame" is not quite what the code does today.

The data crypto paths warn and drop rather than expand. tkip, ccmp and
gcmp all WARN_ON(skb_tailroom(skb) < tail) and return -1, and
ieee80211_tx_h_michael_mic_add() returns TX_DROP. So either those grow a
pskb_expand_head fallback, or the WARN_ONs become plain checks.

And the counter is also the cloned skb unshare trigger in
ieee80211_skb_resize(). Without it a header released clone with slack at
the tail skips the unshare, and software crypto then writes the MIC into
memory it does not own alone, which skb_tailroom() would not catch. An
unshare in the encrypt path would.

With that the counter is only an optimisation, so no objection from me.

Best regards,
Luka Gejak

  reply	other threads:[~2026-10-14 10:27 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-13 12:59 wifi: cfg80211/mac80211: key handling improvements Johannes Berg
2026-10-13 12:59 ` [PATCH 01/11] wifi: cfg80211: reject WEP/TKIP with multi-link Johannes Berg
2026-10-13 12:59 ` [PATCH 02/11] wifi: mac80211: replace ieee80211_iter_keys_rcu() Johannes Berg
2026-10-13 12:59 ` [PATCH 03/11] wifi: cfg80211: support C/B/I/GTK per AP in client modes Johannes Berg
2026-10-13 12:59 ` [PATCH 04/11] wifi: mac80211: clarify per-STA GTK handling code/docs Johannes Berg
2026-10-13 12:59 ` [PATCH 05/11] wifi: mac80211: use per-STA GTK in client mode Johannes Berg
2026-10-13 12:59 ` [PATCH 06/11] wifi: mac80211: allow drivers to install AP GTK on STA Johannes Berg
2026-10-13 12:59 ` [PATCH 07/11] wifi: mac80211: remove lookup_key() dead code Johannes Berg
2026-10-13 12:59 ` [PATCH 08/11] wifi: mac80211: unify key finding/installation Johannes Berg
2026-10-13 12:59 ` [PATCH 09/11] wifi: mac80211: refactor tailroom needed checks Johannes Berg
2026-10-13 12:59 ` [PATCH 10/11] wifi: mac80211: avoid tailroom accounting on new key installation Johannes Berg
2026-10-13 18:22   ` Johannes Berg
2026-10-14 10:27     ` Luka Gejak [this message]
2026-10-13 12:59 ` [PATCH 11/11] wifi: mac80211: optimize RCU on group key rekeying Johannes Berg

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DM4HN6UYFJPO.2KDDCT5UHA52W@linux.dev \
    --to=luka.gejak@linux.dev \
    --cc=johannes@sipsolutions.net \
    --cc=linux-wireless@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.