I was going to not worry about it, but now that I'm replying anyway ...
> +++ b/net/mac80211/iface.c
> @@ -43,6 +43,7 @@
> */
>
> static void ieee80211_iface_work(struct work_struct *work);
> +static void ieee80211_set_vif_encap_ops(struct ieee80211_sub_if_data *sdata);
Do we really need that, can can't reorder things?
> +static bool ieee80211_iftype_supports_encap_offload(enum nl80211_iftype iftype)
> +{
> + switch (iftype) {
> + case NL80211_IFTYPE_AP:
> + case NL80211_IFTYPE_P2P_GO:
> + case NL80211_IFTYPE_P2P_CLIENT:
The P2P ones cannot happen here due to the iftype munging that mac80211
does, suggest you add a comment about that and remove them.
> + list_for_each_entry(key, &sdata->key_list, list) {
> + if (key->conf.cipher == WLAN_CIPHER_SUITE_AES_CMAC ||
> + key->conf.cipher == WLAN_CIPHER_SUITE_BIP_GMAC_128 ||
> + key->conf.cipher == WLAN_CIPHER_SUITE_BIP_GMAC_256 ||
> + key->conf.cipher == WLAN_CIPHER_SUITE_BIP_CMAC_256)
> + continue;
I had to read that a few times to understand it ... maybe add a comment
that the management frame keys aren't relevant? :)
But anyway, what I was really not sure about is this function:
> +static void ieee80211_set_vif_encap_ops(struct ieee80211_sub_if_data *sdata)
> +{
> + struct ieee80211_local *local = sdata->local;
> + struct ieee80211_sub_if_data *bss = sdata;
> + struct ieee80211_key *key;
> + bool enabled;
> +
> + if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN) {
> + if (!sdata->bss)
> + return;
> +
> + bss = container_of(sdata->bss, struct ieee80211_sub_if_data, u.ap);
> + }
> +
> + if (!ieee80211_hw_check(&local->hw, SUPPORTS_TX_ENCAP_OFFLOAD) ||
> + !ieee80211_iftype_supports_encap_offload(bss->vif.type))
> + return;
There are a lot of returns here, some of which make sense, but the
sdata->bss one seems dynamic and doesn't really make sense? Could it
change?
Or maybe we don't care because if it's not linked to a BSS it cannot be
transmitting?
johannes
