On Sun, 2022-09-04 at 03:14 +0000, Sriram R (QUIC) wrote:
> >
> > Though I've actually thought for a while now that we have so many capability
> > checks, we might benefit from making those static keys for systems that only
> > have a single wifi NIC (or multiple similar ones), which is quite many I guess.
> Sure, we could check for fast-xmit cap. But, I added 'enabled' so as to avoid
> accessing local->hw in all these functions and also in case if we need a
> selective enable/disable of this cache in future.
Right, that's what I meant wrt. cache locality - I guess we can keep it
this way. Though we probably check local so often I'm not sure it
matters, and if we ever do revive the static keys idea (I did have a
patch at some point), it'd be better with the flag check.
> > > + if (key) {
> > > + hdr = (struct ieee80211_hdr *)mhdr->hdr;
> > > + hdr->frame_control |= cpu_to_le16(IEEE80211_FCTL_PROTECTED);
> > > + }
> >
> > Isn't "if (key)" equivalent to "if (pn_offs)" at this point, so you can move it into
> > the same if?
> The pn_offs will be set only on IEEE80211_KEY_FLAG_GENERATE_IV is set in key->conf.flags
> So had separate checks.
Ack.
> > > + /* availability of hdr entry ensures the mpath and sta are valid
> > > + * hence validation can be avoided.
> >
> > What about key? :)
> Hmm.. I was relying on the availability of cache entry to ensure sta and key are valid.
And I thought you're right - but you didn't mention it in the comment,
so I wanted to double-check.
> But I see your point and realize the entry gets removed later in __sta_info_destroy_part2()
> whereas the key can get cleared before the entry is flushed.
> May be I'll see if I could flush the cache entry in __sta_info_destroy_part1() so that
> the entry itself is not visible. Hope that solves if such race occurs.
>
Alright, let's see. But you can also remove a key without removing a
STA, so maybe there's some flushing needed there too?
johannes
