So here the reasoning for why I named them as they are and why I prefer
the names used in the patch.
First, many drivers will handle SET_KEY and SET_KEY_RX_ONLY with the
same code and not differentiate between those at all. Using EXT_as a
prefix for the "normal" command is therefore a nice way to imply the
command can only be used with Extended Key ID and still link it to the
original command.
But more important for me was the clash between what the command spells
and what it does in the COMPAT mode: SET_KEY_RX_ONLY would then be used
to install a TX only key which never can be used by the card for Rx.
So I decided to rename it to EXT_SET_KEY, just indicating that this
command adds a new key to the card for Extended Key ID and drop the
confusing reference to Rx.
Wait, what? In compat mode SET_KEY_RX_ONLY installs a TX-only key? Ah,
you mean before you changed this.
Why don't we split out compat mode then?
But I see where you're coming from with the EXT_ now. I need to think of
it less as an "extension" now, but as "extended key ID". I'm not really
entirely sure that makes sense - even what we think of as "extended key
ID" now might be the new normal soon? But then again the spec does the
same thing.
Long story short: Using SET_KEY_RXONLY and SET_KEY_TX is not wrong, but
I would rate them more confusing.
Fair enough.
I've had second (or more like tenths..) thoughts about this API.
If you like any of the solutions below more than the others I'll use
that in the next patch, of course...
In a nutshell, the point of EXT_SET_KEY is to install a key, prepare it
for Tx but NOT use it for Tx. The driver can use it for Rx (NATIVE) or
just do nothing with the (in COMPAT mode) Tx only key till mac80211
starts using it for Tx later.
1) Assuming we keep the RX_ONLY key flag we could just drop EXT_SET_KEY
and use the "normal" SET_KEY with the flag moved to ieee80211_key_conf.
We really just want to install the key to the driver here, only drivers
like ath10k need a way to figure out it must not be used for Tx.
2) We also can drop the Rx only flag/extra command and simply use
SET_KEY. ath10k will then have to honor the keyID mac80211 prepared when
it want to support Extended Key ID. (ath10k seems to need a firmware
update either way, so that could be acceptable.)
That said I've now decided to simply name the calls in the next patch:
SET_KEY
-> no RX only flag!
SET_KEY_RX
-> SET_KEY, only key must not be used for Tx
DISABLE_KEY,
-> unchanged
ACTIVATE_KEY
-> so far named EXT_KEY_RX_TX
DISABLE_KEY_RX
-> so far named EXT_DISABLE_KEY_RX
We can then continue the discussion with a new patch and the other fixes
in it when needed.
To really clean that up we would have to change the existing names to
something like ADD_KEY, REMOVE_KEY. We could then name the new calls
ADD_KEY_RX, ACTIVATE_KEY and DISABLE_KEY_RX. But that seems to be overkill.
Alexander
Alexander
