Search Linux Wireless

Re: [PATCH] mac80211: allow AP_VLAN operation on crypto controlled devices

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



yes you just missed that ap_vlan is used for 4addr ap's / wds too so that might be related to the weired handling

Sebastian

Am 02.12.2018 um 14:02 schrieb Alexander Wetzel:
Hello,

From: Manikanta Pubbisetty <mpubbise@xxxxxxxxxxxxxx>

In the current implementation, mac80211 advertises the support of
AP_VLANs based on the driver's support for AP mode; it also
blocks encrypted AP_VLAN operation on devices advertising
SW_CRYPTO_CONTROL.

The implementation seems weird in it's current form and could be
often confusing, this is because there can be drivers advertising
both SW_CRYPTO_CONTROL and AP mode support (ex: ath10k) in which case
AP_VLAN will still be supported but only in open BSS and not in
secured BSS.

When SW_CRYPTO_CONTROL is enabled, it makes more sense if the decision
to support AP_VLANs is left to the driver. Mac80211 can then allow
AP_VLAN operations depending on the driver support.

This first part of the patch contradicts my current understanding of how Software crypto fallback can be triggered: We have a driver actively telling us to only fall back to sw crypto when it returns 1 on set_key, BUT we ignore that when the interface is set to @NL80211_IFTYPE_AP_VLAN and allow software encryption unconditionally?

Here the code:
        case WLAN_CIPHER_SUITE_GCMP:
        case WLAN_CIPHER_SUITE_GCMP_256:
                /* all of these we can do in software - if driver can */
                if (ret == 1)
                        return 0;
                if (ieee80211_hw_check(&key->local->hw,
                               SW_CRYPTO_CONTROL)) {
                        if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN)
                                return 0;
                        return -EINVAL;
                }
                return 0;
        default:
                return -EINVAL;
        }


Wouldn't it be preferable to just return "ret" or "-EINVAL" instead of "0" when the interface has @NL80211_IFTYPE_AP_VLAN set?
As it is this basically overrides SW_CRYPTO_CONTROL in AP Vlan mode!

For me it looks like the old behavior in this section was already fine and does not hurt the intention of this patch: A driver setting SW_CRYPTO_CONTROL won't get support for AP VLANs as long as the driver is not opting in to it.

Therefore I would like to undo this part of the patch again:

-        if (ieee80211_hw_check(&key->local->hw,
                       SW_CRYPTO_CONTROL))
+        if (ieee80211_hw_check(&key->local->hw,
                       SW_CRYPTO_CONTROL)) {
+            if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN)
+                return 0;
             return -EINVAL;
+        }


Do I miss something here and would anyone have issues when I revert that in another patch?

Alexander




[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Wireless Personal Area Network]     [Linux Bluetooth]     [Wireless Regulations]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Hiking]     [MIPS Linux]     [ARM Linux]     [Linux RAID]

  Powered by Linux