Mutlicast/broadcast traffic destined for a particular vlan group will
always be encrypted in software. To enable dynamic VLANs, it requires
driver support for sending software encrypted packets.
In ath10k, sending sw encrypted frames is allowed only when we insmod
the driver with cryptmode param set to 1, this configuration disables
hardware crypto and enables RAW mode implicitly. Since, enabling raw
mode has performance impact, this cannot be considered as an ideal
solution for supporting VLANs in the driver.
As an alternative take, in this approach, cryptographic keys for
unicast traffic(per peer PTKs) and keys for non-vlan group traffic
will be configured in hardware, allowing hardware encryption for unicast
and non-vlan group traffic. Only vlan group traffic will be encrypted in
software and pushed to the target with encap mode set to RAW in the TX
descriptors.
Not all firmwares can support this type of key configuration(having few
keys installed in hardware and few only in software); for this purpose a
new WMI service flag "WMI_SERVICE_PER_PACKET_SW_ENCRYPT" is introduced to
advertise this support.
Also, adding the logic required to send sw encrypted frames in raw mode.
Tested this change on QCA9984(firmware version 10.4-3.5.3-00057).
Signed-off-by: Manikanta Pubbisetty <[email protected]>
---
drivers/net/wireless/ath/ath10k/core.h | 1 +
drivers/net/wireless/ath/ath10k/mac.c | 26 ++++++++++++++++++++++++--
drivers/net/wireless/ath/ath10k/wmi.h | 21 +++++++++++++++++++++
3 files changed, 46 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index e4ac8f2..105438d 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -122,6 +122,7 @@ enum ath10k_skb_flags {
ATH10K_SKB_F_DELIVER_CAB = BIT(2),
ATH10K_SKB_F_MGMT = BIT(3),
ATH10K_SKB_F_QOS = BIT(4),
+ ATH10K_SKB_F_RAW_TX = BIT(5),
};
struct ath10k_skb_cb {
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index fc3320f..694c0aa 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -3362,6 +3362,7 @@ ath10k_mac_tx_h_get_txmode(struct ath10k *ar,
struct sk_buff *skb)
{
const struct ieee80211_hdr *hdr = (void *)skb->data;
+ const struct ath10k_skb_cb *skb_cb = ATH10K_SKB_CB(skb);
__le16 fc = hdr->frame_control;
if (!vif || vif->type == NL80211_IFTYPE_MONITOR)
@@ -3403,7 +3404,8 @@ ath10k_mac_tx_h_get_txmode(struct ath10k *ar,
if (ieee80211_is_data_present(fc) && sta && sta->tdls)
return ATH10K_HW_TXRX_ETHERNET;
- if (test_bit(ATH10K_FLAG_RAW_MODE, &ar->dev_flags))
+ if (test_bit(ATH10K_FLAG_RAW_MODE, &ar->dev_flags) ||
+ skb_cb->flags & ATH10K_SKB_F_RAW_TX)
return ATH10K_HW_TXRX_RAW;
return ATH10K_HW_TXRX_NATIVE_WIFI;
@@ -3513,6 +3515,9 @@ static void ath10k_mac_tx_h_fill_cb(struct ath10k *ar,
{
struct ieee80211_hdr *hdr = (void *)skb->data;
struct ath10k_skb_cb *cb = ATH10K_SKB_CB(skb);
+ const struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
+ bool is_data = ieee80211_is_data(hdr->frame_control) ||
+ ieee80211_is_data_qos(hdr->frame_control);
cb->flags = 0;
if (!ath10k_tx_h_use_hwcrypto(vif, skb))
@@ -3524,6 +3529,16 @@ static void ath10k_mac_tx_h_fill_cb(struct ath10k *ar,
if (ieee80211_is_data_qos(hdr->frame_control))
cb->flags |= ATH10K_SKB_F_QOS;
+ /* Data frames encrypted in software will be posted to firmware
+ * with tx encap mode set to RAW. One such case would be the
+ * multicast traffic generated for a VLAN group.
+ */
+ if (is_data && ieee80211_has_protected(hdr->frame_control) &&
+ !info->control.hw_key) {
+ cb->flags |= ATH10K_SKB_F_NO_HWCRYPT;
+ cb->flags |= ATH10K_SKB_F_RAW_TX;
+ }
+
cb->vif = vif;
cb->txq = txq;
}
@@ -3632,6 +3647,7 @@ static int ath10k_mac_tx(struct ath10k *ar,
{
struct ieee80211_hw *hw = ar->hw;
struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
+ const struct ath10k_skb_cb *skb_cb = ATH10K_SKB_CB(skb);
int ret;
/* We should disable CCK RATE due to P2P */
@@ -3649,7 +3665,8 @@ static int ath10k_mac_tx(struct ath10k *ar,
ath10k_tx_h_8023(skb);
break;
case ATH10K_HW_TXRX_RAW:
- if (!test_bit(ATH10K_FLAG_RAW_MODE, &ar->dev_flags)) {
+ if (!test_bit(ATH10K_FLAG_RAW_MODE, &ar->dev_flags) &&
+ !(skb_cb->flags & ATH10K_SKB_F_RAW_TX)) {
WARN_ON_ONCE(1);
ieee80211_free_txskb(hw, skb);
return -ENOTSUPP;
@@ -8455,6 +8472,11 @@ int ath10k_mac_register(struct ath10k *ar)
goto err_dfs_detector_exit;
}
+ if (test_bit(WMI_SERVICE_PER_PACKET_SW_ENCRYPT, ar->wmi.svc_map)) {
+ ar->hw->wiphy->interface_modes |= BIT(NL80211_IFTYPE_AP_VLAN);
+ ar->hw->wiphy->software_iftypes |= BIT(NL80211_IFTYPE_AP_VLAN);
+ }
+
if (!ath_is_world_regd(&ar->ath_common.regulatory)) {
ret = regulatory_hint(ar->hw->wiphy,
ar->ath_common.regulatory.alpha2);
diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h
index 3cc129d..e359b6af 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.h
+++ b/drivers/net/wireless/ath/ath10k/wmi.h
@@ -202,6 +202,10 @@ enum wmi_service {
WMI_SERVICE_HOST_DFS_CHECK_SUPPORT,
WMI_SERVICE_TPC_STATS_FINAL,
WMI_SERVICE_RESET_CHIP,
+ WMI_SERVICE_CFR_CAPTURE_SUPPORT,
+ WMI_SERVICE_TX_DATA_ACK_RSSI,
+ WMI_SERVICE_CFR_CAPTURE_IND_MSG_TYPE_1,
+ WMI_SERVICE_PER_PACKET_SW_ENCRYPT,
/* keep last */
WMI_SERVICE_MAX,
@@ -349,6 +353,10 @@ enum wmi_10_4_service {
WMI_10_4_SERVICE_HTT_MGMT_TX_COMP_VALID_FLAGS,
WMI_10_4_SERVICE_HOST_DFS_CHECK_SUPPORT,
WMI_10_4_SERVICE_TPC_STATS_FINAL,
+ WMI_10_4_SERVICE_CFR_CAPTURE_SUPPORT,
+ WMI_10_4_SERVICE_TX_DATA_ACK_RSSI,
+ WMI_10_4_SERVICE_CFR_CAPTURE_IND_MSG_TYPE_1,
+ WMI_10_4_SERVICE_PER_PACKET_SW_ENCRYPT,
};
static inline char *wmi_service_name(int service_id)
@@ -461,6 +469,11 @@ static inline char *wmi_service_name(int service_id)
SVCSTR(WMI_SERVICE_HTT_MGMT_TX_COMP_VALID_FLAGS);
SVCSTR(WMI_SERVICE_HOST_DFS_CHECK_SUPPORT);
SVCSTR(WMI_SERVICE_TPC_STATS_FINAL);
+ SVCSTR(WMI_SERVICE_CFR_CAPTURE_SUPPORT);
+ SVCSTR(WMI_SERVICE_TX_DATA_ACK_RSSI);
+ SVCSTR(WMI_SERVICE_CFR_CAPTURE_IND_MSG_TYPE_1);
+ SVCSTR(WMI_SERVICE_PER_PACKET_SW_ENCRYPT);
+
default:
return NULL;
}
@@ -769,6 +782,14 @@ static inline void wmi_10_4_svc_map(const __le32 *in, unsigned long *out,
WMI_SERVICE_HOST_DFS_CHECK_SUPPORT, len);
SVCMAP(WMI_10_4_SERVICE_TPC_STATS_FINAL,
WMI_SERVICE_TPC_STATS_FINAL, len);
+ SVCMAP(WMI_10_4_SERVICE_CFR_CAPTURE_SUPPORT,
+ WMI_SERVICE_CFR_CAPTURE_SUPPORT, len);
+ SVCMAP(WMI_10_4_SERVICE_TX_DATA_ACK_RSSI,
+ WMI_SERVICE_TX_DATA_ACK_RSSI, len);
+ SVCMAP(WMI_10_4_SERVICE_CFR_CAPTURE_IND_MSG_TYPE_1,
+ WMI_SERVICE_CFR_CAPTURE_IND_MSG_TYPE_1, len);
+ SVCMAP(WMI_10_4_SERVICE_PER_PACKET_SW_ENCRYPT,
+ WMI_SERVICE_PER_PACKET_SW_ENCRYPT, len);
}
#undef SVCMAP
--
2.7.4
Manikanta Pubbisetty <[email protected]> writes:
> Mutlicast/broadcast traffic destined for a particular vlan group will
> always be encrypted in software. To enable dynamic VLANs, it requires
> driver support for sending software encrypted packets.
>
> In ath10k, sending sw encrypted frames is allowed only when we insmod
> the driver with cryptmode param set to 1, this configuration disables
> hardware crypto and enables RAW mode implicitly. Since, enabling raw
> mode has performance impact, this cannot be considered as an ideal
> solution for supporting VLANs in the driver.
>
> As an alternative take, in this approach, cryptographic keys for
> unicast traffic(per peer PTKs) and keys for non-vlan group traffic
> will be configured in hardware, allowing hardware encryption for unicast
> and non-vlan group traffic. Only vlan group traffic will be encrypted in
> software and pushed to the target with encap mode set to RAW in the TX
> descriptors.
>
> Not all firmwares can support this type of key configuration(having few
> keys installed in hardware and few only in software); for this purpose a
> new WMI service flag "WMI_SERVICE_PER_PACKET_SW_ENCRYPT" is introduced to
> advertise this support.
>
> Also, adding the logic required to send sw encrypted frames in raw mode.
>
> Tested this change on QCA9984(firmware version 10.4-3.5.3-00057).
>
> Signed-off-by: Manikanta Pubbisetty <[email protected]>
Your name in patchwork is wrong and hence my script uses the wrong
name. Please fix it by registering to patchwork[1] where it's possible
to change your name during registration, but only one time. If that
doesn't work then send a request to [email protected] and the admins
can fix it.
[1] https://patchwork.kernel.org/register/
--
Kalle Valo
this patch makes no sense at some points. AP_VLAN must be enabled always
(it is enabled by mac80211 by default, but is now disabled in very
latest git version for drivers which announce sw_crypto support)
if its disabled wds ap / wds sta operation will not work anymore since
mac80211 uses AP_VLAN for the local wds sta interfaces
Sebastian
Am 20.04.2018 um 15:57 schrieb Manikanta Pubbisetty:
> Mutlicast/broadcast traffic destined for a particular vlan group will
> always be encrypted in software. To enable dynamic VLANs, it requires
> driver support for sending software encrypted packets.
>
> In ath10k, sending sw encrypted frames is allowed only when we insmod
> the driver with cryptmode param set to 1, this configuration disables
> hardware crypto and enables RAW mode implicitly. Since, enabling raw
> mode has performance impact, this cannot be considered as an ideal
> solution for supporting VLANs in the driver.
>
> As an alternative take, in this approach, cryptographic keys for
> unicast traffic(per peer PTKs) and keys for non-vlan group traffic
> will be configured in hardware, allowing hardware encryption for unicast
> and non-vlan group traffic. Only vlan group traffic will be encrypted in
> software and pushed to the target with encap mode set to RAW in the TX
> descriptors.
>
> Not all firmwares can support this type of key configuration(having few
> keys installed in hardware and few only in software); for this purpose a
> new WMI service flag "WMI_SERVICE_PER_PACKET_SW_ENCRYPT" is introduced to
> advertise this support.
>
> Also, adding the logic required to send sw encrypted frames in raw mode.
>
> Tested this change on QCA9984(firmware version 10.4-3.5.3-00057).
>
> Signed-off-by: Manikanta Pubbisetty <[email protected]>
> ---
> drivers/net/wireless/ath/ath10k/core.h | 1 +
> drivers/net/wireless/ath/ath10k/mac.c | 26 ++++++++++++++++++++++++--
> drivers/net/wireless/ath/ath10k/wmi.h | 21 +++++++++++++++++++++
> 3 files changed, 46 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
> index e4ac8f2..105438d 100644
> --- a/drivers/net/wireless/ath/ath10k/core.h
> +++ b/drivers/net/wireless/ath/ath10k/core.h
> @@ -122,6 +122,7 @@ enum ath10k_skb_flags {
> ATH10K_SKB_F_DELIVER_CAB = BIT(2),
> ATH10K_SKB_F_MGMT = BIT(3),
> ATH10K_SKB_F_QOS = BIT(4),
> + ATH10K_SKB_F_RAW_TX = BIT(5),
> };
>
> struct ath10k_skb_cb {
> diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
> index fc3320f..694c0aa 100644
> --- a/drivers/net/wireless/ath/ath10k/mac.c
> +++ b/drivers/net/wireless/ath/ath10k/mac.c
> @@ -3362,6 +3362,7 @@ ath10k_mac_tx_h_get_txmode(struct ath10k *ar,
> struct sk_buff *skb)
> {
> const struct ieee80211_hdr *hdr = (void *)skb->data;
> + const struct ath10k_skb_cb *skb_cb = ATH10K_SKB_CB(skb);
> __le16 fc = hdr->frame_control;
>
> if (!vif || vif->type == NL80211_IFTYPE_MONITOR)
> @@ -3403,7 +3404,8 @@ ath10k_mac_tx_h_get_txmode(struct ath10k *ar,
> if (ieee80211_is_data_present(fc) && sta && sta->tdls)
> return ATH10K_HW_TXRX_ETHERNET;
>
> - if (test_bit(ATH10K_FLAG_RAW_MODE, &ar->dev_flags))
> + if (test_bit(ATH10K_FLAG_RAW_MODE, &ar->dev_flags) ||
> + skb_cb->flags & ATH10K_SKB_F_RAW_TX)
> return ATH10K_HW_TXRX_RAW;
>
> return ATH10K_HW_TXRX_NATIVE_WIFI;
> @@ -3513,6 +3515,9 @@ static void ath10k_mac_tx_h_fill_cb(struct ath10k *ar,
> {
> struct ieee80211_hdr *hdr = (void *)skb->data;
> struct ath10k_skb_cb *cb = ATH10K_SKB_CB(skb);
> + const struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
> + bool is_data = ieee80211_is_data(hdr->frame_control) ||
> + ieee80211_is_data_qos(hdr->frame_control);
>
> cb->flags = 0;
> if (!ath10k_tx_h_use_hwcrypto(vif, skb))
> @@ -3524,6 +3529,16 @@ static void ath10k_mac_tx_h_fill_cb(struct ath10k *ar,
> if (ieee80211_is_data_qos(hdr->frame_control))
> cb->flags |= ATH10K_SKB_F_QOS;
>
> + /* Data frames encrypted in software will be posted to firmware
> + * with tx encap mode set to RAW. One such case would be the
> + * multicast traffic generated for a VLAN group.
> + */
> + if (is_data && ieee80211_has_protected(hdr->frame_control) &&
> + !info->control.hw_key) {
> + cb->flags |= ATH10K_SKB_F_NO_HWCRYPT;
> + cb->flags |= ATH10K_SKB_F_RAW_TX;
> + }
> +
> cb->vif = vif;
> cb->txq = txq;
> }
> @@ -3632,6 +3647,7 @@ static int ath10k_mac_tx(struct ath10k *ar,
> {
> struct ieee80211_hw *hw = ar->hw;
> struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
> + const struct ath10k_skb_cb *skb_cb = ATH10K_SKB_CB(skb);
> int ret;
>
> /* We should disable CCK RATE due to P2P */
> @@ -3649,7 +3665,8 @@ static int ath10k_mac_tx(struct ath10k *ar,
> ath10k_tx_h_8023(skb);
> break;
> case ATH10K_HW_TXRX_RAW:
> - if (!test_bit(ATH10K_FLAG_RAW_MODE, &ar->dev_flags)) {
> + if (!test_bit(ATH10K_FLAG_RAW_MODE, &ar->dev_flags) &&
> + !(skb_cb->flags & ATH10K_SKB_F_RAW_TX)) {
> WARN_ON_ONCE(1);
> ieee80211_free_txskb(hw, skb);
> return -ENOTSUPP;
> @@ -8455,6 +8472,11 @@ int ath10k_mac_register(struct ath10k *ar)
> goto err_dfs_detector_exit;
> }
>
> + if (test_bit(WMI_SERVICE_PER_PACKET_SW_ENCRYPT, ar->wmi.svc_map)) {
> + ar->hw->wiphy->interface_modes |= BIT(NL80211_IFTYPE_AP_VLAN);
> + ar->hw->wiphy->software_iftypes |= BIT(NL80211_IFTYPE_AP_VLAN);
> + }
> +
> if (!ath_is_world_regd(&ar->ath_common.regulatory)) {
> ret = regulatory_hint(ar->hw->wiphy,
> ar->ath_common.regulatory.alpha2);
> diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h
> index 3cc129d..e359b6af 100644
> --- a/drivers/net/wireless/ath/ath10k/wmi.h
> +++ b/drivers/net/wireless/ath/ath10k/wmi.h
> @@ -202,6 +202,10 @@ enum wmi_service {
> WMI_SERVICE_HOST_DFS_CHECK_SUPPORT,
> WMI_SERVICE_TPC_STATS_FINAL,
> WMI_SERVICE_RESET_CHIP,
> + WMI_SERVICE_CFR_CAPTURE_SUPPORT,
> + WMI_SERVICE_TX_DATA_ACK_RSSI,
> + WMI_SERVICE_CFR_CAPTURE_IND_MSG_TYPE_1,
> + WMI_SERVICE_PER_PACKET_SW_ENCRYPT,
>
> /* keep last */
> WMI_SERVICE_MAX,
> @@ -349,6 +353,10 @@ enum wmi_10_4_service {
> WMI_10_4_SERVICE_HTT_MGMT_TX_COMP_VALID_FLAGS,
> WMI_10_4_SERVICE_HOST_DFS_CHECK_SUPPORT,
> WMI_10_4_SERVICE_TPC_STATS_FINAL,
> + WMI_10_4_SERVICE_CFR_CAPTURE_SUPPORT,
> + WMI_10_4_SERVICE_TX_DATA_ACK_RSSI,
> + WMI_10_4_SERVICE_CFR_CAPTURE_IND_MSG_TYPE_1,
> + WMI_10_4_SERVICE_PER_PACKET_SW_ENCRYPT,
> };
>
> static inline char *wmi_service_name(int service_id)
> @@ -461,6 +469,11 @@ static inline char *wmi_service_name(int service_id)
> SVCSTR(WMI_SERVICE_HTT_MGMT_TX_COMP_VALID_FLAGS);
> SVCSTR(WMI_SERVICE_HOST_DFS_CHECK_SUPPORT);
> SVCSTR(WMI_SERVICE_TPC_STATS_FINAL);
> + SVCSTR(WMI_SERVICE_CFR_CAPTURE_SUPPORT);
> + SVCSTR(WMI_SERVICE_TX_DATA_ACK_RSSI);
> + SVCSTR(WMI_SERVICE_CFR_CAPTURE_IND_MSG_TYPE_1);
> + SVCSTR(WMI_SERVICE_PER_PACKET_SW_ENCRYPT);
> +
> default:
> return NULL;
> }
> @@ -769,6 +782,14 @@ static inline void wmi_10_4_svc_map(const __le32 *in, unsigned long *out,
> WMI_SERVICE_HOST_DFS_CHECK_SUPPORT, len);
> SVCMAP(WMI_10_4_SERVICE_TPC_STATS_FINAL,
> WMI_SERVICE_TPC_STATS_FINAL, len);
> + SVCMAP(WMI_10_4_SERVICE_CFR_CAPTURE_SUPPORT,
> + WMI_SERVICE_CFR_CAPTURE_SUPPORT, len);
> + SVCMAP(WMI_10_4_SERVICE_TX_DATA_ACK_RSSI,
> + WMI_SERVICE_TX_DATA_ACK_RSSI, len);
> + SVCMAP(WMI_10_4_SERVICE_CFR_CAPTURE_IND_MSG_TYPE_1,
> + WMI_SERVICE_CFR_CAPTURE_IND_MSG_TYPE_1, len);
> + SVCMAP(WMI_10_4_SERVICE_PER_PACKET_SW_ENCRYPT,
> + WMI_SERVICE_PER_PACKET_SW_ENCRYPT, len);
> }
>
> #undef SVCMAP
--
Mit freundlichen Grüssen / Regards
Sebastian Gottschall / CTO
NewMedia-NET GmbH - DD-WRT
Firmensitz: Stubenwaldallee 21a, 64625 Bensheim
Registergericht: Amtsgericht Darmstadt, HRB 25473
Geschäftsführer: Peter Steinhäuser, Christian Scheele
http://www.dd-wrt.com
email: [email protected]
Tel.: +496251-582650 / Fax: +496251-5826565
consider my comment regarding vlan_ap.
this patch will break wds ap / wds sta support with latest mac80211 (see
also my post on the wireless mailing list about the breaking patch in
mac80211)
so AP_VLAN must be masked always for all chipsets. otherwise wds breaks
and this is not just a guess. i tested it yesterday using this patch and
found
the cause of the issue
the following lines
+ if (test_bit(WMI_SERVICE_PER_PACKET_SW_ENCRYPT, ar->wmi.svc_map)) {
+ ar->hw->wiphy->interface_modes |= BIT(NL80211_IFTYPE_AP_VLAN);
+ ar->hw->wiphy->software_iftypes |= BIT(NL80211_IFTYPE_AP_VLAN);
+ }
must be just
+ ar->hw->wiphy->interface_modes |= BIT(NL80211_IFTYPE_AP_VLAN);
+ ar->hw->wiphy->software_iftypes |= BIT(NL80211_IFTYPE_AP_VLAN);
everthing else will cause a regression
Am 24.04.2018 um 10:09 schrieb Kalle Valo:
> Manikanta Pubbisetty <[email protected]> writes:
>
>> Mutlicast/broadcast traffic destined for a particular vlan group will
>> always be encrypted in software. To enable dynamic VLANs, it requires
>> driver support for sending software encrypted packets.
>>
>> In ath10k, sending sw encrypted frames is allowed only when we insmod
>> the driver with cryptmode param set to 1, this configuration disables
>> hardware crypto and enables RAW mode implicitly. Since, enabling raw
>> mode has performance impact, this cannot be considered as an ideal
>> solution for supporting VLANs in the driver.
>>
>> As an alternative take, in this approach, cryptographic keys for
>> unicast traffic(per peer PTKs) and keys for non-vlan group traffic
>> will be configured in hardware, allowing hardware encryption for unicast
>> and non-vlan group traffic. Only vlan group traffic will be encrypted in
>> software and pushed to the target with encap mode set to RAW in the TX
>> descriptors.
>>
>> Not all firmwares can support this type of key configuration(having few
>> keys installed in hardware and few only in software); for this purpose a
>> new WMI service flag "WMI_SERVICE_PER_PACKET_SW_ENCRYPT" is introduced to
>> advertise this support.
>>
>> Also, adding the logic required to send sw encrypted frames in raw mode.
>>
>> Tested this change on QCA9984(firmware version 10.4-3.5.3-00057).
>>
>> Signed-off-by: Manikanta Pubbisetty <[email protected]>
> Your name in patchwork is wrong and hence my script uses the wrong
> name. Please fix it by registering to patchwork[1] where it's possible
> to change your name during registration, but only one time. If that
> doesn't work then send a request to [email protected] and the admins
> can fix it.
>
> [1] https://patchwork.kernel.org/register/
>
--
Mit freundlichen Grüssen / Regards
Sebastian Gottschall / CTO
NewMedia-NET GmbH - DD-WRT
Firmensitz: Stubenwaldallee 21a, 64625 Bensheim
Registergericht: Amtsgericht Darmstadt, HRB 25473
Geschäftsführer: Peter Steinhäuser, Christian Scheele
http://www.dd-wrt.com
email: [email protected]
Tel.: +496251-582650 / Fax: +496251-5826565
Am 24.04.2018 um 11:52 schrieb Kalle Valo:
> Sebastian Gottschall <[email protected]> writes:
>
>> consider my comment regarding vlan_ap.
> I am considering comment, I just go one issue at a time and haven't had
> a time to look at your comment. But PLEASE do not top post, it's very
> annoying and creates a mess in patchwork.
>
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#do_not_top_post_and_edit_your_quotes
i almost do top posts for a single reason. you have to scroll down a
long time sometimes to get the essential information.
i dont know why most people in my country prefer top posting. i will try
to remember and change it in future
>
--
Mit freundlichen Grüssen / Regards
Sebastian Gottschall / CTO
NewMedia-NET GmbH - DD-WRT
Firmensitz: Stubenwaldallee 21a, 64625 Bensheim
Registergericht: Amtsgericht Darmstadt, HRB 25473
Geschäftsführer: Peter Steinhäuser, Christian Scheele
http://www.dd-wrt.com
email: [email protected]
Tel.: +496251-582650 / Fax: +496251-5826565
Sebastian Gottschall <[email protected]> writes:
> consider my comment regarding vlan_ap.
I am considering comment, I just go one issue at a time and haven't had
a time to look at your comment. But PLEASE do not top post, it's very
annoying and creates a mess in patchwork.
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#do_not_top_post_and_edit_your_quotes
--
Kalle Valo
Yes Sebastian, Your point is valid. This would break the 4-addr
operation on other ath10k devices which does not support the new WMI
service.
I have another approach to solve this problem, I will come with a small
patch and see what johannes has to say on that approach.
Manikanta
On 4/24/2018 2:39 PM, Sebastian Gottschall wrote:
> consider my comment regarding vlan_ap.
> this patch will break wds ap / wds sta support with latest mac80211
> (see also my post on the wireless mailing list about the breaking
> patch in mac80211)
> so AP_VLAN must be masked always for all chipsets. otherwise wds
> breaks and this is not just a guess. i tested it yesterday using this
> patch and found
> the cause of the issue
>
> the following lines
>
> + if (test_bit(WMI_SERVICE_PER_PACKET_SW_ENCRYPT,
> ar->wmi.svc_map)) {
> + ar->hw->wiphy->interface_modes |= BIT(NL80211_IFTYPE_AP_VLAN);
> + ar->hw->wiphy->software_iftypes |= BIT(NL80211_IFTYPE_AP_VLAN);
> + }
>
>
> must be just
>
> + ar->hw->wiphy->interface_modes |= BIT(NL80211_IFTYPE_AP_VLAN);
> + ar->hw->wiphy->software_iftypes |= BIT(NL80211_IFTYPE_AP_VLAN);
>
> everthing else will cause a regression
>
> Am 24.04.2018 um 10:09 schrieb Kalle Valo:
>> Manikanta Pubbisetty <[email protected]> writes:
>>
>>> Mutlicast/broadcast traffic destined for a particular vlan group will
>>> always be encrypted in software. To enable dynamic VLANs, it requires
>>> driver support for sending software encrypted packets.
>>>
>>> In ath10k, sending sw encrypted frames is allowed only when we insmod
>>> the driver with cryptmode param set to 1, this configuration disables
>>> hardware crypto and enables RAW mode implicitly. Since, enabling raw
>>> mode has performance impact, this cannot be considered as an ideal
>>> solution for supporting VLANs in the driver.
>>>
>>> As an alternative take, in this approach, cryptographic keys for
>>> unicast traffic(per peer PTKs) and keys for non-vlan group traffic
>>> will be configured in hardware, allowing hardware encryption for
>>> unicast
>>> and non-vlan group traffic. Only vlan group traffic will be
>>> encrypted in
>>> software and pushed to the target with encap mode set to RAW in the TX
>>> descriptors.
>>>
>>> Not all firmwares can support this type of key configuration(having few
>>> keys installed in hardware and few only in software); for this
>>> purpose a
>>> new WMI service flag "WMI_SERVICE_PER_PACKET_SW_ENCRYPT" is
>>> introduced to
>>> advertise this support.
>>>
>>> Also, adding the logic required to send sw encrypted frames in raw
>>> mode.
>>>
>>> Tested this change on QCA9984(firmware version 10.4-3.5.3-00057).
>>>
>>> Signed-off-by: Manikanta Pubbisetty <[email protected]>
>> Your name in patchwork is wrong and hence my script uses the wrong
>> name. Please fix it by registering to patchwork[1] where it's possible
>> to change your name during registration, but only one time. If that
>> doesn't work then send a request to [email protected] and the admins
>> can fix it.
>>
>> [1] https://patchwork.kernel.org/register/
>>
>
> Do you know why it actually broke it? I mean, we should've turned off
> the strict requirement for sw crypto control only for the GTK, and that
> shouldn't matter so much?
With the change db3bdcb9c3ff, AP/VLAN support is advertised by the
driver conditionally; the primary reason for doing this is to support
VLAN operations on sw crypto controlled devices.
AP also creates AP/VLAN devices for supporting 4-addr clients and since
the driver now advertises AP/VLAN support conditionally, the 4-addr
operation which has no relation to the VLANs(Per VLAN GTKs) was broken
on some ath10k devices.
>> I have couple of ideas on how to address the problem,
>>
>> 1) Add a new hw_flag and based on the hardware flag, allow/disallow the
>> creation of AP_VLAN interface.
>>
>> + * @IEEE80211_HW_SUPPORTS_SW_ENCRYPT: Device is capable of transmitting
>> + * frames encrypted in software, only valid when SW_CRYPTO_CONTROL
>> + * is enabled. Based on this flag, mac80211 can allow/disallow VLAN
>> + * operations in the BSS.
> Based on the name and initial description, this sounds equivalent to
> just turning off SW_CRYPTO_CONTROL. I think that's not the intent, but
> would need some renaming.
I can rename it to something which is very specific to VLAN support on
sw crypto controlled devices if that is okay.
>> 2) Allow mac80211 to call set_key for GTKs on AP_VLAN interfaces for
>> crypto controlled devices, let the driver decide whether to return '1'
>> or some error code based on their support for transmitting sw encrypted
>> frames. I am little skeptical with this approach as drivers are totally
>> unaware of AP_VLAN interfaces.
> No, that won't work.
>
> I'm unsure how 4-addr VLAN can work with ath10k either way?
> Maybe it just doesn't normally need a GTK, so nothing broke before, but
> your other patch changed things to remove VLAN and then of course it's
> no longer available?
>
> But then I don't understand the complaint that
>
> So maybe the solution should be to add a separate flag for whether or
> not 4-addr VLAN is supported?
IIUC, the combination of 4-addr and VLAN doesn't work even without this
change db3bdcb9c3ff (" mac80211: allow AP_VLAN operation on crypto
controlled devices ").
AP/VLAN devices are used separately to either support 4-addr operation
or VLAN (separating the clients into VLAN groups each having unique GTK),
I believe both are mutually exclusive.
One reason why the combination of 4-addr+VLAN doesn't work could be that
a single AP/VLAN interface can cater to several wireless clients but to
support 4-addr operation every client device should have an exclusive
AP/VLAN interface.
It is possible where few clients in a specific VLAN group can use 4-addr
mode and few might not, if this is the case then AP has to create
individual AP/VLAN device for each 4-addr client and since these clients
belong to specific VLAN group, all of these clients should be tied
to AP/VLAN device created for VLAN operation(Created for maintaining
unique GTKs). I am not sure if the current stack supports this complex
combination, I could not make it work in my testing though.
>>>> + * @IEEE80211_HW_SUPPORTS_SW_ENCRYPT: Device is capable of transmitting
>>>> + * frames encrypted in software, only valid when SW_CRYPTO_CONTROL
>>>> + * is enabled. Based on this flag, mac80211 can allow/disallow VLAN
>>>> + * operations in the BSS.
>>> Based on the name and initial description, this sounds equivalent to
>>> just turning off SW_CRYPTO_CONTROL. I think that's not the intent, but
>>> would need some renaming.
>> I can rename it to something which is very specific to VLAN support on
>> sw crypto controlled devices if that is okay.
> I don't think that makes sense. If we split the capability of AP_VLAN
> and AP_VLAN_FOR_4ADDR at the "root", then we don't need to play with all
> these things. Yes, this is slightly awkward for userspace, and perhaps
> with the interface combination checks, but I'd like you to look at that.
>
Makes sense, I had this thought to split the VLAN and 4-addr
functionality, but since we need to fiddle with userspace, I refrained.
Anyways, I will check all the possibilities of splitting these
functionalities.
Thanks,
Manikanta
Am 23.05.2018 um 12:39 schrieb Johannes Berg:
> On Wed, 2018-05-23 at 16:09 +0530, Manikanta Pubbisetty wrote:
>>>>>> + * @IEEE80211_HW_SUPPORTS_SW_ENCRYPT: Device is capable of transmitting
>>>>>> + * frames encrypted in software, only valid when SW_CRYPTO_CONTROL
>>>>>> + * is enabled. Based on this flag, mac80211 can allow/disallow VLAN
>>>>>> + * operations in the BSS.
>>>>> Based on the name and initial description, this sounds equivalent to
>>>>> just turning off SW_CRYPTO_CONTROL. I think that's not the intent, but
>>>>> would need some renaming.
>>>> I can rename it to something which is very specific to VLAN support on
>>>> sw crypto controlled devices if that is okay.
>>> I don't think that makes sense. If we split the capability of AP_VLAN
>>> and AP_VLAN_FOR_4ADDR at the "root", then we don't need to play with all
>>> these things. Yes, this is slightly awkward for userspace, and perhaps
>>> with the interface combination checks, but I'd like you to look at that.
>>>
>> Makes sense, I had this thought to split the VLAN and 4-addr
>> functionality, but since we need to fiddle with userspace, I refrained.
>> Anyways, I will check all the possibilities of splitting these
>> functionalities.
> I'm not sure we really have to - it's likely that wpa_s/hostapd don't
> check the capabilities?
mmh not sure. it might not check the capabilitiy, but it sets the
interface type to IFTYPE_AP_VLAN
for wds sta interfaces. that might collide
see the following snippet taken from hostapd code
static int i802_set_wds_sta(void *priv, const u8 *addr, int aid, int val,
const char *bridge_ifname, char *ifname_wds)
{
struct i802_bss *bss = priv;
struct wpa_driver_nl80211_data *drv = bss->drv;
char name[IFNAMSIZ + 1];
os_snprintf(name, sizeof(name), "%s.sta%d", bss->ifname, aid);
if (ifname_wds)
os_strlcpy(ifname_wds, name, IFNAMSIZ + 1);
wpa_printf(MSG_DEBUG, "nl80211: Set WDS STA addr=" MACSTR
" aid=%d val=%d name=%s", MAC2STR(addr), aid, val,
name);
if (val) {
if (!if_nametoindex(name)) {
if (nl80211_create_iface(drv, name,
NL80211_IFTYPE_AP_VLAN,
bss->addr, 1, NULL,
NULL, 0) <
0)
return -1;
if (bridge_ifname &&
linux_br_add_if(drv->global->ioctl_sock,
bridge_ifname, name) < 0)
return -1;
}
if (linux_set_iface_flags(drv->global->ioctl_sock,
name, 1)) {
wpa_printf(MSG_ERROR, "nl80211: Failed to set
WDS STA "
"interface %s up", name);
}
return i802_set_sta_vlan(priv, addr, name, 0);
} else {
if (bridge_ifname)
linux_br_del_if(drv->global->ioctl_sock, bridge_ifname,
name);
i802_set_sta_vlan(priv, addr, bss->ifname, 0);
nl80211_remove_iface(drv, if_nametoindex(name));
return 0;
}
}
>
> johannes
>
did someone notice that GTK rekeying is broken in 4addr mode for 10.4.
firmwares since a long time.
i have a test with 2 9984 devices. one is 4addr ap, the second 4addr
sta. it disconnects on rekeying and reauthenticates. (reproducable with
99x0 as well)
this does not occur on 10.2 based chipsets like 988x. it seems to be a
internal firmware problem since the beginning. i wrote already a long
time ago about it,
but no solution was provided. maybe its also finally time to take care
about this issue (cause unknown)
Sebastian
Am 04.05.2018 um 08:50 schrieb Manikanta Pubbisetty:
> Johannes,
>
> It seems like commit db3bdcb9c3ff ("mac80211: allow AP_VLAN operation
> on crypto controlled devices") has broken 4-addr operation on crypto
> controlled devices as reported by sebastian.
> The commit was mainly focused in addressing the problem in supporting
> VLANs on crypto controlled devices but since 4-addr mode is also
> dependent on AP_VLAN interface, this commit breaks the 4-addr AP mode.
>
> I have couple of ideas on how to address the problem,
>
> 1) Add a new hw_flag and based on the hardware flag, allow/disallow
> the creation of AP_VLAN interface.
>
> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
> index d2279b2..301d9c38 100644
> --- a/include/net/mac80211.h
> +++ b/include/net/mac80211.h
> @@ -2084,6 +2084,11 @@ struct ieee80211_txq {
> * @IEEE80211_HW_DOESNT_SUPPORT_QOS_NDP: The driver (or firmware)
> doesn't
> * support QoS NDP for AP probing - that's most likely a driver bug.
> *
> + * @IEEE80211_HW_SUPPORTS_SW_ENCRYPT: Device is capable of transmitting
> + * frames encrypted in software, only valid when SW_CRYPTO_CONTROL
> + * is enabled. Based on this flag, mac80211 can allow/disallow VLAN
> + * operations in the BSS.
> + *
> * @NUM_IEEE80211_HW_FLAGS: number of hardware flags, used for sizing
> arrays
> */
> enum ieee80211_hw_flags {
> @@ -2129,6 +2134,7 @@ enum ieee80211_hw_flags {
> IEEE80211_HW_SUPPORTS_TDLS_BUFFER_STA,
> IEEE80211_HW_DEAUTH_NEED_MGD_TX_PREP,
> IEEE80211_HW_DOESNT_SUPPORT_QOS_NDP,
> + IEEE80211_HW_SUPPORTS_SW_ENCRYPT,
>
> /* keep last, obviously */
> NUM_IEEE80211_HW_FLAGS
> diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
> index 555e389..c825d27 100644
> --- a/net/mac80211/iface.c
> +++ b/net/mac80211/iface.c
> @@ -1736,6 +1736,11 @@ int ieee80211_if_add(struct ieee80211_local
> *local, const char *name,
>
> ASSERT_RTNL();
>
> + if ((type == NL80211_IFTYPE_AP_VLAN) && !params->use_4addr &&
> + ieee80211_hw_check(local->hw, SW_CRYPTO_CONTROL) &&
> + !ieee80211_hw_check(local->hw, SUPPORTS_SW_ENCRYPT))
> + return -EOPNOTSUPP;
> +
> if (type == NL80211_IFTYPE_P2P_DEVICE || type ==
> NL80211_IFTYPE_NAN) {
> struct wireless_dev *wdev;
>
> 2) Allow mac80211 to call set_key for GTKs on AP_VLAN interfaces for
> crypto controlled devices, let the driver decide whether to return '1'
> or some error code based on their support for transmitting sw
> encrypted frames. I am little skeptical with this approach as drivers
> are totally unaware of AP_VLAN interfaces.
>
> diff --git a/net/mac80211/key.c b/net/mac80211/key.c
> index ee0d0cc..0ff5597 100644
> --- a/net/mac80211/key.c
> +++ b/net/mac80211/key.c
> @@ -167,7 +167,8 @@ static int ieee80211_key_enable_hw_accel(struct
> ieee80211_key *key)
> * The driver doesn't know anything about VLAN
> interfaces.
> * Hence, don't send GTKs for VLAN interfaces to the
> driver.
> */
> - if (!(key->conf.flags & IEEE80211_KEY_FLAG_PAIRWISE))
> + if ((!(key->conf.flags & IEEE80211_KEY_FLAG_PAIRWISE) &&
> + !ieee80211_hw_check(&key->local->hw,
> SW_CRYPTO_CONTROL)))
> goto out_unsupported;
> }
>
> Please let me know which is the better way to deal the problem.
>
> Thanks,
> Manikanta
>
>
> On 2018-04-24 14:39, Sebastian Gottschall wrote:
>> consider my comment regarding vlan_ap.
>> this patch will break wds ap / wds sta support with latest mac80211
>> (see also my post on the wireless mailing list about the breaking
>> patch in mac80211)
>> so AP_VLAN must be masked always for all chipsets. otherwise wds
>> breaks and this is not just a guess. i tested it yesterday using this
>> patch and found
>> the cause of the issue
>>
>> the following lines
>>
>> + if (test_bit(WMI_SERVICE_PER_PACKET_SW_ENCRYPT,
>> ar->wmi.svc_map)) {
>> + ar->hw->wiphy->interface_modes |= BIT(NL80211_IFTYPE_AP_VLAN);
>> + ar->hw->wiphy->software_iftypes |= BIT(NL80211_IFTYPE_AP_VLAN);
>> + }
>>
>>
>> must be just
>>
>> + ar->hw->wiphy->interface_modes |= BIT(NL80211_IFTYPE_AP_VLAN);
>> + ar->hw->wiphy->software_iftypes |= BIT(NL80211_IFTYPE_AP_VLAN);
>>
>> everthing else will cause a regression
>>
>> Am 24.04.2018 um 10:09 schrieb Kalle Valo:
>>> Manikanta Pubbisetty <[email protected]> writes:
>>>
>>>> Mutlicast/broadcast traffic destined for a particular vlan group will
>>>> always be encrypted in software. To enable dynamic VLANs, it requires
>>>> driver support for sending software encrypted packets.
>>>>
>>>> In ath10k, sending sw encrypted frames is allowed only when we insmod
>>>> the driver with cryptmode param set to 1, this configuration disables
>>>> hardware crypto and enables RAW mode implicitly. Since, enabling raw
>>>> mode has performance impact, this cannot be considered as an ideal
>>>> solution for supporting VLANs in the driver.
>>>>
>>>> As an alternative take, in this approach, cryptographic keys for
>>>> unicast traffic(per peer PTKs) and keys for non-vlan group traffic
>>>> will be configured in hardware, allowing hardware encryption for
>>>> unicast
>>>> and non-vlan group traffic. Only vlan group traffic will be
>>>> encrypted in
>>>> software and pushed to the target with encap mode set to RAW in the TX
>>>> descriptors.
>>>>
>>>> Not all firmwares can support this type of key configuration(having
>>>> few
>>>> keys installed in hardware and few only in software); for this
>>>> purpose a
>>>> new WMI service flag "WMI_SERVICE_PER_PACKET_SW_ENCRYPT" is
>>>> introduced to
>>>> advertise this support.
>>>>
>>>> Also, adding the logic required to send sw encrypted frames in raw
>>>> mode.
>>>>
>>>> Tested this change on QCA9984(firmware version 10.4-3.5.3-00057).
>>>>
>>>> Signed-off-by: Manikanta Pubbisetty <[email protected]>
>>> Your name in patchwork is wrong and hence my script uses the wrong
>>> name. Please fix it by registering to patchwork[1] where it's possible
>>> to change your name during registration, but only one time. If that
>>> doesn't work then send a request to [email protected] and the admins
>>> can fix it.
>>>
>>> [1] https://patchwork.kernel.org/register/
>>>
>
--
Mit freundlichen Grüssen / Regards
Sebastian Gottschall / CTO
NewMedia-NET GmbH - DD-WRT
Firmensitz: Stubenwaldallee 21a, 64625 Bensheim
Registergericht: Amtsgericht Darmstadt, HRB 25473
Geschäftsführer: Peter Steinhäuser, Christian Scheele
http://www.dd-wrt.com
email: [email protected]
Tel.: +496251-582650 / Fax: +496251-5826565
On Mon, 2018-04-23 at 21:18 +0200, Sebastian Gottschall wrote:
> this patch makes no sense at some points. AP_VLAN must be enabled always
> (it is enabled by mac80211 by default, but is now disabled in very
> latest git version for drivers which announce sw_crypto support)
> if its disabled wds ap / wds sta operation will not work anymore since
> mac80211 uses AP_VLAN for the local wds sta interfaces
You'd do you well to learn the correct terminology used in Linux if you
try to communicate with us...
What you say there makes no sense, WDS is a separate mode. Maybe you
mean 4-addr mode?
johannes
Johannes,
It seems like commit db3bdcb9c3ff ("mac80211: allow AP_VLAN operation on
crypto controlled devices") has broken 4-addr operation on crypto
controlled devices as reported by sebastian.
The commit was mainly focused in addressing the problem in supporting
VLANs on crypto controlled devices but since 4-addr mode is also
dependent on AP_VLAN interface, this commit breaks the 4-addr AP mode.
I have couple of ideas on how to address the problem,
1) Add a new hw_flag and based on the hardware flag, allow/disallow the
creation of AP_VLAN interface.
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index d2279b2..301d9c38 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -2084,6 +2084,11 @@ struct ieee80211_txq {
* @IEEE80211_HW_DOESNT_SUPPORT_QOS_NDP: The driver (or firmware)
doesn't
* support QoS NDP for AP probing - that's most likely a driver
bug.
*
+ * @IEEE80211_HW_SUPPORTS_SW_ENCRYPT: Device is capable of transmitting
+ * frames encrypted in software, only valid when SW_CRYPTO_CONTROL
+ * is enabled. Based on this flag, mac80211 can allow/disallow VLAN
+ * operations in the BSS.
+ *
* @NUM_IEEE80211_HW_FLAGS: number of hardware flags, used for sizing
arrays
*/
enum ieee80211_hw_flags {
@@ -2129,6 +2134,7 @@ enum ieee80211_hw_flags {
IEEE80211_HW_SUPPORTS_TDLS_BUFFER_STA,
IEEE80211_HW_DEAUTH_NEED_MGD_TX_PREP,
IEEE80211_HW_DOESNT_SUPPORT_QOS_NDP,
+ IEEE80211_HW_SUPPORTS_SW_ENCRYPT,
/* keep last, obviously */
NUM_IEEE80211_HW_FLAGS
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 555e389..c825d27 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -1736,6 +1736,11 @@ int ieee80211_if_add(struct ieee80211_local
*local, const char *name,
ASSERT_RTNL();
+ if ((type == NL80211_IFTYPE_AP_VLAN) && !params->use_4addr &&
+ ieee80211_hw_check(local->hw, SW_CRYPTO_CONTROL) &&
+ !ieee80211_hw_check(local->hw, SUPPORTS_SW_ENCRYPT))
+ return -EOPNOTSUPP;
+
if (type == NL80211_IFTYPE_P2P_DEVICE || type ==
NL80211_IFTYPE_NAN) {
struct wireless_dev *wdev;
2) Allow mac80211 to call set_key for GTKs on AP_VLAN interfaces for
crypto controlled devices, let the driver decide whether to return '1'
or some error code based on their support for transmitting sw encrypted
frames. I am little skeptical with this approach as drivers are totally
unaware of AP_VLAN interfaces.
diff --git a/net/mac80211/key.c b/net/mac80211/key.c
index ee0d0cc..0ff5597 100644
--- a/net/mac80211/key.c
+++ b/net/mac80211/key.c
@@ -167,7 +167,8 @@ static int ieee80211_key_enable_hw_accel(struct
ieee80211_key *key)
* The driver doesn't know anything about VLAN
interfaces.
* Hence, don't send GTKs for VLAN interfaces to the
driver.
*/
- if (!(key->conf.flags & IEEE80211_KEY_FLAG_PAIRWISE))
+ if ((!(key->conf.flags & IEEE80211_KEY_FLAG_PAIRWISE) &&
+ !ieee80211_hw_check(&key->local->hw,
SW_CRYPTO_CONTROL)))
goto out_unsupported;
}
Please let me know which is the better way to deal the problem.
Thanks,
Manikanta
On 2018-04-24 14:39, Sebastian Gottschall wrote:
> consider my comment regarding vlan_ap.
> this patch will break wds ap / wds sta support with latest mac80211
> (see also my post on the wireless mailing list about the breaking
> patch in mac80211)
> so AP_VLAN must be masked always for all chipsets. otherwise wds
> breaks and this is not just a guess. i tested it yesterday using this
> patch and found
> the cause of the issue
>
> the following lines
>
> + if (test_bit(WMI_SERVICE_PER_PACKET_SW_ENCRYPT,
> ar->wmi.svc_map)) {
> + ar->hw->wiphy->interface_modes |= BIT(NL80211_IFTYPE_AP_VLAN);
> + ar->hw->wiphy->software_iftypes |=
> BIT(NL80211_IFTYPE_AP_VLAN);
> + }
>
>
> must be just
>
> + ar->hw->wiphy->interface_modes |= BIT(NL80211_IFTYPE_AP_VLAN);
> + ar->hw->wiphy->software_iftypes |=
> BIT(NL80211_IFTYPE_AP_VLAN);
>
> everthing else will cause a regression
>
> Am 24.04.2018 um 10:09 schrieb Kalle Valo:
>> Manikanta Pubbisetty <[email protected]> writes:
>>
>>> Mutlicast/broadcast traffic destined for a particular vlan group will
>>> always be encrypted in software. To enable dynamic VLANs, it requires
>>> driver support for sending software encrypted packets.
>>>
>>> In ath10k, sending sw encrypted frames is allowed only when we insmod
>>> the driver with cryptmode param set to 1, this configuration disables
>>> hardware crypto and enables RAW mode implicitly. Since, enabling raw
>>> mode has performance impact, this cannot be considered as an ideal
>>> solution for supporting VLANs in the driver.
>>>
>>> As an alternative take, in this approach, cryptographic keys for
>>> unicast traffic(per peer PTKs) and keys for non-vlan group traffic
>>> will be configured in hardware, allowing hardware encryption for
>>> unicast
>>> and non-vlan group traffic. Only vlan group traffic will be encrypted
>>> in
>>> software and pushed to the target with encap mode set to RAW in the
>>> TX
>>> descriptors.
>>>
>>> Not all firmwares can support this type of key configuration(having
>>> few
>>> keys installed in hardware and few only in software); for this
>>> purpose a
>>> new WMI service flag "WMI_SERVICE_PER_PACKET_SW_ENCRYPT" is
>>> introduced to
>>> advertise this support.
>>>
>>> Also, adding the logic required to send sw encrypted frames in raw
>>> mode.
>>>
>>> Tested this change on QCA9984(firmware version 10.4-3.5.3-00057).
>>>
>>> Signed-off-by: Manikanta Pubbisetty <[email protected]>
>> Your name in patchwork is wrong and hence my script uses the wrong
>> name. Please fix it by registering to patchwork[1] where it's possible
>> to change your name during registration, but only one time. If that
>> doesn't work then send a request to [email protected] and the admins
>> can fix it.
>>
>> [1] https://patchwork.kernel.org/register/
>>
Am 18.05.2018 um 11:54 schrieb Johannes Berg:
> On Fri, 2018-05-04 at 12:20 +0530, Manikanta Pubbisetty wrote:
>> Johannes,
>>
>> It seems like commit db3bdcb9c3ff ("mac80211: allow AP_VLAN operation on
>> crypto controlled devices") has broken 4-addr operation on crypto
>> controlled devices as reported by sebastian.
>> The commit was mainly focused in addressing the problem in supporting
>> VLANs on crypto controlled devices but since 4-addr mode is also
>> dependent on AP_VLAN interface, this commit breaks the 4-addr AP mode.
> Ok.
>
> Do you know why it actually broke it? I mean, we should've turned off
> the strict requirement for sw crypto control only for the GTK, and that
> shouldn't matter so much?
>
>> I have couple of ideas on how to address the problem,
>>
>> 1) Add a new hw_flag and based on the hardware flag, allow/disallow the
>> creation of AP_VLAN interface.
>>
>> + * @IEEE80211_HW_SUPPORTS_SW_ENCRYPT: Device is capable of transmitting
>> + * frames encrypted in software, only valid when SW_CRYPTO_CONTROL
>> + * is enabled. Based on this flag, mac80211 can allow/disallow VLAN
>> + * operations in the BSS.
> Based on the name and initial description, this sounds equivalent to
> just turning off SW_CRYPTO_CONTROL. I think that's not the intent, but
> would need some renaming.
>
>> 2) Allow mac80211 to call set_key for GTKs on AP_VLAN interfaces for
>> crypto controlled devices, let the driver decide whether to return '1'
>> or some error code based on their support for transmitting sw encrypted
>> frames. I am little skeptical with this approach as drivers are totally
>> unaware of AP_VLAN interfaces.
> No, that won't work.
>
> I'm unsure how 4-addr VLAN can work with ath10k either way?
>
> Maybe it just doesn't normally need a GTK, so nothing broke before, but
> your other patch changed things to remove VLAN and then of course it's
> no longer available?
>
> But then I don't understand the complaint that
>
> So maybe the solution should be to add a separate flag for whether or
> not 4-addr VLAN is supported?
>
> johannes
let me explain. the vlan mode is used to create local interfaces in
4addr mode
like wlan0.sta0, wlan0.sta1 per peer. this is required to put these
peers into the local linux bridge since the local ap interface cannot
handle the bridging capabilities like correct forwarding, stp or even
filtering. this is a long term behaviour since the beginning of ath9k.
so the ap_vlan feature is used to pass the frames per peer in a
indepenened way. you may ask felix fietkau, since he developed it
originally in madwifi
and later in mac80211 / ath9k. so ap_vlan capability is a requiredment
for all 4addr capable wireless drivers.
example of a 4addr capable ap in ath10k with 2 connected 4addr stations
root@apreithalle:~# brctl show
bridge name bridge id STP enabled interfaces
br0 8000.dcef09e4ce07 no ath0
ath0.1
ath0.2
ath0.sta1
ath0.sta4
ath1
ath1.2
eth0
eth1
>
On Wed, 2018-05-23 at 16:09 +0530, Manikanta Pubbisetty wrote:
> > > > > + * @IEEE80211_HW_SUPPORTS_SW_ENCRYPT: Device is capable of transmitting
> > > > > + * frames encrypted in software, only valid when SW_CRYPTO_CONTROL
> > > > > + * is enabled. Based on this flag, mac80211 can allow/disallow VLAN
> > > > > + * operations in the BSS.
> > > >
> > > > Based on the name and initial description, this sounds equivalent to
> > > > just turning off SW_CRYPTO_CONTROL. I think that's not the intent, but
> > > > would need some renaming.
> > >
> > > I can rename it to something which is very specific to VLAN support on
> > > sw crypto controlled devices if that is okay.
> >
> > I don't think that makes sense. If we split the capability of AP_VLAN
> > and AP_VLAN_FOR_4ADDR at the "root", then we don't need to play with all
> > these things. Yes, this is slightly awkward for userspace, and perhaps
> > with the interface combination checks, but I'd like you to look at that.
> >
>
> Makes sense, I had this thought to split the VLAN and 4-addr
> functionality, but since we need to fiddle with userspace, I refrained.
> Anyways, I will check all the possibilities of splitting these
> functionalities.
I'm not sure we really have to - it's likely that wpa_s/hostapd don't
check the capabilities?
johannes
Am 18.05.2018 um 11:53 schrieb Johannes Berg:
> On Mon, 2018-04-23 at 21:18 +0200, Sebastian Gottschall wrote:
>> this patch makes no sense at some points. AP_VLAN must be enabled always
>> (it is enabled by mac80211 by default, but is now disabled in very
>> latest git version for drivers which announce sw_crypto support)
>> if its disabled wds ap / wds sta operation will not work anymore since
>> mac80211 uses AP_VLAN for the local wds sta interfaces
> You'd do you well to learn the correct terminology used in Linux if you
> try to communicate with us...
>
> What you say there makes no sense, WDS is a separate mode. Maybe you
> mean 4-addr mode?
yes 4-addr mode which is common known as WDS mode. (terminology used by
all sorts of vendors)
Sebastian
>
> johannes
>
On Mon, 2018-05-21 at 12:12 +0530, Manikanta Pubbisetty wrote:
> > Do you know why it actually broke it? I mean, we should've turned off
> > the strict requirement for sw crypto control only for the GTK, and that
> > shouldn't matter so much?
>
> With the change db3bdcb9c3ff, AP/VLAN support is advertised by the
> driver conditionally; the primary reason for doing this is to support
> VLAN operations on sw crypto controlled devices.
Right, or, well *not* supporting it.
> AP also creates AP/VLAN devices for supporting 4-addr clients and since
> the driver now advertises AP/VLAN support conditionally, the 4-addr
> operation which has no relation to the VLANs(Per VLAN GTKs) was broken
> on some ath10k devices.
Right. Like I said, splitting those two capabilities somehow would be
best.
> > > + * @IEEE80211_HW_SUPPORTS_SW_ENCRYPT: Device is capable of transmitting
> > > + * frames encrypted in software, only valid when SW_CRYPTO_CONTROL
> > > + * is enabled. Based on this flag, mac80211 can allow/disallow VLAN
> > > + * operations in the BSS.
> >
> > Based on the name and initial description, this sounds equivalent to
> > just turning off SW_CRYPTO_CONTROL. I think that's not the intent, but
> > would need some renaming.
>
> I can rename it to something which is very specific to VLAN support on
> sw crypto controlled devices if that is okay.
I don't think that makes sense. If we split the capability of AP_VLAN
and AP_VLAN_FOR_4ADDR at the "root", then we don't need to play with all
these things. Yes, this is slightly awkward for userspace, and perhaps
with the interface combination checks, but I'd like you to look at that.
johannes
On Fri, 2018-05-04 at 12:20 +0530, Manikanta Pubbisetty wrote:
> Johannes,
>
> It seems like commit db3bdcb9c3ff ("mac80211: allow AP_VLAN operation on
> crypto controlled devices") has broken 4-addr operation on crypto
> controlled devices as reported by sebastian.
> The commit was mainly focused in addressing the problem in supporting
> VLANs on crypto controlled devices but since 4-addr mode is also
> dependent on AP_VLAN interface, this commit breaks the 4-addr AP mode.
Ok.
Do you know why it actually broke it? I mean, we should've turned off
the strict requirement for sw crypto control only for the GTK, and that
shouldn't matter so much?
> I have couple of ideas on how to address the problem,
>
> 1) Add a new hw_flag and based on the hardware flag, allow/disallow the
> creation of AP_VLAN interface.
>
> + * @IEEE80211_HW_SUPPORTS_SW_ENCRYPT: Device is capable of transmitting
> + * frames encrypted in software, only valid when SW_CRYPTO_CONTROL
> + * is enabled. Based on this flag, mac80211 can allow/disallow VLAN
> + * operations in the BSS.
Based on the name and initial description, this sounds equivalent to
just turning off SW_CRYPTO_CONTROL. I think that's not the intent, but
would need some renaming.
> 2) Allow mac80211 to call set_key for GTKs on AP_VLAN interfaces for
> crypto controlled devices, let the driver decide whether to return '1'
> or some error code based on their support for transmitting sw encrypted
> frames. I am little skeptical with this approach as drivers are totally
> unaware of AP_VLAN interfaces.
No, that won't work.
I'm unsure how 4-addr VLAN can work with ath10k either way?
Maybe it just doesn't normally need a GTK, so nothing broke before, but
your other patch changed things to remove VLAN and then of course it's
no longer available?
But then I don't understand the complaint that
So maybe the solution should be to add a separate flag for whether or
not 4-addr VLAN is supported?
johannes
On 5/23/2018 4:09 PM, Johannes Berg wrote:
> On Wed, 2018-05-23 at 16:09 +0530, Manikanta Pubbisetty wrote:
>>>>>> + * @IEEE80211_HW_SUPPORTS_SW_ENCRYPT: Device is capable of transmitting
>>>>>> + * frames encrypted in software, only valid when SW_CRYPTO_CONTROL
>>>>>> + * is enabled. Based on this flag, mac80211 can allow/disallow VLAN
>>>>>> + * operations in the BSS.
>>>>> Based on the name and initial description, this sounds equivalent to
>>>>> just turning off SW_CRYPTO_CONTROL. I think that's not the intent, but
>>>>> would need some renaming.
>>>> I can rename it to something which is very specific to VLAN support on
>>>> sw crypto controlled devices if that is okay.
>>> I don't think that makes sense. If we split the capability of AP_VLAN
>>> and AP_VLAN_FOR_4ADDR at the "root", then we don't need to play with all
>>> these things. Yes, this is slightly awkward for userspace, and perhaps
>>> with the interface combination checks, but I'd like you to look at that.
>>>
>> Makes sense, I had this thought to split the VLAN and 4-addr
>> functionality, but since we need to fiddle with userspace, I refrained.
>> Anyways, I will check all the possibilities of splitting these
>> functionalities.
> I'm not sure we really have to - it's likely that wpa_s/hostapd don't
> check the capabilities?
>
> johannes
IIUC, hostapd will just invoke a NL command to create the AP/VLAN
interface, the capabilities are checked mostly in cfg80211.
If we are planning to split the functionality, possibly something like
having two separate IF-TYPES(one for VLAN and one for 4-addr) instead of
one(which is the case now),
we should probably change the relevant areas in hostapd as well, not
really sure of the complexity of the work involved in userspace though.
On Thu, 2018-05-24 at 06:41 +0200, Sebastian Gottschall wrote:
>
> mmh not sure. it might not check the capabilitiy, but it sets the
> interface type to IFTYPE_AP_VLAN
> for wds sta interfaces. that might collide
That was my point - if it doesn't check the capability then we can
remove it without impacting backward compatibility, and add it back in
another bit saying "AP-VLAN for 4-addr only".
johannes
On 2018-08-16 13:57, Johannes Berg wrote:
> On Tue, 2018-08-14 at 18:23 +0530, Manikanta Pubbisetty wrote:
>
>> > I don't think that makes sense. If we split the capability of AP_VLAN
>> > and AP_VLAN_FOR_4ADDR at the "root", then we don't need to play with all
>> > these things. Yes, this is slightly awkward for userspace, and perhaps
>> > with the interface combination checks, but I'd like you to look at that.
>
>> I was working on splitting the 4-addr functionality from AP/VLAN
>> iftype;
>> I have introduced a new iftype NL80211_IFTYPE_AP_4ADDR and moved the
>> 4-addr handling from AP/VLAN to this new iftype. But this approach
>> breaks the backward compatibility with older userspace applications.
>
> Yeah ...
>
> I'm confused and no longer sure what I was thinking, nor even what
> we're
> trying to achieve here...
>
I had introduced a change db3bdcb9c3ff (" mac80211: allow AP_VLAN
operation on crypto controlled devices ") for supporting VLAN
functionality on ath10k devices; this commit has broken 4 addr support
on ath10k devices as I was advertising the AP/VLAN support
conditionally. Since 4 addr operation is tied to AP/VLAN support, with
this change, only the chips which support VLAN functionality can support
4 addr operation but other ath10k chips don't.
From what I can understand from our previous discussions, we had to
separate the 4addr support from the AP/VLAN iftype but doing so could
lead to backward compatibility issues. I have the code which separates
the 4addr functionality from AP/VLAN but the bigger problem is the
backward compatibility.
I am hoping that now I have set the context correctly :)
>> Since I am completely moving the 4-addr handling to the new type,
>> older
>> applications which do not understand this new type will simply fail
>> and
>> 4-addr mode will be completely broken.
>>
>> Currently, whenever a 4-addr client attempts a connection, hostapd
>> just
>> creates a AP/VLAN interface and moves the 4-addr client to the AP/VLAN
>> iface; there are no other checks. I had no other option other than
>> going
>> with a new iftype for 4-addr handling.
>>
>> Is there a way we can maintain backward compatibility with this
>> approach? Retaining the 4-addr handling in AP/VLAN and duplicating the
>> same functionality to the new iftype seems will work but I am not sure
>> if this is the right approach.
>
> I think we have to keep the 4-addr handling in AP_VLAN iftype either
> way, to not break existing hostapd. We could introduce a separate
> AP_VLAN_NO_4ADDR and then require updating hostapd to get non-4addr
> VLAN, but that also seems awkward.
>
> Since hostapd doesn't currently check anything...
>
> Ok, no, I'm confused now. If we just clear WIPHY_FLAG_4ADDR_AP, don't
> we
> get what we want? 4-addr AP_VLAN interfaces would no longer be
> permitted
> to be created?
As I explained above, the agenda is to provide the driver (in this case,
ath10k) a better control for advertising the device capabilities; only
few ath10k chips can support VLAN functionality but all of them can
support 4 addr operation.
Manikanta
On 5/23/2018 3:20 PM, Johannes Berg wrote:
> On Mon, 2018-05-21 at 12:12 +0530, Manikanta Pubbisetty wrote:
>>> Do you know why it actually broke it? I mean, we should've turned off
>>> the strict requirement for sw crypto control only for the GTK, and that
>>> shouldn't matter so much?
>> With the change db3bdcb9c3ff, AP/VLAN support is advertised by the
>> driver conditionally; the primary reason for doing this is to support
>> VLAN operations on sw crypto controlled devices.
> Right, or, well *not* supporting it.
>
>> AP also creates AP/VLAN devices for supporting 4-addr clients and since
>> the driver now advertises AP/VLAN support conditionally, the 4-addr
>> operation which has no relation to the VLANs(Per VLAN GTKs) was broken
>> on some ath10k devices.
> Right. Like I said, splitting those two capabilities somehow would be
> best.
>
>>>> + * @IEEE80211_HW_SUPPORTS_SW_ENCRYPT: Device is capable of transmitting
>>>> + * frames encrypted in software, only valid when SW_CRYPTO_CONTROL
>>>> + * is enabled. Based on this flag, mac80211 can allow/disallow VLAN
>>>> + * operations in the BSS.
>>> Based on the name and initial description, this sounds equivalent to
>>> just turning off SW_CRYPTO_CONTROL. I think that's not the intent, but
>>> would need some renaming.
>> I can rename it to something which is very specific to VLAN support on
>> sw crypto controlled devices if that is okay.
> I don't think that makes sense. If we split the capability of AP_VLAN
> and AP_VLAN_FOR_4ADDR at the "root", then we don't need to play with all
> these things. Yes, this is slightly awkward for userspace, and perhaps
> with the interface combination checks, but I'd like you to look at that.
Hi Johannes,
I was working on splitting the 4-addr functionality from AP/VLAN iftype;
I have introduced a new iftype NL80211_IFTYPE_AP_4ADDR and moved the
4-addr handling from AP/VLAN to this new iftype. But this approach
breaks the backward compatibility with older userspace applications.
Since I am completely moving the 4-addr handling to the new type, older
applications which do not understand this new type will simply fail and
4-addr mode will be completely broken.
Currently, whenever a 4-addr client attempts a connection, hostapd just
creates a AP/VLAN interface and moves the 4-addr client to the AP/VLAN
iface; there are no other checks. I had no other option other than going
with a new iftype for 4-addr handling.
Is there a way we can maintain backward compatibility with this
approach? Retaining the 4-addr handling in AP/VLAN and duplicating the
same functionality to the new iftype seems will work but I am not sure
if this is the right approach.
Thanks,
Manikanta
Manikanta Pubbisetty <[email protected]> writes:
> On 8/16/2018 1:57 PM, Johannes Berg wrote:
>
> On Tue, 2018-08-14 at 18:23 +0530, Manikanta Pubbisetty wrote:
>
>
> I don't think that makes sense. If we split the capability of AP_VLAN
> and AP_VLAN_FOR_4ADDR at the "root", then we don't need to play with all
> these things. Yes, this is slightly awkward for userspace, and perhaps
> with the interface combination checks, but I'd like you to look at that.
>
>
>
> I was working on splitting the 4-addr functionality from AP/VLAN iftype;
> I have introduced a new iftype NL80211_IFTYPE_AP_4ADDR and moved the
> 4-addr handling from AP/VLAN to this new iftype. But this approach
> breaks the backward compatibility with older userspace applications.
>
>
> Yeah ...
>
> I'm confused and no longer sure what I was thinking, nor even what we're
> trying to achieve here...
>
> I had introduced a change db3bdcb9c3ff (" mac80211: allow AP_VLAN
> operation on crypto controlled devices ") for supporting VLAN
> functionality on ath10k devices; this commit has broken 4 addr support
> on ath10k devices as I was advertising the AP/VLAN support
> conditionally. Since 4 addr operation is tied to AP/VLAN support, with
> this change, only the chips which support VLAN functionality can
> support 4 addr operation but other ath10k chips don't.
Manikanta, please set up your Thunderbird so that it uses the standard
'>' quotation style. This mail is impossible to read as I don't know
which part is written by you and which part by Johannes.
--
Kalle Valo
On Tue, 2018-08-14 at 18:23 +0530, Manikanta Pubbisetty wrote:
> > I don't think that makes sense. If we split the capability of AP_VLAN
> > and AP_VLAN_FOR_4ADDR at the "root", then we don't need to play with all
> > these things. Yes, this is slightly awkward for userspace, and perhaps
> > with the interface combination checks, but I'd like you to look at that.
> I was working on splitting the 4-addr functionality from AP/VLAN iftype;
> I have introduced a new iftype NL80211_IFTYPE_AP_4ADDR and moved the
> 4-addr handling from AP/VLAN to this new iftype. But this approach
> breaks the backward compatibility with older userspace applications.
Yeah ...
I'm confused and no longer sure what I was thinking, nor even what we're
trying to achieve here...
> Since I am completely moving the 4-addr handling to the new type, older
> applications which do not understand this new type will simply fail and
> 4-addr mode will be completely broken.
>
> Currently, whenever a 4-addr client attempts a connection, hostapd just
> creates a AP/VLAN interface and moves the 4-addr client to the AP/VLAN
> iface; there are no other checks. I had no other option other than going
> with a new iftype for 4-addr handling.
>
> Is there a way we can maintain backward compatibility with this
> approach? Retaining the 4-addr handling in AP/VLAN and duplicating the
> same functionality to the new iftype seems will work but I am not sure
> if this is the right approach.
I think we have to keep the 4-addr handling in AP_VLAN iftype either
way, to not break existing hostapd. We could introduce a separate
AP_VLAN_NO_4ADDR and then require updating hostapd to get non-4addr
VLAN, but that also seems awkward.
Since hostapd doesn't currently check anything...
Ok, no, I'm confused now. If we just clear WIPHY_FLAG_4ADDR_AP, don't we
get what we want? 4-addr AP_VLAN interfaces would no longer be permitted
to be created?
johannes
On 9/3/2018 4:09 PM, Johannes Berg wrote:
> Hi,
>
> Sorry ... this got delayed again.
>
>> I had introduced a change db3bdcb9c3ff (" mac80211: allow AP_VLAN
>> operation on crypto controlled devices ") for supporting VLAN
>> functionality on ath10k devices; this commit has broken 4 addr support
>> on ath10k devices as I was advertising the AP/VLAN support
>> conditionally. Since 4 addr operation is tied to AP/VLAN support, with
>> this change, only the chips which support VLAN functionality can support
>> 4 addr operation but other ath10k chips don't.
> Right.
>
>> From what I can understand from our previous discussions, we had to
>> separate the 4addr support from the AP/VLAN iftype but doing so could
>> lead to backward compatibility issues. I have the code which separates
>> the 4addr functionality from AP/VLAN but the bigger problem is the
>> backward compatibility.
> Ok.
>
>> I am hoping that now I have set the context correctly :)
> Thanks!
>
>>> I think we have to keep the 4-addr handling in AP_VLAN iftype either
>>> way, to not break existing hostapd. We could introduce a separate
>>> AP_VLAN_NO_4ADDR and then require updating hostapd to get non-4addr
>>> VLAN, but that also seems awkward.
>>>
>>> Since hostapd doesn't currently check anything...
>>>
>>> Ok, no, I'm confused now. If we just clear WIPHY_FLAG_4ADDR_AP, don't
>>> we
>>> get what we want? 4-addr AP_VLAN interfaces would no longer be
>>> permitted
>>> to be created?
>> As I explained above, the agenda is to provide the driver (in this case,
>> ath10k) a better control for advertising the device capabilities; only
>> few ath10k chips can support VLAN functionality but all of them can
>> support 4 addr operation.
> So the problematic part isn't actually the *4-addr* (fake) VLAN, the
> problematic part is the actual AP_VLAN - I suppose because that uses a
> separate GTK.
>
>
> So I guess the only, mostly backward compatible way to really separate
> the two would be to
>
> 1) move WIPHY_FLAG_4ADDR_{AP,STATION} to be nl80211 ext feature flags,
> I guess the STATION always should've been since there's nothing that
> indicates support for it today in the API
>
> along with one of:
>
> 2a) Add a new AP_VLAN ext feature flag that indicates the AP_VLAN is not
> only supported for 4-addr
>
> 2b) Allow creating an AP_VLAN interface in 4-addr mode in
> nl80211_new_interface() even when AP_VLAN is not in the supported
> interface combinations, if (and only if) WIPHY_FLAG_4ADDR_AP is set
> (or rather the new extended feature flag after doing 1). Update the
> language in the documentation somewhere indicating this.
>
>
> Hostapd clearly deals with both 2a and 2b since it never bothers to
> check the combinations. As a result, I prefer 2b rather than 2a since it
> doesn't add any weird combinations to the API that would be impossible.
Sure Johannes!!
Thanks,
Manikanta
Hi,
Sorry ... this got delayed again.
> I had introduced a change db3bdcb9c3ff (" mac80211: allow AP_VLAN
> operation on crypto controlled devices ") for supporting VLAN
> functionality on ath10k devices; this commit has broken 4 addr support
> on ath10k devices as I was advertising the AP/VLAN support
> conditionally. Since 4 addr operation is tied to AP/VLAN support, with
> this change, only the chips which support VLAN functionality can support
> 4 addr operation but other ath10k chips don't.
Right.
> From what I can understand from our previous discussions, we had to
> separate the 4addr support from the AP/VLAN iftype but doing so could
> lead to backward compatibility issues. I have the code which separates
> the 4addr functionality from AP/VLAN but the bigger problem is the
> backward compatibility.
Ok.
> I am hoping that now I have set the context correctly :)
Thanks!
> > I think we have to keep the 4-addr handling in AP_VLAN iftype either
> > way, to not break existing hostapd. We could introduce a separate
> > AP_VLAN_NO_4ADDR and then require updating hostapd to get non-4addr
> > VLAN, but that also seems awkward.
> >
> > Since hostapd doesn't currently check anything...
> >
> > Ok, no, I'm confused now. If we just clear WIPHY_FLAG_4ADDR_AP, don't
> > we
> > get what we want? 4-addr AP_VLAN interfaces would no longer be
> > permitted
> > to be created?
>
> As I explained above, the agenda is to provide the driver (in this case,
> ath10k) a better control for advertising the device capabilities; only
> few ath10k chips can support VLAN functionality but all of them can
> support 4 addr operation.
So the problematic part isn't actually the *4-addr* (fake) VLAN, the
problematic part is the actual AP_VLAN - I suppose because that uses a
separate GTK.
So I guess the only, mostly backward compatible way to really separate
the two would be to
1) move WIPHY_FLAG_4ADDR_{AP,STATION} to be nl80211 ext feature flags,
I guess the STATION always should've been since there's nothing that
indicates support for it today in the API
along with one of:
2a) Add a new AP_VLAN ext feature flag that indicates the AP_VLAN is not
only supported for 4-addr
2b) Allow creating an AP_VLAN interface in 4-addr mode in
nl80211_new_interface() even when AP_VLAN is not in the supported
interface combinations, if (and only if) WIPHY_FLAG_4ADDR_AP is set
(or rather the new extended feature flag after doing 1). Update the
language in the documentation somewhere indicating this.
Hostapd clearly deals with both 2a and 2b since it never bothers to
check the combinations. As a result, I prefer 2b rather than 2a since it
doesn't add any weird combinations to the API that would be impossible.
johannes