From: Manikanta Pubbisetty <[email protected]>
Extending SW_CRYPTO_CONTROL interface so that drivers can advertise
the interface types on which they can support software encryption.
Driver's job is not done by advertising the supported vif types alone,
they should also return -EOPNOTSUPP from set_key.
Mac80211 will make the fallback decision to sw ecryption based
on the return type of set_key callback and the driver's support for
software encryption.
This change is done with the sole reason of adding the support of
VLANs for drivers which enable SW_CRYPTO_CONTROL(ex:ath10k).
With the current logic, configuring GTKs for specific VLAN groups will
always fail with the drivers enabling SW_CRYPTO_CONTROL. I understand
that the driver can return 1 from set_key to enable software encryption
in mac80211, but GTKs for VLANs are never passed down to the driver.
Since the return value is initialized to -EOPNOTSUPP, with this approach,
we get away with the failure.
Signed-off-by: Manikanta Pubbisetty <[email protected]>
---
include/net/mac80211.h | 5 +++++
net/mac80211/key.c | 12 +++++++++---
2 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 2fd59ed..3df6bee5 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -2237,6 +2237,10 @@ enum ieee80211_hw_flags {
* supported by HW.
* @max_nan_de_entries: maximum number of NAN DE functions supported by the
* device.
+ * @supp_sw_crypto_iftypes: supported interface types for software crypto,
+ * this field is applicable for devices advertising SW_CRYPTO_CONTROL
+ * hwflag. Drivers may also set the interface types on which mac80211
+ * can fallback to software encryption if set_key returns -EOPNOTSUPP.
*/
struct ieee80211_hw {
struct ieee80211_conf conf;
@@ -2272,6 +2276,7 @@ struct ieee80211_hw {
u8 n_cipher_schemes;
const struct ieee80211_cipher_scheme *cipher_schemes;
u8 max_nan_de_entries;
+ u16 supp_sw_crypto_iftypes;
};
static inline bool _ieee80211_hw_check(struct ieee80211_hw *hw,
diff --git a/net/mac80211/key.c b/net/mac80211/key.c
index aee05ec..a1011c4 100644
--- a/net/mac80211/key.c
+++ b/net/mac80211/key.c
@@ -126,7 +126,7 @@ static void decrease_tailroom_need_count(struct ieee80211_sub_if_data *sdata,
static int ieee80211_key_enable_hw_accel(struct ieee80211_key *key)
{
- struct ieee80211_sub_if_data *sdata;
+ struct ieee80211_sub_if_data *sdata = key->sdata;
struct sta_info *sta;
int ret = -EOPNOTSUPP;
@@ -162,7 +162,6 @@ static int ieee80211_key_enable_hw_accel(struct ieee80211_key *key)
if (sta && !sta->uploaded)
goto out_unsupported;
- sdata = key->sdata;
if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN) {
/*
* The driver doesn't know anything about VLAN interfaces.
@@ -214,8 +213,15 @@ static int ieee80211_key_enable_hw_accel(struct ieee80211_key *key)
/* 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 (ieee80211_hw_check(&key->local->hw, SW_CRYPTO_CONTROL)) {
+ if ((ret == -EOPNOTSUPP) &&
+ (sdata->local->hw.supp_sw_crypto_iftypes &
+ (1 << sdata->vif.type)))
+ return 0;
return -EINVAL;
+ }
+
return 0;
default:
return -EINVAL;
--
2.7.4
On 2018-03-22 13:36, Johannes Berg wrote:
> On Thu, 2018-03-22 at 11:51 +0530, [email protected] wrote:
>>
>> > So IMHO - just get rid of the bitmap and hard-code AP_VLAN.
>> >
>>
>> I agree with you only partially.
>>
>> Today, I do not see any driver advertising SW_CRYPTO_CONTROL other
>> than
>> ath10k. There could be some driver which would want to advertise
>> SW_CRYPTO_CONTROL and do not support the software encryption for VLAN
>> devices. In that case, hard-coding doesn't seem to solve the problem
>> completely right? No?
>
> Well, my point is that such a hypothetical driver is completely
> irrelevant because it doesn't make any sense to have this behaviour -
> it would mean it cannot support AP_VLAN with encryption, so it might as
> well not support AP_VLAN at all.
>
>> Or you meant to say that driver should advertise the support for
>> AP_VLANs only if it can support encryption on AP_VLAN devices?
>
> Right.
>
>> If this
>> the case, then I could see some code in ieee80211_register_hw which
>> says
>> this,
>>
>> /* if low-level driver supports AP, we also support VLAN */
>> if (local->hw.wiphy->interface_modes &
>> BIT(NL80211_IFTYPE_AP)) {
>> hw->wiphy->interface_modes |=
>> BIT(NL80211_IFTYPE_AP_VLAN);
>> hw->wiphy->software_iftypes |=
>> BIT(NL80211_IFTYPE_AP_VLAN);
>> }
>
> Yes, but if such a driver comes along we can change this.
>
It makes sense, I will send out the change by hard-coding only for
AP-VLAN interface.
--
mkp
On 2018-03-21 13:23, Johannes Berg wrote:
> On Fri, 2018-03-09 at 11:57 +0530, [email protected] wrote:
>> From: Manikanta Pubbisetty <[email protected]>
>>
>> Extending SW_CRYPTO_CONTROL interface so that drivers can advertise
>> the interface types on which they can support software encryption.
>> Driver's job is not done by advertising the supported vif types alone,
>> they should also return -EOPNOTSUPP from set_key.
>>
>> Mac80211 will make the fallback decision to sw ecryption based
>> on the return type of set_key callback and the driver's support for
>> software encryption.
>>
>> This change is done with the sole reason of adding the support of
>> VLANs for drivers which enable SW_CRYPTO_CONTROL(ex:ath10k).
>>
>> With the current logic, configuring GTKs for specific VLAN groups will
>> always fail with the drivers enabling SW_CRYPTO_CONTROL. I understand
>> that the driver can return 1 from set_key to enable software
>> encryption
>> in mac80211, but GTKs for VLANs are never passed down to the driver.
>> Since the return value is initialized to -EOPNOTSUPP, with this
>> approach,
>> we get away with the failure.
>
> Is there much value in having this control to start with, rather than
> saying it's *always* allowed for AP_VLAN interfaces?
>
> I mean - if the driver wants to support (encryption on) AP_VLAN
> interfaces with SW_CRYPTO_CONTROL it basically has to set this to allow
> it, which is kinda pointless - it's hard to imagine a driver that wants
> to support AP_VLAN only for unencrypted, so if it doesn't support this
> it might as well just disable AP_VLAN support entirely.
>
> So IMHO - just get rid of the bitmap and hard-code AP_VLAN.
>
I agree with you only partially.
Today, I do not see any driver advertising SW_CRYPTO_CONTROL other than
ath10k. There could be some driver which would want to advertise
SW_CRYPTO_CONTROL and do not support the software encryption for VLAN
devices. In that case, hard-coding doesn't seem to solve the problem
completely right? No?
In some way driver has to advertise that it supports encryption on
AP_VLANs, No?
Or you meant to say that driver should advertise the support for
AP_VLANs only if it can support encryption on AP_VLAN devices? If this
the case, then I could see some code in ieee80211_register_hw which says
this,
/* if low-level driver supports AP, we also support VLAN */
if (local->hw.wiphy->interface_modes & BIT(NL80211_IFTYPE_AP)) {
hw->wiphy->interface_modes |=
BIT(NL80211_IFTYPE_AP_VLAN);
hw->wiphy->software_iftypes |=
BIT(NL80211_IFTYPE_AP_VLAN);
}
Please correct if I misinterpreted your comment.
--
mkp
On Thu, 2018-03-22 at 11:51 +0530, [email protected] wrote:
>
> > So IMHO - just get rid of the bitmap and hard-code AP_VLAN.
> >
>
> I agree with you only partially.
>
> Today, I do not see any driver advertising SW_CRYPTO_CONTROL other than
> ath10k. There could be some driver which would want to advertise
> SW_CRYPTO_CONTROL and do not support the software encryption for VLAN
> devices. In that case, hard-coding doesn't seem to solve the problem
> completely right? No?
Well, my point is that such a hypothetical driver is completely
irrelevant because it doesn't make any sense to have this behaviour -
it would mean it cannot support AP_VLAN with encryption, so it might as
well not support AP_VLAN at all.
> Or you meant to say that driver should advertise the support for
> AP_VLANs only if it can support encryption on AP_VLAN devices?
Right.
> If this
> the case, then I could see some code in ieee80211_register_hw which says
> this,
>
> /* if low-level driver supports AP, we also support VLAN */
> if (local->hw.wiphy->interface_modes & BIT(NL80211_IFTYPE_AP)) {
> hw->wiphy->interface_modes |=
> BIT(NL80211_IFTYPE_AP_VLAN);
> hw->wiphy->software_iftypes |=
> BIT(NL80211_IFTYPE_AP_VLAN);
> }
Yes, but if such a driver comes along we can change this.
johannes
On Fri, 2018-03-09 at 11:57 +0530, [email protected] wrote:
> From: Manikanta Pubbisetty <[email protected]>
>
> Extending SW_CRYPTO_CONTROL interface so that drivers can advertise
> the interface types on which they can support software encryption.
> Driver's job is not done by advertising the supported vif types alone,
> they should also return -EOPNOTSUPP from set_key.
>
> Mac80211 will make the fallback decision to sw ecryption based
> on the return type of set_key callback and the driver's support for
> software encryption.
>
> This change is done with the sole reason of adding the support of
> VLANs for drivers which enable SW_CRYPTO_CONTROL(ex:ath10k).
>
> With the current logic, configuring GTKs for specific VLAN groups will
> always fail with the drivers enabling SW_CRYPTO_CONTROL. I understand
> that the driver can return 1 from set_key to enable software encryption
> in mac80211, but GTKs for VLANs are never passed down to the driver.
> Since the return value is initialized to -EOPNOTSUPP, with this approach,
> we get away with the failure.
Is there much value in having this control to start with, rather than
saying it's *always* allowed for AP_VLAN interfaces?
I mean - if the driver wants to support (encryption on) AP_VLAN
interfaces with SW_CRYPTO_CONTROL it basically has to set this to allow
it, which is kinda pointless - it's hard to imagine a driver that wants
to support AP_VLAN only for unencrypted, so if it doesn't support this
it might as well just disable AP_VLAN support entirely.
So IMHO - just get rid of the bitmap and hard-code AP_VLAN.
johannes